[PATCH v2 0/3] migration/block-dirty-bitmap: fix postcopy abort with bitmaps on several nodes

[PATCH v2 0/3] migration/block-dirty-bitmap: fix postcopy abort with bitmaps on several nodes

[Denis V. Lunev]

Postcopy migration of dirty bitmaps kills the destination as soon as
bitmaps live on more than one node:

  qemu-system-x86_64: ../block.c:6400: bdrv_lookup_bs: Assertion
  `qemu_in_main_thread()' failed.

The postcopy listen thread resolves node names with bdrv_lookup_bs()
without the BQL. With a single node the name is sent only during setup,
which runs in the main thread, so the existing test never gets there.

We hit it in production with libvirt: a CBT backup leaves a persistent
bitmap on every disk, libvirt enables the dirty-bitmaps capability, and
the next live migration of any VM with more than one disk aborts the
destination after the guest has already been switched over to it.

Patch 1 names the counted string buffer size that patch 2 needs. Patch 2
resolves every chunk but START from the bitmaps that START set up,
instead of from the block graph. Patch 3 extends
migrate-bitmaps-postcopy-test with a bitmap on a second node, and it
fails without patch 2.

Changes in v2:
- new patch 1: add QEMU_COUNTED_STRING_SIZE instead of bare 256 (Eric)
- patch 2: drop the "Error: " prefix from the new error message (Eric)
- patch 2: move node_alias to the end of LoadBitmapState
- patch 3: add Eric's Tested-by and Reviewed-by

v1: https://lore.kernel.org/qemu-devel/20260924115611.4098417-1-den@openvz.org/

Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Fabiano Rosas <farosas@suse.de>
Cc: Eric Blake <eblake@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Denis V. Lunev (3):
  migration: add QEMU_COUNTED_STRING_SIZE for counted string buffers
  migration/block-dirty-bitmap: fix postcopy abort with bitmaps on
    several nodes
  iotests/migrate-bitmaps-postcopy-test: add a bitmap on a second node

 include/migration/qemu-file-types.h           |  6 +-
 migration/block-dirty-bitmap.c                | 62 +++++++++++++++++--
 migration/qemu-file.c                         |  5 +-
 .../tests/migrate-bitmaps-postcopy-test       | 19 ++++++
 4 files changed, 83 insertions(+), 9 deletions(-)


base-commit: 942229961efea74b153c44c59188eb08375f10f1
-- 
2.53.0

[PATCH v2 1/3] migration: add QEMU_COUNTED_STRING_SIZE for counted string buffers

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

A counted string carries its length in one byte, so the buffer that
qemu_get_counted_string() fills must hold 256 bytes. The dirty bitmap
load state spells that size as a bare 256 for both of its alias buffers.
Name it next to the qemu_get_counted_string() prototype and use it there,
in the length assert of qemu_put_counted_string() and in DBMLoadState.

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Fabiano Rosas <farosas@suse.de>
Cc: Eric Blake <eblake@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: qemu-stable@nongnu.org
---
 include/migration/qemu-file-types.h | 6 +++++-
 migration/block-dirty-bitmap.c      | 4 ++--
 migration/qemu-file.c               | 5 +++--
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/include/migration/qemu-file-types.h b/include/migration/qemu-file-types.h
index adec5abc07..2a073bed29 100644
--- a/include/migration/qemu-file-types.h
+++ b/include/migration/qemu-file-types.h
@@ -163,7 +163,11 @@ static inline void qemu_get_sbe64s(QEMUFile *f, int64_t *pv)
     qemu_get_be64s(f, (uint64_t *)pv);
 }
 
-size_t coroutine_mixed_fn qemu_get_counted_string(QEMUFile *f, char buf[256]);
+/* Buffer size for a string whose length is sent as a single byte */
+#define QEMU_COUNTED_STRING_SIZE 256
+
+size_t coroutine_mixed_fn
+qemu_get_counted_string(QEMUFile *f, char buf[QEMU_COUNTED_STRING_SIZE]);
 
 void qemu_put_counted_string(QEMUFile *f, const char *name);
 
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 1b8f39c12b..1386f43635 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -145,8 +145,8 @@ typedef struct LoadBitmapState {
 /* State of the dirty bitmap migration (DBM) during load process */
 typedef struct DBMLoadState {
     uint32_t flags;
-    char node_alias[256];
-    char bitmap_alias[256];
+    char node_alias[QEMU_COUNTED_STRING_SIZE];
+    char bitmap_alias[QEMU_COUNTED_STRING_SIZE];
     char bitmap_name[BDRV_BITMAP_MAX_NAME_SIZE + 1];
     BlockDriverState *bs;
     BdrvDirtyBitmap *bitmap;
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 7735e61854..df3f96fb24 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -829,7 +829,8 @@ uint64_t qemu_get_be64(QEMUFile *f)
  *          else 0
  *          (Note a 0 length string will return 0 either way)
  */
-size_t coroutine_mixed_fn qemu_get_counted_string(QEMUFile *f, char buf[256])
+size_t coroutine_mixed_fn
+qemu_get_counted_string(QEMUFile *f, char buf[QEMU_COUNTED_STRING_SIZE])
 {
     size_t len = qemu_get_byte(f);
     size_t res = qemu_get_buffer(f, (uint8_t *)buf, len);
@@ -847,7 +848,7 @@ void qemu_put_counted_string(QEMUFile *f, const char *str)
 {
     size_t len = strlen(str);
 
-    assert(len < 256);
+    assert(len < QEMU_COUNTED_STRING_SIZE);
     qemu_put_byte(f, len);
     qemu_put_buffer(f, (const uint8_t *)str, len);
 }

base-commit: 942229961efea74b153c44c59188eb08375f10f1
-- 
2.53.0

[PATCH v2 2/3] migration/block-dirty-bitmap: fix postcopy abort with bitmaps on several nodes

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

With dirty bitmaps on more than one node, the source switches nodes
during the postcopy bulk phase and sends the node name again. The
destination reads those chunks in the postcopy listen thread, which does
not hold the BQL, and resolves the name through bdrv_lookup_bs():

  qemu-system-x86_64: ../block.c:6400: bdrv_lookup_bs: Assertion
  `qemu_in_main_thread()' failed.

