[Qemu-devel] [PATCH for-4.2 00/13] qcow2: Let check -r all repair some snapshot bits

[Max Reitz]

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