[PATCH v2 0/5] migration/postcopy: report destination states only after the run state change

[PATCH v2 0/5] migration/postcopy: report destination states only after the run state change

[Denis V. Lunev]

On the postcopy destination, the postcopy-active and completed states
can be reported while the VM is still in RUN_STATE_INMIGRATE. The run
state only changes in loadvm_postcopy_handle_run_bh(), but
loadvm_postcopy_handle_run() switches to postcopy-active before it
schedules the BH, and the listen thread can report completed before the
BH has run. Precopy does not have this problem: the run state changes
before completed is reported.

We hit this in production. A VM with a persistent dirty bitmap migrates
its bitmaps in postcopy, so libvirt runs the Finish phase as soon as the
source enters postcopy-active. Our libvirt reattaches disks on the
destination in Finish, and the blockdev-add calls land while the
destination is still in RUN_STATE_INMIGRATE. The new nodes are created
inactive, the device cannot be attached to them ("Block node is
read-only"), and the guest is lost on both hosts. A bitmap-only
postcopy makes the window wide, because the listen thread has almost
nothing to load.

The series orders both destination states after the run state change
and adds a qtest check that fails on master; fast snapshot load
(postcopy-ram with mapped-ram) is not covered. The series also gives the
source's postcopy-active the same meaning. It takes Peter's idea from
the review of Juraj's RFC [1]: record the package-loaded ping and send
the pong later [2]. Here the pong is sent from the run BH, after the run
state change, rather than from loadvm_postcopy_handle_run(). Juraj
confirmed that this fixes the unrecoverable source he reported [3]. The
late pong keeps the source in postcopy-device longer, so it can be
dropped without affecting the rest.

The late pong has two more consequences. A bitmap-only postcopy with a
return path deadlocks unless dirty_bitmap_load() stops holding its lock
while it reads the stream. That fix was sent separately [4] and this
series is based on it. The new iotest case covers that configuration;
feel free to drop it. And an autostarting destination that dies between
vm_start() and the pong lets the source resume after the guest has
briefly run there. This cannot happen with -S, which libvirt uses.

[1] https://lore.kernel.org/qemu-devel/20260423154525.10292-1-jmarcin@redhat.com/
[2] https://lore.kernel.org/qemu-devel/afJg6VzsaslzZQW1@x1.local/
[3] https://lore.kernel.org/qemu-devel/20260929132150.2215007-1-jmarcin@redhat.com/
[4] https://lore.kernel.org/qemu-devel/20261110155113.2721134-1-den@openvz.org/

Many thanks to Juraj for the postcopy-device state in 7b842fe354c. It is
what gives the source a point to wait at, and this series only moves
that point to where the destination has really left
RUN_STATE_INMIGRATE.

v2:
- the run BH sets main_thread_load_event instead of a second event
  (Peter)
- the destination's switch to postcopy-active keeps the explicit
  POSTCOPY_DEVICE check (Peter)
- new iotest case for a bitmap postcopy with a return path
- the commit message of the late pong is updated; the code is
  unchanged, so I kept the tags
- rebased

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

Based-on: <20261110155113.2721134-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: Juraj Marcin <jmarcin@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: Eric Blake <eblake@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Hanna Reitz <hreitz@redhat.com>
Cc: qemu-block@nongnu.org

Denis V. Lunev (5):
  migration/postcopy: report COMPLETED only after the run BH has run
  migration/postcopy: set destination postcopy-active in the run BH
  tests/qtest/migration: check the destination run state in postcopy
    tests
  migration/postcopy: answer the package-loaded ping from the run BH
  iotests/migrate-bitmaps-postcopy-test: cover postcopy with a return
    path

 migration/migration.c                         |  1 +
 migration/migration.h                         |  4 +-
 migration/savevm.c                            | 25 +++++++++---
 .../tests/migrate-bitmaps-postcopy-test       | 39 +++++++++++++++++++
 .../tests/migrate-bitmaps-postcopy-test.out   |  4 +-
 tests/qtest/migration/framework.c             |  6 ++-
 tests/qtest/migration/migration-qmp.c         | 10 ++++-
 tests/qtest/migration/migration-qmp.h         |  3 +-
 tests/qtest/migration/migration-util.c        |  8 ++++
 tests/qtest/migration/migration-util.h        |  2 +
 10 files changed, 88 insertions(+), 14 deletions(-)


base-commit: 723622bedb0b423de7d1c61ebbe6e7a71934949c
prerequisite-patch-id: 67b7337e86202ef7509fc53ff8915a814a477e3b
-- 
2.53.0

[PATCH v2 1/5] migration/postcopy: report COMPLETED only after the run BH has run

[Denis V. Lunev]

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

The postcopy listen thread waits for main_thread_load_event before it
reports COMPLETED. That event is set as soon as qemu_loadvm_state_main()
returns in the main thread, while the run state is changed later, in
loadvm_postcopy_handle_run_bh(). The listen thread can therefore report
COMPLETED while the VM is still in RUN_STATE_INMIGRATE, easily so in a
bitmap-only postcopy where it has little to load.

Management reacting to COMPLETED then finds the VM still incoming: a
blockdev-add creates an inactive node, since bds_tree_init() forces
BDRV_O_INACTIVE in RUN_STATE_INMIGRATE, and attaching a device to it
fails with "Block node is read-only". Precopy gets this right:
process_incoming_migration_bh() changes the run state before it reports
COMPLETED.

Once CMD_POSTCOPY_RUN has scheduled the run BH, set the event at the
end of the BH instead. qemu_loadvm_state() keeps setting it in every
other case, including a load that failed before CMD_POSTCOPY_RUN. The
postcopy state it checks only becomes RUNNING on the main thread, right
before the BH is scheduled.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Fabiano Rosas <farosas@suse.de>
Cc: Juraj Marcin <jmarcin@redhat.com>
---
 migration/migration.h | 2 +-
 migration/savevm.c    | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/migration/migration.h b/migration/migration.h
index 057c85d611..b223480d28 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -127,7 +127,7 @@ struct MigrationIncomingState {
     QemuEvent  thread_sync_event;
     /*
      * Free at the start of the main state load, set as the main thread finishes
-     * loading state.
+     * loading state, or in postcopy once the run BH has changed the run state.
      */
     QemuEvent main_thread_load_event;
 
diff --git a/migration/savevm.c b/migration/savevm.c
index fe94ba96e4..7d0276c1d7 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2263,6 +2263,7 @@ static void loadvm_postcopy_handle_run_bh(void *opaque)
     }
 
     trace_vmstate_downtime_checkpoint("dst-postcopy-bh-vm-started");
+    qemu_event_set(&mis->main_thread_load_event);
 }
 
 /* After all discards we can start running and asking for pages */
