Marking the bitmaps that are found in_use in the image as inconsistent
works in the most cases. However, in some cases, like migration,
it's critical for bdrv_dirty_bitmap_check() to always pass.

Instead of asking the user to manually request the deletion of
inconsistent bitmaps, delete them automatically if we can,
while respecting the possible edge cases like repairing an image with
overlaps.

Originally-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
---
Fixed a small typo.

 block/qcow2-bitmap.c                          | 111 ++++++++++++++++--
 block/qcow2.c                                 |   9 ++
 block/qcow2.h                                 |   2 +
 tests/qemu-iotests/tests/migrate-bitmaps-test |   2 +
 tests/qemu-iotests/tests/qemu-img-bitmaps     |   2 +-
 tests/qemu-iotests/tests/qemu-img-bitmaps.out |  36 +++---
 .../qemu-iotests/tests/qemu-img-close-errors  |   3 +-
 7 files changed, 132 insertions(+), 33 deletions(-)

diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 256ec99878..ccc6f731b4 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -30,6 +30,8 @@
 #include "block/dirty-bitmap.h"
 #include "qapi/error.h"
 #include "qemu/cutils.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
 
 #include "qcow2.h"
 
@@ -958,6 +960,48 @@ static void set_readonly_helper(gpointer bitmap, gpointer value)
     bdrv_dirty_bitmap_set_readonly(bitmap, (bool)value);
 }
 