The bdrv_find_dirty_bitmap() call right after it needs the BQL or the
dirty bitmap lock as well. Taking the BQL there would deadlock: the
listen thread holds s->lock, and dirty_bitmap_mig_before_vm_start()
takes s->lock with the BQL held.

Every node and bitmap that a non-START chunk refers to was set up by a
START chunk, and those are loaded under the BQL during setup. Record the
node alias there, and resolve all other chunks against s->bitmaps
instead of the block graph.

Bitmaps on a single node never resend the node name after setup, which
is why migrate-bitmaps-postcopy-test does not catch this.

Fixes: f791bf7f93f2 ("assertions for block global state API")
Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Fabiano Rosas <farosas@suse.de>
Cc: Eric Blake <eblake@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: qemu-stable@nongnu.org
---
 migration/block-dirty-bitmap.c | 58 +++++++++++++++++++++++++++++++---
 1 file changed, 54 insertions(+), 4 deletions(-)

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 1386f43635..35623379eb 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -140,6 +140,7 @@ typedef struct LoadBitmapState {
     BdrvDirtyBitmap *bitmap;
     bool migrated;
     bool enabled;
+    char node_alias[QEMU_COUNTED_STRING_SIZE];
 } LoadBitmapState;
 
 /* State of the dirty bitmap migration (DBM) during load process */
@@ -859,6 +860,7 @@ static int dirty_bitmap_load_start(QEMUFile *f, DBMLoadState *s)
     }
 
     b = g_new(LoadBitmapState, 1);
+    g_strlcpy(b->node_alias, s->node_alias, sizeof(b->node_alias));
     b->bs = s->bs;
     b->bitmap = s->bitmap;
     b->migrated = false;
@@ -1056,6 +1058,53 @@ static int dirty_bitmap_load_bits(QEMUFile *f, DBMLoadState *s)
     return 0;
 }
 