@@ -3148,7 +3149,10 @@ int qemu_loadvm_state(QEMUFile *f, Error **errp)
     cpu_synchronize_all_pre_loadvm();
 
     ret = qemu_loadvm_state_main(f, mis, errp);
-    qemu_event_set(&mis->main_thread_load_event);
+    /* Otherwise loadvm_postcopy_handle_run_bh() sets it */
+    if (postcopy_state_get() != POSTCOPY_INCOMING_RUNNING) {
+        qemu_event_set(&mis->main_thread_load_event);
+    }
 
     trace_qemu_loadvm_state_post_main(ret);
 

base-commit: 723622bedb0b423de7d1c61ebbe6e7a71934949c
prerequisite-patch-id: 67b7337e86202ef7509fc53ff8915a814a477e3b
-- 
2.53.0

[PATCH v2 2/5] migration/postcopy: set destination postcopy-active in the run BH

[Denis V. Lunev]

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

Commit 7b842fe354c ("migration: Introduce POSTCOPY_DEVICE state") made
the destination enter POSTCOPY_ACTIVE in loadvm_postcopy_handle_run(),
before it schedules loadvm_postcopy_handle_run_bh(), and only the BH
starts the VM or moves it to RUN_STATE_PAUSED. Management reacting to
the destination's postcopy-active event can therefore still find the
VM in RUN_STATE_INMIGRATE, where blockdev-add creates inactive nodes
and attaching a device to them fails with "Block node is read-only".

