[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole
From: |
Kevin Wolf |
Subject: |
Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole |
Date: |
Fri, 11 Jun 2021 13:21:45 +0200 |
Am 11.06.2021 um 10:14 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 11.06.2021 11:09, Kevin Wolf wrote:
> > Am 10.06.2021 um 22:46 hat Eric Blake geschrieben:
> > > 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.
> >
> > A negative value isn't any less unexpected than a missing key. I don't
> > think any existing tool would be able to handle it. Encoding different
> > meanings in a single value isn't very QAPI-like either. Usually strings
> > that are parsed are the problem, but negative integers really isn't that
> > much different. I don't really like this solution.
> >
> > Leaving out the depth feels like a better suggestion to me.
> >
> > But anyway, this seems to only happen at the end of the backing chain.
> > So if the backing chain consistents of n images, why not report 'depth':
> > n + 1? So, in the above example, you would get 1. I think this has the
> > best chances of tools actually working correctly with the new output,
> > even though it's still not unlikely to break something.
> >
>
> Did you consider just add a new field?
>
> So, "depth" keeps its meaning "which level provides data".
>
> And we add additional optional field like
>
> absolutely-completely-absent: bool
>
> Which is true if data is nowhere in the backing chain.
Or how about exposing BDRV_BLOCK_ALLOCATED as 'allocated': 'bool'? Which
I think is what the conclusion was already for NBD, so doing the same in
'qemu-img map' would be consistent.
This is, of course, almost the same as 'absolutely-completely-absent',
just without the negating the flag.
Kevin
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, (continued)
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, Nir Soffer, 2021/06/08
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, Eric Blake, 2021/06/10
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, Nir Soffer, 2021/06/10
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, Eric Blake, 2021/06/10
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, Kevin Wolf, 2021/06/11
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, Vladimir Sementsov-Ogievskiy, 2021/06/11
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, Nir Soffer, 2021/06/11
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, Vladimir Sementsov-Ogievskiy, 2021/06/11
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole,
Kevin Wolf <=
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, Vladimir Sementsov-Ogievskiy, 2021/06/11
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, Eric Blake, 2021/06/11
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, Eric Blake, 2021/06/11
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, Nir Soffer, 2021/06/11
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, Eric Blake, 2021/06/11
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, Nir Soffer, 2021/06/11
- Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole, Eric Blake, 2021/06/11