+/*
+ * Only START chunks are guaranteed to be loaded under the BQL; the rest may
+ * come from the postcopy listen thread and must not touch the block graph.
+ */
+static BlockDriverState *dirty_bitmap_load_find_bs(DBMLoadState *s,
+                                                   const char *device,
+                                                   const char *node_name,
+                                                   Error **errp)
+{
+    GSList *item;
+
+    if (s->flags & DIRTY_BITMAP_MIG_FLAG_START) {
+        return bdrv_lookup_bs(device, node_name, errp);
+    }
+
+    for (item = s->bitmaps; item; item = g_slist_next(item)) {
+        LoadBitmapState *b = item->data;
+
+        if (!strcmp(b->node_alias, s->node_alias)) {
+            return b->bs;
+        }
+    }
+
+    error_setg(errp, "no migrated bitmaps on node alias '%s'", s->node_alias);
+    return NULL;
+}
+
+static BdrvDirtyBitmap *dirty_bitmap_load_find_bitmap(DBMLoadState *s)
+{
+    GSList *item;
+
+    if (s->flags & DIRTY_BITMAP_MIG_FLAG_START) {
+        return bdrv_find_dirty_bitmap(s->bs, s->bitmap_name);
+    }
+
+    for (item = s->bitmaps; item; item = g_slist_next(item)) {
+        LoadBitmapState *b = item->data;
+
+        if (b->bs == s->bs &&
+            !strcmp(bdrv_dirty_bitmap_name(b->bitmap), s->bitmap_name)) {
+            return b->bitmap;
+        }
+    }
+
+    return NULL;
+}
+
 static int dirty_bitmap_load_header(QEMUFile *f, DBMLoadState *s,
                                     GHashTable *alias_map)
 {
@@ -1084,11 +1133,12 @@ static int dirty_bitmap_load_header(QEMUFile *f, DBMLoadState *s,
                     s->bs = NULL;
                 } else {
                     bitmap_alias_map = amin->subtree;
-                    s->bs = bdrv_lookup_bs(NULL, amin->string, &local_err);
+                    s->bs = dirty_bitmap_load_find_bs(s, NULL, amin->string,
+                                                      &local_err);
                 }
             } else {
-                s->bs = bdrv_lookup_bs(s->node_alias, s->node_alias,
-                                       &local_err);
+                s->bs = dirty_bitmap_load_find_bs(s, s->node_alias,
+                                                  s->node_alias, &local_err);
             }
             if (!s->bs) {
                 error_report_err(local_err);
@@ -1139,7 +1189,7 @@ static int dirty_bitmap_load_header(QEMUFile *f, DBMLoadState *s,
 
         if (!s->cancelled) {
             g_strlcpy(s->bitmap_name, bitmap_name, sizeof(s->bitmap_name));
-            s->bitmap = bdrv_find_dirty_bitmap(s->bs, s->bitmap_name);
+            s->bitmap = dirty_bitmap_load_find_bitmap(s);
 
             /*
              * bitmap may be NULL here, it wouldn't be an error if it is the
-- 
2.53.0

[PATCH v2 3/3] iotests/migrate-bitmaps-postcopy-test: add a bitmap on a second node

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

All bitmaps in the test live on drive0, so the source never switches
nodes after setup and the destination never resolves a node name from
the postcopy listen thread. Add a second drive with one bitmap and check
that it arrives intact.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Tested-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Fabiano Rosas <farosas@suse.de>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 .../tests/migrate-bitmaps-postcopy-test       | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
index b2b37ada4b..36c281596d 100755
--- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
+++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
@@ -27,6 +27,8 @@ debug = False
 
 disk_a = os.path.join(iotests.test_dir, 'disk_a')
 disk_b = os.path.join(iotests.test_dir, 'disk_b')
+disk_a1 = os.path.join(iotests.test_dir, 'disk_a1')
+disk_b1 = os.path.join(iotests.test_dir, 'disk_b1')
 size = '256G'
 fifo = os.path.join(iotests.test_dir, 'mig_fifo')
 
@@ -97,16 +99,23 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
         self.vm_b.shutdown()
         os.remove(disk_a)
         os.remove(disk_b)
+        os.remove(disk_a1)
+        os.remove(disk_b1)
         os.remove(fifo)
 
     def setUp(self):
         os.mkfifo(fifo)
         qemu_img('create', '-f', iotests.imgfmt, disk_a, size)
         qemu_img('create', '-f', iotests.imgfmt, disk_b, size)
+        qemu_img('create', '-f', iotests.imgfmt, disk_a1, '1G')
+        qemu_img('create', '-f', iotests.imgfmt, disk_b1, '1G')
         self.vm_a = iotests.VM(path_suffix='a').add_drive(disk_a,
                                                           'discard=unmap')
         self.vm_b = iotests.VM(path_suffix='b').add_drive(disk_b,
                                                           'discard=unmap')
+        # A bitmap on a second node makes postcopy switch nodes after setup
+        self.vm_a.add_drive(disk_a1)
+        self.vm_b.add_drive(disk_b1)
         self.vm_b.add_incoming("exec: cat '" + fifo + "'")
         self.vm_a.launch()
         self.vm_b.launch()
@@ -127,6 +136,13 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
                                node='drive0', name='bitmap0')
         empty_sha256 = result['return']['sha256']
 
+        self.vm_a.cmd('block-dirty-bitmap-add', node='drive1',
+                      name='bitmap0', granularity=granularity)
+        self.vm_a.hmp_qemu_io('drive1', 'write 0 64k')
+        result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256',
+                               node='drive1', name='bitmap0')
+        self.drive1_sha256 = result['return']['sha256']
+
         apply_discards(self.vm_a, discards1)
 
         result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256',
@@ -212,6 +228,9 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
 
         # check that bitmaps are migrated and persistence works
         check_bitmaps(self.vm_b, nb_bitmaps)
+        result = self.vm_b.qmp('x-debug-block-dirty-bitmap-sha256',
+                               node='drive1', name='bitmap0')
+        self.assert_qmp(result, 'return/sha256', self.drive1_sha256)
         self.vm_b.shutdown()
         # recreate vm_b, so there is no incoming option, which prevents
         # loading bitmaps from disk
-- 
2.53.0

Re: [PATCH v2 1/3] migration: add QEMU_COUNTED_STRING_SIZE for counted string buffers

[Eric Blake]

On Tue, Oct 06, 2026 at 07:07:36PM +0200, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
> 
> A counted string carries its length in one byte, so the buffer that
> qemu_get_counted_string() fills must hold 256 bytes. The dirty bitmap
> load state spells that size as a bare 256 for both of its alias buffers.
> Name it next to the qemu_get_counted_string() prototype and use it there,
> in the length assert of qemu_put_counted_string() and in DBMLoadState.
> 
> Suggested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Fabiano Rosas <farosas@suse.de>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Cc: qemu-stable@nongnu.org
> ---
>  include/migration/qemu-file-types.h | 6 +++++-
>  migration/block-dirty-bitmap.c      | 4 ++--
>  migration/qemu-file.c               | 5 +++--
>  3 files changed, 10 insertions(+), 5 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH v2 2/3] migration/block-dirty-bitmap: fix postcopy abort with bitmaps on several nodes

[Eric Blake]

On Tue, Oct 06, 2026 at 07:07:37PM +0200, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
> 
> With dirty bitmaps on more than one node, the source switches nodes
> during the postcopy bulk phase and sends the node name again. The
> destination reads those chunks in the postcopy listen thread, which does
> not hold the BQL, and resolves the name through bdrv_lookup_bs():
> 
>   qemu-system-x86_64: ../block.c:6400: bdrv_lookup_bs: Assertion
>   `qemu_in_main_thread()' failed.
> 
> The bdrv_find_dirty_bitmap() call right after it needs the BQL or the
> dirty bitmap lock as well. Taking the BQL there would deadlock: the
> listen thread holds s->lock, and dirty_bitmap_mig_before_vm_start()
> takes s->lock with the BQL held.
> 
> Every node and bitmap that a non-START chunk refers to was set up by a
> START chunk, and those are loaded under the BQL during setup. Record the
> node alias there, and resolve all other chunks against s->bitmaps
> instead of the block graph.
> 
> Bitmaps on a single node never resend the node name after setup, which
> is why migrate-bitmaps-postcopy-test does not catch this.
> 
> Fixes: f791bf7f93f2 ("assertions for block global state API")
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Fabiano Rosas <farosas@suse.de>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Cc: qemu-stable@nongnu.org
> ---
>  migration/block-dirty-bitmap.c | 58 +++++++++++++++++++++++++++++++---
>  1 file changed, 54 insertions(+), 4 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH v2 0/3] migration/block-dirty-bitmap: fix postcopy abort with bitmaps on several nodes

[Eric Blake]

On Tue, Oct 06, 2026 at 07:07:35PM +0200, Denis V. Lunev wrote:
> Postcopy migration of dirty bitmaps kills the destination as soon as
> bitmaps live on more than one node:
> 
>   qemu-system-x86_64: ../block.c:6400: bdrv_lookup_bs: Assertion
>   `qemu_in_main_thread()' failed.
> 

> Changes in v2:
> - new patch 1: add QEMU_COUNTED_STRING_SIZE instead of bare 256 (Eric)
> - patch 2: drop the "Error: " prefix from the new error message (Eric)
> - patch 2: move node_alias to the end of LoadBitmapState
> - patch 3: add Eric's Tested-by and Reviewed-by

I'm queueing this series through my NBD tree, and running tests now in
preparation for a pull request.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH v2 3/3] iotests/migrate-bitmaps-postcopy-test: add a bitmap on a second node

[Eric Blake]

On Tue, Oct 06, 2026 at 07:07:38PM +0200, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
> 
> All bitmaps in the test live on drive0, so the source never switches
> nodes after setup and the destination never resolves a node name from
> the postcopy listen thread. Add a second drive with one bitmap and check
> that it arrives intact.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Tested-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Fabiano Rosas <farosas@suse.de>
> Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  .../tests/migrate-bitmaps-postcopy-test       | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)

While playing with this patch series, I sent SIGINT mid-test, and then
all subsequent runs failed...

>      def setUp(self):
>          os.mkfifo(fifo)

because fifo was not cleanly removed on the preceeding run, but mkfifo
wasn't expecting it to already exist.  I'm squashing in this fix, even
though it was a pre-existing issue:

diff --git i/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test w/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
index b2b37ada4b2..817a9216e58 100755
--- i/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
+++ w/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
@@ -100,6 +100,8 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
         os.remove(fifo)

     def setUp(self):
+        if os.path.exists(fifo):
+            os.remove(fifo)
         os.mkfifo(fifo)
         qemu_img('create', '-f', iotests.imgfmt, disk_a, size)
         qemu_img('create', '-f', iotests.imgfmt, disk_b, size)


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org