Move the transition to the end of the run BH, after vm_start() or
runstate_set(), as process_incoming_migration_bh() does for precopy
COMPLETED. Without a return path the listen thread enters
POSTCOPY_ACTIVE directly, and the fast snapshot load path switches
right after scheduling the BH; in both cases the state is no longer
POSTCOPY_DEVICE when the BH runs and the transition is a no-op.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Fabiano Rosas <farosas@suse.de>
Cc: Juraj Marcin <jmarcin@redhat.com>
---
 migration/savevm.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 7d0276c1d7..600db7a1ab 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2263,6 +2263,12 @@ static void loadvm_postcopy_handle_run_bh(void *opaque)
     }
 
     trace_vmstate_downtime_checkpoint("dst-postcopy-bh-vm-started");
+
+    /* Already POSTCOPY_ACTIVE without a return path or in fast snapshot load */
+    if (mis->state == MIGRATION_STATUS_POSTCOPY_DEVICE) {
+        migrate_set_state(&mis->state, MIGRATION_STATUS_POSTCOPY_DEVICE,
+                          MIGRATION_STATUS_POSTCOPY_ACTIVE);
+    }
     qemu_event_set(&mis->main_thread_load_event);
 }
 
@@ -2277,11 +2283,6 @@ static int loadvm_postcopy_handle_run(MigrationIncomingState *mis, Error **errp)
         return -1;
     }
 
-    /* We might be already in POSTCOPY_ACTIVE if there is no return path */
-    if (mis->state == MIGRATION_STATUS_POSTCOPY_DEVICE) {
-        migrate_set_state(&mis->state, MIGRATION_STATUS_POSTCOPY_DEVICE,
-                          MIGRATION_STATUS_POSTCOPY_ACTIVE);
-    }
     postcopy_state_set(POSTCOPY_INCOMING_RUNNING);
     migration_bh_schedule(loadvm_postcopy_handle_run_bh, mis);
 
-- 
2.53.0

[PATCH v2 3/5] tests/qtest/migration: check the destination run state in postcopy tests

[Denis V. Lunev]

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

The destination must not report postcopy-active or completed while it
is still in RUN_STATE_INMIGRATE. The destination autostarts in these
tests, so the run state change emits RESUME and the order is visible in
the event stream: record a MIGRATION postcopy-active or completed event
seen before RESUME and fail the postcopy tests on it once the
destination has completed.

migrate_postcopy_start() waited for the destination's RESUME with
qtest_qmp_eventwait(), which reads events past the event callback and
drops the ones it does not want. Poll query-status instead, so every
destination event reaches the callback.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Fabiano Rosas <farosas@suse.de>
Cc: Juraj Marcin <jmarcin@redhat.com>
---
 tests/qtest/migration/framework.c      |  6 ++++--
 tests/qtest/migration/migration-qmp.c  | 10 ++++++++--
 tests/qtest/migration/migration-qmp.h  |  3 ++-
 tests/qtest/migration/migration-util.c |  8 ++++++++
 tests/qtest/migration/migration-util.h |  2 ++
 5 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
