On Fri, 3 Jul 2026 11:05:39 +0300 Avihai Horon <[email protected]> wrote:
> 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/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 Hello, This patch looks good to me. Regards, Lukas Straub > > [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 > > > [...]
pgpu0guIS0tEc.pgp
Description: OpenPGP digital signature
