Re: [Qemu-devel] [PATCH 6/8] block: Let format drivers pass @exact

[Maxim Levitsky <mlevitsk@redhat.com>]

On Wed, 2019-09-18 at 11:51 +0200, Max Reitz wrote:
> When truncating a format node, the @exact parameter is generally handled
> simply by virtue of the format storing the new size in the image
> metadata.  Such formats do not need to pass on the parameter to their
> file nodes.
> 
> There are exceptions, though:
> - raw and crypto cannot store the image size, and thus must pass on
>   @exact.
> 
> - When using qcow2 with an external data file, it just makes sense to
>   keep its size in sync with the qcow2 virtual disk (because the
>   external data file is the virtual disk).  Therefore, we should pass
>   @exact when truncating it.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/crypto.c     |  2 +-
>  block/qcow2.c      | 15 ++++++++++++++-
>  block/raw-format.c |  2 +-
>  3 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/block/crypto.c b/block/crypto.c
> index e5a1a2cdf3..24823835c1 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -311,7 +311,7 @@ block_crypto_co_truncate(BlockDriverState *bs, int64_t offset, bool exact,
>  
>      offset += payload_offset;
>  
> -    return bdrv_co_truncate(bs->file, offset, false, prealloc, errp);
> +    return bdrv_co_truncate(bs->file, offset, exact, prealloc, errp);
>  }
>  
>  static void block_crypto_close(BlockDriverState *bs)
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 157b9b75d9..4ef19dd29a 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3822,6 +3822,13 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
>          if ((last_cluster + 1) * s->cluster_size < old_file_size) {
>              Error *local_err = NULL;
>  
> +            /*
> +             * Do not pass @exact here: It will not help the user if
> +             * we get an error here just because they wanted to shrink
> +             * their qcow2 image (on a block device) with qemu-img.
> +             * (And on the qcow2 layer, the @exact requirement is
> +             * always fulfilled, so there is no need to pass it on.)
> +             */
>              bdrv_co_truncate(bs->file, (last_cluster + 1) * s->cluster_size,
>                               false, PREALLOC_MODE_OFF, &local_err);
>              if (local_err) {
> @@ -3840,7 +3847,12 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
>      switch (prealloc) {
>      case PREALLOC_MODE_OFF:
>          if (has_data_file(bs)) {
> -            ret = bdrv_co_truncate(s->data_file, offset, false, prealloc, errp);
> +            /*
> +             * If the caller wants an exact resize, the external data
> +             * file should be resized to the exact target size, too,
> +             * so we pass @exact here.
> +             */
> +            ret = bdrv_co_truncate(s->data_file, offset, exact, prealloc, errp);
>              if (ret < 0) {
>                  goto fail;
>              }
> @@ -3925,6 +3937,7 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
>          /* Allocate the data area */
>          new_file_size = allocation_start +
>                          nb_new_data_clusters * s->cluster_size;
> +        /* Image file grows, so @exact does not matter */
>          ret = bdrv_co_truncate(bs->file, new_file_size, false, prealloc, errp);
> 
>          if (ret < 0) {
>              error_prepend(errp, "Failed to resize underlying file: ");
> diff --git a/block/raw-format.c b/block/raw-format.c
> index 57d84d0bae..3a76ec7dd2 100644
> --- a/block/raw-format.c
> +++ b/block/raw-format.c
> @@ -387,7 +387,7 @@ static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
>  
>      s->size = offset;
>      offset += s->offset;
> -    return bdrv_co_truncate(bs->file, offset, false, prealloc, errp);
> +    return bdrv_co_truncate(bs->file, offset, exact, prealloc, errp);
>  }
>  
>  static void raw_eject(BlockDriverState *bs, bool eject_flag)


Looks all right.
Reviewed-by: Maxim Levitsky <maximlevitsk@redhat.com>

Best regards,
	Maxim Levitsky