[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 04/10] block/vpc: return ZERO block-status when appropriat
Re: [PATCH v3 04/10] block/vpc: return ZERO block-status when appropriate
Mon, 6 Jul 2020 12:17:20 +0300
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0
06.07.2020 11:28, Max Reitz wrote:
On 28.05.20 11:43, Vladimir Sementsov-Ogievskiy wrote:
In case when get_image_offset() returns -1, we do zero out the
corresponding chunk of qiov. So, this should be reported as ZERO.
Note that this changes visible output of "qemu-img map --output=json"
and "qemu-io -c map" commands. For qemu-img map, the change is obvious:
we just mark as zero what is really zero. For qemu-io it's less
obvious: what was unallocated now is allocated.
There is an inconsistency in understanding of unallocated regions in
Qemu: backing-supporting format-drivers return 0 block-status to report
go-to-backing logic for this area. Some protocol-drivers (iscsi) return
0 to report fs-unallocated-non-zero status (i.e., don't occupy space on
disk, read result is undefined).
BDRV_BLOCK_ALLOCATED is defined as something more close to
go-to-backing logic. Still it is calculated as ZERO | DATA, so 0 from
iscsi is treated as unallocated. It doesn't influence backing-chain
behavior, as iscsi can't have backing file. But it does influence
"qemu-io -c map".
We should solve this inconsistency at some future point. Now, let's
just make backing-not-supporting format drivers (vdi in the previous
patch and vpc now) to behave more like backing-supporting drivers
and not report 0 block-status. More over, returning ZERO status is
absolutely valid thing, and again, corresponds to how the other
format-drivers (backing-supporting) work.
After block-status update, it never reports 0, so setting
unallocated_blocks_are_zero doesn't make sense (as the only user of it
is bdrv_co_block_status and it checks unallocated_blocks_are_zero only
for unallocated areas). Drop it.
Signed-off-by: Vladimir Sementsov-Ogievskiy <email@example.com>
Reviewed-by: Eric Blake <firstname.lastname@example.org>
block/vpc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
This breaks iotest 146, I’m afraid. Now everything is reported as
allocated, which is of course what we kind of want, but I suppose it
renders the test a bit useless.
It seems to me like the best thing to do would be to replace the
“qemu-io -o $opts -c map” calls by
“qemu-img map --output=json --image-opts $opts”, so that we get
“zero: true” instead of “not allocated” in the output.
A bit of a problem with that approach are the “allocated” entries,
because those are split into 2 MB chunks, so the output gets long. But,
I’ve attached a diff to that effect. Would you be OK with squashing
Yes, this looks reasonable, thanks a lot!