[PATCH v6 07/25] parallels: do not let the check die on what it is meant to report

From: Denis V. Lunev <[email protected]>

parallels_check_duplicate() sizes its local bitmap by the end of the
payload and asserts that every BAT entry fits into it. An entry
pointing at a cluster which runs past the end of the image file does
not fit, so a plain 'qemu-img check' on such an image dies:

    qemu-img: block/parallels.c:843: parallels_check_duplicate:
    Assertion `ret != -E2BIG' failed.

A repairing check survives by accident, as
parallels_check_outside_image() clears the entry before the duplicate
check gets to see it. There is nothing for the duplicate check to do
with such a cluster anyway, as the corruption has already been
reported, so skip it.

The answer parallels_mark_used() gives has to stay out of ret, which is
what the function returns once the loop is over. -EBUSY for the
duplicate this function exists to find is left in ret whenever the last
allocated BAT entry is the duplicated one and the check is not
repairing. 'qemu-img check' then ends with

    ERROR duplicate offset in BAT entry 1
    qemu-img: Check failed: Device or resource busy

and prints no summary at all, so the corruption it just found is
reported as a failure to look. Keep the answer in a variable of its
own, as the only errors worth returning from here are the I/O ones,
and those leave the loop where they happen.

The tests write two clusters and damage the second entry, one by
cutting the cluster in half so that it no longer fits the file, one by
pointing it at the first cluster.

Fixes: a398275e88 ("parallels: create mark_used() helper which sets bit in used bitmap")
Cc: Stefan Hajnoczi <[email protected]>
Signed-off-by: Denis V. Lunev <[email protected]>
---
 block/parallels.c                             | 12 ++++---
 tests/qemu-iotests/tests/parallels-checks     | 33 ++++++++++++++++++
 tests/qemu-iotests/tests/parallels-checks.out | 34 +++++++++++++++++++
 3 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index c96bed5ed3..cacf23143b 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -872,14 +872,16 @@ parallels_check_duplicate(BlockDriverState *bs, BdrvCheckResult *res,
     buf = qemu_blockalign(bs, s->cluster_size);
 
     for (i = 0; i < s->bat_size; i++) {
+        int used;
+
         host_off = bat2sect(s, i) << BDRV_SECTOR_BITS;
         if (host_off == 0) {
             continue;
         }
 
-        ret = parallels_mark_used(bs, bitmap, bitmap_size, host_off, 1);
-        assert(ret != -E2BIG);
-        if (ret == 0) {
+        used = parallels_mark_used(bs, bitmap, bitmap_size, host_off, 1);
+        if (used == 0 || used == -E2BIG) {
+            /* parallels_check_outside_image() reports the -E2BIG one */
             continue;
         }
 
@@ -937,8 +939,8 @@ parallels_check_duplicate(BlockDriverState *bs, BdrvCheckResult *res,
          * considered, and the bitmap size doesn't change. This specifically
          * means that -E2BIG is OK.
          */
-        ret = parallels_mark_used(bs, bitmap, bitmap_size, host_off, 1);
-        if (ret == -EBUSY) {
+        used = parallels_mark_used(bs, bitmap, bitmap_size, host_off, 1);
+        if (used == -EBUSY) {
             res->check_errors++;
             goto out_repair_bat;
         }
diff --git a/tests/qemu-iotests/tests/parallels-checks b/tests/qemu-iotests/tests/parallels-checks
index 99af4c5f52..cf90eaf152 100755
--- a/tests/qemu-iotests/tests/parallels-checks
+++ b/tests/qemu-iotests/tests/parallels-checks
@@ -335,6 +335,39 @@ echo "== both of them survive the close =="
               -c "read -P 0x22 $CLUSTER_SIZE $CLUSTER_SIZE" \
               "$TEST_IMG"; } 2>&1 | _filter_qemu_io | _filter_testdir
 
+# Clear image
+_make_test_img $SIZE
+
+echo "== TEST A CLUSTER WHICH RUNS PAST THE END OF THE FILE =="
+
+echo "== write two clusters =="
+{ $QEMU_IO -c "write -P 0x11 0 $CLUSTER_SIZE" \
+           -c "write -P 0x22 $CLUSTER_SIZE $CLUSTER_SIZE" \
+           "$TEST_IMG"; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo "== cut the second one in half =="
+file_size=`stat --printf="%s" "$TEST_IMG"`
+truncate -s $((file_size - CLUSTER_SIZE / 2)) "$TEST_IMG"
+
+echo "== the check completes and reports the cluster =="
+_check_test_img
+
+# Clear image
+_make_test_img $SIZE
+
+echo "== TEST A DUPLICATE IN THE LAST ALLOCATED BAT ENTRY =="
+
+echo "== write two clusters =="
+{ $QEMU_IO -c "write -P 0x11 0 $CLUSTER_SIZE" \
+           -c "write -P 0x22 $CLUSTER_SIZE $CLUSTER_SIZE" \
+           "$TEST_IMG"; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo "== point the second entry at the first cluster =="
+poke_file "$TEST_IMG" "$(($BAT_OFFSET + 4))" "\x01\x00\x00\x00"
+
+echo "== the check completes and reports the duplicate =="
+_check_test_img
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/tests/parallels-checks.out b/tests/qemu-iotests/tests/parallels-checks.out
index 51eb3f1ef1..645c4b3679 100644
--- a/tests/qemu-iotests/tests/parallels-checks.out
+++ b/tests/qemu-iotests/tests/parallels-checks.out
@@ -199,4 +199,38 @@ read 1048576/1048576 bytes at offset 0
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 1048576/1048576 bytes at offset 1048576
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4194304
+== TEST A CLUSTER WHICH RUNS PAST THE END OF THE FILE ==
+== write two clusters ==
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1048576/1048576 bytes at offset 1048576
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+== cut the second one in half ==
+== the check completes and reports the cluster ==
+ERROR cluster 1 is outside image
+ERROR space leaked at the end of the image 524288
+
+1 errors were found on the image.
+Data may be corrupted, or further writes to the image may corrupt it.
+
+1 leaked clusters were found on the image.
+This means waste of disk space, but no harm to data.
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4194304
+== TEST A DUPLICATE IN THE LAST ALLOCATED BAT ENTRY ==
+== write two clusters ==
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1048576/1048576 bytes at offset 1048576
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+== point the second entry at the first cluster ==
+== the check completes and reports the duplicate ==
+ERROR space leaked at the end of the image 1048576
+ERROR duplicate offset in BAT entry 1
+
+1 errors were found on the image.
+Data may be corrupted, or further writes to the image may corrupt it.
+
+1 leaked clusters were found on the image.
+This means waste of disk space, but no harm to data.
 *** done
-- 
2.53.0