Re: [Qemu-devel] [PATCH 02/13] block: Freeze the backing chain for the duration of the commit job

[Kevin Wolf]

Am 17.01.2019 um 16:33 hat Alberto Garcia geschrieben:
> Signed-off-by: Alberto Garcia <address@hidden>
> ---
>  block/commit.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/block/commit.c b/block/commit.c
> index 53148e610b..8824d135e0 100644
> --- a/block/commit.c
> +++ b/block/commit.c
> @@ -73,6 +73,8 @@ static int commit_prepare(Job *job)
>  {
>      CommitBlockJob *s = container_of(job, CommitBlockJob, common.job);
>  
> +    bdrv_unfreeze_backing_chain(s->commit_top_bs, s->base_bs);
> +
>      /* Remove base node parent that still uses BLK_PERM_WRITE/RESIZE before
>       * the normal backing chain can be restored. */
>      blk_unref(s->base);
> @@ -89,6 +91,8 @@ static void commit_abort(Job *job)
>      CommitBlockJob *s = container_of(job, CommitBlockJob, common.job);
>      BlockDriverState *top_bs = blk_bs(s->top);
>  
> +    bdrv_unfreeze_backing_chain(s->commit_top_bs, s->base_bs);

commit_abort() will run if commit_prepare() returned an error, so we may
end up unfreezing twice. Maybe move it into the 'if (s->base)' block a
few lines down.

>      /* Make sure commit_top_bs and top stay around until bdrv_replace_node() 
> */
>      bdrv_ref(top_bs);
>      bdrv_ref(s->commit_top_bs);
> @@ -336,6 +340,10 @@ void commit_start(const char *job_id, BlockDriverState 
> *bs,
>          }
>      }
>  
> +    if (bdrv_freeze_backing_chain(commit_top_bs, base, errp) < 0) {
> +        goto fail;
> +    }

Don't error paths need to unfreeze after this?

Kevin