[Qemu-devel] [PATCH for-4.1? 0/4] block: Fix three .bdrv_has_zero_init()s

[Max Reitz]

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.)


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(-)

-- 
2.21.0