index b7aef6b21c..1cf18233f7 100644
--- a/tests/qtest/migration/framework.c
+++ b/tests/qtest/migration/framework.c
@@ -667,6 +667,8 @@ static void migrate_postcopy_complete(QTestState *from, QTestState *to,
     MigrationTestEnv *env = migration_get_env();
 
     wait_for_migration_complete(from);
+    wait_for_migration_complete(to);
+    g_assert(!dst_state.migrated_before_resume);
 
     if (args->start.suspend_me) {
         /* wakeup succeeds only if guest is suspended */
@@ -695,7 +697,7 @@ void test_postcopy_common(MigrateCommon *args)
     if (migrate_postcopy_prepare(&from, &to, &hook_data, args)) {
         return;
     }
-    migrate_postcopy_start(from, to, &src_state);
+    migrate_postcopy_start(from, to, &src_state, &dst_state);
     migrate_postcopy_complete(from, to, hook_data, args);
 }
 
@@ -842,7 +844,7 @@ void test_postcopy_recovery_common(MigrateCommon *args,
     migrate_set_parameter_int(from, "max-postcopy-bandwidth", 4096);
 
     /* Now we start the postcopy */
-    migrate_postcopy_start(from, to, &src_state);
+    migrate_postcopy_start(from, to, &src_state, &dst_state);
 
     /*
      * Wait until postcopy is really started; we can only run the
diff --git a/tests/qtest/migration/migration-qmp.c b/tests/qtest/migration/migration-qmp.c
index 437b5eaeff..77befe1027 100644
--- a/tests/qtest/migration/migration-qmp.c
+++ b/tests/qtest/migration/migration-qmp.c
@@ -549,10 +549,16 @@ void migrate_cancel(QTestState *who)
 }
 
 void migrate_postcopy_start(QTestState *from, QTestState *to,
-                            QTestMigrationState *src_state)
+                            QTestMigrationState *src_state,
+                            QTestMigrationState *dst_state)
 {
     qtest_qmp_assert_success(from, "{ 'execute': 'migrate-start-postcopy' }");
 
     wait_for_stop(from, src_state);
-    qtest_qmp_eventwait(to, "RESUME");
+
+    /* Poll so that every destination event passes the event callback */
+    while (!dst_state->resume_seen) {
+        qtest_qmp_assert_success(to, "{ 'execute': 'query-status' }");
+        g_usleep(1000);
+    }
 }
diff --git a/tests/qtest/migration/migration-qmp.h b/tests/qtest/migration/migration-qmp.h
index 940ffd5950..f275cd7692 100644
--- a/tests/qtest/migration/migration-qmp.h
+++ b/tests/qtest/migration/migration-qmp.h
@@ -46,6 +46,7 @@ void migrate_continue(QTestState *who, const char *state);
 void migrate_recover(QTestState *who, const char *uri);
 void migrate_cancel(QTestState *who);
 void migrate_postcopy_start(QTestState *from, QTestState *to,
-                            QTestMigrationState *src_state);
+                            QTestMigrationState *src_state,
+                            QTestMigrationState *dst_state);
 
 #endif /* MIGRATION_QMP_H */
diff --git a/tests/qtest/migration/migration-util.c b/tests/qtest/migration/migration-util.c
index db66d8b24f..4b5663c5a6 100644
--- a/tests/qtest/migration/migration-util.c
+++ b/tests/qtest/migration/migration-util.c
@@ -178,6 +178,14 @@ bool migrate_watch_for_events(QTestState *who, const char *name,
     } else if (g_str_equal(name, "RESUME")) {
         state->resume_seen = true;
         return true;
+    } else if (g_str_equal(name, "MIGRATION") && !state->resume_seen) {
+        const char *status = qdict_get_str(qdict_get_qdict(event, "data"),
+                                           "status");
+
+        if (g_str_equal(status, "postcopy-active") ||
+            g_str_equal(status, "completed")) {
+            state->migrated_before_resume = true;
+        }
     }
 
     return false;
