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

[Eric Blake]

On 11/17/2017 12:17 PM, Max Reitz wrote:
> 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.

Oh my.  Thanks for the rapid testing.

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

Isn't sha256 supposed to be a byte-based hash that is not endian
specific?  What causes that difference?

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

At any rate, I concur with this conclusion; I'll post a followup that
filters out the hash (for this test, we only care that the existence of
a hash proves the bitmap exists; unlike 165 where we want to validate
that it is actually tracking correct information).

I missed Kevin's -rc2 pull, unless he wants to send a v2; but we also
have time (it's not the end of the world if the fix goes in -rc3).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature