On 16.07.19 18:54, Kevin Wolf wrote: > Am 15.07.2019 um 12:45 hat Max Reitz geschrieben: >> If a qcow2 file is preallocated, it can no longer guarantee that it >> initially appears as filled with zeroes. >> >> So implement .bdrv_has_zero_init() by checking whether the file is >> preallocated; if so, forward the call to the underlying storage node, >> except for when it is encrypted: Encrypted preallocated images always >> return effectively random data, so .bdrv_has_zero_init() must always >> return 0 for them. >> >> Reported-by: Stefano Garzarella <sgarz...@redhat.com> >> Signed-off-by: Max Reitz <mre...@redhat.com> > > Hm... This patch only really works directly after image creation (which > is indeed where .bdrv_has_zero_init is used). Why do we have to have a > full qcow2_is_zero() that loops over the whole image just to find out > whether it's preallocated? Wouldn't looking at a single data cluster be > enough?
Hm. I would like to agree (because you’re right), but now I see that the callers of bdrv_has_zero_init() don’t necessarily hold to that convention. For example, qemu-img convert has the -n flag, but that doesn’t stop it from invoking bdrv_has_zero_init(). Which is a bug, of course. $ ./qemu-img create -f qcow2 src.qcow2 64M $ ./qemu-img create -f qcow2 dest.qcow2 64M $ ./qemu-io -c 'write -P 42 0 64M' dest.qcow2 $ ./qemu-img convert -n src.qcow2 dest.qcow2 $ ./qemu-img compare src.qcow2 dest.qcow2 Content mismatch at offset 0! Aw, man, why does this keep happening... :-/ OK, so qemu-img convert -n is easy to fix. But there are more callers: mirror: Uses this function to inquire whether it needs to zero the target before actually doing something useful. There is no guarantee that the target is a new image. Well, it just isn’t with mode=existing or blockdev-mirror. parallels: Whether to write zeroes to newly added image areas. That actually sounds correct, because those new areas cannot point to any data yet. Well, maybe not correct, because bdrv_has_zero_init() is not the same as “when this image grows, new areas will be zero”, but at least bdrv_hsa_zero_init() will return false if the the latter is false. vhdx: Similarly to parallels, it uses this information to check whether it needs to zero new areas when growing an image file. raw/vmdk/vpc: Just passing through info from their storage child. Hm, OK. So mirror and qemu-img need fixing. That sounds possible. Max
signature.asc
Description: OpenPGP digital signature