diff --git a/tests/qtest/migration/migration-util.h b/tests/qtest/migration/migration-util.h
index 694773e594..f195aeea48 100644
--- a/tests/qtest/migration/migration-util.h
+++ b/tests/qtest/migration/migration-util.h
@@ -22,6 +22,8 @@ typedef struct QTestMigrationState {
     bool resume_seen;
     bool suspend_seen;
     bool suspend_me;
+    /* postcopy-active or completed reported before RESUME */
+    bool migrated_before_resume;
 } QTestMigrationState;
 
 bool migrate_watch_for_events(QTestState *who, const char *name,
-- 
2.53.0

[PATCH v2 4/5] migration/postcopy: answer the package-loaded ping from the run BH

[Denis V. Lunev]

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

Commit 7b842fe354c ("migration: Introduce POSTCOPY_DEVICE state") keeps
the source in POSTCOPY_DEVICE until the destination answers the
QEMU_VM_PING_PACKAGED_LOADED ping packaged right before
CMD_POSTCOPY_RUN. The destination answers it from the generic
MIG_CMD_PING handler as soon as it reads it, so the source's
postcopy-active means the device state is loaded, not that the
destination has left RUN_STATE_INMIGRATE. Management acting on the
destination once the source reports postcopy-active can still race
with loadvm_postcopy_handle_run_bh().

Answering early also leaves the source unrecoverable when the
destination dies in the run BH, for example while synchronizing the CPU
state: by then the source has moved to POSTCOPY_ACTIVE and can no
longer fail the migration and resume the VM itself.

Record the ping in the MIG_CMD_PING handler instead of answering it,
and send the pong at the end of the run BH, after the run state change.

The wire format is unchanged: the pong only arrives later, and an
older destination keeps answering at once. The cost is that the source
stays in POSTCOPY_DEVICE for the duration of the run BH, including
block layer activation when the destination autostarts.

An autostarting destination that dies between vm_start() and the pong
now leaves the source in POSTCOPY_DEVICE, so the source resumes after
the guest has briefly run on the destination. The pong cannot go out
before vm_start(), which is where the run state changes. With -S, as
libvirt starts the destination, the run BH does not start the VM.

Suggested-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Tested-by: Juraj Marcin <jmarcin@redhat.com>
Cc: Fabiano Rosas <farosas@suse.de>
---
 migration/migration.c | 1 +
 migration/migration.h | 2 ++
 migration/savevm.c    | 8 ++++++++
 3 files changed, 11 insertions(+)

diff --git a/migration/migration.c b/migration/migration.c
index 5686209ab1..138cc307cb 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -514,6 +514,7 @@ void migration_incoming_state_destroy(void)
     }
 
     qemu_event_reset(&mis->main_thread_load_event);
+    mis->postcopy_package_loaded_ping = false;
 
     if (mis->page_requested) {
         g_tree_destroy(mis->page_requested);
diff --git a/migration/migration.h b/migration/migration.h
index b223480d28..4adec429e9 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -130,6 +130,8 @@ struct MigrationIncomingState {
      * loading state, or in postcopy once the run BH has changed the run state.
      */
     QemuEvent main_thread_load_event;
+    /* QEMU_VM_PING_PACKAGED_LOADED received, answered by the run BH */
+    bool postcopy_package_loaded_ping;
 
     /* For network announces */
     AnnounceTimer  announce_timer;
diff --git a/migration/savevm.c b/migration/savevm.c
index 600db7a1ab..c1aa3d054d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2269,6 +2269,10 @@ static void loadvm_postcopy_handle_run_bh(void *opaque)
         migrate_set_state(&mis->state, MIGRATION_STATUS_POSTCOPY_DEVICE,
                           MIGRATION_STATUS_POSTCOPY_ACTIVE);
     }
+    if (mis->postcopy_package_loaded_ping) {
+        mis->postcopy_package_loaded_ping = false;
+        migrate_send_rp_pong(mis, QEMU_VM_PING_PACKAGED_LOADED);
+    }
     qemu_event_set(&mis->main_thread_load_event);
 }
 
@@ -2620,6 +2624,10 @@ static int loadvm_process_command(QEMUFile *f, Error **errp)
                        tmp32);
             return -1;
         }
+        if (tmp32 == QEMU_VM_PING_PACKAGED_LOADED) {
+            mis->postcopy_package_loaded_ping = true;
+            return 0;
+        }
         migrate_send_rp_pong(mis, tmp32);
         return 0;
 
-- 
2.53.0

[PATCH v2 5/5] iotests/migrate-bitmaps-postcopy-test: cover postcopy with a return path

[Denis V. Lunev]

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

The test migrates through an exec: fifo, which has no return path, so
the source never waits for the destination's package-loaded pong.
libvirt enables return-path for every migration that is not tunnelled.

Add a case that migrates one bitmap over a unix socket with return-path,
unthrottled, and waits for the source to enter postcopy-active, so that
the bitmap really goes through postcopy, and then for both sides to
complete. It times out when the destination answers the pong from the
run BH while dirty_bitmap_load() holds its lock across stream reads.

