Hi, As Eric reports in https://bugzilla.redhat.com/show_bug.cgi?id=1727347, qemu-img amend has a bug when it comes to converting qcow2 v2 images to v3: In v3, every snapshot table entry requires at least 16 bytes of extra metadata to be present, which isn’t the case for v2 images. Currently, qemu-img amend doesn’t take care of updating the snapshot table, so the image is a bit corrupt afterwards (luckily, qemu doesn’t take notice, though).
This yields the following patches: - Patch 3: Helper patch - Patch 4: Helper patch, so we can actually do more than just to bump up the version number when upgrading a qcow2 image from v2 to v3 - Patch 5: The fix Eric also points out that qemu-img check does not see any problem with such images and doesn’t fix them, so: - Patch 11: Makes qemu-img check report if a snapshot table entry has too little extra data, and repair it with -r all (Patches 6 and 7 add the necessary infrastructure so we can check and repair the snapshot table at all.) Then I got the glorious idea of “Hey, if I want to see how much extra data a snapshot table entry has outside of qcow2_read_snapshots(), I should add a field that reports that value to QCowSnapshot. And if I do that, I might as well make the qcow2 driver interpret the specification a bit more literally, namely it should ignore all unknown extra data, that is (as I interpret it), keep it in memory and write it back when updating the snapshot table.” That led to patch 2. Maybe you find that stupid, in which case we can totally drop patch 2 (with some changes to other patches). Anyway. Because of this, qcow2_read_snapshots() suddenly got more error case, so I thought now would be a good time to give it an Error ** parameter. Cue patch 1. At this point: (1) I had infrastructure for repairing a snapshot table in qemu-img check -r all, and (2) I had added a new error case if a snapshot table entry has a suspiciously large amount of extra data. I decided that this should be repairable, too. This is done by patch 8. (If we drop patch 2, this will go, too.) Now I was really into it, so I decided even more things needed fixing! Namely the final two reasons why we would reject a snapshot table: (1) It has too many snapshots (patch 10), (2) It is too long overall (patch 9). Finally, patch 13 adds an overly complicated test (using the new peek_file* functions added in patch 12). Max Reitz (13): qcow2: Add Error ** to qcow2_read_snapshots() qcow2: Keep unknown extra snapshot data qcow2: Make qcow2_write_snapshots() public qcow2: Put qcow2_upgrade() into an own function qcow2: Write v3-compliant snapshot list on upgrade qcow2: Separate qcow2_check_read_snapshot_table() qcow2: Add qcow2_check_fix_snapshot_table() qcow2: Fix broken snapshot table entries qcow2: Fix overly long snapshot tables qcow2: Repair snapshot table with too many entries qcow2: Fix v3 snapshot table entry compliancy iotests: Add peek_file* functions iotests: Test qcow2's snapshot table handling block/qcow2.h | 15 +- block/qcow2-snapshot.c | 302 +++++++++++++++++++++-- block/qcow2.c | 152 ++++++++++-- tests/qemu-iotests/261 | 449 +++++++++++++++++++++++++++++++++++ tests/qemu-iotests/261.out | 321 +++++++++++++++++++++++++ tests/qemu-iotests/common.rc | 20 ++ tests/qemu-iotests/group | 1 + 7 files changed, 1219 insertions(+), 41 deletions(-) create mode 100755 tests/qemu-iotests/261 create mode 100644 tests/qemu-iotests/261.out -- 2.21.0