[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 06/14] blockjob: Add .commit and .abort block
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH v6 06/14] blockjob: Add .commit and .abort block job actions |
Date: |
Mon, 21 Sep 2015 18:29:17 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
On 09/15/2015 02:11 AM, Fam Zheng wrote:
> Reviewed-by: Max Reitz <address@hidden>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
> 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 <address@hidden>
- [Qemu-devel] [PATCH v6 00/14] block: incremental backup transactions using BlockJobTxn, Fam Zheng, 2015/09/15
- [Qemu-devel] [PATCH v6 01/14] qapi: Add transaction support to block-dirty-bitmap operations, Fam Zheng, 2015/09/15
- [Qemu-devel] [PATCH v6 02/14] iotests: add transactional incremental backup test, Fam Zheng, 2015/09/15
- [Qemu-devel] [PATCH v6 04/14] backup: Extract dirty bitmap handling as a separate function, Fam Zheng, 2015/09/15
- [Qemu-devel] [PATCH v6 03/14] block: rename BlkTransactionState and BdrvActionOps, Fam Zheng, 2015/09/15
- [Qemu-devel] [PATCH v6 05/14] blockjob: Introduce reference count, Fam Zheng, 2015/09/15
- [Qemu-devel] [PATCH v6 06/14] blockjob: Add .commit and .abort block job actions, Fam Zheng, 2015/09/15
- Re: [Qemu-devel] [PATCH v6 06/14] blockjob: Add .commit and .abort block job actions,
John Snow <=
- [Qemu-devel] [PATCH v6 07/14] blockjob: Add "completed" and "ret" in BlockJob, Fam Zheng, 2015/09/15
- [Qemu-devel] [PATCH v6 08/14] blockjob: Simplify block_job_finish_sync, Fam Zheng, 2015/09/15
- [Qemu-devel] [PATCH v6 09/14] block: Add block job transactions, Fam Zheng, 2015/09/15
- [Qemu-devel] [PATCH v6 10/14] blockdev: make BlockJobTxn available to qmp 'transaction', Fam Zheng, 2015/09/15