The existing cases are not run with a return path instead. They
throttle the source, which lets the run BH take the lock between
sections, and with a return path the source notices the destination
shutdown and resumes the VM, as each VM has its own image.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Fabiano Rosas <farosas@suse.de>
Cc: Juraj Marcin <jmarcin@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: Eric Blake <eblake@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Hanna Reitz <hreitz@redhat.com>
Cc: qemu-block@nongnu.org
---
 .../tests/migrate-bitmaps-postcopy-test       | 39 +++++++++++++++++++
 .../tests/migrate-bitmaps-postcopy-test.out   |  4 +-
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
index 53f7dafc79..a6e21dec0a 100755
--- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
+++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
@@ -31,6 +31,7 @@ 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')
+mig_sock = os.path.join(iotests.sock_dir, 'mig_sock')
 
 granularity = 512
 nb_bitmaps = 15
@@ -299,6 +300,44 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
         check_bitmaps(self.vm_b, 0)
 
 
+# libvirt's configuration: the source waits for the destination's pong
+class TestDirtyBitmapPostcopyReturnPath(iotests.QMPTestCase):
+    def setUp(self):
+        qemu_img('create', '-f', iotests.imgfmt, disk_a, '1G')
+        qemu_img('create', '-f', iotests.imgfmt, disk_b, '1G')
+        self.vm_a = iotests.VM(path_suffix='a').add_drive(disk_a)
+        self.vm_b = iotests.VM(path_suffix='b').add_drive(disk_b)
+        self.vm_b.add_incoming('unix:' + mig_sock)
+        self.vm_a.launch()
+        self.vm_b.launch()
+
+    def tearDown(self):
+        self.vm_a.shutdown()
+        self.vm_b.shutdown()
+        os.remove(disk_a)
+        os.remove(disk_b)
+
+    def test_postcopy_success(self):
+        self.vm_a.cmd('block-dirty-bitmap-add', node='drive0',
+                      name='bitmap0', persistent=True)
+
+        caps = [{'capability': 'dirty-bitmaps', 'state': True},
+                {'capability': 'events', 'state': True},
+                {'capability': 'return-path', 'state': True}]
+        self.vm_a.cmd('migrate-set-capabilities', capabilities=caps)
+        self.vm_b.cmd('migrate-set-capabilities', capabilities=caps)
+
+        self.vm_a.cmd('migrate', uri='unix:' + mig_sock)
+        self.vm_a.cmd('migrate-start-postcopy')
+
+        match = {'data': {'status': 'postcopy-active'}}
+        self.vm_a.event_wait('MIGRATION', match=match)
+        match = {'data': {'status': 'completed'}}
+        self.vm_b.event_wait('MIGRATION', match=match)
+        self.vm_a.event_wait('MIGRATION', match=match)
+        check_bitmaps(self.vm_b, 1)
+
+
 if __name__ == '__main__':
     iotests.main(supported_fmts=['qcow2'],
                  unsupported_imgopts=['compat'],
diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test.out b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test.out
index 8d7e996700..89968f35d7 100644
--- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test.out
+++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test.out
@@ -1,5 +1,5 @@
-...
+....
 ----------------------------------------------------------------------
-Ran 3 tests
+Ran 4 tests
 
 OK
-- 
2.53.0

Re: [PATCH v2 1/5] migration/postcopy: report COMPLETED only after the run BH has run

[Eric Blake]

On Tue, Nov 10, 2026 at 07:32:53PM +0100, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
> 
> The postcopy listen thread waits for main_thread_load_event before it
> reports COMPLETED. That event is set as soon as qemu_loadvm_state_main()
> returns in the main thread, while the run state is changed later, in
> loadvm_postcopy_handle_run_bh(). The listen thread can therefore report
> COMPLETED while the VM is still in RUN_STATE_INMIGRATE, easily so in a
> bitmap-only postcopy where it has little to load.
> 
> Management reacting to COMPLETED then finds the VM still incoming: a
> blockdev-add creates an inactive node, since bds_tree_init() forces
> BDRV_O_INACTIVE in RUN_STATE_INMIGRATE, and attaching a device to it
> fails with "Block node is read-only". Precopy gets this right:
> process_incoming_migration_bh() changes the run state before it reports
> COMPLETED.
> 
> Once CMD_POSTCOPY_RUN has scheduled the run BH, set the event at the
> end of the BH instead. qemu_loadvm_state() keeps setting it in every
> other case, including a load that failed before CMD_POSTCOPY_RUN. The
> postcopy state it checks only becomes RUNNING on the main thread, right
> before the BH is scheduled.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Fabiano Rosas <farosas@suse.de>
> Cc: Juraj Marcin <jmarcin@redhat.com>
> ---
>  migration/migration.h | 2 +-
>  migration/savevm.c    | 6 +++++-
>  2 files changed, 6 insertions(+), 2 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/5] migration/postcopy: set destination postcopy-active in the run BH

