On Mon, Jul 15, 2019 at 12:45:04PM +0200, Max Reitz wrote: > Hi, > > .bdrv_has_zero_init() must not return 1 if the (newly created[1]) image > may not return zeroes when read. > > [1] This is guaranteed by the caller. > > If the image is preallocated, this generally depends on the protocol > layer, because the format layer will just allocate the necessary > metadata, make it point to data blocks and leave their initialization to > the protocol layer. For example, qcow2: > > - leaves the blocks uninitialized with preallocation=metadata, > - and passes preallocation=falloc and =full on to the protocol node. > > In either case, the data then stored in these blocks fully depends on > the protocol level. > > Therefore, format drivers have to pass through .bdrv_has_zero_init() to > the data storage node when dealing with preallocated images. > > Protocol drivers OTOH have to be accurate in what they return from > .bdrv_has_zero_init(). They are free to return 0 even for preallocated > images. > > > So let’s look at the existing .bdrv_has_zero_init() implementations: > > - file-posix: Always returns 1 (for regular files). Correct, because it > makes sure the image always reads as 0, preallocated or not. > > - file-win32: Same. (But doesn’t even support preallocation.) > > - gluster: Always returns 0. Safe. > > - nfs: Only returns 1 for regular files, similarly to file-posix. Seems > reasonable. > > - parallels: Always returns 1. This format does not support > preallocation, but apparently ensures that it always writes out data > that reads back as 0 (where necessary), because if the protocol node > does not have_zero_init, it explicitly writes zeroes to it instead of > just truncating it. > So this driver seems OK, too. > > - qcow2: Always returns 1. This is wrong for preallocated images, and > really wrong for preallocated encrypted images. Addressed by patch 1. > > - qcow: Always returns 1. Has no preallocation support, so that seems > OK. > > - qed: Same as qcow. > > - raw: Always forwards the value from the filtered node. OK. > > - rbd: Always returns 1. This is a protocol driver, so I’ll just trust > it knows what it’s doing. > > - sheepdog: Always returns 1. From the fact that its preallocation code > simply reads the image and writes it back, this seems correct to me. > > - ssh: Same as nfs. > > - vdi: Always returns 1. It does support preallocation=metadata, in > which case this may be wrong. Addressed by patch 2. > > - vhdx: Similar to vdi, except it doesn’t support @preallocation, but > has its own option “subformat=fixed”. Addressed by patch 3. > > - vmdk: Hey, this one is already exactly what we want. If any of the > extents is flat, it goes to the respective protocol node, and if that > does not have_zero_init, it returns 0. Great. > (Added in commit da7a50f9385.) > > - vpc: Hey, this one, too. With subformat=fixed, it returns what the > protocol node has to say about has_zero_init. > (Added in commit 72c6cc94daa.) > > So that leaves three cases to fix, which are the first three patches in > this series. The final patch adds a test case for qcow2. (It’s > difficult to test the other drivers, because that would require a > protocol driver with image creation support and has_zero_init=0, which > is not so easily available.)
Acked-by: Stefano Garzarella <sgarz...@redhat.com> Tested-by: Stefano Garzarella <sgarz...@redhat.com> Thanks, Stefano > > > Max Reitz (4): > qcow2: Fix .bdrv_has_zero_init() > vdi: Fix .bdrv_has_zero_init() > vhdx: Fix .bdrv_has_zero_init() > iotests: Convert to preallocated encrypted qcow2 > > block/qcow2.c | 90 +++++++++++++++++++++++++++++++++++++- > block/vdi.c | 13 +++++- > block/vhdx.c | 21 ++++++++- > tests/qemu-iotests/188 | 20 ++++++++- > tests/qemu-iotests/188.out | 4 ++ > 5 files changed, 144 insertions(+), 4 deletions(-) >
