[PATCH 2/2] qcow2: repair a dirty image when it becomes writable

A dirty image must be repaired before anything allocates a cluster in
it. qcow2_do_open() does that, but only for a node that is writable
from the start. A node opened read-only skips it, and nothing revisits
the question once that node becomes writable, which block-commit does
routinely: commit_active_start() and commit_start() reopen the base
read-write for the duration of the job.

With lazy refcounts the on-disk refcount block then still accounts for
the metadata clusters only, so the allocator restarts at the front of
the image and hands out clusters that L2 entries point at. Two guest
offsets end up sharing one host cluster. Nothing fails, the corrupt bit
stays clear, and a clean close clears the dirty bit, so no later open
repairs the image either.

Do the repair in qcow2_reopen_commit_post(). bdrv_reopen_prepare() runs
before bdrv_list_refresh_perms(), so it holds no BLK_PERM_WRITE and,
with auto-read-only, bs->file may still have an O_RDONLY descriptor.
commit_post cannot reject the reopen, so signal corruption if the
repair fails rather than let writes alias live clusters. An inactive
node is skipped: bdrv_activate() calls qcow2_do_open() again through
qcow2_co_invalidate_cache().

Signed-off-by: Denis V. Lunev <[email protected]>
CC: Kevin Wolf <[email protected]>
CC: Hanna Reitz <[email protected]>
---
 block/qcow2.c              | 14 ++++++++++++++
 tests/qemu-iotests/039     | 24 ++++++++++++++++++++++++
 tests/qemu-iotests/039.out | 17 +++++++++++++++++
 3 files changed, 55 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 1543255eba..e660655a0d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2147,8 +2147,22 @@ static void qcow2_reopen_commit(BDRVReopenState *state)
 
 static void qcow2_reopen_commit_post(BDRVReopenState *state)
 {
+    BDRVQcow2State *s = state->bs->opaque;
+
     GRAPH_RDLOCK_GUARD_MAINLOOP();
 
+    if (bdrv_is_writable(state->bs) &&
+        (s->incompatible_features & QCOW2_INCOMPAT_DIRTY)) {
+        BdrvCheckResult result = {0};
+        int ret;
+
+        ret = bdrv_check(state->bs, &result, BDRV_FIX_ERRORS | BDRV_FIX_LEAKS);
+        if (ret < 0 || result.check_errors) {
+            qcow2_signal_corruption(state->bs, true, -1, -1,
+                                    "Could not repair dirty image");
+        }
+    }
+
     if (state->flags & BDRV_O_RDWR) {
         Error *local_err = NULL;
 
diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index a5be81bc4a..255dc2e7e5 100755
--- a/tests/qemu-iotests/039
+++ b/tests/qemu-iotests/039
@@ -137,6 +137,30 @@ $QEMU_IO -c "write 0 512" "$TEST_IMG" | _filter_qemu_io
 # The dirty bit must not be set
 _qcow2_dump_header | grep incompatible_features
 
+echo
+echo "== Reopening a dirty image read/write should repair it =="
+
+_make_test_img -o "compat=1.1,lazy_refcounts=on" $size
+
+_NO_VALGRIND \
+$QEMU_IO -c "write -P 0x5a 0 512" \
+         -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \
+    | _filter_qemu_io
+
+# The dirty bit must be set
+_qcow2_dump_header | grep incompatible_features
+
+# The refcounts are only resolved when the node becomes writable. Without
+# that, this write is allocated on top of the cluster at offset 0 and reading
+# it back returns the data written here.
+$QEMU_IO -r -c "reopen -w" \
+            -c "write -P 0xb1 1M 512" \
+            -c "read -P 0x5a 0 512" "$TEST_IMG" | _filter_qemu_io
+
+# The dirty bit must not be set
+_qcow2_dump_header | grep incompatible_features
+_check_test_img
+
 echo
 echo "== Creating an image file with lazy_refcounts=off =="
 
diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
index 3c71e5a3dd..62073916a9 100644
--- a/tests/qemu-iotests/039.out
+++ b/tests/qemu-iotests/039.out
@@ -64,6 +64,23 @@ wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 incompatible_features     []
 
+== Reopening a dirty image read/write should repair it ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+wrote 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+./common.rc: Killed ( VALGRIND_QEMU="${VALGRIND_QEMU_IO}" _qemu_proc_exec "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
+incompatible_features     [0]
+ERROR cluster 5 refcount=0 reference=1
+Rebuilding refcount structure
+Repairing cluster 1 refcount=1 reference=0
+Repairing cluster 2 refcount=1 reference=0
+wrote 512/512 bytes at offset 1048576
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+incompatible_features     []
+No errors were found on the image.
+
 == Creating an image file with lazy_refcounts=off ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
-- 
2.53.0