On Mon, 09/21 18:29, John Snow wrote:
>
>
> On 09/15/2015 02:11 AM, Fam Zheng wrote:
> > Reviewed-by: Max Reitz <mre...@redhat.com>
> > Signed-off-by: Fam Zheng <f...@redhat.com>
> > ---
> > include/block/blockjob.h | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/include/block/blockjob.h b/include/block/blockjob.h
> > index 3e7ad21..a7b497c 100644
> > --- a/include/block/blockjob.h
> > +++ b/include/block/blockjob.h
> > @@ -50,6 +50,24 @@ typedef struct BlockJobDriver {
> > * manually.
> > */
> > void (*complete)(BlockJob *job, Error **errp);
> > +
> > + /**
> > + * If the callback is not NULL, it will be invoked when all the jobs
> > + * belonging to the same transaction complete; or upon this job's
> > + * completion if it is not in a transaction. Skipped if NULL.
> > + *
> > + * Exactly one of .commit() and .abort() will be called for each job.
> > + */
> > + void (*commit)(BlockJob *job);
> > +
>
> I find this phrasing strange, but maybe it's just me. "Exactly one of
> commit and abort will be called for each job" implies [to me] that it'd
> be possible to call commit for one, but abort for different jobs [in a
> transaction] -- but clearly we don't mean that. It is the "for each job"
> that implies an iteration over a collection to me.
>
> Just above we say "[commit] will be invoked when all the jobs belonging
> to the same transaction are complete" which itself implies either all
> jobs will be committed or all jobs will be aborted.
>
> Maybe:
>
> "All jobs will complete with a call to either .commit() or .abort() but
> never both."
>
> But I might be being too bikesheddy.
>
> > + /**
> > + * If the callback is not NULL, it will be invoked when any job in the
> > + * same transaction fails; or upon this job's failure (due to error or
> > + * cancellation) if it is not in a transaction. Skipped if NULL.
> > + *
> > + * Exactly one of .commit() and .abort() will be called for each job.
> > + */
> > + void (*abort)(BlockJob *job);
> > } BlockJobDriver;
> >
> > /**
> >
>
> I'm probably just too picky.
>
> Reviewed-by: John Snow <js...@redhat.com>
>
No problem, It makese sense, I'll use your words :)
Thanks.
Fam
