Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole

From: Eric Blake <eblake@redhat.com>
To: Nir Soffer <nsoffer@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	Nir Soffer <nirsof@gmail.com>, qemu-block <qemu-block@nongnu.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Max Reitz <mreitz@redhat.com>
Subject: Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole
Date: Thu, 10 Jun 2021 15:46:17 -0500	[thread overview]
Message-ID: <20210610204617.fuj4ivqrixpz4qfj@redhat.com> (raw)
In-Reply-To: <CAMRbyysoYhcyiP2mWubfZsj09k=Ea_3-RPr+Tt7KvoE1z3jrNA@mail.gmail.com>

On Thu, Jun 10, 2021 at 11:09:05PM +0300, Nir Soffer wrote:
> > But:
> >
> > $ qemu-img map --output=json -f qcow2 json:'{"driver":"qcow2","backing":null, \
> >   "file":{"driver":"file","filename":"top.qcow2"}}'
> > [{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false},
> > { "start": 65536, "length": 65536, "depth": 0, "zero": false, "data": true, "offset": 327680},
> > { "start": 131072, "length": 131072, "depth": 0, "zero": true, "data": false}]
> >
> > also reports the entire file at "depth":0, which is misleading, since
> > we have just been arguing from the qemu:allocation-depth perspective
> > (and also from bdrv_block_status) that the qcow2 image is NOT 100%
> > allocated (in the sense where allocation == data comes locally).
> > Perhaps it might be better if we tweaked the above qemu-img map to
> > produce:
> >
> > [{ "start": 0, "length": 65536, "depth": -1, "zero": true, "data": false},
> > { "start": 65536, "length": 65536, "depth": 0, "zero": false, "data": true, "offset": 327680},
> > { "start": 131072, "length": 65536, "depth": 0, "zero": true, "data": false},
> > { "start": 196608, "length": 65536, "depth": -1, "zero": true, "data": false}]
> 
> It will be more consistent with "offset" to drop "depth" from output
> if we don't have it:
> 
>     [{ "start": 0, "length": 65536, "zero": true, "data": false},
>      { "start": 65536, "length": 65536, "depth": 0, "zero": false,
> "data": true, "offset": 327680},
>      { "start": 131072, "length": 65536, "depth": 0, "zero": true,
> "data": false},
>      { "start": 196608, "length": 65536, "zero": true, "data": false}]

Yes, that might work as well.  But we didn't previously document
depth to be optional.  Removing something from output risks breaking
more downstream tools that expect it to be non-optional, compared to
providing a new value.

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