[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [RFC v3 01/14] blockjobs: add manual prope
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [Qemu-devel] [RFC v3 01/14] blockjobs: add manual property |
Date: |
Mon, 29 Jan 2018 18:46:58 +0100 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
Am 29.01.2018 um 18:34 hat John Snow geschrieben:
> On 01/29/2018 11:59 AM, Kevin Wolf wrote:
> > Am 27.01.2018 um 03:05 hat John Snow geschrieben:
> >> This property will be used to opt-in to the new BlockJobs workflow
> >> that allows a tighter, more explicit control over transitions from
> >> one runstate to another.
> >>
> >> Signed-off-by: John Snow <address@hidden>
> >
> >> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
> >> index 00403d9482..b94d0c9fa6 100644
> >> --- a/include/block/blockjob.h
> >> +++ b/include/block/blockjob.h
> >> @@ -141,6 +141,11 @@ typedef struct BlockJob {
> >> */
> >> QEMUTimer sleep_timer;
> >>
> >> + /* Set to true when management API has requested 2.12+ job lifetime
> >> + * management semantics.
> >> + */
> >> + bool manual;
> >
> > Wouldn't it make more sense to describe what "2.12+ job lifetime
> > management semantics" actually are? Maybe then it would be easy to find
> > a more specific name, too, like manual_completion.
> >
>
> Sure. At the time I wrote it, I wasn't sure what they were. Maybe I'll
> find out after the review; but I'll make a note to document it here.
>
> > In fact, I wonder if the opposite flag wouldn't be nicer, i.e. having a
> > bool auto_completion (or finalization or whatever that extra step was
> > called in the final draft), defaulting to true.
> >
>
> I could do that, if you feel it'd be more readable. In terms of style I
> tend to prefer new additions default to false as that feels more
> permanently reliable, but it's only a preference.
Yeah, that is one way to look at it. The other one is that options
should generally be positive, i.e. true enables a feature rather than
disabling it. If I look at the interface without its history, the
feature (the extra thing on top of the basic state machine) that is
enabled or disabled is automatic completion.
This is why my preference would be the other way round, but it's still
only a preference. :-)
After reading a few more patches, it also seems to me that you are
looking a bit differently at the whole state machine than I am. So you
seem to assume two different state machines depending on whether manual
is set, whereas I assume all jobs share the same state machine and only
some transitions are optionally manual or automatic.
Kevin
[Qemu-block] [RFC v3 05/14] blockjobs: add block_job_dismiss, John Snow, 2018/01/26
[Qemu-block] [RFC v3 04/14] blockjobs: RFC add block_job_verb permission table, John Snow, 2018/01/26
[Qemu-block] [RFC v3 06/14] blockjobs: add CONCLUDED state, John Snow, 2018/01/26
[Qemu-block] [RFC v3 09/14] blockjobs: add prepare callback, John Snow, 2018/01/26
[Qemu-block] [RFC v3 07/14] blockjobs: ensure abort is called for cancelled jobs, John Snow, 2018/01/26
[Qemu-block] [RFC v3 08/14] blockjobs: add commit, abort, clean helpers, John Snow, 2018/01/26
[Qemu-block] [RFC v3 11/14] blockjobs: add PENDING status and event, John Snow, 2018/01/26