Re: [Qemu-devel] [PATCH v2 01/11] qemu-img: Fix bdrv_has_zero_init() use in convert

[Maxim Levitsky <mlevitsk@redhat.com>]

On Wed, 2019-07-24 at 19:12 +0200, Max Reitz wrote:
> bdrv_has_zero_init() only has meaning for newly created images or image
> areas.  If qemu-img convert did not create the image itself, it cannot
> rely on bdrv_has_zero_init()'s result to carry any meaning.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qemu-img.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 79983772de..0f4be80c10 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1578,6 +1578,7 @@ typedef struct ImgConvertState {
>      bool has_zero_init;
>      bool compressed;
>      bool unallocated_blocks_are_zero;
> +    bool target_is_new;
>      bool target_has_backing;
>      int64_t target_backing_sectors; /* negative if unknown */
>      bool wr_in_order;
> @@ -1975,9 +1976,11 @@ static int convert_do_copy(ImgConvertState *s)
>      int64_t sector_num = 0;
>  
>      /* Check whether we have zero initialisation or can get it efficiently */
> -    s->has_zero_init = s->min_sparse && !s->target_has_backing
> -                     ? bdrv_has_zero_init(blk_bs(s->target))
> -                     : false;
> +    if (s->target_is_new && s->min_sparse && !s->target_has_backing) {
> +        s->has_zero_init = bdrv_has_zero_init(blk_bs(s->target));
> +    } else {
> +        s->has_zero_init = false;
> +    }
>  
>      if (!s->has_zero_init && !s->target_has_backing &&
>          bdrv_can_write_zeroes_with_unmap(blk_bs(s->target)))
> @@ -2423,6 +2426,8 @@ static int img_convert(int argc, char **argv)
>          }
>      }
>  
> +    s.target_is_new = !skip_create;
> +
>      flags = s.min_sparse ? (BDRV_O_RDWR | BDRV_O_UNMAP) : BDRV_O_RDWR;
>      ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
>      if (ret < 0) {


Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
	Maxim Levitsky