[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 04/13] block/commit: refactor commit to use j
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v2 04/13] block/commit: refactor commit to use job callbacks |
Date: |
Mon, 27 Aug 2018 14:07:21 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 2018-08-24 00:22, John Snow wrote:
> Use the component callbacks; prepare, abort, and clean.
>
> NB: prepare is only called when the job has not yet failed;
> and abort can be called after prepare.
>
> complete -> prepare -> abort -> clean
> complete -> abort -> clean
>
> Signed-off-by: John Snow <address@hidden>
> ---
> block/commit.c | 90
> ++++++++++++++++++++++++++++++++--------------------------
> 1 file changed, 49 insertions(+), 41 deletions(-)
>
> diff --git a/block/commit.c b/block/commit.c
> index b6e8969877..9bf50385cf 100644
> --- a/block/commit.c
> +++ b/block/commit.c
[...]
> @@ -68,61 +69,65 @@ static int coroutine_fn commit_populate(BlockBackend *bs,
> BlockBackend *base,
[...]
> +static void commit_abort(Job *job)
> +{
> + CommitBlockJob *s = container_of(job, CommitBlockJob, common.job);
> + BlockDriverState *top_bs = blk_bs(s->top);
> +
> + /* Make sure commit_top_bs and top stay around until bdrv_replace_node()
> */
> + bdrv_ref(top_bs);
> + bdrv_ref(s->commit_top_bs);
> +
> + if (s->base) {
> + blk_unref(s->base);
> }
>
> + /* free the blockers on the intermediate nodes so that bdrv_replace_nodes
> + * can succeed */
> + block_job_remove_all_bdrv(&s->common);
> +
> + /* If bdrv_drop_intermediate() didn't already do that, remove the commit
> + * filter driver from the backing chain. Do this as the final step so
> that
> + * the 'consistent read' permission can be granted.
I'd suggest rewording this to "If bdrv_drop_intermediate() failed (or
was not invoked at all), remove the commit filter driver from the
backing chain now."
Right now it sounds like maybe bdrv_drop_intermediate() removes the
filter, and maybe it doesn't. But actually it is supposed to always
remove it -- but it may fail, and then we have to force-remove the filter.
Apart from that:
Reviewed-by: Max Reitz <address@hidden>
> + *
> + * XXX Can (or should) we somehow keep 'consistent read' blocked even
> + * after the failed/cancelled commit job is gone? If we already wrote
> + * something to base, the intermediate images aren't valid any more. */
> + bdrv_child_try_set_perm(s->commit_top_bs->backing, 0, BLK_PERM_ALL,
> + &error_abort);
> + bdrv_replace_node(s->commit_top_bs, backing_bs(s->commit_top_bs),
> + &error_abort);
> +
> + bdrv_unref(s->commit_top_bs);
> + bdrv_unref(top_bs);
> +}
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v2 02/13] block/mirror: add block job creation flags, (continued)
- [Qemu-devel] [PATCH v2 10/13] qapi/block-commit: expose new job properties, John Snow, 2018/08/23
- [Qemu-devel] [PATCH v2 05/13] block/mirror: conservative mirror_exit refactor, John Snow, 2018/08/23
- [Qemu-devel] [PATCH v2 09/13] jobs: remove .exit callback, John Snow, 2018/08/23
- [Qemu-devel] [PATCH v2 07/13] tests/blockjob: replace Blockjob with Job, John Snow, 2018/08/23
- [Qemu-devel] [PATCH v2 04/13] block/commit: refactor commit to use job callbacks, John Snow, 2018/08/23
- Re: [Qemu-devel] [PATCH v2 04/13] block/commit: refactor commit to use job callbacks,
Max Reitz <=
- [Qemu-devel] [PATCH v2 13/13] block/backup: qapi documentation fixup, John Snow, 2018/08/23
- [Qemu-devel] [PATCH v2 12/13] qapi/block-stream: expose new job properties, John Snow, 2018/08/23
- [Qemu-devel] [PATCH v2 11/13] qapi/block-mirror: expose new job properties, John Snow, 2018/08/23