[Qemu-block] [PATCH v2 00/16] qcow2: Let check -r all repair some snapshot bits

[Max Reitz]

Hi,

See the v1 cover letter here if you haven’t already:
https://lists.nongnu.org/archive/html/qemu-block/2019-07/msg01290.html

I kept all patches from the previous version because Eric deemed the
patches I might have dropped useful.

I kept the way I implemented fixing overly long snapshot tables, namely
by discarding all snapshots past the maximum end.  Eric criticized this,
because this is indiscriminate (and the last ones have been created the
most recently, but I’m not sure whether we’d generally rather keep the
oldest or the most recent ones.  Maybe best would be to drop every
second snapshot? :-)).
I kept it as it is, because everything else would require user
interaction.  We currently have no interactive check mode, and I feel
like this is not a place where we need it badly: We introduced our
current snapshot limits in such a way that no sane image would ever hit
them.  If you do hit them, changes are that your image is already
corrupted, so you probably no longer care about your snapshots anyway
and just want to copy the active layer off.

Furthermore, I felt like an interactive mode would be pretty hard to
test thoroughly.  I don’t want to introduce code that probably nobody
will ever use and that is hardly tested.

I also tried Eric’s suggestion of using bdrv_preadv() in
qcow2_read_snapshots() to read all variable data at once, and while it
was very promising, it doesn’t allow us to skip data (which we must do
when we want to truncate extra data down to a sane size).  So all in all
I couldn’t make it work.


So, the changes in v2:
- Patch 1: Eric asked from some magic numbers to be removed (very
           reasonable), and endof() will help with that.
- Patch 2: See, it helps!
- Patch 4: Rebased conflicts (because of patch 2), and I should use
           BDRV_REQUEST_MAX_BYTES instead of INT_MAX when I mean the
           former
- Patch 7: Drop magic number [Eric]
- Patch 8: Drop magic number [Eric]
- Patch 10: Instead of dropping all unknown extra data when there is too
            much, just truncate it down to the maximum [Eric]
- Patch 11: When some snapshot table entry has too much extra data and
            we truncate it, that shortens the snapshot table length, so
            we should keep that in mind when calculating whether the
            table is too long
- Patch 12: Rebase conflicts, and dropped a magic number [Eric]
- Patch 14: Drop magic number [Eric]
- Patch 16: Simplified padding calculation [Eric], fixed one test case
            (I used “sn-size” instead of “sn_size”, oops), and added a
            test case for what happens when a snapshot table entry is
            too big and contains an entry with too much extra data (the
            latter should be fixed first and then we should look into
            whether the table is still too long)


git-backport-diff against v1:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/16:[down] 'include: Move endof() up from hw/virtio/virtio.h'
002/16:[down] 'qcow2: Use endof()'
003/16:[----] [--] 'qcow2: Add Error ** to qcow2_read_snapshots()'
004/16:[0012] [FC] 'qcow2: Keep unknown extra snapshot data'
005/16:[----] [--] 'qcow2: Make qcow2_write_snapshots() public'
006/16:[----] [--] 'qcow2: Put qcow2_upgrade() into an own function'
007/16:[0005] [FC] 'qcow2: Write v3-compliant snapshot list on upgrade'
008/16:[0004] [FC] 'qcow2: Separate qcow2_check_read_snapshot_table()'
009/16:[----] [--] 'qcow2: Add qcow2_check_fix_snapshot_table()'
010/16:[0038] [FC] 'qcow2: Fix broken snapshot table entries'
011/16:[down] 'qcow2: Keep track of the snapshot table length'
012/16:[0003] [FC] 'qcow2: Fix overly long snapshot tables'
013/16:[----] [--] 'qcow2: Repair snapshot table with too many entries'
014/16:[0005] [FC] 'qcow2: Fix v3 snapshot table entry compliancy'
015/16:[----] [--] 'iotests: Add peek_file* functions'
016/16:[0125] [FC] 'iotests: Test qcow2's snapshot table handling'


Max Reitz (16):
  include: Move endof() up from hw/virtio/virtio.h
  qcow2: Use endof()
  qcow2: Add Error ** to qcow2_read_snapshots()
  qcow2: Keep unknown extra snapshot data
  qcow2: Make qcow2_write_snapshots() public
  qcow2: Put qcow2_upgrade() into its 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: Keep track of the snapshot table length
  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 +-
 include/hw/virtio/virtio.h   |   7 -
 include/qemu/compiler.h      |   7 +
 block/qcow2-snapshot.c       | 323 +++++++++++++++++++--
 block/qcow2.c                | 155 +++++++++--
 hw/block/virtio-blk.c        |   4 +-
 hw/net/virtio-net.c          |  10 +-
 tests/qemu-iotests/261       | 523 +++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/261.out   | 346 +++++++++++++++++++++++
 tests/qemu-iotests/common.rc |  20 ++
 tests/qemu-iotests/group     |   1 +
 11 files changed, 1354 insertions(+), 57 deletions(-)
 create mode 100755 tests/qemu-iotests/261
 create mode 100644 tests/qemu-iotests/261.out

-- 
2.21.0