[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH for-2.11] qcow2: fix image corruption on commit
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [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
signature.asc
Description: OpenPGP digital signature