[PATCH] block/qcow2-bitmap: remove inconsistent bitmaps when loading

Polina Vishneva posted 1 patch 2 weeks, 5 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260120171226.1042260-1-polina.vishneva@virtuozzo.com
Maintainers: Eric Blake <eblake@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, John Snow <jsnow@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
There is a newer version of this series
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(-)
[PATCH] block/qcow2-bitmap: remove inconsistent bitmaps when loading
Posted by Polina Vishneva 2 weeks, 5 days ago
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>
---
 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..6ab857ad1a 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
+             * whenbdrv_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

Re: [PATCH] block/qcow2-bitmap: remove inconsistent bitmaps when loading
Posted by Jean-Louis Dupond 2 weeks, 4 days ago
Would this also solve https://gitlab.com/qemu-project/qemu/-/issues/2909 
perhaps?

On 20/01/2026 18:20, Polina Vishneva wrote:
> 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>
> ---
>   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..6ab857ad1a 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -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
> +             * whenbdrv_dirty_bitmap_check() could be called on a bitmap
Small typo here btw :)
> +             * 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;


Thanks
Jean-Louis
Re: [PATCH] block/qcow2-bitmap: remove inconsistent bitmaps when loading
Posted by Polina Vishneva 2 weeks, 3 days ago
On Wed, 2026-01-21 at 15:22 +0100, Jean-Louis Dupond wrote:
> Would this also solve https://gitlab.com/qemu-project/qemu/-/issues/2909 
> perhaps?

Unfortunately, it won't, because in your case the error happens in
in bitmap_list_load(), before we even start traversing the bitmaps.
Also these changes only affect in_use bitmaps, if they are corrupted
in some other way, nothing will change.

>
> On 20/01/2026 18:20, Polina Vishneva wrote:
> > +             * whenbdrv_dirty_bitmap_check() could be called on a bitmap
> Small typo here btw :)

Thanks!

Best regards,
Polina.