Re: [Qemu-devel] [PATCH 4/7] block/commit: utilize job_exit shim

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]

On 2018-08-17 21:04, John Snow wrote:
> Change the manual deferment to commit_complete into the implicit
> callback to job_exit, renaming commit_complete to commit_exit.
> 
> This conversion does change the timing of when job_completed is
> called to after the bdrv_replace_node and bdrv_unref calls, which
> could have implications for bjob->blk which will now be put down
> after this cleanup.
> 
> Kevin highlights that we did not take any permissions for that backend
> at job creation time, so it is safe to reorder these operations.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  block/commit.c | 20 ++++----------------
>  1 file changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/block/commit.c b/block/commit.c
> index 4a17bb73ec..93c3b6a39e 100644
> --- a/block/commit.c
> +++ b/block/commit.c

[...]

> @@ -93,8 +87,8 @@ static void commit_complete(Job *job, void *opaque)
>  
>      if (!job_is_cancelled(job) && ret == 0) {
>          /* success */
> -        ret = bdrv_drop_intermediate(s->commit_top_bs, base,
> -                                     s->backing_file_str);
> +        job->ret = bdrv_drop_intermediate(s->commit_top_bs, base,
> +                                          s->backing_file_str);

This makes me ask myself why .exit() doesn't just return an int, like
.run().  And takes an Error **.

Max

>      } else {
>          /* XXX Can (or should) we somehow keep 'consistent read' blocked even
>           * after the failed/cancelled commit job is gone? If we already wrote


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]