Re: [Qemu-devel] [PATCH v3 2/3] block: resize backing image during active layer commit, if needed

[Benoît Canet]

Le Thursday 23 Jan 2014 à 16:48:56 (-0500), Jeff Cody a écrit :
> If the top image to commit is the active layer, and also larger than
> the base image, then an I/O error will likely be returned during
> block-commit.
> 
> For instance, if we have a base image with a virtual size 10G, and a
> active layer image of size 20G, then committing the snapshot via
> 'block-commit' will likely fail.
> 
> This will automatically attempt to resize the base image, if the
> active layer image to be committed is larger.
> 
> Signed-off-by: Jeff Cody <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> ---
>  block/mirror.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index 2932bab..2a33aa6 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -630,11 +630,52 @@ void commit_active_start(BlockDriverState *bs, 
> BlockDriverState *base,
>                           BlockDriverCompletionFunc *cb,
>                           void *opaque, Error **errp)
>  {
> +    int64_t length, base_length;
> +    int orig_base_flags;
> +
> +    orig_base_flags = bdrv_get_flags(base);
> +
>      if (bdrv_reopen(base, bs->open_flags, errp)) {
>          return;
>      }
> +
> +    length = bdrv_getlength(bs);
> +    if (length < 0) {
> +        error_setg(errp, "Unable to determine length of %s", bs->filename);
> +        goto error_restore_flags;
> +    }
> +
> +    base_length = bdrv_getlength(base);
> +    if (base_length < 0) {
> +        error_setg(errp, "Unable to determine length of %s", base->filename);
> +        goto error_restore_flags;
> +    }
> +
> +    if (length > base_length) {
> +        if (bdrv_truncate(base, length) < 0) {
> +            error_setg(errp, "Top image %s is larger than base image %s, and 
> "
> +                             "resize of base image failed",
> +                             bs->filename, base->filename);
> +            goto error_restore_flags;
> +        }


> +    } else if (length < 0) {
> +        goto error_restore_flags;
> +    }
The code already test for (length < 0) ten lines above. Is this branch really
needed ?

Best regards

Benoît

> +
> +
>      bdrv_ref(base);
>      mirror_start_job(bs, base, speed, 0, 0,
>                       on_error, on_error, cb, opaque, errp,
>                       &commit_active_job_driver, false, base);
> +    if (error_is_set(errp)) {
> +        goto error_restore_flags;
> +    }
> +
> +    return;
> +
> +error_restore_flags:
> +    /* ignore error and errp for bdrv_reopen, because we want to propagate
> +     * the original error */
> +    bdrv_reopen(base, orig_base_flags, NULL);
> +    return;
>  }
> -- 
> 1.8.3.1
> 
>