[Eric Blake]

On Tue, Nov 10, 2026 at 07:32:54PM +0100, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
> 
> Commit 7b842fe354c ("migration: Introduce POSTCOPY_DEVICE state") made
> the destination enter POSTCOPY_ACTIVE in loadvm_postcopy_handle_run(),
> before it schedules loadvm_postcopy_handle_run_bh(), and only the BH
> starts the VM or moves it to RUN_STATE_PAUSED. Management reacting to
> the destination's postcopy-active event can therefore still find the
> VM in RUN_STATE_INMIGRATE, where blockdev-add creates inactive nodes
> and attaching a device to them fails with "Block node is read-only".
> 
> Move the transition to the end of the run BH, after vm_start() or
> runstate_set(), as process_incoming_migration_bh() does for precopy
> COMPLETED. Without a return path the listen thread enters
> POSTCOPY_ACTIVE directly, and the fast snapshot load path switches
> right after scheduling the BH; in both cases the state is no longer
> POSTCOPY_DEVICE when the BH runs and the transition is a no-op.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Fabiano Rosas <farosas@suse.de>
> Cc: Juraj Marcin <jmarcin@redhat.com>
> ---
>  migration/savevm.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)

I'll defer to Peter's judgment on this one and 4/5, but the
descriptions in the cover letter and commit message make sense to
me. Meanwhile, I'm setting up to play with 3 and 5/5 to see if I can
review those tests for catching the issues documented here, even if
I'm less comfortable being the sole reviewer on the code changes that
got them to pass.

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

Re: [PATCH v2 3/5] tests/qtest/migration: check the destination run state in postcopy tests

[Eric Blake]

On Tue, Nov 10, 2026 at 07:32:55PM +0100, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
> 
> The destination must not report postcopy-active or completed while it
> is still in RUN_STATE_INMIGRATE. The destination autostarts in these
> tests, so the run state change emits RESUME and the order is visible in
> the event stream: record a MIGRATION postcopy-active or completed event
> seen before RESUME and fail the postcopy tests on it once the
> destination has completed.
> 
> migrate_postcopy_start() waited for the destination's RESUME with
> qtest_qmp_eventwait(), which reads events past the event callback and
> drops the ones it does not want. Poll query-status instead, so every
> destination event reaches the callback.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Fabiano Rosas <farosas@suse.de>
> Cc: Juraj Marcin <jmarcin@redhat.com>
> ---
>  tests/qtest/migration/framework.c      |  6 ++++--
>  tests/qtest/migration/migration-qmp.c  | 10 ++++++++--
>  tests/qtest/migration/migration-qmp.h  |  3 ++-
>  tests/qtest/migration/migration-util.c |  8 ++++++++
>  tests/qtest/migration/migration-util.h |  2 ++
>  5 files changed, 24 insertions(+), 5 deletions(-)

Took me a while to figure out that I needed a temporary:

$ sudo sysctl -w vm.unprivileged_userfaultfd=1

to get the test to pass/fail rather than skip, but with that done, I
can confirm that the test was sufficient to show failure without 2/5,
and pass when the series is applied in order. I'd be remiss if I did
not mention how handy it was to have AI help me dig up the specific
configuration and tests I needed to run for double-checking your patch
(thankfully, much different than the ethical worries about using AI to
write a patch in the first place).

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 v2 5/5] iotests/migrate-bitmaps-postcopy-test: cover postcopy with a return path

[Eric Blake]

