qemu-img: Fail fast on convert --bitmaps with inconsistent bitmap

Waiting until the end of the convert operation (a potentially
time-consuming task) to finally detect that we can't copy a bitmap is
bad, comparing to failing fast up front.  Furthermore, this prevents
us from leaving a file behind with a bitmap that is not marked as
inconsistent even though it does not have sane contents.

This fixes the problems exposed in the previous patch to the iotest:
it adds a fast failure up front, and even if we don't fail early, it
ensures that any bitmap we add but do not properly populate is removed
again rather than left behind incomplete.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210709153951.2801666-3-eblake@redhat.com>
[eblake: add a hint to the warning message, simplify name computation]
Reviewed-by: Nir Soffer <nsoffer@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

diff --git a/qemu-img.c b/qemu-img.c
index 797742a44331622d5f9b18d0a878ef587ffa26e5..c5496e82e025718e4829cd9a6cd693b6c5b5c932 100644 (file)
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2101,6 +2101,29 @@ static int convert_do_copy(ImgConvertState *s)
     return s->ret;
 }
 
+/* Check that bitmaps can be copied, or output an error */
+static int convert_check_bitmaps(BlockDriverState *src)
+{
+    BdrvDirtyBitmap *bm;
+
+    if (!bdrv_supports_persistent_dirty_bitmap(src)) {
+        error_report("Source lacks bitmap support");
+        return -1;
+    }
+    FOR_EACH_DIRTY_BITMAP(src, bm) {
+        if (!bdrv_dirty_bitmap_get_persistence(bm)) {
+            continue;
+        }
+        if (bdrv_dirty_bitmap_inconsistent(bm)) {
+            error_report("Cannot copy inconsistent bitmap '%s'",
+                         bdrv_dirty_bitmap_name(bm));
+            error_printf("Try 'qemu-img bitmap --remove' to delete it\n");
+            return -1;
+        }
+    }
+    return 0;
+}
+
 static int convert_copy_bitmaps(BlockDriverState *src, BlockDriverState *dst)
 {
     BdrvDirtyBitmap *bm;
@@ -2127,6 +2150,7 @@ static int convert_copy_bitmaps(BlockDriverState *src, BlockDriverState *dst)
                               &err);
         if (err) {
             error_reportf_err(err, "Failed to populate bitmap %s: ", name);
+            qmp_block_dirty_bitmap_remove(dst->node_name, name, NULL);
             return -1;
         }
     }
@@ -2554,9 +2578,8 @@ static int img_convert(int argc, char **argv)
             ret = -1;
             goto out;
         }
-        if (!bdrv_supports_persistent_dirty_bitmap(blk_bs(s.src[0]))) {
-            error_report("Source lacks bitmap support");
-            ret = -1;
+        ret = convert_check_bitmaps(blk_bs(s.src[0]));
+        if (ret < 0) {
             goto out;
         }
     }

diff --git a/tests/qemu-iotests/tests/qemu-img-bitmaps b/tests/qemu-iotests/tests/qemu-img-bitmaps
index 409c4497a3037604320fbdf027af6ca090e342b0..09c3d395d1eb20453962aa8fbe194f19686b1369 100755 (executable)
--- a/tests/qemu-iotests/tests/qemu-img-bitmaps
+++ b/tests/qemu-iotests/tests/qemu-img-bitmaps
@@ -140,11 +140,10 @@ $QEMU_IO -c abort "$TEST_IMG" 2>/dev/null
 $QEMU_IMG bitmap --add "$TEST_IMG" b4
 $QEMU_IMG bitmap --remove "$TEST_IMG" b1
 _img_info --format-specific | _filter_irrelevant_img_info
+# Proof that we fail fast if bitmaps can't be copied
 echo
 $QEMU_IMG convert --bitmaps -O qcow2 "$TEST_IMG" "$TEST_IMG.copy" &&
     echo "unexpected success"
-# Bug - even though we failed at conversion, we left a file around with
-# a bitmap marked as not corrupt
 TEST_IMG=$TEST_IMG.copy _img_info --format-specific \
     | _filter_irrelevant_img_info
 

diff --git a/tests/qemu-iotests/tests/qemu-img-bitmaps.out b/tests/qemu-iotests/tests/qemu-img-bitmaps.out
index 6824d41128935ac712010572ed1f35201081aae1..d99c279d0c63e566d926fdec09c7f04f85b67be3 100644 (file)
--- a/tests/qemu-iotests/tests/qemu-img-bitmaps.out
+++ b/tests/qemu-iotests/tests/qemu-img-bitmaps.out
@@ -144,22 +144,7 @@ Format specific information:
             granularity: 65536
     corrupt: false
 
-qemu-img: Failed to populate bitmap b0: Bitmap 'b0' is inconsistent and cannot be used
-Try block-dirty-bitmap-remove to delete this bitmap from disk
-image: TEST_DIR/t.IMGFMT.copy
-file format: IMGFMT
-virtual size: 10 MiB (10485760 bytes)
-cluster_size: 65536
-Format specific information:
-    bitmaps:
-        [0]:
-            flags:
-            name: b0
-            granularity: 65536
-        [1]:
-            flags:
-                [0]: auto
-            name: b4
-            granularity: 65536
-    corrupt: false
+qemu-img: Cannot copy inconsistent bitmap 'b0'
+Try 'qemu-img bitmap --remove' to delete it
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT.copy': Could not open 'TEST_DIR/t.IMGFMT.copy': No such file or directory
 *** done