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

[PATCH 0/2] 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 resolves every chunk but START from the bitmaps that START set
up, instead of from the block graph. Patch 2 extends
migrate-bitmaps-postcopy-test with a bitmap on a second node, and it
fails without patch 1.

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 (2):
  migration/block-dirty-bitmap: fix postcopy abort with bitmaps on
    several nodes
  iotests/migrate-bitmaps-postcopy-test: add a bitmap on a second node

 migration/block-dirty-bitmap.c                | 59 +++++++++++++++++--
 .../tests/migrate-bitmaps-postcopy-test       | 19 ++++++
 2 files changed, 74 insertions(+), 4 deletions(-)


base-commit: 942229961efea74b153c44c59188eb08375f10f1
-- 
2.53.0

[PATCH 1/2] 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 | 59 +++++++++++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 4 deletions(-)

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 1b8f39c12b..61691ee4cc 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -136,6 +136,7 @@ typedef struct DBMSaveState {
 } DBMSaveState;
 
 typedef struct LoadBitmapState {
+    char node_alias[256];
     BlockDriverState *bs;
     BdrvDirtyBitmap *bitmap;
     bool migrated;
@@ -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,54 @@ 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, "Error: 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 +1134,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 +1190,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 2/2] 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>
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>
---
 .../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 0/2] migration/block-dirty-bitmap: fix postcopy abort with bitmaps on several nodes

[Denis V. Lunev]

On 9/24/26 13:56, Denis V. Lunev wrote:
> This email originated from an IP that might not be authorized by the domain it was sent from.
> Do not click links or open attachments unless it is an email you expected to receive.
> 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 resolves every chunk but START from the bitmaps that START set
> up, instead of from the block graph. Patch 2 extends
> migrate-bitmaps-postcopy-test with a bitmap on a second node, and it
> fails without patch 1.
>
> 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 (2):
>   migration/block-dirty-bitmap: fix postcopy abort with bitmaps on
>     several nodes
>   iotests/migrate-bitmaps-postcopy-test: add a bitmap on a second node
>
>  migration/block-dirty-bitmap.c                | 59 +++++++++++++++++--
>  .../tests/migrate-bitmaps-postcopy-test       | 19 ++++++
>  2 files changed, 74 insertions(+), 4 deletions(-)
>
>
> base-commit: 942229961efea74b153c44c59188eb08375f10f1
ping

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

[Eric Blake]

On Thu, Sep 24, 2026 at 01:56:11PM +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>
> 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>
> ---
>  .../tests/migrate-bitmaps-postcopy-test       | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)

I confirmed that this reveals the bug when applied in isolation
without 1/2, therefore:

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

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

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

[Eric Blake]

On Thu, Sep 24, 2026 at 01:56:10PM +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 | 59 +++++++++++++++++++++++++++++++---
>  1 file changed, 55 insertions(+), 4 deletions(-)
> 
> diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
> index 1b8f39c12b..61691ee4cc 100644
> --- a/migration/block-dirty-bitmap.c
> +++ b/migration/block-dirty-bitmap.c
> @@ -136,6 +136,7 @@ typedef struct DBMSaveState {
>  } DBMSaveState;
>  
>  typedef struct LoadBitmapState {
> +    char node_alias[256];

Is making this fixed-length rather than malloc'd char* correct?

/me checks

migration/block-dirty-bitmap.c:    char node_alias[256];

Yep. I'm a bit surprised we did it that way earlier, but this is
correct. That said, if enough structs reference the same size, we may
want a #define or enum named value in a header, rather than a magic
number at each use site.

> +/*
> + * 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, "Error: no migrated bitmaps on node alias '%s'",
> +               s->node_alias);

Drop the "Error: " prefix; it will be redundant if the caller reports
rather than ignores the error.

Otherwise, this makes sense.

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