+int qcow2_remove_in_use_bitmaps(BlockDriverState *bs)
+{
+    BDRVQcow2State *s = bs->opaque;
+    Qcow2BitmapList *bm_list;
+    Qcow2Bitmap *bm, *next;
+    bool removed_any = false;
+
+    if (s->nb_bitmaps == 0) {
+        return 0;
+    }
+
+    bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
+                               s->bitmap_directory_size, NULL);
+    if (bm_list == NULL) {
+        return 0;
+    }
+
+    QSIMPLEQ_FOREACH_SAFE(bm, bm_list, entry, next) {
+        if (bm->flags & BME_FLAG_IN_USE) {
+            qemu_log("Removing inconsistent bitmap '%s' at image '%s'\n",
+                     bm->name, bs->filename);
+
+            QSIMPLEQ_REMOVE(bm_list, bm, Qcow2Bitmap, entry);
+            free_bitmap_clusters(bs, &bm->table);
+            bitmap_free(bm);
+            removed_any = true;
+        }
+    }
+
+    int ret = 0;
+    if (removed_any) {
+        ret = update_ext_header_and_dir(bs, bm_list);
+        if (ret < 0) {
+            error_report("Can't update bitmap directory after removing "
+                         "inconsistent bitmaps");
+        }
+    }
+
+    bitmap_list_free(bm_list);
+    return ret;
+}
+
 /*
  * Return true on success, false on failure.
  * If header_updated is not NULL then it is set appropriately regardless of
@@ -969,9 +1013,9 @@ qcow2_load_dirty_bitmaps(BlockDriverState *bs,
 {
     BDRVQcow2State *s = bs->opaque;
     Qcow2BitmapList *bm_list;
-    Qcow2Bitmap *bm;
+    Qcow2Bitmap *bm, *next;
     GSList *created_dirty_bitmaps = NULL;
-    bool needs_update = false;
+    bool needs_update = false, removed_persistent_bitmaps = false;
 
     if (header_updated) {
         *header_updated = false;
@@ -988,7 +1032,7 @@ qcow2_load_dirty_bitmaps(BlockDriverState *bs,
         return false;
     }
 
-    QSIMPLEQ_FOREACH(bm, bm_list, entry) {
+    QSIMPLEQ_FOREACH_SAFE(bm, bm_list, entry, next) {
         BdrvDirtyBitmap *bitmap;
 
         if ((bm->flags & BME_FLAG_IN_USE) &&
@@ -1008,6 +1052,32 @@ qcow2_load_dirty_bitmaps(BlockDriverState *bs,
             continue;
         }
 
+        if ((bm->flags & BME_FLAG_IN_USE) && can_write(bs) &&
+            !(s->flags & BDRV_O_CHECK))
+        {
+            /*
+             * Remove inconsistent bitmaps.
+             * This is to avoid errors on migrations, when
+             * when bdrv_dirty_bitmap_check() could be called on a bitmap
+             * marked as inconsistent, causing an error and requiring the user
+             * to manually request inconsistent bitmap deletion.
+             *
+             * In case we have a corrupted image, there's no guarantee that
+             * update_ext_header_and_dir() will succeed.
+             * This would render some images impossible to repair.
+             * Therefore, skip it on the image open if we're in the check mode
+             * (and do it later when the image is successfully repaired).
+             */
+            qemu_log("Removing inconsistent bitmap '%s' at image '%s'\n",
+                     bm->name, bs->filename);
+
+            QSIMPLEQ_REMOVE(bm_list, bm, Qcow2Bitmap, entry);
+            free_bitmap_clusters(bs, &bm->table);
+            bitmap_free(bm);
+            removed_persistent_bitmaps = true;
+            continue;
+        }
+
         bitmap = load_bitmap(bs, bm, errp);
         if (bitmap == NULL) {
             goto fail;
@@ -1015,6 +1085,7 @@ qcow2_load_dirty_bitmaps(BlockDriverState *bs,
 
         bdrv_dirty_bitmap_set_persistence(bitmap, true);
         if (bm->flags & BME_FLAG_IN_USE) {
+            /* Reached in check mode or readonly mode */
             bdrv_dirty_bitmap_set_inconsistent(bitmap);
         } else {
             /* NB: updated flags only get written if can_write(bs) is true. */
@@ -1028,15 +1099,31 @@ qcow2_load_dirty_bitmaps(BlockDriverState *bs,
             g_slist_append(created_dirty_bitmaps, bitmap);
     }
 
-    if (needs_update && can_write(bs)) {
-        /* in_use flags must be updated */
-        int ret = update_ext_header_and_dir_in_place(bs, bm_list);
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "Can't update bitmap directory");
-            goto fail;
-        }
-        if (header_updated) {
-            *header_updated = true;
+    if (can_write(bs)) {
+        if (removed_persistent_bitmaps) {
+            /*
+             * Bitmaps must be removed
+             * (possibly along with updating in_use flags)
+             */
+            int ret = update_ext_header_and_dir(bs, bm_list);
+            if (ret < 0) {
+                error_setg_errno(errp, -ret, "Can't update bitmap directory "
+                                 "after removing inconsistent bitmaps");
+                goto fail;
+            }
+            if (header_updated) {
+                *header_updated = true;
+            }
+        } else if (needs_update) {
+            /* Only in_use flags must be updated */
+            int ret = update_ext_header_and_dir_in_place(bs, bm_list);
+            if (ret < 0) {
+                error_setg_errno(errp, -ret, "Can't update bitmap directory");
+                goto fail;
+            }
+            if (header_updated) {
+                *header_updated = true;
+            }
         }
     }
 
diff --git a/block/qcow2.c b/block/qcow2.c
index e29810d86a..eb6da74d34 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -634,6 +634,15 @@ qcow2_co_check_locked(BlockDriverState *bs, BdrvCheckResult *result,
     }
 
     if (fix && result->check_errors == 0 && result->corruptions == 0) {
+        /*
+         * Run the removal of inconsistent bitmaps that we've skipped on
+         * qcow2_load_dirty_bitmaps().
+         */
+        ret = qcow2_remove_in_use_bitmaps(bs);
+        if (ret < 0) {
+            return ret;
+        }
+
         ret = qcow2_mark_clean(bs);
         if (ret < 0) {
             return ret;
diff --git a/block/qcow2.h b/block/qcow2.h
index 96db7c51ec..542630bfbd 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -1039,6 +1039,8 @@ bool coroutine_fn GRAPH_RDLOCK
 qcow2_load_dirty_bitmaps(BlockDriverState *bs, bool *header_updated,
                          Error **errp);
 
+int GRAPH_RDLOCK qcow2_remove_in_use_bitmaps(BlockDriverState *bs);
+
 bool GRAPH_RDLOCK
 qcow2_get_bitmap_info_list(BlockDriverState *bs,
                            Qcow2BitmapInfoList **info_list, Error **errp);
diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-test b/tests/qemu-iotests/tests/migrate-bitmaps-test
index 8fb4099201..dc69bb9433 100755
--- a/tests/qemu-iotests/tests/migrate-bitmaps-test
+++ b/tests/qemu-iotests/tests/migrate-bitmaps-test
@@ -197,6 +197,8 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
             log = self.vm_b.get_log()
             log = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', log)
             log = re.sub(r'\[I \+\d+\.\d+\] CLOSED\n?$', '', log)
+            if persistent and shared_storage and pre_shutdown:
+                log = re.sub(r'Removed inconsistent bitmap.*\n', '', log)
             self.assertEqual(log, '')
 
             # recreate vm_b, as we don't want -incoming option (this will lead
diff --git a/tests/qemu-iotests/tests/qemu-img-bitmaps b/tests/qemu-iotests/tests/qemu-img-bitmaps
index 7a3fe8c3d3..90743d4eb9 100755
--- a/tests/qemu-iotests/tests/qemu-img-bitmaps
+++ b/tests/qemu-iotests/tests/qemu-img-bitmaps
@@ -137,7 +137,7 @@ echo
 
 # Prepare image with corrupted bitmap
 $QEMU_IO -c abort "$TEST_IMG" 2>/dev/null
-$QEMU_IMG bitmap --add "$TEST_IMG" b4
+$QEMU_IMG bitmap --add "$TEST_IMG" b4 2>&1 | _filter_testdir
 $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
diff --git a/tests/qemu-iotests/tests/qemu-img-bitmaps.out b/tests/qemu-iotests/tests/qemu-img-bitmaps.out
index 74b81f703b..8062ae1114 100644
--- a/tests/qemu-iotests/tests/qemu-img-bitmaps.out
+++ b/tests/qemu-iotests/tests/qemu-img-bitmaps.out
@@ -118,6 +118,10 @@ Format specific information:
 
 === Check handling of inconsistent bitmap ===
 
+Removing inconsistent bitmap 'b1' at image 'TEST_DIR/t.qcow2'
+Removing inconsistent bitmap 'b2' at image 'TEST_DIR/t.qcow2'
+Removing inconsistent bitmap 'b0' at image 'TEST_DIR/t.qcow2'
+qemu-img: Operation remove on bitmap b1 failed: Dirty bitmap 'b1' not found
 image: TEST_DIR/t.IMGFMT
 file format: IMGFMT
 virtual size: 10 MiB (10485760 bytes)
@@ -127,29 +131,13 @@ backing file format: IMGFMT
 Format specific information:
     bitmaps:
         [0]:
-            flags:
-                [0]: in-use
-                [1]: auto
-            name: b2
-            granularity: 65536
-        [1]:
-            flags:
-                [0]: in-use
-            name: b0
-            granularity: 65536
-        [2]:
             flags:
                 [0]: auto
             name: b4
             granularity: 65536
     corrupt: false
 
-qemu-img: Cannot copy inconsistent bitmap 'b0'
-Try --skip-broken-bitmaps, or use '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
-
-qemu-img: warning: Skipping inconsistent bitmap 'b0'
-qemu-img: warning: Skipping inconsistent bitmap 'b2'
+unexpected success
 image: TEST_DIR/t.IMGFMT.copy
 file format: IMGFMT
 virtual size: 10 MiB (10485760 bytes)
@@ -174,10 +162,20 @@ Format specific information:
                 [0]: auto
             name: b4
             granularity: 65536
-        [1]:
+    corrupt: false
+
+qemu-img: Operation remove on bitmap b0 failed: Dirty bitmap 'b0' not found
+qemu-img: Operation remove on bitmap b2 failed: Dirty bitmap 'b2' not found
+image: TEST_DIR/t.IMGFMT.copy
+file format: IMGFMT
+virtual size: 10 MiB (10485760 bytes)
+cluster_size: 65536
+Format specific information:
+    bitmaps:
+        [0]:
             flags:
                 [0]: auto
-            name: b2
+            name: b4
             granularity: 65536
     corrupt: false
 *** done
diff --git a/tests/qemu-iotests/tests/qemu-img-close-errors b/tests/qemu-iotests/tests/qemu-img-close-errors
index 50bfb6cfa2..d72567171d 100755
--- a/tests/qemu-iotests/tests/qemu-img-close-errors
+++ b/tests/qemu-iotests/tests/qemu-img-close-errors
@@ -81,7 +81,8 @@ for max_bitmap in 6 7; do
     $QEMU_IMG commit -d "$TEST_IMG" 2>&1 | _filter_generated_node_ids
     echo "qemu-img commit exit code: ${PIPESTATUS[0]}"
 
-    $QEMU_IMG bitmap --add "$BASE_JSON" "good-bitmap"
+    $QEMU_IMG bitmap --add "$BASE_JSON" "good-bitmap" 2>&1 | \
+        sed '/^Removing inconsistent bitmap.*/d'
     echo "qemu-img bitmap --add exit code: $?"
 
     $QEMU_IMG bitmap --merge "good-bitmap" -b "$TEST_IMG" "$BASE_JSON" \
-- 
2.52.0