On 6/30/2026 7:30 AM, Cédric Le Goater wrote:
External email: Use caution opening links or attachments
From: Avihai Horon <[email protected]>
Before switchover, the source needs one last exact pending query so
modules can flush dirty state. This is currently done ad hoc in modules
handlers. For example, RAM syncs its dirty bitmap in its save_complete
handler.
This should be a general concept relevant for any module, so extract it
to migration core instead by running a final save_query_pending before
switchover.
The final query requires special handling by modules (e.g., it's called
with BQL locked, during VM stop), so extend save_query_pending
SaveVMHandlers callback and qemu_savevm_query_pending() with a "final"
flag so migration modules can tell the last pending query during
switchover from periodic iteration queries.
Reviewed-by: Peter Xu <[email protected]>
Signed-off-by: Avihai Horon <[email protected]>
Reviewed-by: Jason J. Herne <[email protected]>
Link:
https://lore.kernel.org/qemu-devel/[email protected]
Signed-off-by: Cédric Le Goater <[email protected]>
---
include/migration/register.h | 41 ++++++++++++++++++----------------
migration/savevm.h | 3 ++-
hw/s390x/s390-stattrib.c | 2 +-
hw/vfio/migration.c | 11 ++++++---
migration/block-dirty-bitmap.c | 11 ++++++---
migration/migration.c | 14 ++++++++++--
migration/ram.c | 40 +++++++++++++++++++--------------
migration/savevm.c | 20 +++++++++++++----
hw/vfio/trace-events | 2 +-
migration/trace-events | 2 +-
10 files changed, 94 insertions(+), 52 deletions(-)
diff --git a/include/migration/register.h b/include/migration/register.h
index
5e5e0ee432e537781ce78e84b2ff2d81c33e8f37..6f632123f1d0710de0556e68ba53e068a1e02640
100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -171,6 +171,28 @@ typedef struct SaveVMHandlers {
*/
bool (*is_active_iterate)(void *opaque);
+ /**
+ * @save_query_pending
+ *
+ * This estimates the remaining data to transfer on the source side.
+ *
+ * When @exact is true, a module must report accurate results. When
+ * @exact is false, a module may report estimates.
+ *
+ * It's highly recommended that modules implement a faster version of
+ * the query path (for example, by proper caching on the counters) if
+ * an accurate query will be time-consuming.
+ *
+ * @opaque: data pointer passed to register_savevm_live()
+ * @pending: pointer to a MigPendingData struct
+ * @exact: set to true for an accurate (slow) query
+ * @final: set to true for the final query during switchover. When final is
+ * true, the query is called with BQL locked. Otherwise, it's called with
+ * BQL unlocked.
+ */
+ void (*save_query_pending)(void *opaque, MigPendingData *pending,
+ bool exact, bool final);
+
/* This runs outside the BQL in the migration case, and
* within the lock in the savevm case. The callback had better only
* use data that is local to the migration thread or protected
@@ -210,25 +232,6 @@ typedef struct SaveVMHandlers {
*/
bool (*save_postcopy_prepare)(QEMUFile *f, void *opaque, Error **errp);
- /**
- * @save_query_pending
- *
- * This estimates the remaining data to transfer on the source side.
- *
- * When @exact is true, a module must report accurate results. When
- * @exact is false, a module may report estimates.
- *
- * It's highly recommended that modules implement a faster version of
- * the query path (for example, by proper caching on the counters) if
- * an accurate query will be time-consuming.
- *
- * @opaque: data pointer passed to register_savevm_live()
- * @pending: pointer to a MigPendingData struct
- * @exact: set to true for an accurate (slow) query
- */
- void (*save_query_pending)(void *opaque, MigPendingData *pending,
- bool exact);
-
/**
* @load_state
*
diff --git a/migration/savevm.h b/migration/savevm.h
index
b6bb4fa977fae84ed121b0668cdefdf49e3d9c67..81abd96dda94c6c95cc40b3c188df23d11c19463
100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -45,7 +45,8 @@ int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy);
void qemu_savevm_state_cleanup(void);
void qemu_savevm_state_complete_postcopy(QEMUFile *f);
int qemu_savevm_state_complete_precopy(MigrationState *s, Error **errp);
-void qemu_savevm_query_pending(MigPendingData *pending, bool exact);
+void qemu_savevm_query_pending_iter(MigPendingData *pending, bool exact);
+void qemu_savevm_query_pending_final(MigPendingData *pending);
int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool
in_postcopy);
bool qemu_savevm_state_postcopy_prepare(QEMUFile *f, Error **errp);
void qemu_savevm_state_end(QEMUFile *f);
diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
index
c334714b31ab4313e26c0cf57091a8cb21e55306..b0f04eb30c7f73e5dd91b1345d71a14cb404a4eb
100644
--- a/hw/s390x/s390-stattrib.c
+++ b/hw/s390x/s390-stattrib.c
@@ -190,7 +190,7 @@ static int cmma_save_setup(QEMUFile *f, void *opaque, Error
**errp)
}
static void cmma_state_pending(void *opaque, MigPendingData *pending,
- bool exact)
+ bool exact, bool final)
{
S390StAttribState *sas = S390_STATTRIB(opaque);
S390StAttribClass *sac = S390_STATTRIB_GET_CLASS(sas);
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index
fb12b9717f773fdde657911517de9d74c1eb3931..195498845ed95d6a55cf68537a100b4e83221fb0
100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -622,13 +622,18 @@ static void vfio_state_pending_sync(VFIODevice *vbasedev)
}
static void vfio_state_pending(void *opaque, MigPendingData *pending,
- bool exact)
+ bool exact, bool final)
{
VFIODevice *vbasedev = opaque;
VFIOMigration *migration = vbasedev->migration;
uint64_t precopy_size, stopcopy_size;
- if (exact) {
+ /*
+ * The final pending query runs during switchover downtime. VFIO does not
+ * need a fresh device pending-data query then to get the latest dirty
+ * data, so avoid the extra work and report the cached counters below.
+ */
+ if (exact && !final) {
vfio_state_pending_sync(vbasedev);
}
@@ -646,7 +651,7 @@ static void vfio_state_pending(void *opaque, MigPendingData
*pending,
trace_vfio_state_pending(vbasedev->name, migration->stopcopy_size,
migration->precopy_init_size,
- migration->precopy_dirty_size, exact);
+ migration->precopy_dirty_size, exact, final);
}
static bool vfio_is_active_iterate(void *opaque)
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index
7ef3759e5348a6907c0fb3773e2c6cbb0850632e..cba54e25cd50da924e1958bdfc23ce060784b0ac
100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -767,13 +767,16 @@ static int dirty_bitmap_save_complete(QEMUFile *f, void
*opaque)
}
static void dirty_bitmap_state_pending(void *opaque, MigPendingData *data,
- bool exact)
+ bool exact, bool final)
{
DBMSaveState *s = &((DBMState *)opaque)->save;
SaveBitmapState *dbms;
uint64_t pending = 0;
- bql_lock();
+ /* Final pending query is called with BQL locked */
+ if (!final) {
+ bql_lock();
+ }
QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
uint64_t gran = bdrv_dirty_bitmap_granularity(dbms->bitmap);
@@ -783,7 +786,9 @@ static void dirty_bitmap_state_pending(void *opaque,
MigPendingData *data,
pending += DIV_ROUND_UP(sectors * BDRV_SECTOR_SIZE, gran);
}
- bql_unlock();
+ if (!final) {
+ bql_unlock();
+ }
trace_dirty_bitmap_state_pending(pending);
diff --git a/migration/migration.c b/migration/migration.c
index
0f9e891cf85c8e5074f7f9800515e244fa413fa2..ea0b236597087cc005541e1916ceabb229afab4b
100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2787,12 +2787,22 @@ static bool migration_switchover_prepare(MigrationState
*s)
static bool migration_switchover_start(MigrationState *s, Error **errp)
{
ERRP_GUARD();
+ MigPendingData pending = {};
if (!migration_switchover_prepare(s)) {
error_setg(errp, "Switchover is interrupted");
return false;
}
+ /*
+ * The final query to the whole system on dirty data to make sure we
+ * collect the latest status of the VM. For precopy, source QEMU will
+ * dump all the dirty data during switchover. For postcopy, this will
+ * properly update all the dirty bitmaps to finally generate the
+ * correct discard bitmaps; see ram_postcopy_send_discard_bitmap().
+ */
+ qemu_savevm_query_pending_final(&pending);
+
/* Inactivate disks except in COLO */
if (!migrate_colo()) {
/*
@@ -3285,7 +3295,7 @@ static void migration_iteration_go_next(MigPendingData
*pending)
/*
* Do a slow sync first before boosting the iteration count.
*/
- qemu_savevm_query_pending(pending, true);
+ qemu_savevm_query_pending_iter(pending, true);
/*
* Update the dirty information for the whole system for this
@@ -3336,7 +3346,7 @@ static MigIterateState
migration_iteration_run(MigrationState *s)
bool complete_ready;
/* Fast path - get the estimated amount of pending data */
- qemu_savevm_query_pending(&pending, false);
+ qemu_savevm_query_pending_iter(&pending, false);
if (in_postcopy) {
/*
diff --git a/migration/ram.c b/migration/ram.c
index
55aa9b47dc8ab8463fd1aeba19726a493b81b070..8918b2f03b6fbcc98532bb45ab905f9bac59384c
100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2686,9 +2686,6 @@ void ram_postcopy_send_discard_bitmap(MigrationState *ms)
RCU_READ_LOCK_GUARD();
- /* This should be our last sync, the src is now paused */
- migration_bitmap_sync_precopy(true);
-
/* Easiest way to make sure we don't resume in the middle of a host-page
*/
rs->pss[RAM_CHANNEL_PRECOPY].last_sent_block = NULL;
rs->last_seen_block = NULL;
@@ -3376,10 +3373,6 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
rs->last_stage = !migration_in_colo_state();
WITH_RCU_READ_LOCK_GUARD() {
- if (!migration_in_postcopy()) {
- migration_bitmap_sync_precopy(true);
- }
-
(Got some technical issues receiving QEMU ML mails that I'm not CCed to, so
replying here for Stefan's mail [1])
Seems like this patch overlooked the COLO case which calls ram_save_complete()
on each checkpoint and expects the RAM complete handler to sync RAM dirty
bitmap.
This patch moved this dirty sync from ram_save_complete() (the removed lines
above) into a qemu_savevm_query_pending_final() called only once at
migration_switchover_start().
I believe this is the root cause for the CI failure reported by Stefan, since I
was able to repro this locally [2] and the diff below [3] fixed the issue [4].
I have zero knowledge in COLO so would really appreciate some feedback from
Peter/COLO folks if that's a valid fix.
Thanks.
[1]
https://lore.kernel.org/qemu-devel/cajsp0qx-8l2csx5q95md5v6nte_4p+k8kr-qgbqkm2bbmtd...@mail.gmail.com/
[2]
QTEST_QEMU_BINARY=./qemu-system-x86_64 ./tests/qtest/migration-test -k --full
-r /x86_64/migration/colo
[...]
# Running /x86_64/migration/colo/plain/secondary_failover
# Using machine type: pc-q35-11.1
# starting QEMU: exec ./qemu-system-x86_64 -qtest unix:/tmp/qtest-1415533.sock
-qtest-log /dev/null -chardev socket,path=/tmp/qtest-1415533.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio
none -run-with exit-with-parent=on -accel kvm -accel tcg -machine pc-q35-11.1,
-name source,debug-threads=on -machine memory-backend=mig.mem -object
memory-backend-ram,id=mig.mem,size=150M,share=off -serial
file:/tmp/migration-test-GF36R3/src_serial -drive
if=none,id=d0,file=/tmp/migration-test-GF36R3/bootsect,format=raw -device
ide-hd,drive=d0,secs=1,cyls=1,heads=1 -snapshot 2>/dev/null -accel qtest
# starting QEMU: exec ./qemu-system-x86_64 -qtest unix:/tmp/qtest-1415533.sock
-qtest-log /dev/null -chardev socket,path=/tmp/qtest-1415533.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio
none -run-with exit-with-parent=on -accel kvm -accel tcg -machine pc-q35-11.1,
-name target,debug-threads=on -machine memory-backend=mig.mem -object
memory-backend-ram,id=mig.mem,size=150M,share=off -serial
file:/tmp/migration-test-GF36R3/dest_serial -incoming defer -drive
if=none,id=d0,file=/tmp/migration-test-GF36R3/bootsect,format=raw -device
ide-hd,drive=d0,secs=1,cyls=1,heads=1 -snapshot 2>/dev/null -a
ccel qtest
Memory content inconsistency at ea2000 first_byte = 53 last_byte = 53 current =
54 hit_edge = 0
Memory content inconsistency at ea3000 first_byte = 53 last_byte = 53 current =
54 hit_edge = 0
Memory content inconsistency at ea4000 first_byte = 53 last_byte = 53 current =
54 hit_edge = 0
Memory content inconsistency at ea5000 first_byte = 53 last_byte = 53 current =
54 hit_edge = 0
Memory content inconsistency at ea6000 first_byte = 53 last_byte = 53 current =
54 hit_edge = 0
Memory content inconsistency at ea7000 first_byte = 53 last_byte = 53 current =
54 hit_edge = 0
Memory content inconsistency at ea8000 first_byte = 53 last_byte = 53 current =
54 hit_edge = 0
Memory content inconsistency at ea9000 first_byte = 53 last_byte = 53 current =
54 hit_edge = 0
Memory content inconsistency at eaa000 first_byte = 53 last_byte = 53 current =
54 hit_edge = 0
Memory content inconsistency at eab000 first_byte = 53 last_byte = 53 current =
54 hit_edge = 0
and in another 13488 pages**
ERROR:../tests/qtest/migration/framework.c:190:check_guests_ram: assertion
failed: (bad == 0)
not ok /x86_64/migration/colo/plain/secondary_failover -
ERROR:../tests/qtest/migration/framework.c:190:check_guests_ram: assertion
failed: (bad == 0)
Bail out!
[3]
diff --git a/migration/colo.c b/migration/colo.c
index 2d36f933cf..5c486c09ef 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -409,6 +409,7 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
QEMUFile *fb)
{
Error *local_err = NULL;
+ MigPendingData pending = {};
int ret = -1;
colo_send_message(s->to_dst_file, COLO_MESSAGE_CHECKPOINT_REQUEST,
@@ -465,6 +466,18 @@ static int colo_do_checkpoint_transaction(MigrationState
*s,
if (migrate_auto_converge()) {
mig_throttle_counter_reset();
}
+
+ /*
+ * Run the final pending query so migration modules can flush their dirty
+ * state (e.g., RAM syncs its dirty bitmap) before this checkpoint's live
+ * state is saved. Unlike a regular switchover, COLO reaches completion
+ * repeatedly for every checkpoint, so this must be done on each one.
+ */
+ if (!qemu_savevm_query_pending_final(s, &pending, &local_err)) {
+ bql_unlock();
+ goto out;
+ }
+
/*
* Only save VM's live state, which not including device state.
* TODO: We may need a timeout mechanism to prevent COLO process
[4]
QTEST_QEMU_BINARY=./qemu-system-x86_64 ./tests/qtest/migration-test -k --full
-r /x86_64/migration/colo
[...]
# Running /x86_64/migration/colo/plain/secondary_failover
# Using machine type: pc-q35-11.1
# starting QEMU: exec ./qemu-system-x86_64 -qtest unix:/tmp/qtest-1417324.sock
-qtest-log /dev/null -chardev socket,path=/tmp/qtest-1417324.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio
none -run-with exit-with-parent=on -accel kvm -accel tcg -machine pc-q35-11.1,
-name source,debug-threads=on -machine memory-backend=mig.mem -object
memory-backend-ram,id=mig.mem,size=150M,share=off -serial
file:/tmp/migration-test-0TMNR3/src_serial -drive
if=none,id=d0,file=/tmp/migration-test-0TMNR3/bootsect,format=raw -device
ide-hd,drive=d0,secs=1,cyls=1,heads=1 -snapshot 2>/dev/null -accel qtest
# starting QEMU: exec ./qemu-system-x86_64 -qtest unix:/tmp/qtest-1417324.sock
-qtest-log /dev/null -chardev socket,path=/tmp/qtest-1417324.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio
none -run-with exit-with-parent=on -accel kvm -accel tcg -machine pc-q35-11.1,
-name target,debug-threads=on -machine memory-backend=mig.mem -object
memory-backend-ram,id=mig.mem,size=150M,share=off -serial
file:/tmp/migration-test-0TMNR3/dest_serial -incoming defer -drive
if=none,id=d0,file=/tmp/migration-test-0TMNR3/bootsect,format=raw -device
ide-hd,drive=d0,secs=1,cyls=1,heads=1 -snapshot 2>/dev/null -a
ccel qtest
ok 2 /x86_64/migration/colo/plain/secondary_failover
