qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH for-4.1? 0/4] block: Fix three .bdrv_has_zero_in


From: Stefano Garzarella
Subject: Re: [Qemu-block] [PATCH for-4.1? 0/4] block: Fix three .bdrv_has_zero_init()s
Date: Mon, 15 Jul 2019 16:49:00 +0200
User-agent: NeoMutt/20180716

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 <address@hidden>
Tested-by: Stefano Garzarella <address@hidden>

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]