Re: [PATCH 7/8] qemu-img: convert: don't use unallocated_blocks_are_zero

[Eric Blake]

On 5/6/20 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
> qemu-img convert wants to distinguish ZERO which comes from short
> backing files. unallocated_blocks_are_zero field of bdi is unrelated:
> space after EOF is always considered to be zero anyway. So, just make
> post_backing_zero true in case of short backing file.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>   qemu-img.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)

This patch should come first in the series.  It would have saved me a 
lot of time in reviewing 1/8.

> diff --git a/qemu-img.c b/qemu-img.c
> index 6a4327aaba..4fe751878b 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1632,7 +1632,6 @@ typedef struct ImgConvertState {
>       BlockBackend *target;
>       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 */
> @@ -1677,7 +1676,7 @@ static int convert_iteration_sectors(ImgConvertState *s, 
> int64_t sector_num)
>   
>       if (s->target_backing_sectors >= 0) {
>           if (sector_num >= s->target_backing_sectors) {
> -            post_backing_zero = s->unallocated_blocks_are_zero;
> +            post_backing_zero = true;
>           } else if (sector_num + n > s->target_backing_sectors) {
>               /* Split requests around target_backing_sectors (because
>                * starting from there, zeros are handled differently) */
> @@ -2580,7 +2579,6 @@ static int img_convert(int argc, char **argv)
>       } else {
>           s.compressed = s.compressed || bdi.needs_compressed_writes;
>           s.cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE;
> -        s.unallocated_blocks_are_zero = bdi.unallocated_blocks_are_zero;
>       }

My question in 1/8 about whether we have iotest coverage of this 
optimization remains, but I concur that the block layer takes care of 
reading zero when encountering unallocated blocks beyond EOF of a short 
backing file, and therefore performing this optimization always rather 
than just when the driver claims that unallocated blocks read as zero 
should be safe.

Reviewed-by: Eric Blake <address@hidden>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org