Re: [Qemu-devel] [PATCH for-2.11] qcow2: fix image corruption on commit with persistent snapshot

[Max Reitz]

On 2017-11-17 17:47, Eric Blake wrote:
> If an image contains persistent snapshots, we cannot use the
> fast path of bdrv_make_empty() to clear the image during
> qemu-img commit, because that will lose the clusters related
> to the bitmaps.
> 
> Also leave a comment in qcow2_read_extensions to remind future
> feature additions to think about fast-path removal, since we
> just barely fixed the same bug for LUKS encryption.
> 
> It's a pain that qemu-img has not yet been taught to manipulate,
> or even at a very minimum display, information about persistent
> bitmaps; instead, we have to use QMP commands.  It's also a
> pain that only qeury-block and x-debug-block-dirty-bitmap-sha256
> will allow bitmap introspection; but the former requires the
> node to be hooked to a block device, and the latter is experimental.
> 
> Signed-off-by: Eric Blake <address@hidden>
> ---
> 
> As promised, based on Dan's similar patch for LUKS encryption
> 
>  block/qcow2.c              |  17 ++--
>  tests/qemu-iotests/176     |  55 ++++++++++--
>  tests/qemu-iotests/176.out | 216 
> ++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 270 insertions(+), 18 deletions(-)

The test fails with -m32 and probably also on Big Endian architectures,
because the bitmap hash differs.

The reason the hash differs for -m32 is because of different bitmap
sizes: The default bitmap granularity is 64 kB, so for 32 bit this means
the bitmap always covers a multiple of 2 MB, whereas for 64 bit, the
bitmap always covers a multiple of 4 MB.
> diff --git a/tests/qemu-iotests/176 b/tests/qemu-iotests/176
> index 950b28720e..0f31a20294 100755
> --- a/tests/qemu-iotests/176
> +++ b/tests/qemu-iotests/176

[...]

> @@ -66,14 +74,29 @@ _supported_os Linux
>  for i in 0 1 2 3; do
> 
>  echo
> -echo "=== Test pass $i ==="
> +echo "=== Test pass $reason.$i ==="
>  echo
> 
>  len=$((2100 * 1024 * 1024 + 512)) # larger than 2G, and not cluster aligned

That means that with -m64, the bitmap will behave as if a 2104 MB image
has been created, but with -m32, it will behave as if it is a 2102 MB
image.  Therefore, you get different hashes (because the 64 bit bitmap
has 32 more zero bits than the 32 bit bitmap).

We could "fix" this by replacing the 2100 by a 2102, so for both bit
widths rounding is the same.  But that would still leave the issue open
for Big Endian architectures (which generate just completely different
hashes) -- and I'm not really a fan of testing this on every possible
architecture and then adding different reference outputs.

Therefore, the best fix is probably to just filter the hashes out (you
don't need the exact value anyway, do you?), and I think it's fine to do
this as a follow-up.

Max

signature.asc
Description: OpenPGP digital signature