[Top][All Lists]

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

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

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH for-2.11] qcow2: fix image corruption on commit with persistent snapshot
Date: Fri, 17 Nov 2017 12:46:09 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

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

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

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