On Tue, Nov 10, 2026 at 07:32:57PM +0100, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
> 
> The test migrates through an exec: fifo, which has no return path, so
> the source never waits for the destination's package-loaded pong.
> libvirt enables return-path for every migration that is not tunnelled.
> 
> Add a case that migrates one bitmap over a unix socket with return-path,
> unthrottled, and waits for the source to enter postcopy-active, so that
> the bitmap really goes through postcopy, and then for both sides to
> complete. It times out when the destination answers the pong from the
> run BH while dirty_bitmap_load() holds its lock across stream reads.
> 
> The existing cases are not run with a return path instead. They
> throttle the source, which lets the run BH take the lock between
> sections, and with a return path the source notices the destination
> shutdown and resumes the VM, as each VM has its own image.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Fabiano Rosas <farosas@suse.de>
> Cc: Juraj Marcin <jmarcin@redhat.com>
> Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Hanna Reitz <hreitz@redhat.com>
> Cc: qemu-block@nongnu.org
> ---
>  .../tests/migrate-bitmaps-postcopy-test       | 39 +++++++++++++++++++
>  .../tests/migrate-bitmaps-postcopy-test.out   |  4 +-
>  2 files changed, 41 insertions(+), 2 deletions(-)

I'm having trouble getting this test to fail when applied in isolation
without the rest of the series.

I even tried amending your patch as follows (to force more migration
traffic, and ensure that postcopy is in the loop), but still could not
get it to fail:

diff --git i/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test w/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
index a6e21dec0a0..7eb729716ff 100755
--- i/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
+++ w/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
@@ -303,8 +303,8 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
 # libvirt's configuration: the source waits for the destination's pong
 class TestDirtyBitmapPostcopyReturnPath(iotests.QMPTestCase):
     def setUp(self):
-        qemu_img('create', '-f', iotests.imgfmt, disk_a, '1G')
-        qemu_img('create', '-f', iotests.imgfmt, disk_b, '1G')
+        qemu_img('create', '-f', iotests.imgfmt, disk_a, '256G')
+        qemu_img('create', '-f', iotests.imgfmt, disk_b, '256G')
         self.vm_a = iotests.VM(path_suffix='a').add_drive(disk_a)
         self.vm_b = iotests.VM(path_suffix='b').add_drive(disk_b)
         self.vm_b.add_incoming('unix:' + mig_sock)
@@ -318,13 +318,17 @@ class TestDirtyBitmapPostcopyReturnPath(iotests.QMPTestCase):
         os.remove(disk_b)

     def test_postcopy_success(self):
-        self.vm_a.cmd('block-dirty-bitmap-add', node='drive0',
-                      name='bitmap0', persistent=True)
+        for i in range(15):
+            self.vm_a.cmd('block-dirty-bitmap-add', node='drive0',
+                          name=f'bitmap{i}', granularity=512, persistent=True)
+        self.vm_a.hmp_qemu_io('drive0', 'write 0 256M')

         caps = [{'capability': 'dirty-bitmaps', 'state': True},
+                {'capability': 'postcopy-ram', 'state': True},
                 {'capability': 'events', 'state': True},
                 {'capability': 'return-path', 'state': True}]
         self.vm_a.cmd('migrate-set-capabilities', capabilities=caps)
+        self.vm_a.cmd('migrate-set-parameters', max_bandwidth=16536)
         self.vm_b.cmd('migrate-set-capabilities', capabilities=caps)

         self.vm_a.cmd('migrate', uri='unix:' + mig_sock)
@@ -332,10 +336,11 @@ class TestDirtyBitmapPostcopyReturnPath(iotests.QMPTestCase):

         match = {'data': {'status': 'postcopy-active'}}
         self.vm_a.event_wait('MIGRATION', match=match)
+        self.vm_a.cmd('migrate-set-parameters', max_bandwidth=524288)
         match = {'data': {'status': 'completed'}}
         self.vm_b.event_wait('MIGRATION', match=match)
         self.vm_a.event_wait('MIGRATION', match=match)
-        check_bitmaps(self.vm_b, 1)
+        check_bitmaps(self.vm_b, 15)


 if __name__ == '__main__':


While it is indeed valuable to add a test covering libvirt's setup, I
was hoping that the test would also make it more obvious why the rest
of the series is necessary; if the test passes without the rest of the
series, it is not testing what you fixed.

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