The following changes since commit 13356edb87506c148b163b8c7eb0695647d00c2a: Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging (2023-01-24 09:45:33 +0000) are available in the Git repository at: https://gitlab.com/juan.quintela/qemu.git tags/next-pull-request for you to fetch changes up to 07181bd32c39007855e25eb5675e9210e9ccd8da: migration: simplify migration_iteration_run() (2023-01-30 08:55:04 +0100) ---------------------------------------------------------------- Migration PULL request Hi - Peter Xu fix for crash on hosts with guest page bigger than host - The parts that are reviewed of my vfio series Please, apply. ---------------------------------------------------------------- Juan Quintela (4): migration: No save_live_pending() method uses the QEMUFile parameter migration: Split save_live_pending() into state_pending_* migration: Remove unused threshold_size parameter migration: simplify migration_iteration_run() Peter Xu (1): migration: Fix migration crash when target psize larger than host docs/devel/migration.rst | 18 ++++++----- docs/devel/vfio-migration.rst | 4 +-- include/migration/register.h | 17 ++++++----- migration/savevm.h | 10 +++--- hw/s390x/s390-stattrib.c | 11 ++++--- hw/vfio/migration.c | 20 ++++++------ migration/block-dirty-bitmap.c | 14 ++++----- migration/block.c | 13 ++++---- migration/migration.c | 42 ++++++++++++++----------- migration/ram.c | 56 +++++++++++++++++++++++++++------- migration/savevm.c | 39 ++++++++++++++++++----- hw/vfio/trace-events | 2 +- migration/trace-events | 7 +++-- 13 files changed, 163 insertions(+), 90 deletions(-) -- 2.39.1 From: Peter Xu <peterx@redhat.com> Commit d9e474ea56 overlooked the case where the target psize is even larger than the host psize. One example is Alpha has 8K page size and migration will start to crash the source QEMU when running Alpha migration on x86. Fix it by detecting that case and set host start/end just to cover the single page to be migrated. This will slightly optimize the common case where host psize equals to guest psize so we don't even need to do the roundups, but that's trivial. Cc: qemu-stable@nongnu.org Reported-by: Thomas Huth <thuth@redhat.com> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1456 Fixes: d9e474ea56 ("migration: Teach PSS about host page") Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/ram.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index XXXXXXX..XXXXXXX 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -XXX,XX +XXX,XX @@ static void pss_host_page_prepare(PageSearchStatus *pss) size_t guest_pfns = qemu_ram_pagesize(pss->block) >> TARGET_PAGE_BITS; pss->host_page_sending = true; - pss->host_page_start = ROUND_DOWN(pss->page, guest_pfns); - pss->host_page_end = ROUND_UP(pss->page + 1, guest_pfns); + if (guest_pfns <= 1) { + /* + * This covers both when guest psize == host psize, or when guest + * has larger psize than the host (guest_pfns==0). + * + * For the latter, we always send one whole guest page per + * iteration of the host page (example: an Alpha VM on x86 host + * will have guest psize 8K while host psize 4K). + */ + pss->host_page_start = pss->page; + pss->host_page_end = pss->page + 1; + } else { + /* + * The host page spans over multiple guest pages, we send them + * within the same host page iteration. + */ + pss->host_page_start = ROUND_DOWN(pss->page, guest_pfns); + pss->host_page_end = ROUND_UP(pss->page + 1, guest_pfns); + } } /* -- 2.39.1 So remove it everywhere. Signed-off-by: Juan Quintela <quintela@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- include/migration/register.h | 2 +- migration/savevm.h | 2 +- hw/s390x/s390-stattrib.c | 3 +-- hw/vfio/migration.c | 2 +- migration/block-dirty-bitmap.c | 2 +- migration/block.c | 2 +- migration/migration.c | 2 +- migration/ram.c | 2 +- migration/savevm.c | 4 ++-- 9 files changed, 10 insertions(+), 11 deletions(-) diff --git a/include/migration/register.h b/include/migration/register.h index XXXXXXX..XXXXXXX 100644 --- a/include/migration/register.h +++ b/include/migration/register.h @@ -XXX,XX +XXX,XX @@ typedef struct SaveVMHandlers { /* This runs outside the iothread lock! */ int (*save_setup)(QEMUFile *f, void *opaque); - void (*save_live_pending)(QEMUFile *f, void *opaque, + void (*save_live_pending)(void *opaque, uint64_t threshold_size, uint64_t *res_precopy_only, uint64_t *res_compatible, diff --git a/migration/savevm.h b/migration/savevm.h index XXXXXXX..XXXXXXX 100644 --- a/migration/savevm.h +++ b/migration/savevm.h @@ -XXX,XX +XXX,XX @@ void qemu_savevm_state_cleanup(void); void qemu_savevm_state_complete_postcopy(QEMUFile *f); int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only, bool inactivate_disks); -void qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size, +void qemu_savevm_state_pending(uint64_t max_size, uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only); diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c index XXXXXXX..XXXXXXX 100644 --- a/hw/s390x/s390-stattrib.c +++ b/hw/s390x/s390-stattrib.c @@ -XXX,XX +XXX,XX @@ static int cmma_save_setup(QEMUFile *f, void *opaque) return 0; } -static void cmma_save_pending(QEMUFile *f, void *opaque, uint64_t max_size, - uint64_t *res_precopy_only, +static void cmma_save_pending(uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only) { diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index XXXXXXX..XXXXXXX 100644 --- a/hw/vfio/migration.c +++ b/hw/vfio/migration.c @@ -XXX,XX +XXX,XX @@ static void vfio_save_cleanup(void *opaque) trace_vfio_save_cleanup(vbasedev->name); } -static void vfio_save_pending(QEMUFile *f, void *opaque, +static void vfio_save_pending(void *opaque, uint64_t threshold_size, uint64_t *res_precopy_only, uint64_t *res_compatible, diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c index XXXXXXX..XXXXXXX 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c @@ -XXX,XX +XXX,XX @@ static int dirty_bitmap_save_complete(QEMUFile *f, void *opaque) return 0; } -static void dirty_bitmap_save_pending(QEMUFile *f, void *opaque, +static void dirty_bitmap_save_pending(void *opaque, uint64_t max_size, uint64_t *res_precopy_only, uint64_t *res_compatible, diff --git a/migration/block.c b/migration/block.c index XXXXXXX..XXXXXXX 100644 --- a/migration/block.c +++ b/migration/block.c @@ -XXX,XX +XXX,XX @@ static int block_save_complete(QEMUFile *f, void *opaque) return 0; } -static void block_save_pending(QEMUFile *f, void *opaque, uint64_t max_size, +static void block_save_pending(void *opaque, uint64_t max_size, uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only) diff --git a/migration/migration.c b/migration/migration.c index XXXXXXX..XXXXXXX 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -XXX,XX +XXX,XX @@ static MigIterateState migration_iteration_run(MigrationState *s) uint64_t pending_size, pend_pre, pend_compat, pend_post; bool in_postcopy = s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE; - qemu_savevm_state_pending(s->to_dst_file, s->threshold_size, &pend_pre, + qemu_savevm_state_pending(s->threshold_size, &pend_pre, &pend_compat, &pend_post); pending_size = pend_pre + pend_compat + pend_post; diff --git a/migration/ram.c b/migration/ram.c index XXXXXXX..XXXXXXX 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -XXX,XX +XXX,XX @@ static int ram_save_complete(QEMUFile *f, void *opaque) return 0; } -static void ram_save_pending(QEMUFile *f, void *opaque, uint64_t max_size, +static void ram_save_pending(void *opaque, uint64_t max_size, uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only) diff --git a/migration/savevm.c b/migration/savevm.c index XXXXXXX..XXXXXXX 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -XXX,XX +XXX,XX @@ flush: * the result is split into the amount for units that can and * for units that can't do postcopy. */ -void qemu_savevm_state_pending(QEMUFile *f, uint64_t threshold_size, +void qemu_savevm_state_pending(uint64_t threshold_size, uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only) @@ -XXX,XX +XXX,XX @@ void qemu_savevm_state_pending(QEMUFile *f, uint64_t threshold_size, continue; } } - se->ops->save_live_pending(f, se->opaque, threshold_size, + se->ops->save_live_pending(se->opaque, threshold_size, res_precopy_only, res_compatible, res_postcopy_only); } -- 2.39.1 We split the function into to: - state_pending_estimate: We estimate the remaining state size without stopping the machine. - state pending_exact: We calculate the exact amount of remaining state. The only "device" that implements different functions for _estimate() and _exact() is ram. Signed-off-by: Juan Quintela <quintela@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- docs/devel/migration.rst | 18 ++++++++------- docs/devel/vfio-migration.rst | 4 ++-- include/migration/register.h | 19 +++++++++------ migration/savevm.h | 12 ++++++---- hw/s390x/s390-stattrib.c | 10 ++++---- hw/vfio/migration.c | 21 +++++++++-------- migration/block-dirty-bitmap.c | 15 ++++++------ migration/block.c | 13 ++++++----- migration/migration.c | 20 +++++++++++----- migration/ram.c | 35 ++++++++++++++++++++-------- migration/savevm.c | 42 +++++++++++++++++++++++++++------- hw/vfio/trace-events | 2 +- migration/trace-events | 7 +++--- 13 files changed, 143 insertions(+), 75 deletions(-) diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/devel/migration.rst +++ b/docs/devel/migration.rst @@ -XXX,XX +XXX,XX @@ An iterative device must provide: - A ``load_setup`` function that initialises the data structures on the destination. - - A ``save_live_pending`` function that is called repeatedly and must - indicate how much more data the iterative data must save. The core - migration code will use this to determine when to pause the CPUs - and complete the migration. + - A ``state_pending_exact`` function that indicates how much more + data we must save. The core migration code will use this to + determine when to pause the CPUs and complete the migration. - - A ``save_live_iterate`` function (called after ``save_live_pending`` - when there is significant data still to be sent). It should send - a chunk of data until the point that stream bandwidth limits tell it - to stop. Each call generates one section. + - A ``state_pending_estimate`` function that indicates how much more + data we must save. When the estimated amount is smaller than the + threshold, we call ``state_pending_exact``. + + - A ``save_live_iterate`` function should send a chunk of data until + the point that stream bandwidth limits tell it to stop. Each call + generates one section. - A ``save_live_complete_precopy`` function that must transmit the last section for the device containing any remaining data. diff --git a/docs/devel/vfio-migration.rst b/docs/devel/vfio-migration.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/devel/vfio-migration.rst +++ b/docs/devel/vfio-migration.rst @@ -XXX,XX +XXX,XX @@ VFIO implements the device hooks for the iterative approach as follows: * A ``load_setup`` function that sets up the migration region on the destination and sets _RESUMING flag in the VFIO device state. -* A ``save_live_pending`` function that reads pending_bytes from the vendor +* A ``state_pending_exact`` function that reads pending_bytes from the vendor driver, which indicates the amount of data that the vendor driver has yet to save for the VFIO device. @@ -XXX,XX +XXX,XX @@ Live migration save path (RUNNING, _SETUP, _RUNNING|_SAVING) | (RUNNING, _ACTIVE, _RUNNING|_SAVING) - If device is active, get pending_bytes by .save_live_pending() + If device is active, get pending_bytes by .state_pending_exact() If total pending_bytes >= threshold_size, call .save_live_iterate() Data of VFIO device for pre-copy phase is copied Iterate till total pending bytes converge and are less than threshold diff --git a/include/migration/register.h b/include/migration/register.h index XXXXXXX..XXXXXXX 100644 --- a/include/migration/register.h +++ b/include/migration/register.h @@ -XXX,XX +XXX,XX @@ typedef struct SaveVMHandlers { /* This runs outside the iothread lock! */ int (*save_setup)(QEMUFile *f, void *opaque); - void (*save_live_pending)(void *opaque, - uint64_t threshold_size, - uint64_t *res_precopy_only, - uint64_t *res_compatible, - uint64_t *res_postcopy_only); /* Note for save_live_pending: * - res_precopy_only is for data which must be migrated in precopy phase * or in stopped state, in other words - before target vm start @@ -XXX,XX +XXX,XX @@ typedef struct SaveVMHandlers { * Sum of res_postcopy_only, res_compatible and res_postcopy_only is the * whole amount of pending data. */ - - + /* This estimates the remaining data to transfer */ + void (*state_pending_estimate)(void *opaque, + uint64_t threshold_size, + uint64_t *res_precopy_only, + uint64_t *res_compatible, + uint64_t *res_postcopy_only); + /* This calculate the exact remaining data to transfer */ + void (*state_pending_exact)(void *opaque, + uint64_t threshold_size, + uint64_t *res_precopy_only, + uint64_t *res_compatible, + uint64_t *res_postcopy_only); LoadStateHandler *load_state; int (*load_setup)(QEMUFile *f, void *opaque); int (*load_cleanup)(void *opaque); diff --git a/migration/savevm.h b/migration/savevm.h index XXXXXXX..XXXXXXX 100644 --- a/migration/savevm.h +++ b/migration/savevm.h @@ -XXX,XX +XXX,XX @@ void qemu_savevm_state_cleanup(void); void qemu_savevm_state_complete_postcopy(QEMUFile *f); int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only, bool inactivate_disks); -void qemu_savevm_state_pending(uint64_t max_size, - uint64_t *res_precopy_only, - uint64_t *res_compatible, - uint64_t *res_postcopy_only); +void qemu_savevm_state_pending_exact(uint64_t threshold_size, + uint64_t *res_precopy_only, + uint64_t *res_compatible, + uint64_t *res_postcopy_only); +void qemu_savevm_state_pending_estimate(uint64_t thershold_size, + uint64_t *res_precopy_only, + uint64_t *res_compatible, + uint64_t *res_postcopy_only); void qemu_savevm_send_ping(QEMUFile *f, uint32_t value); void qemu_savevm_send_open_return_path(QEMUFile *f); int qemu_savevm_send_packaged(QEMUFile *f, const uint8_t *buf, size_t len); diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c index XXXXXXX..XXXXXXX 100644 --- a/hw/s390x/s390-stattrib.c +++ b/hw/s390x/s390-stattrib.c @@ -XXX,XX +XXX,XX @@ static int cmma_save_setup(QEMUFile *f, void *opaque) return 0; } -static void cmma_save_pending(uint64_t *res_precopy_only, - uint64_t *res_compatible, - uint64_t *res_postcopy_only) +static void cmma_state_pending(void * opaque, + uint64_t *res_precopy_only, + uint64_t *res_compatible, + uint64_t *res_postcopy_only) { S390StAttribState *sas = S390_STATTRIB(opaque); S390StAttribClass *sac = S390_STATTRIB_GET_CLASS(sas); @@ -XXX,XX +XXX,XX @@ static SaveVMHandlers savevm_s390_stattrib_handlers = { .save_setup = cmma_save_setup, .save_live_iterate = cmma_save_iterate, .save_live_complete_precopy = cmma_save_complete, - .save_live_pending = cmma_save_pending, + .state_pending_exact = cmma_state_pending, + .state_pending_estimate = cmma_state_pending, .save_cleanup = cmma_save_cleanup, .load_state = cmma_load, .is_active = cmma_active, diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index XXXXXXX..XXXXXXX 100644 --- a/hw/vfio/migration.c +++ b/hw/vfio/migration.c @@ -XXX,XX +XXX,XX @@ static void vfio_save_cleanup(void *opaque) trace_vfio_save_cleanup(vbasedev->name); } -static void vfio_save_pending(void *opaque, - uint64_t threshold_size, - uint64_t *res_precopy_only, - uint64_t *res_compatible, - uint64_t *res_postcopy_only) +static void vfio_state_pending(void *opaque, + uint64_t threshold_size, + uint64_t *res_precopy_only, + uint64_t *res_compatible, + uint64_t *res_postcopy_only) { VFIODevice *vbasedev = opaque; VFIOMigration *migration = vbasedev->migration; @@ -XXX,XX +XXX,XX @@ static void vfio_save_pending(void *opaque, *res_precopy_only += migration->pending_bytes; - trace_vfio_save_pending(vbasedev->name, *res_precopy_only, + trace_vfio_state_pending(vbasedev->name, *res_precopy_only, *res_postcopy_only, *res_compatible); } @@ -XXX,XX +XXX,XX @@ static int vfio_save_iterate(QEMUFile *f, void *opaque) } /* - * Reset pending_bytes as .save_live_pending is not called during savevm or - * snapshot case, in such case vfio_update_pending() at the start of this - * function updates pending_bytes. + * Reset pending_bytes as state_pending* are not called during + * savevm or snapshot case, in such case vfio_update_pending() at + * the start of this function updates pending_bytes. */ migration->pending_bytes = 0; trace_vfio_save_iterate(vbasedev->name, data_size); @@ -XXX,XX +XXX,XX @@ static int vfio_load_state(QEMUFile *f, void *opaque, int version_id) static SaveVMHandlers savevm_vfio_handlers = { .save_setup = vfio_save_setup, .save_cleanup = vfio_save_cleanup, - .save_live_pending = vfio_save_pending, + .state_pending_exact = vfio_state_pending, + .state_pending_estimate = vfio_state_pending, .save_live_iterate = vfio_save_iterate, .save_live_complete_precopy = vfio_save_complete_precopy, .save_state = vfio_save_state, diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c index XXXXXXX..XXXXXXX 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c @@ -XXX,XX +XXX,XX @@ static int dirty_bitmap_save_complete(QEMUFile *f, void *opaque) return 0; } -static void dirty_bitmap_save_pending(void *opaque, - uint64_t max_size, - uint64_t *res_precopy_only, - uint64_t *res_compatible, - uint64_t *res_postcopy_only) +static void dirty_bitmap_state_pending(void *opaque, + uint64_t max_size, + uint64_t *res_precopy_only, + uint64_t *res_compatible, + uint64_t *res_postcopy_only) { DBMSaveState *s = &((DBMState *)opaque)->save; SaveBitmapState *dbms; @@ -XXX,XX +XXX,XX @@ static void dirty_bitmap_save_pending(void *opaque, qemu_mutex_unlock_iothread(); - trace_dirty_bitmap_save_pending(pending, max_size); + trace_dirty_bitmap_state_pending(pending); *res_postcopy_only += pending; } @@ -XXX,XX +XXX,XX @@ static SaveVMHandlers savevm_dirty_bitmap_handlers = { .save_live_complete_postcopy = dirty_bitmap_save_complete, .save_live_complete_precopy = dirty_bitmap_save_complete, .has_postcopy = dirty_bitmap_has_postcopy, - .save_live_pending = dirty_bitmap_save_pending, + .state_pending_exact = dirty_bitmap_state_pending, + .state_pending_estimate = dirty_bitmap_state_pending, .save_live_iterate = dirty_bitmap_save_iterate, .is_active_iterate = dirty_bitmap_is_active_iterate, .load_state = dirty_bitmap_load, diff --git a/migration/block.c b/migration/block.c index XXXXXXX..XXXXXXX 100644 --- a/migration/block.c +++ b/migration/block.c @@ -XXX,XX +XXX,XX @@ static int block_save_complete(QEMUFile *f, void *opaque) return 0; } -static void block_save_pending(void *opaque, uint64_t max_size, - uint64_t *res_precopy_only, - uint64_t *res_compatible, - uint64_t *res_postcopy_only) +static void block_state_pending(void *opaque, uint64_t max_size, + uint64_t *res_precopy_only, + uint64_t *res_compatible, + uint64_t *res_postcopy_only) { /* Estimate pending number of bytes to send */ uint64_t pending; @@ -XXX,XX +XXX,XX @@ static void block_save_pending(void *opaque, uint64_t max_size, pending = BLK_MIG_BLOCK_SIZE; } - trace_migration_block_save_pending(pending); + trace_migration_block_state_pending(pending); /* We don't do postcopy */ *res_precopy_only += pending; } @@ -XXX,XX +XXX,XX @@ static SaveVMHandlers savevm_block_handlers = { .save_setup = block_save_setup, .save_live_iterate = block_save_iterate, .save_live_complete_precopy = block_save_complete, - .save_live_pending = block_save_pending, + .state_pending_exact = block_state_pending, + .state_pending_estimate = block_state_pending, .load_state = block_load, .save_cleanup = block_migration_cleanup, .is_active = block_is_active, diff --git a/migration/migration.c b/migration/migration.c index XXXXXXX..XXXXXXX 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -XXX,XX +XXX,XX @@ typedef enum { */ static MigIterateState migration_iteration_run(MigrationState *s) { - uint64_t pending_size, pend_pre, pend_compat, pend_post; + uint64_t pend_pre, pend_compat, pend_post; bool in_postcopy = s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE; - qemu_savevm_state_pending(s->threshold_size, &pend_pre, - &pend_compat, &pend_post); - pending_size = pend_pre + pend_compat + pend_post; + qemu_savevm_state_pending_estimate(s->threshold_size, &pend_pre, + &pend_compat, &pend_post); + uint64_t pending_size = pend_pre + pend_compat + pend_post; - trace_migrate_pending(pending_size, s->threshold_size, - pend_pre, pend_compat, pend_post); + trace_migrate_pending_estimate(pending_size, s->threshold_size, + pend_pre, pend_compat, pend_post); + + if (pend_pre + pend_compat <= s->threshold_size) { + qemu_savevm_state_pending_exact(s->threshold_size, &pend_pre, + &pend_compat, &pend_post); + pending_size = pend_pre + pend_compat + pend_post; + trace_migrate_pending_exact(pending_size, s->threshold_size, + pend_pre, pend_compat, pend_post); + } if (pending_size && pending_size >= s->threshold_size) { /* Still a significant amount to transfer */ diff --git a/migration/ram.c b/migration/ram.c index XXXXXXX..XXXXXXX 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -XXX,XX +XXX,XX @@ static int ram_save_complete(QEMUFile *f, void *opaque) return 0; } -static void ram_save_pending(void *opaque, uint64_t max_size, - uint64_t *res_precopy_only, - uint64_t *res_compatible, - uint64_t *res_postcopy_only) +static void ram_state_pending_estimate(void *opaque, uint64_t max_size, + uint64_t *res_precopy_only, + uint64_t *res_compatible, + uint64_t *res_postcopy_only) { RAMState **temp = opaque; RAMState *rs = *temp; - uint64_t remaining_size; - remaining_size = rs->migration_dirty_pages * TARGET_PAGE_SIZE; + uint64_t remaining_size = rs->migration_dirty_pages * TARGET_PAGE_SIZE; - if (!migration_in_postcopy() && - remaining_size < max_size) { + if (migrate_postcopy_ram()) { + /* We can do postcopy, and all the data is postcopiable */ + *res_postcopy_only += remaining_size; + } else { + *res_precopy_only += remaining_size; + } +} + +static void ram_state_pending_exact(void *opaque, uint64_t max_size, + uint64_t *res_precopy_only, + uint64_t *res_compatible, + uint64_t *res_postcopy_only) +{ + RAMState **temp = opaque; + RAMState *rs = *temp; + + uint64_t remaining_size = rs->migration_dirty_pages * TARGET_PAGE_SIZE; + + if (!migration_in_postcopy()) { qemu_mutex_lock_iothread(); WITH_RCU_READ_LOCK_GUARD() { migration_bitmap_sync_precopy(rs); @@ -XXX,XX +XXX,XX @@ static SaveVMHandlers savevm_ram_handlers = { .save_live_complete_postcopy = ram_save_complete, .save_live_complete_precopy = ram_save_complete, .has_postcopy = ram_has_postcopy, - .save_live_pending = ram_save_pending, + .state_pending_exact = ram_state_pending_exact, + .state_pending_estimate = ram_state_pending_estimate, .load_state = ram_load, .save_cleanup = ram_save_cleanup, .load_setup = ram_load_setup, diff --git a/migration/savevm.c b/migration/savevm.c index XXXXXXX..XXXXXXX 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -XXX,XX +XXX,XX @@ flush: * the result is split into the amount for units that can and * for units that can't do postcopy. */ -void qemu_savevm_state_pending(uint64_t threshold_size, - uint64_t *res_precopy_only, - uint64_t *res_compatible, - uint64_t *res_postcopy_only) +void qemu_savevm_state_pending_estimate(uint64_t threshold_size, + uint64_t *res_precopy_only, + uint64_t *res_compatible, + uint64_t *res_postcopy_only) { SaveStateEntry *se; @@ -XXX,XX +XXX,XX @@ void qemu_savevm_state_pending(uint64_t threshold_size, QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { - if (!se->ops || !se->ops->save_live_pending) { + if (!se->ops || !se->ops->state_pending_exact) { continue; } if (se->ops->is_active) { @@ -XXX,XX +XXX,XX @@ void qemu_savevm_state_pending(uint64_t threshold_size, continue; } } - se->ops->save_live_pending(se->opaque, threshold_size, - res_precopy_only, res_compatible, - res_postcopy_only); + se->ops->state_pending_exact(se->opaque, threshold_size, + res_precopy_only, res_compatible, + res_postcopy_only); + } +} + +void qemu_savevm_state_pending_exact(uint64_t threshold_size, + uint64_t *res_precopy_only, + uint64_t *res_compatible, + uint64_t *res_postcopy_only) +{ + SaveStateEntry *se; + + *res_precopy_only = 0; + *res_compatible = 0; + *res_postcopy_only = 0; + + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { + if (!se->ops || !se->ops->state_pending_estimate) { + continue; + } + if (se->ops->is_active) { + if (!se->ops->is_active(se->opaque)) { + continue; + } + } + se->ops->state_pending_estimate(se->opaque, threshold_size, + res_precopy_only, res_compatible, + res_postcopy_only); } } diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events index XXXXXXX..XXXXXXX 100644 --- a/hw/vfio/trace-events +++ b/hw/vfio/trace-events @@ -XXX,XX +XXX,XX @@ vfio_save_cleanup(const char *name) " (%s)" vfio_save_buffer(const char *name, uint64_t data_offset, uint64_t data_size, uint64_t pending) " (%s) Offset 0x%"PRIx64" size 0x%"PRIx64" pending 0x%"PRIx64 vfio_update_pending(const char *name, uint64_t pending) " (%s) pending 0x%"PRIx64 vfio_save_device_config_state(const char *name) " (%s)" -vfio_save_pending(const char *name, uint64_t precopy, uint64_t postcopy, uint64_t compatible) " (%s) precopy 0x%"PRIx64" postcopy 0x%"PRIx64" compatible 0x%"PRIx64 +vfio_state_pending(const char *name, uint64_t precopy, uint64_t postcopy, uint64_t compatible) " (%s) precopy 0x%"PRIx64" postcopy 0x%"PRIx64" compatible 0x%"PRIx64 vfio_save_iterate(const char *name, int data_size) " (%s) data_size %d" vfio_save_complete_precopy(const char *name) " (%s)" vfio_load_device_config_state(const char *name) " (%s)" diff --git a/migration/trace-events b/migration/trace-events index XXXXXXX..XXXXXXX 100644 --- a/migration/trace-events +++ b/migration/trace-events @@ -XXX,XX +XXX,XX @@ migrate_fd_cleanup(void) "" migrate_fd_error(const char *error_desc) "error=%s" migrate_fd_cancel(void) "" migrate_handle_rp_req_pages(const char *rbname, size_t start, size_t len) "in %s at 0x%zx len 0x%zx" -migrate_pending(uint64_t size, uint64_t max, uint64_t pre, uint64_t compat, uint64_t post) "pending size %" PRIu64 " max %" PRIu64 " (pre = %" PRIu64 " compat=%" PRIu64 " post=%" PRIu64 ")" +migrate_pending_exact(uint64_t size, uint64_t max, uint64_t pre, uint64_t compat, uint64_t post) "exact pending size %" PRIu64 " max %" PRIu64 " (pre = %" PRIu64 " compat=%" PRIu64 " post=%" PRIu64 ")" +migrate_pending_estimate(uint64_t size, uint64_t max, uint64_t pre, uint64_t compat, uint64_t post) "estimate pending size %" PRIu64 " max %" PRIu64 " (pre = %" PRIu64 " compat=%" PRIu64 " post=%" PRIu64 ")" migrate_send_rp_message(int msg_type, uint16_t len) "%d: len %d" migrate_send_rp_recv_bitmap(char *name, int64_t size) "block '%s' size 0x%"PRIi64 migration_completion_file_err(void) "" @@ -XXX,XX +XXX,XX @@ send_bitmap_bits(uint32_t flags, uint64_t start_sector, uint32_t nr_sectors, uin dirty_bitmap_save_iterate(int in_postcopy) "in postcopy: %d" dirty_bitmap_save_complete_enter(void) "" dirty_bitmap_save_complete_finish(void) "" -dirty_bitmap_save_pending(uint64_t pending, uint64_t max_size) "pending %" PRIu64 " max: %" PRIu64 +dirty_bitmap_state_pending(uint64_t pending) "pending %" PRIu64 dirty_bitmap_load_complete(void) "" dirty_bitmap_load_bits_enter(uint64_t first_sector, uint32_t nr_sectors) "chunk: %" PRIu64 " %" PRIu32 dirty_bitmap_load_bits_zeroes(void) "" @@ -XXX,XX +XXX,XX @@ migration_block_save_device_dirty(int64_t sector) "Error reading sector %" PRId6 migration_block_flush_blks(const char *action, int submitted, int read_done, int transferred) "%s submitted %d read_done %d transferred %d" migration_block_save(const char *mig_stage, int submitted, int transferred) "Enter save live %s submitted %d transferred %d" migration_block_save_complete(void) "Block migration completed" -migration_block_save_pending(uint64_t pending) "Enter save live pending %" PRIu64 +migration_block_state_pending(uint64_t pending) "Enter save live pending %" PRIu64 # page_cache.c migration_pagecache_init(int64_t max_num_items) "Setting cache buckets to %" PRId64 -- 2.39.1 Until previous commit, save_live_pending() was used for ram. Now with the split into state_pending_estimate() and state_pending_exact() it is not needed anymore, so remove them. Signed-off-by: Juan Quintela <quintela@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- include/migration/register.h | 2 -- migration/savevm.h | 6 ++---- hw/vfio/migration.c | 1 - migration/block-dirty-bitmap.c | 1 - migration/block.c | 2 +- migration/migration.c | 10 ++++------ migration/ram.c | 4 ++-- migration/savevm.c | 11 ++++------- migration/trace-events | 4 ++-- 9 files changed, 15 insertions(+), 26 deletions(-) diff --git a/include/migration/register.h b/include/migration/register.h index XXXXXXX..XXXXXXX 100644 --- a/include/migration/register.h +++ b/include/migration/register.h @@ -XXX,XX +XXX,XX @@ typedef struct SaveVMHandlers { */ /* This estimates the remaining data to transfer */ void (*state_pending_estimate)(void *opaque, - uint64_t threshold_size, uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only); /* This calculate the exact remaining data to transfer */ void (*state_pending_exact)(void *opaque, - uint64_t threshold_size, uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only); diff --git a/migration/savevm.h b/migration/savevm.h index XXXXXXX..XXXXXXX 100644 --- a/migration/savevm.h +++ b/migration/savevm.h @@ -XXX,XX +XXX,XX @@ void qemu_savevm_state_cleanup(void); void qemu_savevm_state_complete_postcopy(QEMUFile *f); int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only, bool inactivate_disks); -void qemu_savevm_state_pending_exact(uint64_t threshold_size, - uint64_t *res_precopy_only, +void qemu_savevm_state_pending_exact(uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only); -void qemu_savevm_state_pending_estimate(uint64_t thershold_size, - uint64_t *res_precopy_only, +void qemu_savevm_state_pending_estimate(uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only); void qemu_savevm_send_ping(QEMUFile *f, uint32_t value); diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index XXXXXXX..XXXXXXX 100644 --- a/hw/vfio/migration.c +++ b/hw/vfio/migration.c @@ -XXX,XX +XXX,XX @@ static void vfio_save_cleanup(void *opaque) } static void vfio_state_pending(void *opaque, - uint64_t threshold_size, uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only) diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c index XXXXXXX..XXXXXXX 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c @@ -XXX,XX +XXX,XX @@ static int dirty_bitmap_save_complete(QEMUFile *f, void *opaque) } static void dirty_bitmap_state_pending(void *opaque, - uint64_t max_size, uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only) diff --git a/migration/block.c b/migration/block.c index XXXXXXX..XXXXXXX 100644 --- a/migration/block.c +++ b/migration/block.c @@ -XXX,XX +XXX,XX @@ static int block_save_complete(QEMUFile *f, void *opaque) return 0; } -static void block_state_pending(void *opaque, uint64_t max_size, +static void block_state_pending(void *opaque, uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only) diff --git a/migration/migration.c b/migration/migration.c index XXXXXXX..XXXXXXX 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -XXX,XX +XXX,XX @@ static MigIterateState migration_iteration_run(MigrationState *s) uint64_t pend_pre, pend_compat, pend_post; bool in_postcopy = s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE; - qemu_savevm_state_pending_estimate(s->threshold_size, &pend_pre, - &pend_compat, &pend_post); + qemu_savevm_state_pending_estimate(&pend_pre, &pend_compat, &pend_post); uint64_t pending_size = pend_pre + pend_compat + pend_post; - trace_migrate_pending_estimate(pending_size, s->threshold_size, + trace_migrate_pending_estimate(pending_size, pend_pre, pend_compat, pend_post); if (pend_pre + pend_compat <= s->threshold_size) { - qemu_savevm_state_pending_exact(s->threshold_size, &pend_pre, - &pend_compat, &pend_post); + qemu_savevm_state_pending_exact(&pend_pre, &pend_compat, &pend_post); pending_size = pend_pre + pend_compat + pend_post; - trace_migrate_pending_exact(pending_size, s->threshold_size, + trace_migrate_pending_exact(pending_size, pend_pre, pend_compat, pend_post); } diff --git a/migration/ram.c b/migration/ram.c index XXXXXXX..XXXXXXX 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -XXX,XX +XXX,XX @@ static int ram_save_complete(QEMUFile *f, void *opaque) return 0; } -static void ram_state_pending_estimate(void *opaque, uint64_t max_size, +static void ram_state_pending_estimate(void *opaque, uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only) @@ -XXX,XX +XXX,XX @@ static void ram_state_pending_estimate(void *opaque, uint64_t max_size, } } -static void ram_state_pending_exact(void *opaque, uint64_t max_size, +static void ram_state_pending_exact(void *opaque, uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only) diff --git a/migration/savevm.c b/migration/savevm.c index XXXXXXX..XXXXXXX 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -XXX,XX +XXX,XX @@ flush: * the result is split into the amount for units that can and * for units that can't do postcopy. */ -void qemu_savevm_state_pending_estimate(uint64_t threshold_size, - uint64_t *res_precopy_only, +void qemu_savevm_state_pending_estimate(uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only) { @@ -XXX,XX +XXX,XX @@ void qemu_savevm_state_pending_estimate(uint64_t threshold_size, *res_compatible = 0; *res_postcopy_only = 0; - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { if (!se->ops || !se->ops->state_pending_exact) { continue; @@ -XXX,XX +XXX,XX @@ void qemu_savevm_state_pending_estimate(uint64_t threshold_size, continue; } } - se->ops->state_pending_exact(se->opaque, threshold_size, + se->ops->state_pending_exact(se->opaque, res_precopy_only, res_compatible, res_postcopy_only); } } -void qemu_savevm_state_pending_exact(uint64_t threshold_size, - uint64_t *res_precopy_only, +void qemu_savevm_state_pending_exact(uint64_t *res_precopy_only, uint64_t *res_compatible, uint64_t *res_postcopy_only) { @@ -XXX,XX +XXX,XX @@ void qemu_savevm_state_pending_exact(uint64_t threshold_size, continue; } } - se->ops->state_pending_estimate(se->opaque, threshold_size, + se->ops->state_pending_estimate(se->opaque, res_precopy_only, res_compatible, res_postcopy_only); } diff --git a/migration/trace-events b/migration/trace-events index XXXXXXX..XXXXXXX 100644 --- a/migration/trace-events +++ b/migration/trace-events @@ -XXX,XX +XXX,XX @@ migrate_fd_cleanup(void) "" migrate_fd_error(const char *error_desc) "error=%s" migrate_fd_cancel(void) "" migrate_handle_rp_req_pages(const char *rbname, size_t start, size_t len) "in %s at 0x%zx len 0x%zx" -migrate_pending_exact(uint64_t size, uint64_t max, uint64_t pre, uint64_t compat, uint64_t post) "exact pending size %" PRIu64 " max %" PRIu64 " (pre = %" PRIu64 " compat=%" PRIu64 " post=%" PRIu64 ")" -migrate_pending_estimate(uint64_t size, uint64_t max, uint64_t pre, uint64_t compat, uint64_t post) "estimate pending size %" PRIu64 " max %" PRIu64 " (pre = %" PRIu64 " compat=%" PRIu64 " post=%" PRIu64 ")" +migrate_pending_exact(uint64_t size, uint64_t pre, uint64_t compat, uint64_t post) "exact pending size %" PRIu64 " (pre = %" PRIu64 " compat=%" PRIu64 " post=%" PRIu64 ")" +migrate_pending_estimate(uint64_t size, uint64_t pre, uint64_t compat, uint64_t post) "estimate pending size %" PRIu64 " (pre = %" PRIu64 " compat=%" PRIu64 " post=%" PRIu64 ")" migrate_send_rp_message(int msg_type, uint16_t len) "%d: len %d" migrate_send_rp_recv_bitmap(char *name, int64_t size) "block '%s' size 0x%"PRIi64 migration_completion_file_err(void) "" -- 2.39.1 Signed-off-by: Juan Quintela <quintela@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- migration/migration.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index XXXXXXX..XXXXXXX 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -XXX,XX +XXX,XX @@ static MigIterateState migration_iteration_run(MigrationState *s) pend_pre, pend_compat, pend_post); } - if (pending_size && pending_size >= s->threshold_size) { - /* Still a significant amount to transfer */ - if (!in_postcopy && pend_pre <= s->threshold_size && - qatomic_read(&s->start_postcopy)) { - if (postcopy_start(s)) { - error_report("%s: postcopy failed to start", __func__); - } - return MIG_ITERATE_SKIP; - } - /* Just another iteration step */ - qemu_savevm_state_iterate(s->to_dst_file, in_postcopy); - } else { + if (pending_size < s->threshold_size) { trace_migration_thread_low_pending(pending_size); migration_completion(s); return MIG_ITERATE_BREAK; } + /* Still a significant amount to transfer */ + if (!in_postcopy && pend_pre <= s->threshold_size && + qatomic_read(&s->start_postcopy)) { + if (postcopy_start(s)) { + error_report("%s: postcopy failed to start", __func__); + } + return MIG_ITERATE_SKIP; + } + + /* Just another iteration step */ + qemu_savevm_state_iterate(s->to_dst_file, in_postcopy); return MIG_ITERATE_RESUME; } -- 2.39.1 The following changes since commit 343a88cb022e5cdb1d839a0499f9a33f8614598d: Merge tag 'firmware-20260519-pull-request' of https://gitlab.com/kraxel/qemu into staging (2026-05-19 09:28:07 -0400) are available in the Git repository at: https://gitlab.com/peterx/qemu.git tags/next-pull-request for you to fetch changes up to 7a4b1c333ffe8358664430e1f5a676e1dee7175c: MAINTAINERS: Update email of Yong Huang (2026-05-20 17:01:37 -0400) ---------------------------------------------------------------- Migration and mem pull request - Peter's fix on 2nd migration crashing if the 1st migration cancelled early - Phil's patch to remove VMS_MULTIPLY_ELEMENTS across tree - Peter's fix on possible division by zero in recent query-migrate change - Aadeshveer's cleanup for current_migration references - Fabiano's fix of auto-converge test - Maciej's maintainer file update for CPR - Fabiano's migration qtest refactor to stick with -incoming defer - Bin's cleanup / fix series all over migration (part of) - hongmianquan's cpr optimization to use ghash for fd bookkeeping - Yong's email address update ---------------------------------------------------------------- Aadeshveer Singh (1): migration: Replace current_migration with migrate_get_current() Bin Guo (6): migration/global_state: replace strcpy("") with explicit NUL termination migration/vmstate: avoid per-element heap churn in vmsd ptr marker field migration/savevm: use stack-allocated bitmap in configuration_validate_capabilities migration/multifd: fix off-by-one in recv channel ID validation migration/multifd: cache migrate_multifd_channels() in send/recv hot paths migration/multifd: cache channel count in multifd_send_sync_main Fabiano Rosas (16): tests/qtest/migration: Fix auto-converge test tests/qtest/migration: Move cpr transfer logic into cpr-tests.c tests/qtest/migration: Make file-tests defer by default tests/qtest/migration: Set file URI by default tests/qtest/migration: Group unix migration tests tests/qtest/migration: Use precopy_unix_common for ignore-shared test tests/qtest/migration: Use a default TCP URI for precopy tests/qtest/migration: Defer by default in precopy_common tests/qtest/migration: Set compression method in compression-tests tests/qtest/migration: Remove multifd compression hook tests/qtest/migration: Use defer for all tests tests/qtest/migration: Use defer for cpr-tests tests/qtest/migration: Use defer for auto-converge tests/qtest/migration: Use defer in dirty_limit test tests/qtest/migration: Stop passing URI into migrate_start tests/qtest/migration: Unify URIs Hyman Huang (1): MAINTAINERS: Update email of Yong Huang Maciej S. Szmigiero (1): MAINTAINERS: Make Maciej CPR maintainer Peter Xu (2): migration: Fix crash on second migration when cancel early migration: Fix possible division by zero on calc expected downtime Philippe Mathieu-Daudé (1): migration: Remove VMS_MULTIPLY_ELEMENTS and VMSTATE_VARRAY_MULTIPLY() hongmianquan (1): migration/cpr: use hashtable for cpr fds MAINTAINERS | 5 +- include/migration/cpr.h | 1 + include/migration/vmstate.h | 22 +--- migration/migration.h | 5 + tests/qtest/migration/framework.h | 26 ++-- migration/cpr-transfer.c | 10 ++ migration/cpr.c | 116 ++++++++++++++--- migration/global_state.c | 2 +- migration/migration.c | 57 +++++++-- migration/multifd.c | 27 ++-- migration/savevm.c | 5 +- migration/vmstate.c | 45 +++---- tests/qtest/migration/colo-tests.c | 16 +-- tests/qtest/migration/compression-tests.c | 34 ++--- tests/qtest/migration/cpr-tests.c | 85 ++++++++++--- tests/qtest/migration/file-tests.c | 56 +-------- tests/qtest/migration/framework.c | 91 +++++--------- tests/qtest/migration/misc-tests.c | 62 ++++------ tests/qtest/migration/precopy-tests.c | 144 ++++------------------ tests/qtest/migration/tls-tests.c | 110 ++--------------- rust/bindings/migration-sys/lib.rs | 8 -- rust/migration/src/vmstate.rs | 3 +- rust/tests/tests/vmstate_tests.rs | 55 --------- 23 files changed, 396 insertions(+), 589 deletions(-) -- 2.53.0 Marc-André reported an issue on QEMU crash when retrying a cancelled migration during early setup phase, see "Link:" for more information, and also easy way to reproduce. This patch is a replacement of the prior fix proposed by not only switching to migration_cleanup(), but also fixing it from CPR side, so that we track hup_source properly to know if src QEMU is waiting or the HUP signal. To put it simple: this chunk of special casing in migration_cancel() should not affect normal migration, but only cpr-transfer migration to cover the small window when the src QEMU is waiting for a HUP signal on cpr channel (so that src QEMU can continue the migration on the main channel). To achieve that, we'll also need to remember to detach the hup_source whenenver invoked: after that point, we should always be able to cleanup the migration. It's not a generic operation to explicitly detach a gsource from its context while in its dispatch() function. But it should be safe, because gsource disptch() will only happen with a boosted refcount for the dispatcher so that the gsource will not be freed until the callback completes. It's also safe to return G_SOURCE_REMOVE after the gsource is detached, as glib will simply ignore the G_SOURCE_REMOVE. One can refer to latest 2.86.5 glib code in g_main_dispatch() for that: https://github.com/GNOME/glib/blob/2.86.5/glib/gmain.c#L3592 When at this, add a bunch of assertions to make sure nothing surprises us. After this patch applied, the 2nd migration will not crash QEMU, instead it'll be in CANCELLING until the socket connection times out (it will take ~2min on my Fedora default kernel). During this process no 2nd migration will be allowed, and after it timed out migration can be restarted. It's because so far we don't have control over socket_connect_outgoing(), or anything yet managed by a task executed in qio_task_run_in_thread(). Speeding up the cancellation to be left for future. I also tested cpr-transfer by only providing cpr channel not the main channel (with -incoming defer), kickoff migration on source, then cancel it on source directly without providing the main channel. It keeps working. I wanted to add an unit test for that but it'll need to refactor current cpr-transfer tests first; let's leave it for later. Link: https://lore.kernel.org/r/20260417184742.293061-1-marcandre.lureau@redhat.com Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com> Tested-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20260421175820.302795-1-peterx@redhat.com Signed-off-by: Peter Xu <peterx@redhat.com> --- include/migration/cpr.h | 1 + migration/migration.h | 5 +++++ migration/cpr-transfer.c | 10 ++++++++++ migration/migration.c | 31 +++++++++++++++++++++++-------- 4 files changed, 39 insertions(+), 8 deletions(-) diff --git a/include/migration/cpr.h b/include/migration/cpr.h index XXXXXXX..XXXXXXX 100644 --- a/include/migration/cpr.h +++ b/include/migration/cpr.h @@ -XXX,XX +XXX,XX @@ QEMUFile *cpr_transfer_input(MigrationChannel *channel, Error **errp); void cpr_transfer_add_hup_watch(MigrationState *s, QIOChannelFunc func, void *opaque); void cpr_transfer_source_destroy(MigrationState *s); +bool cpr_transfer_source_active(MigrationState *s); void cpr_exec_init(void); QEMUFile *cpr_exec_output(Error **errp); diff --git a/migration/migration.h b/migration/migration.h index XXXXXXX..XXXXXXX 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -XXX,XX +XXX,XX @@ struct MigrationState { bool postcopy_package_loaded; + /* + * When set, it means cpr-transfer is waiting for the HUP signal from + * destination to continue the 2nd step of migration via the main + * channel. + */ GSource *hup_source; /* diff --git a/migration/cpr-transfer.c b/migration/cpr-transfer.c index XXXXXXX..XXXXXXX 100644 --- a/migration/cpr-transfer.c +++ b/migration/cpr-transfer.c @@ -XXX,XX +XXX,XX @@ */ #include "qemu/osdep.h" +#include "qemu/main-loop.h" #include "qapi/clone-visitor.h" #include "qapi/error.h" #include "qapi/qapi-visit-migration.h" @@ -XXX,XX +XXX,XX @@ QEMUFile *cpr_transfer_input(MigrationChannel *channel, Error **errp) void cpr_transfer_add_hup_watch(MigrationState *s, QIOChannelFunc func, void *opaque) { + assert(bql_locked()); s->hup_source = qio_channel_create_watch(cpr_state_ioc(), G_IO_HUP); g_source_set_callback(s->hup_source, (GSourceFunc)func, @@ -XXX,XX +XXX,XX @@ void cpr_transfer_add_hup_watch(MigrationState *s, QIOChannelFunc func, void cpr_transfer_source_destroy(MigrationState *s) { + assert(bql_locked()); if (s->hup_source) { g_source_destroy(s->hup_source); g_source_unref(s->hup_source); s->hup_source = NULL; } } + +bool cpr_transfer_source_active(MigrationState *s) +{ + /* Whenever the HUP gsource is available, it's active. */ + assert(bql_locked()); + return s->hup_source; +} diff --git a/migration/migration.c b/migration/migration.c index XXXXXXX..XXXXXXX 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -XXX,XX +XXX,XX @@ void migration_cancel(void) } /* - * If migration_connect_outgoing has not been called, then there - * is no path that will complete the cancellation. Do it now. - */ - if (setup && !s->to_dst_file) { - migrate_set_state(&s->state, MIGRATION_STATUS_CANCELLING, - MIGRATION_STATUS_CANCELLED); - cpr_state_close(); - cpr_transfer_source_destroy(s); + * This is cpr-transfer specific processing. + * + * If this is true, it means cpr-transfer migration is waiting for the + * destination to send HUP event on CPR channel to continue the next + * phase. If so, do the cleanup proactively to avoid get stuck in + * CANCELLING state. + */ + if (cpr_transfer_source_active(s)) { + assert(migrate_mode() == MIG_MODE_CPR_TRANSFER); + assert(setup && !s->to_dst_file); + migration_cleanup(s); + /* Now all things should have been released */ + assert(!cpr_transfer_source_active(s)); } } @@ -XXX,XX +XXX,XX @@ static gboolean migration_connect_outgoing_cb(QIOChannel *channel, MigrationState *s = migrate_get_current(); Error *local_err = NULL; + /* + * Detach and release the GSource right after use. We rely on this to + * detect this small cpr-transfer window of "waiting for HUP event". + */ + cpr_transfer_source_destroy(s); + migration_connect_outgoing(s, opaque, &local_err); if (local_err) { migration_connect_error_propagate(s, local_err); } + /* + * This is redundant as we do cpr_transfer_source_destroy() at the + * entry, but it's benign; glib will just skip the detach. + */ return G_SOURCE_REMOVE; } -- 2.53.0 From: Philippe Mathieu-Daudé <philmd@linaro.org> Commit c1eb3ac3468 ("target/sparc: Replace VMSTATE_VARRAY_MULTIPLY -> VMSTATE_UINTTL_ARRAY") removed the last use of the VMSTATE_VARRAY_MULTIPLY() macro. We can now remove it as unnecessary, along with the VMS_MULTIPLY_ELEMENTS flag and the associated tests. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> Acked-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> Link: https://lore.kernel.org/r/20260507070228.48877-1-philmd@linaro.org Signed-off-by: Peter Xu <peterx@redhat.com> --- include/migration/vmstate.h | 22 ++---------- migration/vmstate.c | 4 --- rust/bindings/migration-sys/lib.rs | 8 ----- rust/migration/src/vmstate.rs | 3 +- rust/tests/tests/vmstate_tests.rs | 55 ------------------------------ 5 files changed, 4 insertions(+), 88 deletions(-) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index XXXXXXX..XXXXXXX 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -XXX,XX +XXX,XX @@ enum VMStateFlags { VMS_ARRAY_OF_POINTER = 0x040, /* The field is an array of variable size. The uint16_t at opaque - * + VMStateField.num_offset (subject to VMS_MULTIPLY_ELEMENTS) + * + VMStateField.num_offset * contains the number of entries in the array. See the VMS_ARRAY * description regarding array handling in general. May not be * combined with VMS_ARRAY or any other VMS_VARRAY*. */ @@ -XXX,XX +XXX,XX @@ enum VMStateFlags { VMS_MULTIPLY = 0x200, /* The field is an array of variable size. The uint8_t at opaque + - * VMStateField.num_offset (subject to VMS_MULTIPLY_ELEMENTS) + * VMStateField.num_offset * contains the number of entries in the array. See the VMS_ARRAY * description regarding array handling in general. May not be * combined with VMS_ARRAY or any other VMS_VARRAY*. */ VMS_VARRAY_UINT8 = 0x400, /* The field is an array of variable size. The uint32_t at opaque - * + VMStateField.num_offset (subject to VMS_MULTIPLY_ELEMENTS) + * + VMStateField.num_offset * contains the number of entries in the array. See the VMS_ARRAY * description regarding array handling in general. May not be * combined with VMS_ARRAY or any other VMS_VARRAY*. */ @@ -XXX,XX +XXX,XX @@ enum VMStateFlags { * cause the individual entries to be allocated. */ VMS_ALLOC = 0x2000, - /* Multiply the number of entries given by the integer at opaque + - * VMStateField.num_offset (see VMS_VARRAY*) with VMStateField.num - * to determine the number of entries in the array. Only valid in - * combination with one of VMS_VARRAY*. */ - VMS_MULTIPLY_ELEMENTS = 0x4000, - /* A structure field that is like VMS_STRUCT, but uses * VMStateField.struct_version_id to tell which version of the * structure we are referencing to use. */ @@ -XXX,XX +XXX,XX @@ extern const VMStateInfo vmstate_info_qlist; .offset = vmstate_offset_2darray(_state, _field, _type, _n1, _n2), \ } -#define VMSTATE_VARRAY_MULTIPLY(_field, _state, _field_num, _multiply, _info, _type) { \ - .name = (stringify(_field)), \ - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\ - .num = (_multiply), \ - .info = &(_info), \ - .size = sizeof(_type), \ - .flags = VMS_VARRAY_UINT32|VMS_MULTIPLY_ELEMENTS, \ - .offset = vmstate_offset_varray(_state, _field, _type), \ -} - #define VMSTATE_SUB_ARRAY(_field, _state, _start, _num, _version, _info, _type) { \ .name = (stringify(_field)), \ .version_id = (_version), \ diff --git a/migration/vmstate.c b/migration/vmstate.c index XXXXXXX..XXXXXXX 100644 --- a/migration/vmstate.c +++ b/migration/vmstate.c @@ -XXX,XX +XXX,XX @@ static int vmstate_n_elems(void *opaque, const VMStateField *field) n_elems = *(uint8_t *)(opaque + field->num_offset); } - if (field->flags & VMS_MULTIPLY_ELEMENTS) { - n_elems *= field->num; - } - trace_vmstate_n_elems(field->name, n_elems); return n_elems; } diff --git a/rust/bindings/migration-sys/lib.rs b/rust/bindings/migration-sys/lib.rs index XXXXXXX..XXXXXXX 100644 --- a/rust/bindings/migration-sys/lib.rs +++ b/rust/bindings/migration-sys/lib.rs @@ -XXX,XX +XXX,XX @@ pub const fn with_varray_flag(mut self, flag: VMStateFlags) -> Self { assert!((self.flags.0 & VMStateFlags::VMS_ARRAY.0) != 0); self.with_varray_flag_unchecked(flag) } - - #[must_use] - pub const fn with_varray_multiply(mut self, num: u32) -> Self { - assert!(num <= 0x7FFF_FFFFu32); - self.flags = VMStateFlags(self.flags.0 | VMStateFlags::VMS_MULTIPLY_ELEMENTS.0); - self.num = num as i32; - self - } } diff --git a/rust/migration/src/vmstate.rs b/rust/migration/src/vmstate.rs index XXXXXXX..XXXXXXX 100644 --- a/rust/migration/src/vmstate.rs +++ b/rust/migration/src/vmstate.rs @@ -XXX,XX +XXX,XX @@ macro_rules! vmstate_of { )$(.with_varray_flag($crate::call_func_with_field!( $crate::vmstate::vmstate_varray_flag, $struct_name, - $($num).+)) - $(.with_varray_multiply($factor))?)? + $($num).+)))? } }; } diff --git a/rust/tests/tests/vmstate_tests.rs b/rust/tests/tests/vmstate_tests.rs index XXXXXXX..XXXXXXX 100644 --- a/rust/tests/tests/vmstate_tests.rs +++ b/rust/tests/tests/vmstate_tests.rs @@ -XXX,XX +XXX,XX @@ fn test_vmstate_varray_uint16_unsafe() { assert!(foo_fields[2].field_exists.is_none()); } -#[test] -fn test_vmstate_varray_multiply() { - let foo_fields: &[VMStateField] = - unsafe { slice::from_raw_parts(VMSTATE_FOOA.as_ref().fields, 5) }; - - // 4th VMStateField ("arr_mul") in VMSTATE_FOOA (corresponding to - // VMSTATE_VARRAY_MULTIPLY) - assert_eq!( - unsafe { CStr::from_ptr(foo_fields[3].name) }.to_bytes_with_nul(), - b"arr_mul\0" - ); - assert_eq!(foo_fields[3].offset, 6); - assert_eq!(foo_fields[3].num_offset, 12); - assert_eq!(foo_fields[3].info, unsafe { &vmstate_info_int8 }); - assert_eq!(foo_fields[3].version_id, 0); - assert_eq!(foo_fields[3].size, 1); - assert_eq!(foo_fields[3].num, 16); - assert_eq!( - foo_fields[3].flags.0, - VMStateFlags::VMS_VARRAY_UINT32.0 | VMStateFlags::VMS_MULTIPLY_ELEMENTS.0 - ); - assert!(foo_fields[3].vmsd.is_null()); - assert!(foo_fields[3].field_exists.is_none()); - - // The last VMStateField in VMSTATE_FOOA. - assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_END); -} - // =========================== Test VMSTATE_FOOB =========================== // Test the use cases of the vmstate macro, corresponding to the following C // macro variants: @@ -XXX,XX +XXX,XX @@ fn test_vmstate_struct_varray_uint8() { assert!(foo_fields[2].field_exists.is_none()); } -#[test] -fn test_vmstate_struct_varray_uint32_multiply() { - let foo_fields: &[VMStateField] = - unsafe { slice::from_raw_parts(VMSTATE_FOOB.as_ref().fields, 7) }; - - // 4th VMStateField ("arr_a_mul") in VMSTATE_FOOB (corresponding to - // (no C version) MULTIPLY variant of VMSTATE_STRUCT_VARRAY_UINT32) - assert_eq!( - unsafe { CStr::from_ptr(foo_fields[3].name) }.to_bytes_with_nul(), - b"arr_a_mul\0" - ); - assert_eq!(foo_fields[3].offset, 64); - assert_eq!(foo_fields[3].num_offset, 124); - assert!(foo_fields[3].info.is_null()); // VMSTATE_STRUCT_VARRAY_UINT8 doesn't set info field. - assert_eq!(foo_fields[3].version_id, 2); - assert_eq!(foo_fields[3].size, 20); - assert_eq!(foo_fields[3].num, 32); - assert_eq!( - foo_fields[3].flags.0, - VMStateFlags::VMS_STRUCT.0 - | VMStateFlags::VMS_VARRAY_UINT32.0 - | VMStateFlags::VMS_MULTIPLY_ELEMENTS.0 - ); - assert_eq!(foo_fields[3].vmsd, VMSTATE_FOOA.as_ref()); - assert!(foo_fields[3].field_exists.is_none()); -} - #[test] fn test_vmstate_macro_array() { let foo_fields: &[VMStateField] = -- 2.53.0 Commit dd4fe8844b changed the reporting of expected downtime behavior, so that the value will be calculated on-demand. One side effect on the change is QEMU will allow the calculation to happen anytime even if there's no transfer happening for a short while. PeterM reported an ubsan report from clang when running migration-test with aarch64 binary on x86_64 hosts. I can also reproduce if I run the test concurrently so some of the src QEMU may not get chance to push any data, causing mbps to be 0: ../migration/migration.c:1051:12: runtime error: -nan is outside the range of representable values of type 'long' Fix it by properly handle both Inf and Nan to return INT64_MAX. Add a rich comment, per PeterM's suggestion. Link: https://lore.kernel.org/r/CAFEAcA-MYH6C39xO0OLx4-M5pKurJpurwRsMqZe9q=W-NShAbw@mail.gmail.com Reported-by: Peter Maydell <peter.maydell@linaro.org> Fixes: dd4fe8844b ("migration: Calculate expected downtime on demand") Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Link: https://lore.kernel.org/r/20260511182432.1333467-1-peterx@redhat.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/migration.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/migration/migration.c b/migration/migration.c index XXXXXXX..XXXXXXX 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -XXX,XX +XXX,XX @@ #include "system/dirtylimit.h" #include "qemu/sockets.h" #include "system/kvm.h" +#include "math.h" #define NOTIFIER_ELEM_INIT(array, elem) \ [elem] = NOTIFIER_WITH_RETURN_LIST_INITIALIZER((array)[elem]) @@ -XXX,XX +XXX,XX @@ static bool migrate_show_downtime(MigrationState *s) /* Return expected downtime (unit: milliseconds) */ int64_t migration_downtime_calc_expected(MigrationState *s) { + double expected_ms; + if (mig_stats.dirty_sync_count <= 1) { return migrate_downtime_limit(); } - return mig_stats.dirty_bytes_last_sync / + expected_ms = mig_stats.dirty_bytes_last_sync / migration_get_switchover_bw(s) * 1000; + + /* + * If we haven't been able to transfer any data, the result here could + * be NaN (for 0 / 0) or infinity (something else / 0). + * + * Return INT64_MAX as our best approximation to "this will take + * forever to complete". If the problem is transient (e.g. we just + * haven't started to transfer yet) we'll recalculate to a more + * accurate figure later. + */ + if (isnan(expected_ms) || expected_ms >= (double)INT64_MAX) { + return INT64_MAX; + } + + return (int64_t) expected_ms; } static void populate_time_info(MigrationInfo *info, MigrationState *s) -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> We fixed the cpu throttling sync thread affecting the dirty-sync-count, but the test still relies on it to gauge for progress. Remove that block from the test with no replacement. While here remove several incorrect or redundant comments. Fixes: 9519d3667a ("migration: Move iteration counter out of RAM") Signed-off-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20260512141338.10089-1-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/precopy-tests.c | 62 ++------------------------- 1 file changed, 3 insertions(+), 59 deletions(-) diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/precopy-tests.c +++ b/tests/qtest/migration/precopy-tests.c @@ -XXX,XX +XXX,XX @@ static void test_precopy_fd_socket(char *name, MigrateCommon *args) } #endif /* _WIN32 */ -/* - * The way auto_converge works, we need to do too many passes to - * run this test. Auto_converge logic is only run once every - * three iterations, so: - * - * - 3 iterations without auto_converge enabled - * - 3 iterations with pct = 5 - * - 3 iterations with pct = 30 - * - 3 iterations with pct = 55 - * - 3 iterations with pct = 80 - * - 3 iterations with pct = 95 (max(95, 80 + 25)) - * - * To make things even worse, we need to run the initial stage at - * 3MB/s so we enter autoconverge even when host is (over)loaded. - */ static void test_auto_converge(char *name, MigrateCommon *args) { g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); QTestState *from, *to; int64_t percentage; - - /* - * We want the test to be stable and as fast as possible. - * E.g., with 1Gb/s bandwidth migration may pass without throttling, - * so we need to decrease a bandwidth. - */ const int64_t init_pct = 5, inc_pct = 25, max_pct = 95; - uint64_t prev_dirty_sync_cnt, dirty_sync_cnt; - int max_try_count, hit = 0; if (migrate_start(&from, &to, uri, &args->start)) { return; @@ -XXX,XX +XXX,XX @@ static void test_auto_converge(char *name, MigrateCommon *args) migrate_set_parameter_int(from, "cpu-throttle-increment", inc_pct); migrate_set_parameter_int(from, "max-cpu-throttle", max_pct); - /* - * Set the initial parameters so that the migration could not converge - * without throttling. - */ migrate_ensure_non_converge(from); /* To check remaining size after precopy */ migrate_set_capability(from, "pause-before-switchover", true); - /* Wait for the first serial output from the source */ wait_for_serial("src_serial"); migrate_qmp(from, to, uri, NULL, "{}"); - /* Wait for throttling begins */ + /* Wait until throttling begins */ percentage = 0; do { percentage = read_migrate_property_int(from, "cpu-throttle-percentage"); @@ -XXX,XX +XXX,XX @@ static void test_auto_converge(char *name, MigrateCommon *args) /* The first percentage of throttling should be at least init_pct */ g_assert_cmpint(percentage, >=, init_pct); - /* - * End the loop when the dirty sync count greater than 1. - */ - while ((dirty_sync_cnt = get_migration_pass(from)) < 2) { - usleep(1000 * 1000); - } - - prev_dirty_sync_cnt = dirty_sync_cnt; - - /* - * The RAMBlock dirty sync count must changes in 5 seconds, here we set - * the timeout to 10 seconds to ensure it changes. - * - * Note that migrate_ensure_non_converge set the max-bandwidth to 3MB/s, - * while the qtest mem is >= 100MB, one iteration takes at least 33s (100/3) - * to complete; this ensures that the RAMBlock dirty sync occurs. - */ - max_try_count = 10; - while (--max_try_count) { - dirty_sync_cnt = get_migration_pass(from); - if (dirty_sync_cnt != prev_dirty_sync_cnt) { - hit = 1; - break; - } - prev_dirty_sync_cnt = dirty_sync_cnt; - sleep(1); - } - g_assert_cmpint(hit, ==, 1); - - /* Now, when we tested that throttling works, let it converge */ + /* throttling always ignores the first pass */ + assert(get_migration_pass(from) == 2); migrate_ensure_converge(from); /* -- 2.53.0 From: Aadeshveer Singh <aadeshveer07@gmail.com> Replaces the direct accesses to global variable `current_migration` with `migrate_get_current()` to ensure consistency across systems. Note: Following this only direct access to `current_migration` will be * `migrate_get_current()` itself * `migration_object_init()` initializes `current_migration` * `migration_shutdown()` to pair up with initialization * `migration_is_running()`, as there might be a case where this function is called by a thread before object initialization Signed-off-by: Aadeshveer Singh <aadeshveer07@gmail.com> Link: https://lore.kernel.org/r/20260513063513.250911-1-aadeshveer07@gmail.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/migration.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index XXXXXXX..XXXXXXX 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -XXX,XX +XXX,XX @@ bool migration_is_running(void) static bool migration_is_active(void) { - MigrationState *s = current_migration; + MigrationState *s = migrate_get_current(); return (s->state == MIGRATION_STATUS_ACTIVE || s->state == MIGRATION_STATUS_POSTCOPY_DEVICE || @@ -XXX,XX +XXX,XX @@ bool migration_in_bg_snapshot(void) bool migration_thread_is_self(void) { - MigrationState *s = current_migration; + MigrationState *s = migrate_get_current(); return qemu_thread_is_self(&s->thread); } @@ -XXX,XX +XXX,XX @@ static MigThrError postcopy_pause(MigrationState *s) void migration_file_set_error(int ret, Error *err) { - MigrationState *s = current_migration; + MigrationState *s = migrate_get_current(); WITH_QEMU_LOCK_GUARD(&s->qemu_file_lock) { if (s->to_dst_file) { -- 2.53.0 From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> Since Steve has retired last year I will take the CPR maintainership - with kind help of Mark who remains a reviewer. Cc: Mark Kanda <mark.kanda@oracle.com> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Link: https://lore.kernel.org/r/ebe67053f4bdf92eedab1e5839603b7137e36970.1778687091.git.maciej.szmigiero@oracle.com Signed-off-by: Peter Xu <peterx@redhat.com> --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ T: git https://gitlab.com/jsnow/qemu.git jobs T: git https://gitlab.com/vsementsov/qemu.git block CheckPoint and Restart (CPR) +M: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> R: Peter Xu <peterx@redhat.com> R: Fabiano Rosas <farosas@suse.de> R: Mark Kanda <mark.kanda@oracle.com> -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> There's some amount of cpr-transfer logic at precopy_common, which in retrospect was a bad idea. For just two tests, that's too much code to be in the common function. Move it to the cpr file. We'll need this cleanup for subsequent improvements. Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20260505160915.25558-2-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/framework.h | 3 -- tests/qtest/migration/cpr-tests.c | 57 ++++++++++++++++++++++++++++--- tests/qtest/migration/framework.c | 36 +++---------------- 3 files changed, 56 insertions(+), 40 deletions(-) diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/framework.h +++ b/tests/qtest/migration/framework.h @@ -XXX,XX +XXX,XX @@ typedef struct { */ const char *connect_channels; - /* Optional: the cpr migration channel, in JSON or dotted keys format */ - const char *cpr_channel; - /* Optional: callback to run at start to set migration parameters */ TestMigrateStartHook start_hook; /* Optional: callback to run at finish to cleanup */ diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/cpr-tests.c +++ b/tests/qtest/migration/cpr-tests.c @@ -XXX,XX +XXX,XX @@ #include "migration/framework.h" #include "migration/migration-qmp.h" #include "migration/migration-util.h" +#include "qapi/error.h" +#include "qobject/qjson.h" +#include "qobject/qlist.h" static char *tmpfs; @@ -XXX,XX +XXX,XX @@ static void test_mode_reboot(char *name, MigrateCommon *args) test_file_common(args, true); } -static void *test_mode_transfer_start(QTestState *from, QTestState *to) +static int test_transfer(MigrateCommon *args, const char *cpr_channel, + bool incoming_defer) { + QTestState *from, *to; + QObject *obj, *out_channels = qobject_from_json(args->connect_channels, + &error_abort); + QList *channels_list; + + /* + * The cpr channel must be included in outgoing channels, but not in + * migrate-incoming channels. + */ + channels_list = qobject_to(QList, out_channels); + obj = migrate_str_to_channel(cpr_channel); + qlist_append(channels_list, obj); + + if (migrate_start(&from, &to, args->listen_uri, &args->start)) { + return -1; + } + migrate_set_parameter_str(from, "mode", "cpr-transfer"); - return NULL; + + wait_for_serial("src_serial"); + + qtest_qmp_assert_success(from, "{ 'execute' : 'stop'}"); + wait_for_stop(from, get_src()); + migrate_ensure_converge(from); + + migrate_qmp(from, to, NULL, out_channels, "{}"); + + qtest_connect(to); + qtest_qmp_handshake(to, NULL); + if (incoming_defer) { + QObject *in_channels = qobject_from_json(args->connect_channels, + &error_abort); + + migrate_incoming_qmp(to, NULL, in_channels, "{}"); + } + + wait_for_migration_complete(from); + wait_for_migration_complete(to); + + qtest_qmp_assert_success(to, "{ 'execute' : 'cont'}"); + + wait_for_resume(to, get_dst()); + wait_for_serial("dest_serial"); + + migrate_end(from, to, true); + + return 0; } /* @@ -XXX,XX +XXX,XX @@ static void test_mode_transfer_common(MigrateCommon *args, bool incoming_defer) args->listen_uri = incoming_defer ? "defer" : uri; args->connect_channels = connect_channels; - args->cpr_channel = cpr_channel; - args->start_hook = test_mode_transfer_start; args->start.opts_source = opts; args->start.opts_target = opts_target; args->start.defer_target_connect = true; args->start.mem_type = MEM_TYPE_MEMFD; - if (test_precopy_common(args) < 0) { + if (test_transfer(args, cpr_channel, incoming_defer) < 0) { close(cpr_sockfd); unlink(cpr_path); } diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/framework.c +++ b/tests/qtest/migration/framework.c @@ -XXX,XX +XXX,XX @@ #include "ppc-util.h" #include "qapi/error.h" #include "qobject/qjson.h" -#include "qobject/qlist.h" #include "qemu/bswap.h" #include "qemu/module.h" #include "qemu/option.h" @@ -XXX,XX +XXX,XX @@ int test_precopy_common(MigrateCommon *args) { QTestState *from, *to; void *data_hook = NULL; - QObject *in_channels = NULL; - QObject *out_channels = NULL; - - g_assert(!args->cpr_channel || args->connect_channels); + QObject *channels = NULL; if (migrate_start(&from, &to, args->listen_uri, &args->start)) { return -1; @@ -XXX,XX +XXX,XX @@ int test_precopy_common(MigrateCommon *args) } } - /* - * The cpr channel must be included in outgoing channels, but not in - * migrate-incoming channels. - */ if (args->connect_channels) { - if (args->start.defer_target_connect && - !strcmp(args->listen_uri, "defer")) { - in_channels = qobject_from_json(args->connect_channels, - &error_abort); - } - out_channels = qobject_from_json(args->connect_channels, &error_abort); - - if (args->cpr_channel) { - QList *channels_list = qobject_to(QList, out_channels); - QObject *obj = migrate_str_to_channel(args->cpr_channel); - - qlist_append(channels_list, obj); - } + channels = qobject_from_json(args->connect_channels, &error_abort); } if (args->result == MIG_TEST_QMP_ERROR) { - migrate_qmp_fail(from, args->connect_uri, out_channels, "{}"); + migrate_qmp_fail(from, args->connect_uri, channels, "{}"); goto finish; } - migrate_qmp(from, to, args->connect_uri, out_channels, "{}"); - - if (args->start.defer_target_connect) { - qtest_connect(to); - qtest_qmp_handshake(to, NULL); - if (!strcmp(args->listen_uri, "defer")) { - migrate_incoming_qmp(to, args->connect_uri, in_channels, "{}"); - } - } + migrate_qmp(from, to, args->connect_uri, channels, "{}"); if (args->result != MIG_TEST_SUCCEED) { bool allow_active = args->result == MIG_TEST_FAIL; -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> All file: tests use listen_uri="defer". Make this the default in the common function. Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20260505160915.25558-3-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/cpr-tests.c | 1 - tests/qtest/migration/file-tests.c | 14 -------------- tests/qtest/migration/framework.c | 2 +- 3 files changed, 1 insertion(+), 16 deletions(-) diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/cpr-tests.c +++ b/tests/qtest/migration/cpr-tests.c @@ -XXX,XX +XXX,XX @@ static void test_mode_reboot(char *name, MigrateCommon *args) FILE_TEST_FILENAME); args->connect_uri = uri; - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_mode_reboot; args->start.mem_type = MEM_TYPE_SHMEM; diff --git a/tests/qtest/migration/file-tests.c b/tests/qtest/migration/file-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/file-tests.c +++ b/tests/qtest/migration/file-tests.c @@ -XXX,XX +XXX,XX @@ static void test_precopy_file(char *name, MigrateCommon *args) g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, FILE_TEST_FILENAME); args->connect_uri = uri; - args->listen_uri = "defer"; - test_file_common(args, true); } @@ -XXX,XX +XXX,XX @@ static void test_precopy_file_offset_fdset(char *name, MigrateCommon *args) g_autofree char *uri = g_strdup_printf("file:/dev/fdset/1,offset=%d", FILE_TEST_OFFSET); args->connect_uri = uri; - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_file_offset_fdset; test_file_common(args, false); @@ -XXX,XX +XXX,XX @@ static void test_precopy_file_offset(char *name, MigrateCommon *args) FILE_TEST_OFFSET); args->connect_uri = uri; - args->listen_uri = "defer"; - test_file_common(args, false); } @@ -XXX,XX +XXX,XX @@ static void test_precopy_file_offset_bad(char *name, MigrateCommon *args) tmpfs, FILE_TEST_FILENAME); args->connect_uri = uri; - args->listen_uri = "defer"; args->result = MIG_TEST_QMP_ERROR; test_file_common(args, false); @@ -XXX,XX +XXX,XX @@ static void test_precopy_file_mapped_ram_live(char *name, MigrateCommon *args) FILE_TEST_FILENAME); args->connect_uri = uri; - args->listen_uri = "defer"; args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true; @@ -XXX,XX +XXX,XX @@ static void test_precopy_file_mapped_ram(char *name, MigrateCommon *args) FILE_TEST_FILENAME); args->connect_uri = uri; - args->listen_uri = "defer"; args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true; @@ -XXX,XX +XXX,XX @@ static void test_multifd_file_mapped_ram_live(char *name, MigrateCommon *args) g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, FILE_TEST_FILENAME); args->connect_uri = uri; - args->listen_uri = "defer"; args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true; @@ -XXX,XX +XXX,XX @@ static void test_multifd_file_mapped_ram(char *name, MigrateCommon *args) FILE_TEST_FILENAME); args->connect_uri = uri; - args->listen_uri = "defer"; args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true; @@ -XXX,XX +XXX,XX @@ static void test_multifd_file_mapped_ram_dio(char *name, MigrateCommon *args) g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, FILE_TEST_FILENAME); args->connect_uri = uri; - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_multifd_mapped_ram_dio; args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true; @@ -XXX,XX +XXX,XX @@ static void test_multifd_file_mapped_ram_fdset(char *name, MigrateCommon *args) FILE_TEST_OFFSET); args->connect_uri = uri; - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_multifd_mapped_ram_fdset; args->end_hook = migrate_hook_end_multifd_mapped_ram_fdset; @@ -XXX,XX +XXX,XX @@ static void test_multifd_file_mapped_ram_fdset_dio(char *name, g_autofree char *uri = g_strdup_printf("file:/dev/fdset/1,offset=%d", FILE_TEST_OFFSET); args->connect_uri = uri; - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_multifd_mapped_ram_fdset_dio; args->end_hook = migrate_hook_end_multifd_mapped_ram_fdset; @@ -XXX,XX +XXX,XX @@ test_precopy_file_mapped_ram_ignore_shared(char *name, MigrateCommon *args) g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, FILE_TEST_FILENAME); args->connect_uri = uri; - args->listen_uri = "defer"; args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true; args->start.caps[MIGRATION_CAPABILITY_X_IGNORE_SHARED] = true; diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/framework.c +++ b/tests/qtest/migration/framework.c @@ -XXX,XX +XXX,XX @@ void test_file_common(MigrateCommon *args, bool stop_src) void *data_hook = NULL; bool check_offset = false; - if (migrate_start(&from, &to, args->listen_uri, &args->start)) { + if (migrate_start(&from, &to, "defer", &args->start)) { return; } -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> Most file: tests use the same URI. Make it a default in the common function. Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20260505160915.25558-4-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/file-tests.c | 29 ----------------------------- tests/qtest/migration/framework.c | 6 ++++++ 2 files changed, 6 insertions(+), 29 deletions(-) diff --git a/tests/qtest/migration/file-tests.c b/tests/qtest/migration/file-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/file-tests.c +++ b/tests/qtest/migration/file-tests.c @@ -XXX,XX +XXX,XX @@ static void test_file_connect_outgoing_fd_leak(char *name, MigrateCommon *args) static void test_precopy_file(char *name, MigrateCommon *args) { - g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, - FILE_TEST_FILENAME); - args->connect_uri = uri; test_file_common(args, true); } @@ -XXX,XX +XXX,XX @@ static void test_precopy_file_offset_bad(char *name, MigrateCommon *args) static void test_precopy_file_mapped_ram_live(char *name, MigrateCommon *args) { - g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, - FILE_TEST_FILENAME); - - args->connect_uri = uri; - args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true; test_file_common(args, false); @@ -XXX,XX +XXX,XX @@ static void test_precopy_file_mapped_ram_live(char *name, MigrateCommon *args) static void test_precopy_file_mapped_ram(char *name, MigrateCommon *args) { - g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, - FILE_TEST_FILENAME); - - args->connect_uri = uri; - args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true; test_file_common(args, true); @@ -XXX,XX +XXX,XX @@ static void test_precopy_file_mapped_ram(char *name, MigrateCommon *args) static void test_multifd_file_mapped_ram_live(char *name, MigrateCommon *args) { - g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, - FILE_TEST_FILENAME); - args->connect_uri = uri; - args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true; @@ -XXX,XX +XXX,XX @@ static void test_multifd_file_mapped_ram_live(char *name, MigrateCommon *args) static void test_multifd_file_mapped_ram(char *name, MigrateCommon *args) { - g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, - FILE_TEST_FILENAME); - - args->connect_uri = uri; - args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true; @@ -XXX,XX +XXX,XX @@ static void *migrate_hook_start_multifd_mapped_ram_dio(QTestState *from, static void test_multifd_file_mapped_ram_dio(char *name, MigrateCommon *args) { - g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, - FILE_TEST_FILENAME); - args->connect_uri = uri; args->start_hook = migrate_hook_start_multifd_mapped_ram_dio; args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true; @@ -XXX,XX +XXX,XX @@ static void migration_test_add_file_smoke(MigrationTestEnv *env) static void test_precopy_file_mapped_ram_ignore_shared(char *name, MigrateCommon *args) { - g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, - FILE_TEST_FILENAME); - args->connect_uri = uri; - args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true; args->start.caps[MIGRATION_CAPABILITY_X_IGNORE_SHARED] = true; diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/framework.c +++ b/tests/qtest/migration/framework.c @@ -XXX,XX +XXX,XX @@ void test_file_common(MigrateCommon *args, bool stop_src) QTestState *from, *to; void *data_hook = NULL; bool check_offset = false; + g_autofree char *uri = NULL; if (migrate_start(&from, &to, "defer", &args->start)) { return; } + if (!args->connect_uri) { + uri = g_strdup_printf("file:%s/%s", tmpfs, FILE_TEST_FILENAME); + args->connect_uri = uri; + } + /* * File migration is never live. We can keep the source VM running * during migration, but the destination will not be running -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> Remove some repetition when defining unix: tests by introducing a _common function. Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20260505160915.25558-5-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/framework.h | 1 + tests/qtest/migration/compression-tests.c | 6 +---- tests/qtest/migration/framework.c | 9 +++++++ tests/qtest/migration/precopy-tests.c | 30 +++-------------------- tests/qtest/migration/tls-tests.c | 12 ++------- 5 files changed, 17 insertions(+), 41 deletions(-) diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/framework.h +++ b/tests/qtest/migration/framework.h @@ -XXX,XX +XXX,XX @@ void test_postcopy_common(MigrateCommon *args); void test_postcopy_recovery_common(MigrateCommon *args, PostcopyRecoveryFailStage fail_stage); int test_precopy_common(MigrateCommon *args); +void test_precopy_unix_common(MigrateCommon *args); void test_file_common(MigrateCommon *args, bool stop_src); void *migrate_hook_start_precopy_tcp_multifd_common(QTestState *from, QTestState *to, diff --git a/tests/qtest/migration/compression-tests.c b/tests/qtest/migration/compression-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/compression-tests.c +++ b/tests/qtest/migration/compression-tests.c @@ -XXX,XX +XXX,XX @@ migrate_hook_start_xbzrle(QTestState *from, static void test_precopy_unix_xbzrle(char *name, MigrateCommon *args) { - g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); - - args->connect_uri = uri; - args->listen_uri = uri; args->start_hook = migrate_hook_start_xbzrle; args->iterations = 2; /* @@ -XXX,XX +XXX,XX @@ static void test_precopy_unix_xbzrle(char *name, MigrateCommon *args) args->start.caps[MIGRATION_CAPABILITY_XBZRLE] = true; - test_precopy_common(args); + test_precopy_unix_common(args); } static void * diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/framework.c +++ b/tests/qtest/migration/framework.c @@ -XXX,XX +XXX,XX @@ finish: return 0; } +void test_precopy_unix_common(MigrateCommon *args) +{ + g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); + + args->listen_uri = uri; + args->connect_uri = uri; + test_precopy_common(args); +} + static void file_dirty_offset_region(void) { g_autofree char *path = g_strdup_printf("%s/%s", tmpfs, FILE_TEST_FILENAME); diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/precopy-tests.c +++ b/tests/qtest/migration/precopy-tests.c @@ -XXX,XX +XXX,XX @@ static char *tmpfs; static void test_precopy_unix_plain(char *name, MigrateCommon *args) { - g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); - - args->listen_uri = uri; - args->connect_uri = uri; /* * The simplest use case of precopy, covering smoke tests of * get-dirty-log dirty tracking. */ args->live = true; - - test_precopy_common(args); + test_precopy_unix_common(args); } static void test_precopy_unix_suspend_live(char *name, MigrateCommon *args) { - g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); - - args->listen_uri = uri; - args->connect_uri = uri; /* * despite being live, the test is fast because the src * suspends immediately. */ args->live = true; - args->start.suspend_me = true; - - test_precopy_common(args); + test_precopy_unix_common(args); } static void test_precopy_unix_suspend_notlive(char *name, MigrateCommon *args) { - g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); - - args->listen_uri = uri; - args->connect_uri = uri; args->start.suspend_me = true; - - test_precopy_common(args); + test_precopy_unix_common(args); } static void test_precopy_unix_dirty_ring(char *name, MigrateCommon *args) { - g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); - - args->listen_uri = uri; - args->connect_uri = uri; /* * Besides the precopy/unix basic test, cover dirty ring interface * rather than get-dirty-log. */ args->live = true; - args->start.use_dirty_ring = true; - - test_precopy_common(args); + test_precopy_unix_common(args); } #ifdef CONFIG_RDMA diff --git a/tests/qtest/migration/tls-tests.c b/tests/qtest/migration/tls-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/tls-tests.c +++ b/tests/qtest/migration/tls-tests.c @@ -XXX,XX +XXX,XX @@ static void test_multifd_postcopy_preempt_recovery_tls_psk(char *name, static void test_precopy_unix_tls_psk(char *name, MigrateCommon *args) { - g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); - - args->connect_uri = uri; - args->listen_uri = uri; args->start_hook = migrate_hook_start_tls_psk_match; args->end_hook = migrate_hook_end_tls_psk; - test_precopy_common(args); + test_precopy_unix_common(args); } #ifdef CONFIG_TASN1 @@ -XXX,XX +XXX,XX @@ static void test_precopy_unix_tls_x509_default_host(char *name, static void test_precopy_unix_tls_x509_override_host(char *name, MigrateCommon *args) { - g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); - - args->connect_uri = uri; - args->listen_uri = uri; args->start_hook = migrate_hook_start_tls_x509_override_host; args->end_hook = migrate_hook_end_tls_x509; - test_precopy_common(args); + test_precopy_unix_common(args); } #endif /* CONFIG_TASN1 */ -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> The ignore-shared test has the same code as the precopy_common test but inverting (probably incorrectly) the order of a few event waits. Change it to use the common code instead. Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20260505160915.25558-6-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/misc-tests.c | 40 ++++++++---------------------- 1 file changed, 11 insertions(+), 29 deletions(-) diff --git a/tests/qtest/migration/misc-tests.c b/tests/qtest/migration/misc-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/misc-tests.c +++ b/tests/qtest/migration/misc-tests.c @@ -XXX,XX +XXX,XX @@ static void test_analyze_script(char *name, MigrateCommon *args) } #endif -static void test_ignore_shared(char *name, MigrateCommon *args) +static void ignore_shared_assert_skipped(QTestState *from, QTestState *to, + void *data) { - g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); - QTestState *from, *to; - - args->start.mem_type = MEM_TYPE_SHMEM; - args->start.caps[MIGRATION_CAPABILITY_X_IGNORE_SHARED] = true; - - if (migrate_start(&from, &to, uri, &args->start)) { - return; - } - - migrate_ensure_non_converge(from); - migrate_prepare_for_dirty_mem(from); - - /* Wait for the first serial output from the source */ - wait_for_serial("src_serial"); - - migrate_qmp(from, to, uri, NULL, "{}"); - - migrate_wait_for_dirty_mem(from, to); - - wait_for_stop(from, get_src()); - - qtest_qmp_eventwait(to, "RESUME"); - - wait_for_serial("dest_serial"); - wait_for_migration_complete(from); - /* Check whether shared RAM has been really skipped */ g_assert_cmpint( read_ram_property_int(from, "transferred"), <, 4 * 1024 * 1024); +} + +static void test_ignore_shared(char *name, MigrateCommon *args) +{ + args->live = true; + args->start.mem_type = MEM_TYPE_SHMEM; + args->start.caps[MIGRATION_CAPABILITY_X_IGNORE_SHARED] = true; + args->end_hook = ignore_shared_assert_skipped; - migrate_end(from, to, true); + test_precopy_unix_common(args); } static void do_test_validate_uuid(MigrateStart *args, bool should_fail) -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> Using a localhost TCP URI for testing is quite common. Set it as a default for precopy tests that don't provide an URI. Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20260505160915.25558-7-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/framework.c | 4 ++++ tests/qtest/migration/precopy-tests.c | 3 --- tests/qtest/migration/tls-tests.c | 9 --------- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/framework.c +++ b/tests/qtest/migration/framework.c @@ -XXX,XX +XXX,XX @@ int test_precopy_common(MigrateCommon *args) void *data_hook = NULL; QObject *channels = NULL; + if (!args->listen_uri) { + args->listen_uri = "tcp:127.0.0.1:0"; + } + if (migrate_start(&from, &to, args->listen_uri, &args->start)) { return -1; } diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/precopy-tests.c +++ b/tests/qtest/migration/precopy-tests.c @@ -XXX,XX +XXX,XX @@ static void test_precopy_rdma_plain_ipv6(char *name, MigrateCommon *args) static void test_precopy_tcp_plain(char *name, MigrateCommon *args) { - args->listen_uri = "tcp:127.0.0.1:0"; - test_precopy_common(args); } static void test_precopy_tcp_switchover_ack(char *name, MigrateCommon *args) { - args->listen_uri = "tcp:127.0.0.1:0"; /* * Source VM must be running in order to consider the switchover ACK * when deciding to do switchover or not. diff --git a/tests/qtest/migration/tls-tests.c b/tests/qtest/migration/tls-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/tls-tests.c +++ b/tests/qtest/migration/tls-tests.c @@ -XXX,XX +XXX,XX @@ static void test_precopy_unix_tls_x509_override_host(char *name, static void test_precopy_tcp_tls_psk_match(char *name, MigrateCommon *args) { - args->listen_uri = "tcp:127.0.0.1:0"; args->start_hook = migrate_hook_start_tls_psk_match; args->end_hook = migrate_hook_end_tls_psk; @@ -XXX,XX +XXX,XX @@ static void test_precopy_tcp_tls_psk_match(char *name, MigrateCommon *args) static void test_precopy_tcp_tls_psk_mismatch(char *name, MigrateCommon *args) { - args->listen_uri = "tcp:127.0.0.1:0"; args->start_hook = migrate_hook_start_tls_psk_mismatch; args->end_hook = migrate_hook_end_tls_psk; args->result = MIG_TEST_FAIL; @@ -XXX,XX +XXX,XX @@ static void *migrate_hook_start_no_tls(QTestState *from, QTestState *to) static void test_precopy_tcp_no_tls(char *name, MigrateCommon *args) { - args->listen_uri = "tcp:127.0.0.1:0"; args->start_hook = migrate_hook_start_no_tls; /* the no_tls start hook requires no cleanup actions */ args->end_hook = NULL; @@ -XXX,XX +XXX,XX @@ static void test_precopy_tcp_tls_no_hostname(char *name, MigrateCommon *args) static void test_precopy_tcp_tls_x509_default_host(char *name, MigrateCommon *args) { - args->listen_uri = "tcp:127.0.0.1:0"; args->start_hook = migrate_hook_start_tls_x509_default_host; args->end_hook = migrate_hook_end_tls_x509; @@ -XXX,XX +XXX,XX @@ static void test_precopy_tcp_tls_x509_default_host(char *name, static void test_precopy_tcp_tls_x509_override_host(char *name, MigrateCommon *args) { - args->listen_uri = "tcp:127.0.0.1:0"; args->start_hook = migrate_hook_start_tls_x509_override_host; args->end_hook = migrate_hook_end_tls_x509; @@ -XXX,XX +XXX,XX @@ static void test_precopy_tcp_tls_x509_mismatch_host(char *name, static void test_precopy_tcp_tls_x509_friendly_client(char *name, MigrateCommon *args) { - args->listen_uri = "tcp:127.0.0.1:0"; args->start_hook = migrate_hook_start_tls_x509_friendly_client; args->end_hook = migrate_hook_end_tls_x509; @@ -XXX,XX +XXX,XX @@ static void test_precopy_tcp_tls_x509_friendly_client(char *name, static void test_precopy_tcp_tls_x509_hostile_client(char *name, MigrateCommon *args) { - args->listen_uri = "tcp:127.0.0.1:0"; args->start_hook = migrate_hook_start_tls_x509_hostile_client; args->end_hook = migrate_hook_end_tls_x509; args->result = MIG_TEST_FAIL; @@ -XXX,XX +XXX,XX @@ static void test_precopy_tcp_tls_x509_hostile_client(char *name, static void test_precopy_tcp_tls_x509_allow_anon_client(char *name, MigrateCommon *args) { - args->listen_uri = "tcp:127.0.0.1:0"; args->start_hook = migrate_hook_start_tls_x509_allow_anon_client; args->end_hook = migrate_hook_end_tls_x509; @@ -XXX,XX +XXX,XX @@ static void test_precopy_tcp_tls_x509_allow_anon_client(char *name, static void test_precopy_tcp_tls_x509_reject_anon_client(char *name, MigrateCommon *args) { - args->listen_uri = "tcp:127.0.0.1:0"; args->start_hook = migrate_hook_start_tls_x509_reject_anon_client; args->end_hook = migrate_hook_end_tls_x509; args->result = MIG_TEST_FAIL; -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> As a design direction, we're restricting the usage of the command line option -incoming <URI>. The alternative -incoming defer should be used instead. Make all precopy_common tests defer by default. Using the defer option means that QEMU will not start the incoming migration automatically. Add the incoming QMP command. With the added command, the invocation at the multifd_common hook becomes redundant, so remove it. Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Lukas Straub <lukasstraub2@web.de> Tested-by: Lukas Straub <lukasstraub2@web.de> Link: https://lore.kernel.org/r/20260505160915.25558-8-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/colo-tests.c | 12 ++++-------- tests/qtest/migration/compression-tests.c | 6 ------ tests/qtest/migration/framework.c | 15 ++++++++------- tests/qtest/migration/precopy-tests.c | 11 +---------- tests/qtest/migration/tls-tests.c | 15 +-------------- 5 files changed, 14 insertions(+), 45 deletions(-) diff --git a/tests/qtest/migration/colo-tests.c b/tests/qtest/migration/colo-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/colo-tests.c +++ b/tests/qtest/migration/colo-tests.c @@ -XXX,XX +XXX,XX @@ static int test_colo_common(MigrateCommon *args, args->start.caps[MIGRATION_CAPABILITY_RETURN_PATH] = true; args->start.caps[MIGRATION_CAPABILITY_X_COLO] = true; - if (migrate_start(&from, &to, args->listen_uri, &args->start)) { + if (migrate_start(&from, &to, "defer", &args->start)) { return -1; } @@ -XXX,XX +XXX,XX @@ static int test_colo_common(MigrateCommon *args, data_hook = args->start_hook(from, to); } + migrate_incoming_qmp(to, args->listen_uri, NULL, "{}"); + migrate_ensure_converge(from); wait_for_serial("src_serial"); @@ -XXX,XX +XXX,XX @@ static void test_colo_plain_common(MigrateCommon *args, test_colo_common(args, failover_during_checkpoint, primary_failover); } -static void *hook_start_multifd(QTestState *from, QTestState *to) -{ - return migrate_hook_start_precopy_tcp_multifd_common(from, to, "none"); -} - static void test_colo_multifd_common(MigrateCommon *args, bool failover_during_checkpoint, bool primary_failover) { - args->listen_uri = "defer"; - args->start_hook = hook_start_multifd; + args->listen_uri = "tcp:127.0.0.1:0"; args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; test_colo_common(args, failover_during_checkpoint, primary_failover); } diff --git a/tests/qtest/migration/compression-tests.c b/tests/qtest/migration/compression-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/compression-tests.c +++ b/tests/qtest/migration/compression-tests.c @@ -XXX,XX +XXX,XX @@ migrate_hook_start_precopy_tcp_multifd_zstd(QTestState *from, static void test_multifd_tcp_zstd(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_precopy_tcp_multifd_zstd; args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_zstd(char *name, MigrateCommon *args) static void test_multifd_postcopy_tcp_zstd(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_precopy_tcp_multifd_zstd, args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; @@ -XXX,XX +XXX,XX @@ migrate_hook_start_precopy_tcp_multifd_qatzip(QTestState *from, static void test_multifd_tcp_qatzip(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_precopy_tcp_multifd_qatzip; args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; @@ -XXX,XX +XXX,XX @@ migrate_hook_start_precopy_tcp_multifd_qpl(QTestState *from, static void test_multifd_tcp_qpl(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_precopy_tcp_multifd_qpl; args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; @@ -XXX,XX +XXX,XX @@ migrate_hook_start_precopy_tcp_multifd_uadk(QTestState *from, static void test_multifd_tcp_uadk(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_precopy_tcp_multifd_uadk; args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; @@ -XXX,XX +XXX,XX @@ migrate_hook_start_precopy_tcp_multifd_zlib(QTestState *from, static void test_multifd_tcp_zlib(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_precopy_tcp_multifd_zlib; args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/framework.c +++ b/tests/qtest/migration/framework.c @@ -XXX,XX +XXX,XX @@ int test_precopy_common(MigrateCommon *args) void *data_hook = NULL; QObject *channels = NULL; - if (!args->listen_uri) { + assert(!args->connect_uri); + + if (args->listen_uri) { + args->connect_uri = args->listen_uri; + } else { args->listen_uri = "tcp:127.0.0.1:0"; } - if (migrate_start(&from, &to, args->listen_uri, &args->start)) { + if (migrate_start(&from, &to, "defer", &args->start)) { return -1; } @@ -XXX,XX +XXX,XX @@ int test_precopy_common(MigrateCommon *args) data_hook = args->start_hook(from, to); } + migrate_incoming_qmp(to, args->listen_uri, NULL, "{}"); + /* Wait for the first serial output from the source */ if (args->result == MIG_TEST_SUCCEED) { wait_for_serial("src_serial"); @@ -XXX,XX +XXX,XX @@ void test_precopy_unix_common(MigrateCommon *args) g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); args->listen_uri = uri; - args->connect_uri = uri; test_precopy_common(args); } @@ -XXX,XX +XXX,XX @@ void *migrate_hook_start_precopy_tcp_multifd_common(QTestState *from, { migrate_set_parameter_str(from, "multifd-compression", method); migrate_set_parameter_str(to, "multifd-compression", method); - - /* Start incoming migration from the 1st socket */ - migrate_incoming_qmp(to, "tcp:127.0.0.1:0", NULL, "{}"); - return NULL; } diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/precopy-tests.c +++ b/tests/qtest/migration/precopy-tests.c @@ -XXX,XX +XXX,XX @@ static void __test_precopy_rdma_plain(MigrateCommon *args, bool ipv6) g_autofree char *uri = g_strdup_printf("rdma:%s:29200", buffer); args->listen_uri = uri; - args->connect_uri = uri; test_precopy_common(args); } @@ -XXX,XX +XXX,XX @@ static void *migrate_hook_start_fd(QTestState *from, " 'arguments': { 'fdname': 'fd-mig' }}"); close(pair[0]); - /* Start incoming migration from the 1st socket */ - migrate_incoming_qmp(to, "fd:fd-mig", NULL, "{}"); - /* Send the 2nd socket to the target */ qtest_qmp_fds_assert_success(from, &pair[1], 1, "{ 'execute': 'getfd'," @@ -XXX,XX +XXX,XX @@ static void migrate_hook_end_fd(QTestState *from, static void test_precopy_fd_socket(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; - args->connect_uri = "fd:fd-mig"; + args->listen_uri = "fd:fd-mig"; args->start_hook = migrate_hook_start_fd; args->end_hook = migrate_hook_end_fd; @@ -XXX,XX +XXX,XX @@ migrate_hook_start_precopy_tcp_multifd_no_zero_page(QTestState *from, static void test_multifd_tcp_uri_none(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_precopy_tcp_multifd; /* * Multifd is more complicated than most of the features, it @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_uri_none(char *name, MigrateCommon *args) static void test_multifd_tcp_zero_page_legacy(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_precopy_tcp_multifd_zero_page_legacy; /* * Multifd is more complicated than most of the features, it @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_zero_page_legacy(char *name, MigrateCommon *args) static void test_multifd_tcp_no_zero_page(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_precopy_tcp_multifd_no_zero_page; /* * Multifd is more complicated than most of the features, it @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_no_zero_page(char *name, MigrateCommon *args) static void test_multifd_tcp_channels_none(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_precopy_tcp_multifd; args->live = true; args->connect_channels = ("[ { 'channel-type': 'main'," diff --git a/tests/qtest/migration/tls-tests.c b/tests/qtest/migration/tls-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/tls-tests.c +++ b/tests/qtest/migration/tls-tests.c @@ -XXX,XX +XXX,XX @@ static void test_precopy_unix_tls_x509_default_host(char *name, { g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); - args->connect_uri = uri; - args->listen_uri = "defer"; + args->listen_uri = uri; args->start_hook = migrate_hook_start_tls_x509_default_host; args->end_hook = migrate_hook_end_tls_x509; args->result = MIG_TEST_FAIL; @@ -XXX,XX +XXX,XX @@ migrate_hook_start_tls_x509_no_host(QTestState *from, QTestState *to) static void test_precopy_tcp_tls_no_hostname(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; - args->connect_uri = "tcp:127.0.0.1:0"; args->start_hook = migrate_hook_start_tls_x509_no_host; args->end_hook = migrate_hook_end_tls_x509; args->result = MIG_TEST_FAIL; @@ -XXX,XX +XXX,XX @@ static void test_precopy_tcp_tls_x509_override_host(char *name, static void test_precopy_tcp_tls_x509_mismatch_host(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; - args->connect_uri = "tcp:127.0.0.1:0"; args->start_hook = migrate_hook_start_tls_x509_mismatch_host; args->end_hook = migrate_hook_end_tls_x509; args->result = MIG_TEST_FAIL; @@ -XXX,XX +XXX,XX @@ migrate_hook_start_multifd_tls_x509_reject_anon_client(QTestState *from, static void test_multifd_tcp_tls_psk_match(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_multifd_tcp_tls_psk_match; args->end_hook = migrate_hook_end_tls_psk; @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_tls_psk_match(char *name, MigrateCommon *args) static void test_multifd_tcp_tls_psk_mismatch(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_multifd_tcp_tls_psk_mismatch; args->end_hook = migrate_hook_end_tls_psk; args->result = MIG_TEST_FAIL; @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_tls_psk_mismatch(char *name, MigrateCommon *args) static void test_multifd_postcopy_tcp_tls_psk_match(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_multifd_tcp_tls_psk_match; args->end_hook = migrate_hook_end_tls_psk; @@ -XXX,XX +XXX,XX @@ static void test_multifd_postcopy_tcp_tls_psk_match(char *name, static void test_multifd_tcp_tls_x509_default_host(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_multifd_tls_x509_default_host; args->end_hook = migrate_hook_end_tls_x509; @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_tls_x509_default_host(char *name, static void test_multifd_tcp_tls_x509_override_host(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_multifd_tls_x509_override_host; args->end_hook = migrate_hook_end_tls_x509; @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_tls_x509_mismatch_host(char *name, * to load migration state, and thus just aborts the migration * without exiting. */ - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_multifd_tls_x509_mismatch_host; args->end_hook = migrate_hook_end_tls_x509; args->result = MIG_TEST_FAIL; @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_tls_x509_mismatch_host(char *name, static void test_multifd_tcp_tls_x509_allow_anon_client(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_multifd_tls_x509_allow_anon_client; args->end_hook = migrate_hook_end_tls_x509; @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_tls_x509_allow_anon_client(char *name, static void test_multifd_tcp_tls_x509_reject_anon_client(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start_hook = migrate_hook_start_multifd_tls_x509_reject_anon_client; args->end_hook = migrate_hook_end_tls_x509; args->result = MIG_TEST_FAIL; -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> Stop calling a common function to set the multifd compression method. The default method is "none", so the common function is not necessary for tests that don't set compression and will be removed. Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20260505160915.25558-9-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/compression-tests.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/tests/qtest/migration/compression-tests.c b/tests/qtest/migration/compression-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/compression-tests.c +++ b/tests/qtest/migration/compression-tests.c @@ -XXX,XX +XXX,XX @@ static char *tmpfs; +static void set_multifd_compression(QTestState *from, QTestState *to, + const char *method) +{ + migrate_set_parameter_str(from, "multifd-compression", method); + migrate_set_parameter_str(to, "multifd-compression", method); +} + #ifdef CONFIG_ZSTD static void * migrate_hook_start_precopy_tcp_multifd_zstd(QTestState *from, @@ -XXX,XX +XXX,XX @@ migrate_hook_start_precopy_tcp_multifd_zstd(QTestState *from, { migrate_set_parameter_int(from, "multifd-zstd-level", 2); migrate_set_parameter_int(to, "multifd-zstd-level", 2); + set_multifd_compression(from, to, "zstd"); - return migrate_hook_start_precopy_tcp_multifd_common(from, to, "zstd"); + return NULL; } static void test_multifd_tcp_zstd(char *name, MigrateCommon *args) @@ -XXX,XX +XXX,XX @@ migrate_hook_start_precopy_tcp_multifd_qatzip(QTestState *from, { migrate_set_parameter_int(from, "multifd-qatzip-level", 2); migrate_set_parameter_int(to, "multifd-qatzip-level", 2); + set_multifd_compression(from, to, "qatzip"); - return migrate_hook_start_precopy_tcp_multifd_common(from, to, "qatzip"); + return NULL; } static void test_multifd_tcp_qatzip(char *name, MigrateCommon *args) @@ -XXX,XX +XXX,XX @@ static void * migrate_hook_start_precopy_tcp_multifd_qpl(QTestState *from, QTestState *to) { - return migrate_hook_start_precopy_tcp_multifd_common(from, to, "qpl"); + set_multifd_compression(from, to, "qpl"); + return NULL; } static void test_multifd_tcp_qpl(char *name, MigrateCommon *args) @@ -XXX,XX +XXX,XX @@ static void * migrate_hook_start_precopy_tcp_multifd_uadk(QTestState *from, QTestState *to) { - return migrate_hook_start_precopy_tcp_multifd_common(from, to, "uadk"); + set_multifd_compression(from, to, "uadk"); + return NULL; } static void test_multifd_tcp_uadk(char *name, MigrateCommon *args) @@ -XXX,XX +XXX,XX @@ migrate_hook_start_precopy_tcp_multifd_zlib(QTestState *from, */ migrate_set_parameter_int(from, "multifd-zlib-level", 2); migrate_set_parameter_int(to, "multifd-zlib-level", 2); + set_multifd_compression(from, to, "zlib"); - return migrate_hook_start_precopy_tcp_multifd_common(from, to, "zlib"); + return NULL; } static void test_multifd_tcp_zlib(char *name, MigrateCommon *args) -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> Take advantage of the default compression method for multifd being "none" and remove the common compression hook. Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20260505160915.25558-10-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/framework.h | 3 -- tests/qtest/migration/framework.c | 9 ---- tests/qtest/migration/precopy-tests.c | 11 ---- tests/qtest/migration/tls-tests.c | 74 +++------------------------ 4 files changed, 8 insertions(+), 89 deletions(-) diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/framework.h +++ b/tests/qtest/migration/framework.h @@ -XXX,XX +XXX,XX @@ void test_postcopy_recovery_common(MigrateCommon *args, int test_precopy_common(MigrateCommon *args); void test_precopy_unix_common(MigrateCommon *args); void test_file_common(MigrateCommon *args, bool stop_src); -void *migrate_hook_start_precopy_tcp_multifd_common(QTestState *from, - QTestState *to, - const char *method); typedef struct QTestMigrationState QTestMigrationState; QTestMigrationState *get_src(void); diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/framework.c +++ b/tests/qtest/migration/framework.c @@ -XXX,XX +XXX,XX @@ finish: migrate_end(from, to, args->result == MIG_TEST_SUCCEED); } -void *migrate_hook_start_precopy_tcp_multifd_common(QTestState *from, - QTestState *to, - const char *method) -{ - migrate_set_parameter_str(from, "multifd-compression", method); - migrate_set_parameter_str(to, "multifd-compression", method); - return NULL; -} - QTestMigrationState *get_src(void) { return &src_state; diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/precopy-tests.c +++ b/tests/qtest/migration/precopy-tests.c @@ -XXX,XX +XXX,XX @@ static void test_auto_converge(char *name, MigrateCommon *args) migrate_end(from, to, true); } -static void * -migrate_hook_start_precopy_tcp_multifd(QTestState *from, - QTestState *to) -{ - return migrate_hook_start_precopy_tcp_multifd_common(from, to, "none"); -} - static void * migrate_hook_start_precopy_tcp_multifd_zero_page_legacy(QTestState *from, QTestState *to) { - migrate_hook_start_precopy_tcp_multifd_common(from, to, "none"); migrate_set_parameter_str(from, "zero-page-detection", "legacy"); return NULL; } @@ -XXX,XX +XXX,XX @@ static void * migrate_hook_start_precopy_tcp_multifd_no_zero_page(QTestState *from, QTestState *to) { - migrate_hook_start_precopy_tcp_multifd_common(from, to, "none"); migrate_set_parameter_str(from, "zero-page-detection", "none"); return NULL; } static void test_multifd_tcp_uri_none(char *name, MigrateCommon *args) { - args->start_hook = migrate_hook_start_precopy_tcp_multifd; /* * Multifd is more complicated than most of the features, it * directly takes guest page buffers when sending, make sure @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_no_zero_page(char *name, MigrateCommon *args) static void test_multifd_tcp_channels_none(char *name, MigrateCommon *args) { - args->start_hook = migrate_hook_start_precopy_tcp_multifd; args->live = true; args->connect_channels = ("[ { 'channel-type': 'main'," " 'addr': { 'transport': 'socket'," diff --git a/tests/qtest/migration/tls-tests.c b/tests/qtest/migration/tls-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/tls-tests.c +++ b/tests/qtest/migration/tls-tests.c @@ -XXX,XX +XXX,XX @@ static void test_precopy_tcp_tls_x509_reject_anon_client(char *name, } #endif /* CONFIG_TASN1 */ -static void * -migrate_hook_start_multifd_tcp_tls_psk_match(QTestState *from, - QTestState *to) -{ - migrate_hook_start_precopy_tcp_multifd_common(from, to, "none"); - return migrate_hook_start_tls_psk_match(from, to); -} - -static void * -migrate_hook_start_multifd_tcp_tls_psk_mismatch(QTestState *from, - QTestState *to) -{ - migrate_hook_start_precopy_tcp_multifd_common(from, to, "none"); - return migrate_hook_start_tls_psk_mismatch(from, to); -} - -#ifdef CONFIG_TASN1 -static void * -migrate_hook_start_multifd_tls_x509_default_host(QTestState *from, - QTestState *to) -{ - migrate_hook_start_precopy_tcp_multifd_common(from, to, "none"); - return migrate_hook_start_tls_x509_default_host(from, to); -} - -static void * -migrate_hook_start_multifd_tls_x509_override_host(QTestState *from, - QTestState *to) -{ - migrate_hook_start_precopy_tcp_multifd_common(from, to, "none"); - return migrate_hook_start_tls_x509_override_host(from, to); -} - -static void * -migrate_hook_start_multifd_tls_x509_mismatch_host(QTestState *from, - QTestState *to) -{ - migrate_hook_start_precopy_tcp_multifd_common(from, to, "none"); - return migrate_hook_start_tls_x509_mismatch_host(from, to); -} - -static void * -migrate_hook_start_multifd_tls_x509_allow_anon_client(QTestState *from, - QTestState *to) -{ - migrate_hook_start_precopy_tcp_multifd_common(from, to, "none"); - return migrate_hook_start_tls_x509_allow_anon_client(from, to); -} - -static void * -migrate_hook_start_multifd_tls_x509_reject_anon_client(QTestState *from, - QTestState *to) -{ - migrate_hook_start_precopy_tcp_multifd_common(from, to, "none"); - return migrate_hook_start_tls_x509_reject_anon_client(from, to); -} -#endif /* CONFIG_TASN1 */ - static void test_multifd_tcp_tls_psk_match(char *name, MigrateCommon *args) { - args->start_hook = migrate_hook_start_multifd_tcp_tls_psk_match; + args->start_hook = migrate_hook_start_tls_psk_match; args->end_hook = migrate_hook_end_tls_psk; args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_tls_psk_match(char *name, MigrateCommon *args) static void test_multifd_tcp_tls_psk_mismatch(char *name, MigrateCommon *args) { - args->start_hook = migrate_hook_start_multifd_tcp_tls_psk_mismatch; + args->start_hook = migrate_hook_start_tls_psk_mismatch; args->end_hook = migrate_hook_end_tls_psk; args->result = MIG_TEST_FAIL; @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_tls_psk_mismatch(char *name, MigrateCommon *args) static void test_multifd_postcopy_tcp_tls_psk_match(char *name, MigrateCommon *args) { - args->start_hook = migrate_hook_start_multifd_tcp_tls_psk_match; + args->start_hook = migrate_hook_start_tls_psk_match; args->end_hook = migrate_hook_end_tls_psk; args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; @@ -XXX,XX +XXX,XX @@ static void test_multifd_postcopy_tcp_tls_psk_match(char *name, static void test_multifd_tcp_tls_x509_default_host(char *name, MigrateCommon *args) { - args->start_hook = migrate_hook_start_multifd_tls_x509_default_host; + args->start_hook = migrate_hook_start_tls_x509_default_host; args->end_hook = migrate_hook_end_tls_x509; args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_tls_x509_default_host(char *name, static void test_multifd_tcp_tls_x509_override_host(char *name, MigrateCommon *args) { - args->start_hook = migrate_hook_start_multifd_tls_x509_override_host; + args->start_hook = migrate_hook_start_tls_x509_override_host; args->end_hook = migrate_hook_end_tls_x509; args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_tls_x509_mismatch_host(char *name, * to load migration state, and thus just aborts the migration * without exiting. */ - args->start_hook = migrate_hook_start_multifd_tls_x509_mismatch_host; + args->start_hook = migrate_hook_start_tls_x509_mismatch_host; args->end_hook = migrate_hook_end_tls_x509; args->result = MIG_TEST_FAIL; @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_tls_x509_mismatch_host(char *name, static void test_multifd_tcp_tls_x509_allow_anon_client(char *name, MigrateCommon *args) { - args->start_hook = migrate_hook_start_multifd_tls_x509_allow_anon_client; + args->start_hook = migrate_hook_start_tls_x509_allow_anon_client; args->end_hook = migrate_hook_end_tls_x509; args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_tls_x509_allow_anon_client(char *name, static void test_multifd_tcp_tls_x509_reject_anon_client(char *name, MigrateCommon *args) { - args->start_hook = migrate_hook_start_multifd_tls_x509_reject_anon_client; + args->start_hook = migrate_hook_start_tls_x509_reject_anon_client; args->end_hook = migrate_hook_end_tls_x509; args->result = MIG_TEST_FAIL; -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> Change all invocations of migrate_start to use defer. The uri parameter will be removed from that function in subsequent patches. Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20260505160915.25558-11-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/misc-tests.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/qtest/migration/misc-tests.c b/tests/qtest/migration/misc-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/misc-tests.c +++ b/tests/qtest/migration/misc-tests.c @@ -XXX,XX +XXX,XX @@ static void test_baddest(char *name, MigrateCommon *args) args->start.hide_stderr = true; - if (migrate_start(&from, &to, "tcp:127.0.0.1:0", &args->start)) { + if (migrate_start(&from, &to, "defer", &args->start)) { return; } + + migrate_incoming_qmp(to, "tcp:127.0.0.1:0", NULL, "{}"); migrate_qmp(from, to, "tcp:127.0.0.1:0", NULL, "{}"); wait_for_migration_fail(from, false); migrate_end(from, to, false); @@ -XXX,XX +XXX,XX @@ static void test_analyze_script(char *name, MigrateCommon *args) return; } - /* dummy url */ - if (migrate_start(&from, &to, "tcp:127.0.0.1:0", &args->start)) { + if (migrate_start(&from, &to, "defer", &args->start)) { return; } @@ -XXX,XX +XXX,XX @@ static void test_analyze_script(char *name, MigrateCommon *args) uri = g_strdup_printf("exec:cat > %s", file); migrate_ensure_converge(from); + migrate_incoming_qmp(to, "tcp:127.0.0.1:0", NULL, "{}"); migrate_qmp(from, to, uri, NULL, "{}"); wait_for_migration_complete(from); @@ -XXX,XX +XXX,XX @@ static void do_test_validate_uri_channel(MigrateCommon *args) QTestState *from, *to; QObject *channels; - if (migrate_start(&from, &to, args->listen_uri, &args->start)) { + if (migrate_start(&from, &to, "defer", &args->start)) { return; } /* Wait for the first serial output from the source */ wait_for_serial("src_serial"); + migrate_incoming_qmp(to, "tcp:127.0.0.1:0", NULL, "{}"); + /* * 'uri' and 'channels' validation is checked even before the migration * starts. @@ -XXX,XX +XXX,XX @@ static void test_validate_caps_pair(char *test_path, MigrateCommon *args) static void test_validate_uri_channels_both_set(char *name, MigrateCommon *args) { - args->listen_uri = "defer", args->connect_uri = "tcp:127.0.0.1:0", args->connect_channels = ("[ { ""'channel-type': 'main'," " 'addr': { 'transport': 'socket'," @@ -XXX,XX +XXX,XX @@ static void test_validate_uri_channels_both_set(char *name, MigrateCommon *args) static void test_validate_uri_channels_none_set(char *name, MigrateCommon *args) { - args->listen_uri = "defer"; args->start.hide_stderr = true; do_test_validate_uri_channel(args); -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20260505160915.25558-12-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/cpr-tests.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/cpr-tests.c +++ b/tests/qtest/migration/cpr-tests.c @@ -XXX,XX +XXX,XX @@ static int test_transfer(MigrateCommon *args, const char *cpr_channel, obj = migrate_str_to_channel(cpr_channel); qlist_append(channels_list, obj); - if (migrate_start(&from, &to, args->listen_uri, &args->start)) { + if (migrate_start(&from, &to, "defer", &args->start)) { return -1; } @@ -XXX,XX +XXX,XX @@ static void test_mode_transfer_common(MigrateCommon *args, bool incoming_defer) int cpr_sockfd = qtest_socket_server(cpr_path); g_assert(cpr_sockfd >= 0); - opts_target = g_strdup_printf("-incoming cpr,addr.transport=socket," - "addr.type=fd,addr.str=%d %s", - cpr_sockfd, opts); + if (incoming_defer) { + opts_target = g_strdup_printf("-incoming cpr,addr.transport=socket," + "addr.type=fd,addr.str=%d %s", + cpr_sockfd, opts); + } else { + opts_target = g_strdup_printf("-incoming %s " + "-incoming cpr,addr.transport=socket," + "addr.type=fd,addr.str=%d %s", + uri, cpr_sockfd, opts); + } - args->listen_uri = incoming_defer ? "defer" : uri; args->connect_channels = connect_channels; args->start.opts_source = opts; @@ -XXX,XX +XXX,XX @@ static void test_cpr_exec(MigrateCommon *args) g_autofree char *filename = g_strdup_printf("%s/%s", tmpfs, FILE_TEST_FILENAME); - if (migrate_start(&from, NULL, args->listen_uri, &args->start)) { + if (migrate_start(&from, NULL, "defer", &args->start)) { return; } @@ -XXX,XX +XXX,XX @@ static void test_mode_exec(char *name, MigrateCommon *args) { g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, FILE_TEST_FILENAME); - g_autofree char *listen_uri = g_strdup_printf("defer"); - args->connect_uri = uri; - args->listen_uri = listen_uri; args->start_hook = test_mode_exec_start; args->start.only_source = true; -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20260505160915.25558-13-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/precopy-tests.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/precopy-tests.c +++ b/tests/qtest/migration/precopy-tests.c @@ -XXX,XX +XXX,XX @@ static void test_auto_converge(char *name, MigrateCommon *args) int64_t percentage; const int64_t init_pct = 5, inc_pct = 25, max_pct = 95; - if (migrate_start(&from, &to, uri, &args->start)) { + if (migrate_start(&from, &to, "defer", &args->start)) { return; } @@ -XXX,XX +XXX,XX @@ static void test_auto_converge(char *name, MigrateCommon *args) wait_for_serial("src_serial"); + migrate_incoming_qmp(to, uri, NULL, "{}"); migrate_qmp(from, to, uri, NULL, "{}"); /* Wait until throttling begins */ -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20260505160915.25558-14-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/precopy-tests.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/precopy-tests.c +++ b/tests/qtest/migration/precopy-tests.c @@ -XXX,XX +XXX,XX @@ static void test_dirty_limit(char *name, MigrateCommon *args) args->start.use_dirty_ring = true; /* Restart dst vm, src vm already show up so we needn't wait anymore */ - if (migrate_start(&from, &to, args->listen_uri, &args->start)) { + if (migrate_start(&from, &to, "defer", &args->start)) { return; } /* Start migrate */ + migrate_incoming_qmp(to, args->listen_uri, NULL, "{}"); migrate_qmp(from, to, args->connect_uri, NULL, "{}"); /* Wait for dirty limit throttle begin */ -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> Don't allow changing the default -incoming URI via migrate_start. The default is now -incoming defer. If a test really needs to alter this (such as with CPR), the target_opts variable is still available to change the command line. (aside from the larger goal of using defer, this change is a step towards allowing migrate_start() to be invoked only once for all tests) Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Lukas Straub <lukasstraub2@web.de> Tested-by: Lukas Straub <lukasstraub2@web.de> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20260505160915.25558-15-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/framework.h | 5 ++--- tests/qtest/migration/colo-tests.c | 2 +- tests/qtest/migration/cpr-tests.c | 6 +++--- tests/qtest/migration/file-tests.c | 3 +-- tests/qtest/migration/framework.c | 17 ++++++++--------- tests/qtest/migration/misc-tests.c | 10 +++++----- tests/qtest/migration/precopy-tests.c | 12 ++++++------ 7 files changed, 26 insertions(+), 29 deletions(-) diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/framework.h +++ b/tests/qtest/migration/framework.h @@ -XXX,XX +XXX,XX @@ void wait_for_serial(const char *side); void migrate_prepare_for_dirty_mem(QTestState *from); void migrate_wait_for_dirty_mem(QTestState *from, QTestState *to); -int migrate_args(char **from, char **to, const char *uri, MigrateStart *args); -int migrate_start(QTestState **from, QTestState **to, const char *uri, - MigrateStart *args); +int migrate_args(char **from, char **to, MigrateStart *args); +int migrate_start(QTestState **from, QTestState **to, MigrateStart *args); void migrate_end(QTestState *from, QTestState *to, bool test_dest); void test_postcopy_common(MigrateCommon *args); diff --git a/tests/qtest/migration/colo-tests.c b/tests/qtest/migration/colo-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/colo-tests.c +++ b/tests/qtest/migration/colo-tests.c @@ -XXX,XX +XXX,XX @@ static int test_colo_common(MigrateCommon *args, args->start.caps[MIGRATION_CAPABILITY_RETURN_PATH] = true; args->start.caps[MIGRATION_CAPABILITY_X_COLO] = true; - if (migrate_start(&from, &to, "defer", &args->start)) { + if (migrate_start(&from, &to, &args->start)) { return -1; } diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/cpr-tests.c +++ b/tests/qtest/migration/cpr-tests.c @@ -XXX,XX +XXX,XX @@ static int test_transfer(MigrateCommon *args, const char *cpr_channel, obj = migrate_str_to_channel(cpr_channel); qlist_append(channels_list, obj); - if (migrate_start(&from, &to, "defer", &args->start)) { + if (migrate_start(&from, &to, &args->start)) { return -1; } @@ -XXX,XX +XXX,XX @@ static void set_cpr_exec_args(QTestState *who, MigrateCommon *args) */ g_assert(args->start.hide_stderr == false); - ret = migrate_args(&from_args, &to_args, args->listen_uri, &args->start); + ret = migrate_args(&from_args, &to_args, &args->start); g_assert(!ret); qtest_from_args = qtest_qemu_args(from_args); @@ -XXX,XX +XXX,XX @@ static void test_cpr_exec(MigrateCommon *args) g_autofree char *filename = g_strdup_printf("%s/%s", tmpfs, FILE_TEST_FILENAME); - if (migrate_start(&from, NULL, "defer", &args->start)) { + if (migrate_start(&from, NULL, &args->start)) { return; } diff --git a/tests/qtest/migration/file-tests.c b/tests/qtest/migration/file-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/file-tests.c +++ b/tests/qtest/migration/file-tests.c @@ -XXX,XX +XXX,XX @@ static void test_file_connect_outgoing_fd_leak(char *name, MigrateCommon *args) return; } - args->listen_uri = "defer"; - if (migrate_start(&from, &to, args->listen_uri, &args->start)) { + if (migrate_start(&from, &to, &args->start)) { return; } diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/framework.c +++ b/tests/qtest/migration/framework.c @@ -XXX,XX +XXX,XX @@ static char *migrate_mem_type_get_opts(MemType type, const char *memory_size) return opts; } -int migrate_args(char **from, char **to, const char *uri, MigrateStart *args) +int migrate_args(char **from, char **to, MigrateStart *args) { /* options for source and target */ g_autofree gchar *arch_opts = NULL; @@ -XXX,XX +XXX,XX @@ int migrate_args(char **from, char **to, const char *uri, MigrateStart *args) "-name target,debug-threads=on " "%s " "-serial file:%s/dest_serial " - "-incoming %s " + "-incoming defer " "%s %s %s %s", kvm_opts ? kvm_opts : "", machine, machine_opts, - memory_backend, tmpfs, uri, + memory_backend, tmpfs, events, arch_opts ? arch_opts : "", args->opts_target ? args->opts_target : "", @@ -XXX,XX +XXX,XX @@ static void migrate_mem_type_cleanup(MemType type) } } -int migrate_start(QTestState **from, QTestState **to, const char *uri, - MigrateStart *args) +int migrate_start(QTestState **from, QTestState **to, MigrateStart *args) { g_autofree gchar *cmd_source = NULL; g_autofree gchar *cmd_target = NULL; @@ -XXX,XX +XXX,XX @@ int migrate_start(QTestState **from, QTestState **to, const char *uri, bootfile_create(qtest_get_arch(), tmpfs, args->suspend_me); src_state.suspend_me = args->suspend_me; - if (migrate_args(&cmd_source, &cmd_target, uri, args)) { + if (migrate_args(&cmd_source, &cmd_target, args)) { return -1; } @@ -XXX,XX +XXX,XX @@ static int migrate_postcopy_prepare(QTestState **from_ptr, args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_BLOCKTIME] = true; args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_RAM] = true; - if (migrate_start(&from, &to, "defer", &args->start)) { + if (migrate_start(&from, &to, &args->start)) { return -1; } @@ -XXX,XX +XXX,XX @@ int test_precopy_common(MigrateCommon *args) args->listen_uri = "tcp:127.0.0.1:0"; } - if (migrate_start(&from, &to, "defer", &args->start)) { + if (migrate_start(&from, &to, &args->start)) { return -1; } @@ -XXX,XX +XXX,XX @@ void test_file_common(MigrateCommon *args, bool stop_src) bool check_offset = false; g_autofree char *uri = NULL; - if (migrate_start(&from, &to, "defer", &args->start)) { + if (migrate_start(&from, &to, &args->start)) { return; } diff --git a/tests/qtest/migration/misc-tests.c b/tests/qtest/migration/misc-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/misc-tests.c +++ b/tests/qtest/migration/misc-tests.c @@ -XXX,XX +XXX,XX @@ static void test_baddest(char *name, MigrateCommon *args) args->start.hide_stderr = true; - if (migrate_start(&from, &to, "defer", &args->start)) { + if (migrate_start(&from, &to, &args->start)) { return; } @@ -XXX,XX +XXX,XX @@ static void test_analyze_script(char *name, MigrateCommon *args) return; } - if (migrate_start(&from, &to, "defer", &args->start)) { + if (migrate_start(&from, &to, &args->start)) { return; } @@ -XXX,XX +XXX,XX @@ static void do_test_validate_uuid(MigrateStart *args, bool should_fail) g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); QTestState *from, *to; - if (migrate_start(&from, &to, "defer", args)) { + if (migrate_start(&from, &to, args)) { return; } @@ -XXX,XX +XXX,XX @@ static void do_test_validate_uri_channel(MigrateCommon *args) QTestState *from, *to; QObject *channels; - if (migrate_start(&from, &to, "defer", &args->start)) { + if (migrate_start(&from, &to, &args->start)) { return; } @@ -XXX,XX +XXX,XX @@ static void test_validate_caps_pair(char *test_path, MigrateCommon *args) args->start.hide_stderr = true; args->start.only_source = true; - if (migrate_start(&from, &to, "defer", &args->start)) { + if (migrate_start(&from, &to, &args->start)) { return; } diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/precopy-tests.c +++ b/tests/qtest/migration/precopy-tests.c @@ -XXX,XX +XXX,XX @@ static void test_auto_converge(char *name, MigrateCommon *args) int64_t percentage; const int64_t init_pct = 5, inc_pct = 25, max_pct = 95; - if (migrate_start(&from, &to, "defer", &args->start)) { + if (migrate_start(&from, &to, &args->start)) { return; } @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_cancel(MigrateCommon *args, bool postcopy_ram) args->start.hide_stderr = true; - if (migrate_start(&from, &to, "defer", &args->start)) { + if (migrate_start(&from, &to, &args->start)) { return; } @@ -XXX,XX +XXX,XX @@ static void test_multifd_tcp_cancel(MigrateCommon *args, bool postcopy_ram) args->start.only_target = true; - if (migrate_start(&from, &to2, "defer", &args->start)) { + if (migrate_start(&from, &to2, &args->start)) { return; } @@ -XXX,XX +XXX,XX @@ static void test_cancel_src_after_status(char *test_path, MigrateCommon *args) args->start.hide_stderr = true; - if (migrate_start(&from, &to, "defer", &args->start)) { + if (migrate_start(&from, &to, &args->start)) { return; } @@ -XXX,XX +XXX,XX @@ static void test_dirty_limit(char *name, MigrateCommon *args) args->connect_uri = uri; /* Start src, dst vm */ - if (migrate_start(&from, &to, "defer", &args->start)) { + if (migrate_start(&from, &to, &args->start)) { return; } @@ -XXX,XX +XXX,XX @@ static void test_dirty_limit(char *name, MigrateCommon *args) args->start.use_dirty_ring = true; /* Restart dst vm, src vm already show up so we needn't wait anymore */ - if (migrate_start(&from, &to, "defer", &args->start)) { + if (migrate_start(&from, &to, &args->start)) { return; } -- 2.53.0 From: Fabiano Rosas <farosas@suse.de> The migration tests have always used localhost migration and therefore the same URI for both sides of migration. Change the listen_uri and connect_uri into a single uri variable. For migrations using sockets, there's the possibility of detecting the socket address the destination side is using. For those, keep using different variables for migrate_qmp and migrate_incoming_qmp. Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Lukas Straub <lukasstraub2@web.de> Tested-by: Lukas Straub <lukasstraub2@web.de> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20260505160915.25558-16-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/framework.h | 14 ++++++------- tests/qtest/migration/colo-tests.c | 8 ++++---- tests/qtest/migration/cpr-tests.c | 6 +++--- tests/qtest/migration/file-tests.c | 10 ++++----- tests/qtest/migration/framework.c | 29 ++++++++++----------------- tests/qtest/migration/misc-tests.c | 4 ++-- tests/qtest/migration/precopy-tests.c | 17 ++++++++-------- tests/qtest/migration/tls-tests.c | 2 +- 8 files changed, 40 insertions(+), 50 deletions(-) diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/framework.h +++ b/tests/qtest/migration/framework.h @@ -XXX,XX +XXX,XX @@ typedef struct { /* Optional: fine tune start parameters */ MigrateStart start; - /* Required: the URI for the dst QEMU to listen on */ - const char *listen_uri; - /* - * Optional: the URI for the src QEMU to connect to - * If NULL, then it will query the dst QEMU for its actual - * listening address and use that as the connect address. - * This allows for dynamically picking a free TCP port. + * Optional: the migration URI. If NULL, the common code should + * provide a default. For socket migration, the source QEMU may + * query the dst QEMU for the listening address and use that as + * the connection address. This allows for dynamically picking a + * free TCP port. */ - const char *connect_uri; + const char *uri; /* * Optional: JSON-formatted list of src QEMU URIs. If a port is diff --git a/tests/qtest/migration/colo-tests.c b/tests/qtest/migration/colo-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/colo-tests.c +++ b/tests/qtest/migration/colo-tests.c @@ -XXX,XX +XXX,XX @@ static int test_colo_common(MigrateCommon *args, data_hook = args->start_hook(from, to); } - migrate_incoming_qmp(to, args->listen_uri, NULL, "{}"); + migrate_incoming_qmp(to, args->uri, NULL, "{}"); migrate_ensure_converge(from); wait_for_serial("src_serial"); - migrate_qmp(from, to, args->connect_uri, NULL, "{}"); + migrate_qmp(from, to, NULL, NULL, "{}"); wait_for_migration_status(from, "colo", NULL); wait_for_resume(to, get_dst()); @@ -XXX,XX +XXX,XX @@ static void test_colo_plain_common(MigrateCommon *args, bool failover_during_checkpoint, bool primary_failover) { - args->listen_uri = "tcp:127.0.0.1:0"; + args->uri = "tcp:127.0.0.1:0"; test_colo_common(args, failover_during_checkpoint, primary_failover); } @@ -XXX,XX +XXX,XX @@ static void test_colo_multifd_common(MigrateCommon *args, bool failover_during_checkpoint, bool primary_failover) { - args->listen_uri = "tcp:127.0.0.1:0"; + args->uri = "tcp:127.0.0.1:0"; args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true; test_colo_common(args, failover_during_checkpoint, primary_failover); } diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/cpr-tests.c +++ b/tests/qtest/migration/cpr-tests.c @@ -XXX,XX +XXX,XX @@ static void test_mode_reboot(char *name, MigrateCommon *args) g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, FILE_TEST_FILENAME); - args->connect_uri = uri; + args->uri = uri; args->start_hook = migrate_hook_start_mode_reboot; args->start.mem_type = MEM_TYPE_SHMEM; @@ -XXX,XX +XXX,XX @@ static void test_cpr_exec(MigrateCommon *args) { QTestState *from, *to; void *data_hook = NULL; - g_autofree char *connect_uri = g_strdup(args->connect_uri); + g_autofree char *connect_uri = g_strdup(args->uri); g_autofree char *filename = g_strdup_printf("%s/%s", tmpfs, FILE_TEST_FILENAME); @@ -XXX,XX +XXX,XX @@ static void test_mode_exec(char *name, MigrateCommon *args) { g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, FILE_TEST_FILENAME); - args->connect_uri = uri; + args->uri = uri; args->start_hook = test_mode_exec_start; args->start.only_source = true; diff --git a/tests/qtest/migration/file-tests.c b/tests/qtest/migration/file-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/file-tests.c +++ b/tests/qtest/migration/file-tests.c @@ -XXX,XX +XXX,XX @@ static void test_precopy_file_offset_fdset(char *name, MigrateCommon *args) { g_autofree char *uri = g_strdup_printf("file:/dev/fdset/1,offset=%d", FILE_TEST_OFFSET); - args->connect_uri = uri; + args->uri = uri; args->start_hook = migrate_hook_start_file_offset_fdset; test_file_common(args, false); @@ -XXX,XX +XXX,XX @@ static void test_precopy_file_offset(char *name, MigrateCommon *args) FILE_TEST_FILENAME, FILE_TEST_OFFSET); - args->connect_uri = uri; + args->uri = uri; test_file_common(args, false); } @@ -XXX,XX +XXX,XX @@ static void test_precopy_file_offset_bad(char *name, MigrateCommon *args) g_autofree char *uri = g_strdup_printf("file:%s/%s,offset=0x20M", tmpfs, FILE_TEST_FILENAME); - args->connect_uri = uri; + args->uri = uri; args->result = MIG_TEST_QMP_ERROR; test_file_common(args, false); @@ -XXX,XX +XXX,XX @@ static void test_multifd_file_mapped_ram_fdset(char *name, MigrateCommon *args) g_autofree char *uri = g_strdup_printf("file:/dev/fdset/1,offset=%d", FILE_TEST_OFFSET); - args->connect_uri = uri; + args->uri = uri; args->start_hook = migrate_hook_start_multifd_mapped_ram_fdset; args->end_hook = migrate_hook_end_multifd_mapped_ram_fdset; @@ -XXX,XX +XXX,XX @@ static void test_multifd_file_mapped_ram_fdset_dio(char *name, { g_autofree char *uri = g_strdup_printf("file:/dev/fdset/1,offset=%d", FILE_TEST_OFFSET); - args->connect_uri = uri; + args->uri = uri; args->start_hook = migrate_hook_start_multifd_mapped_ram_fdset_dio; args->end_hook = migrate_hook_end_multifd_mapped_ram_fdset; diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/framework.c +++ b/tests/qtest/migration/framework.c @@ -XXX,XX +XXX,XX @@ int test_precopy_common(MigrateCommon *args) QTestState *from, *to; void *data_hook = NULL; QObject *channels = NULL; - - assert(!args->connect_uri); - - if (args->listen_uri) { - args->connect_uri = args->listen_uri; - } else { - args->listen_uri = "tcp:127.0.0.1:0"; - } + const char *listen_uri = args->uri ?: "tcp:127.0.0.1:0"; if (migrate_start(&from, &to, &args->start)) { return -1; @@ -XXX,XX +XXX,XX @@ int test_precopy_common(MigrateCommon *args) data_hook = args->start_hook(from, to); } - migrate_incoming_qmp(to, args->listen_uri, NULL, "{}"); + migrate_incoming_qmp(to, listen_uri, NULL, "{}"); /* Wait for the first serial output from the source */ if (args->result == MIG_TEST_SUCCEED) { @@ -XXX,XX +XXX,XX @@ int test_precopy_common(MigrateCommon *args) } if (args->result == MIG_TEST_QMP_ERROR) { - migrate_qmp_fail(from, args->connect_uri, channels, "{}"); + migrate_qmp_fail(from, args->uri, channels, "{}"); goto finish; } - migrate_qmp(from, to, args->connect_uri, channels, "{}"); + migrate_qmp(from, to, args->uri, channels, "{}"); if (args->result != MIG_TEST_SUCCEED) { bool allow_active = args->result == MIG_TEST_FAIL; @@ -XXX,XX +XXX,XX @@ void test_precopy_unix_common(MigrateCommon *args) { g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); - args->listen_uri = uri; + args->uri = uri; test_precopy_common(args); } @@ -XXX,XX +XXX,XX @@ void test_file_common(MigrateCommon *args, bool stop_src) return; } - if (!args->connect_uri) { + if (!args->uri) { uri = g_strdup_printf("file:%s/%s", tmpfs, FILE_TEST_FILENAME); - args->connect_uri = uri; + args->uri = uri; } /* @@ -XXX,XX +XXX,XX @@ void test_file_common(MigrateCommon *args, bool stop_src) */ g_assert_false(args->live); - if (g_strrstr(args->connect_uri, "offset=")) { + if (g_strrstr(args->uri, "offset=")) { check_offset = true; /* * This comes before the start_hook because it's equivalent to @@ -XXX,XX +XXX,XX @@ void test_file_common(MigrateCommon *args, bool stop_src) } if (args->result == MIG_TEST_QMP_ERROR) { - migrate_qmp_fail(from, args->connect_uri, NULL, "{}"); + migrate_qmp_fail(from, args->uri, NULL, "{}"); goto finish; } - migrate_qmp(from, to, args->connect_uri, NULL, "{}"); + migrate_qmp(from, to, args->uri, NULL, "{}"); wait_for_migration_complete(from); /* * We need to wait for the source to finish before starting the * destination. */ - migrate_incoming_qmp(to, args->connect_uri, NULL, "{}"); + migrate_incoming_qmp(to, args->uri, NULL, "{}"); wait_for_migration_complete(to); if (stop_src) { diff --git a/tests/qtest/migration/misc-tests.c b/tests/qtest/migration/misc-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/misc-tests.c +++ b/tests/qtest/migration/misc-tests.c @@ -XXX,XX +XXX,XX @@ static void do_test_validate_uri_channel(MigrateCommon *args) channels = args->connect_channels ? qobject_from_json(args->connect_channels, &error_abort) : NULL; - migrate_qmp_fail(from, args->connect_uri, channels, "{}"); + migrate_qmp_fail(from, args->uri, channels, "{}"); migrate_end(from, to, false); } @@ -XXX,XX +XXX,XX @@ static void test_validate_caps_pair(char *test_path, MigrateCommon *args) static void test_validate_uri_channels_both_set(char *name, MigrateCommon *args) { - args->connect_uri = "tcp:127.0.0.1:0", + args->uri = "tcp:127.0.0.1:0", args->connect_channels = ("[ { ""'channel-type': 'main'," " 'addr': { 'transport': 'socket'," " 'type': 'inet'," diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/precopy-tests.c +++ b/tests/qtest/migration/precopy-tests.c @@ -XXX,XX +XXX,XX @@ static void __test_precopy_rdma_plain(MigrateCommon *args, bool ipv6) **/ g_autofree char *uri = g_strdup_printf("rdma:%s:29200", buffer); - args->listen_uri = uri; + args->uri = uri; test_precopy_common(args); } @@ -XXX,XX +XXX,XX @@ static void migrate_hook_end_fd(QTestState *from, static void test_precopy_fd_socket(char *name, MigrateCommon *args) { - args->listen_uri = "fd:fd-mig"; + args->uri = "fd:fd-mig"; args->start_hook = migrate_hook_start_fd; args->end_hook = migrate_hook_end_fd; @@ -XXX,XX +XXX,XX @@ static void test_dirty_limit(char *name, MigrateCommon *args) args->start.hide_stderr = true; args->start.use_dirty_ring = true; - args->connect_uri = uri; + args->uri = uri; /* Start src, dst vm */ if (migrate_start(&from, &to, &args->start)) { @@ -XXX,XX +XXX,XX @@ static void test_dirty_limit(char *name, MigrateCommon *args) migrate_dirty_limit_wait_showup(from, dirtylimit_period, dirtylimit_value); /* Start migrate */ - migrate_incoming_qmp(to, args->connect_uri, NULL, "{}"); - migrate_qmp(from, to, args->connect_uri, NULL, "{}"); + migrate_incoming_qmp(to, args->uri, NULL, "{}"); + migrate_qmp(from, to, args->uri, NULL, "{}"); /* Wait for dirty limit throttle begin */ throttle_us_per_full = 0; @@ -XXX,XX +XXX,XX @@ static void test_dirty_limit(char *name, MigrateCommon *args) /* Assert dirty limit is not in service */ g_assert_cmpint(throttle_us_per_full, ==, 0); - args->listen_uri = uri; - args->connect_uri = uri; + args->uri = uri; args->start.only_target = true; args->start.use_dirty_ring = true; @@ -XXX,XX +XXX,XX @@ static void test_dirty_limit(char *name, MigrateCommon *args) } /* Start migrate */ - migrate_incoming_qmp(to, args->listen_uri, NULL, "{}"); - migrate_qmp(from, to, args->connect_uri, NULL, "{}"); + migrate_incoming_qmp(to, args->uri, NULL, "{}"); + migrate_qmp(from, to, args->uri, NULL, "{}"); /* Wait for dirty limit throttle begin */ throttle_us_per_full = 0; diff --git a/tests/qtest/migration/tls-tests.c b/tests/qtest/migration/tls-tests.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration/tls-tests.c +++ b/tests/qtest/migration/tls-tests.c @@ -XXX,XX +XXX,XX @@ static void test_precopy_unix_tls_x509_default_host(char *name, { g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); - args->listen_uri = uri; + args->uri = uri; args->start_hook = migrate_hook_start_tls_x509_default_host; args->end_hook = migrate_hook_end_tls_x509; args->result = MIG_TEST_FAIL; -- 2.53.0 From: Bin Guo <guobin@linux.alibaba.com> Drop the unnecessary strcpy of an empty literal (and its spurious (char *)& cast) in favor of a direct NUL store, which avoids the libc call and hides no bugs behind a cast. Signed-off-by: Bin Guo <guobin@linux.alibaba.com> Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20260518110112.21395-3-guobin@linux.alibaba.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/global_state.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migration/global_state.c b/migration/global_state.c index XXXXXXX..XXXXXXX 100644 --- a/migration/global_state.c +++ b/migration/global_state.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_globalstate = { void register_global_state(void) { /* We would use it independently that we receive it */ - strcpy((char *)&global_state.runstate, ""); + global_state.runstate[0] = '\0'; global_state.received = false; vmstate_register(NULL, 0, &vmstate_globalstate, &global_state); } -- 2.53.0 From: Bin Guo <guobin@linux.alibaba.com> For every NULL slot in a VMS_ARRAY_OF_POINTER (or every entry of a dynamic array), the saver allocates a 1-element fake VMStateField via g_new0 and frees it again right after the save. For arrays of thousands of entries this is thousands of malloc/free pairs on the hot save path. Replace the heap-allocated marker with a stack-resident field populated by an init helper. The caller passes a pointer to a local VMStateField, the helper fills it in (still asserting the precondition), and no g_free is needed. Signed-off-by: Bin Guo <guobin@linux.alibaba.com> Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20260518110112.21395-4-guobin@linux.alibaba.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/vmstate.c | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/migration/vmstate.c b/migration/vmstate.c index XXXXXXX..XXXXXXX 100644 --- a/migration/vmstate.c +++ b/migration/vmstate.c @@ -XXX,XX +XXX,XX @@ vmstate_field_exists(const VMStateDescription *vmsd, const VMStateField *field, * array of a VMS_ARRAY_OF_POINTER VMSD field. It's needed because we * can't dereference the NULL pointer. */ -static const VMStateField * -vmsd_create_ptr_marker_field(const VMStateField *field) +static void +vmsd_init_ptr_marker_field(VMStateField *fake, const VMStateField *field) { - VMStateField *fake = g_new0(VMStateField, 1); - /* It can only happen on an array of pointers! */ assert(field->flags & VMS_ARRAY_OF_POINTER); - /* Some of fake's properties should match the original's */ - fake->name = field->name; - fake->version_id = field->version_id; - - /* Do not need "field_exists" check as it always exists */ - fake->field_exists = NULL; - - /* See vmstate_info_ptr_marker - use 1 byte to represent ptr status */ - fake->size = 1; - fake->info = &vmstate_info_ptr_marker; - fake->flags = VMS_SINGLE; - - /* All the rest fields shouldn't matter.. */ - - return (const VMStateField *)fake; + /* See vmstate_info_ptr_marker - 1 byte represents ptr status */ + *fake = (VMStateField) { + .name = field->name, + .version_id = field->version_id, + /* Marker always exists, no field_exists callback needed */ + .field_exists = NULL, + .size = 1, + .info = &vmstate_info_ptr_marker, + .flags = VMS_SINGLE, + /* All other fields stay zero-initialised */ + }; } static int vmstate_n_elems(void *opaque, const VMStateField *field) @@ -XXX,XX +XXX,XX @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd, for (i = 0; i < n_elems; i++) { void *curr_elem = first_elem + size * i; const VMStateField *inner_field; + VMStateField marker_field; /* maximum number of elements to compress in the JSON blob */ int max_elems = vmsd_can_compress(field) ? (n_elems - i) : 1; bool use_marker_field, is_null = false; @@ -XXX,XX +XXX,XX @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd, use_marker_field = use_dynamic_array || is_null; if (use_marker_field) { - inner_field = vmsd_create_ptr_marker_field(field); + vmsd_init_ptr_marker_field(&marker_field, field); + inner_field = &marker_field; } else { inner_field = field; } @@ -XXX,XX +XXX,XX @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd, inner_field, vmdesc_loop, i, max_elems, errp); - /* If we used a fake temp field.. free it now */ - if (use_marker_field) { - g_clear_pointer((gpointer *)&inner_field, g_free); - } - if (!ok) { goto out; } -- 2.53.0 From: Bin Guo <guobin@linux.alibaba.com> configuration_validate_capabilities() allocates a bitmap on the heap to track source capabilities via bitmap_new()/g_free(). Since MIGRATION_CAPABILITY__MAX is a small compile-time constant (< 64), a heap allocation for a bitmap this small is wasteful: it adds malloc/free overhead and a potential cache miss for a transient 8-byte allocation. Replace with DECLARE_BITMAP() on the stack and bitmap_zero() to initialize. This eliminates the heap round-trip entirely. Signed-off-by: Bin Guo <guobin@linux.alibaba.com> Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20260518110112.21395-5-guobin@linux.alibaba.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/savevm.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/migration/savevm.c b/migration/savevm.c index XXXXXXX..XXXXXXX 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -XXX,XX +XXX,XX @@ static bool configuration_validate_capabilities(SaveState *state) { bool ret = true; MigrationState *s = migrate_get_current(); - unsigned long *source_caps_bm; + DECLARE_BITMAP(source_caps_bm, MIGRATION_CAPABILITY__MAX); int i; - source_caps_bm = bitmap_new(MIGRATION_CAPABILITY__MAX); + bitmap_zero(source_caps_bm, MIGRATION_CAPABILITY__MAX); for (i = 0; i < state->caps_count; i++) { MigrationCapability capability = state->capabilities[i]; set_bit(capability, source_caps_bm); @@ -XXX,XX +XXX,XX @@ static bool configuration_validate_capabilities(SaveState *state) } } - g_free(source_caps_bm); return ret; } -- 2.53.0 From: Bin Guo <guobin@linux.alibaba.com> multifd_recv_initial_packet() validates the channel ID received from the source against the configured number of channels. The current check uses '>' which allows msg.id == N to pass through. This ID is then used to index multifd_recv_state->params[msg.id], which was allocated with g_new0(MultiFDRecvParams, N) -- an out-of-bounds access. A malicious or buggy source could send id == N and cause heap corruption on the destination. Fix by changing '>' to '>='. Also fix the error message to say "exceeds channel count" for accuracy. Signed-off-by: Bin Guo <guobin@linux.alibaba.com> Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20260518110112.21395-6-guobin@linux.alibaba.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/multifd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/migration/multifd.c b/migration/multifd.c index XXXXXXX..XXXXXXX 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -XXX,XX +XXX,XX @@ static int multifd_recv_initial_packet(QIOChannel *c, Error **errp) return -1; } - if (msg.id > migrate_multifd_channels()) { - error_setg(errp, "multifd: received channel id %u is greater than " - "number of channels %u", msg.id, migrate_multifd_channels()); + if (msg.id >= migrate_multifd_channels()) { + error_setg(errp, "multifd: received channel id %u exceeds " + "channel count %u", msg.id, migrate_multifd_channels()); return -1; } -- 2.53.0 From: Bin Guo <guobin@linux.alibaba.com> multifd_send() and multifd_recv() are on the per-page-batch hot path of live migration. Both functions call migrate_multifd_channels() multiple times (3-4 calls each) for modulo arithmetic in the round-robin channel selection loop. Each call goes through migrate_get_current() -> dereference MigrationState -> read parameters.multifd_channels. While each individual call is cheap, these functions execute for every page batch during the entire migration, easily millions of times. Cache the return value in a local variable at function entry. The channel count is fixed for the duration of a migration and cannot change mid-flight. For multifd_send(): 3 calls reduced to 1. For multifd_recv(): 4 calls reduced to 1. Signed-off-by: Bin Guo <guobin@linux.alibaba.com> Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20260518110112.21395-8-guobin@linux.alibaba.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/multifd.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/migration/multifd.c b/migration/multifd.c index XXXXXXX..XXXXXXX 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -XXX,XX +XXX,XX @@ bool multifd_send(MultiFDSendData **send_data) /* We wait here, until at least one channel is ready */ qemu_sem_wait(&multifd_send_state->channels_ready); + int thread_count = migrate_multifd_channels(); + /* * next_channel can remain from a previous migration that was * using more channels, so ensure it doesn't overflow if the * limit is lower now. */ - next_channel %= migrate_multifd_channels(); - for (i = next_channel;; i = (i + 1) % migrate_multifd_channels()) { + next_channel %= thread_count; + for (i = next_channel;; i = (i + 1) % thread_count) { if (multifd_send_should_exit()) { return false; } @@ -XXX,XX +XXX,XX @@ bool multifd_send(MultiFDSendData **send_data) * sender thread can clear it. */ if (qatomic_read(&p->pending_job) == false) { - next_channel = (i + 1) % migrate_multifd_channels(); + next_channel = (i + 1) % thread_count; break; } } @@ -XXX,XX +XXX,XX @@ bool multifd_recv(void) int i; static int next_recv_channel; MultiFDRecvParams *p = NULL; + int thread_count = migrate_multifd_channels(); MultiFDRecvData *data = multifd_recv_state->data; /* @@ -XXX,XX +XXX,XX @@ bool multifd_recv(void) * using more channels, so ensure it doesn't overflow if the * limit is lower now. */ - next_recv_channel %= migrate_multifd_channels(); - for (i = next_recv_channel;; i = (i + 1) % migrate_multifd_channels()) { + next_recv_channel %= thread_count; + for (i = next_recv_channel;; i = (i + 1) % thread_count) { if (multifd_recv_should_exit()) { return false; } @@ -XXX,XX +XXX,XX @@ bool multifd_recv(void) p = &multifd_recv_state->params[i]; if (qatomic_read(&p->pending_job) == false) { - next_recv_channel = (i + 1) % migrate_multifd_channels(); + next_recv_channel = (i + 1) % thread_count; break; } } -- 2.53.0 From: Bin Guo <guobin@linux.alibaba.com> multifd_send_sync_main() is called once per RAM synchronization round during live migration. It iterates over all multifd channels twice (signal loop + wait loop), calling migrate_multifd_channels() independently in each loop header. Cache migrate_multifd_channels() in a local thread_count variable at function entry, matching the pattern already used in multifd_send_setup() and multifd_recv_setup(). This eliminates 2 redundant config lookups per sync call. Signed-off-by: Bin Guo <guobin@linux.alibaba.com> Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20260518110112.21395-9-guobin@linux.alibaba.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/multifd.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/migration/multifd.c b/migration/multifd.c index XXXXXXX..XXXXXXX 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -XXX,XX +XXX,XX @@ static int multifd_zero_copy_flush(QIOChannel *c) int multifd_send_sync_main(MultiFDSyncReq req) { int i; + int thread_count; bool flush_zero_copy; assert(req != MULTIFD_SYNC_NONE); + thread_count = migrate_multifd_channels(); flush_zero_copy = migrate_zero_copy_send(); - for (i = 0; i < migrate_multifd_channels(); i++) { + for (i = 0; i < thread_count; i++) { MultiFDSendParams *p = &multifd_send_state->params[i]; if (multifd_send_should_exit()) { @@ -XXX,XX +XXX,XX @@ int multifd_send_sync_main(MultiFDSyncReq req) qatomic_set(&p->pending_sync, req); qemu_sem_post(&p->sem); } - for (i = 0; i < migrate_multifd_channels(); i++) { + for (i = 0; i < thread_count; i++) { MultiFDSendParams *p = &multifd_send_state->params[i]; if (multifd_send_should_exit()) { -- 2.53.0 From: hongmianquan <hongmianquan@bytedance.com> Use a GHashTable to store cpr fds to reduce the time consumption of `cpr_find_fd` in scenarios with a large number of fds. The time complexity for `cpr_find_fd` is reduced from O(N) to O(1). Keep cpr fds lookups in a GHashTable during normal runtime while preserving the existing QLIST migration ABI. Build a temporary QLIST from the hash table in pre_save and rebuild the hash table from the loaded QLIST in post_load. To demonstrate the performance improvement, we tested the total time consumed by `cpr_find_fd` (called N times for N fds) under our real-world business scenarios with different numbers of file descriptors. The results are measured in nanoseconds: | Number of FDs | Total time with QLIST (ns) | Total time with GHashTable (ns) | |---------------|----------------------------|---------------------------------| | 540 | 936,753 | 393,358 | | 2,870 | 24,102,342 | 2,212,113 | | 7,530 | 152,715,916 | 5,474,310 | As shown in the data, the lookup time grows exponentially with the QLIST as the number of fds increases. With the GHashTable, the time consumption remains linear (O(1) per lookup), significantly reducing the downtime during the CPR process. Signed-off-by: hongmianquan <hongmianquan@bytedance.com> Link: https://lore.kernel.org/r/20260519134315.27997-1-hongmianquan@bytedance.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/cpr.c | 116 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 98 insertions(+), 18 deletions(-) diff --git a/migration/cpr.c b/migration/cpr.c index XXXXXXX..XXXXXXX 100644 --- a/migration/cpr.c +++ b/migration/cpr.c @@ -XXX,XX +XXX,XX @@ /* cpr state container for all information to be saved. */ CprState cpr_state; +static GHashTable *cpr_fds_hash; /****************************************************************************/ @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_cpr_fd = { } }; +static guint cpr_fd_hash(gconstpointer v) +{ + const CprFd *elem = v; + + return g_str_hash(elem->name) ^ elem->id; +} + +static gboolean cpr_fd_equal(gconstpointer a, gconstpointer b) +{ + const CprFd *elem_a = a; + const CprFd *elem_b = b; + + return !strcmp(elem_a->name, elem_b->name) && elem_a->id == elem_b->id; +} + +static void cpr_fd_destroy(gpointer data) +{ + CprFd *elem = data; + + g_free(elem->name); + g_free(elem); +} + +static GHashTable *get_cpr_fds_hash(void) +{ + if (!cpr_fds_hash) { + cpr_fds_hash = g_hash_table_new_full(cpr_fd_hash, cpr_fd_equal, + cpr_fd_destroy, NULL); + } + + return cpr_fds_hash; +} + +static void cpr_fd_hash_insert(CprFd *elem) +{ + /* Use the same CprFd as key and value. */ + g_hash_table_insert(get_cpr_fds_hash(), elem, elem); +} + +static int cpr_fd_pre_save(void *opaque) +{ + CprState *state = (CprState *)opaque; + GHashTableIter iter; + CprFd *elem; + + QLIST_INIT(&state->fds); + + g_hash_table_iter_init(&iter, get_cpr_fds_hash()); + while (g_hash_table_iter_next(&iter, (gpointer *)&elem, NULL)) { + QLIST_INSERT_HEAD(&state->fds, elem, next); + } + + return 0; +} + +static int cpr_fd_post_load(void *opaque, int version_id) +{ + CprState *state = (CprState *)opaque; + CprFd *elem; + + while ((elem = QLIST_FIRST(&state->fds))) { + QLIST_REMOVE(elem, next); + + /* + * Preserve legacy QLIST lookup semantics if duplicate keys exist in + * the incoming stream: the first matching entry wins. + */ + if (g_hash_table_contains(get_cpr_fds_hash(), elem)) { + cpr_fd_destroy(elem); + continue; + } + + cpr_fd_hash_insert(elem); + } + + return 0; +} + void cpr_save_fd(const char *name, int id, int fd) { CprFd *elem = g_new0(CprFd, 1); @@ -XXX,XX +XXX,XX @@ void cpr_save_fd(const char *name, int id, int fd) elem->namelen = strlen(name) + 1; elem->id = id; elem->fd = fd; - QLIST_INSERT_HEAD(&cpr_state.fds, elem, next); + cpr_fd_hash_insert(elem); } -static CprFd *find_fd(CprFdList *head, const char *name, int id) +static CprFd *find_fd(const char *name, int id) { - CprFd *elem; + CprFd key = { + .name = (char *)name, + .id = id, + }; - QLIST_FOREACH(elem, head, next) { - if (!strcmp(elem->name, name) && elem->id == id) { - return elem; - } - } - return NULL; + return g_hash_table_lookup(get_cpr_fds_hash(), &key); } void cpr_delete_fd(const char *name, int id) { - CprFd *elem = find_fd(&cpr_state.fds, name, id); + CprFd key = { + .name = (char *)name, + .id = id, + }; - if (elem) { - QLIST_REMOVE(elem, next); - g_free(elem->name); - g_free(elem); - } + g_hash_table_remove(get_cpr_fds_hash(), &key); trace_cpr_delete_fd(name, id); } int cpr_find_fd(const char *name, int id) { - CprFd *elem = find_fd(&cpr_state.fds, name, id); + CprFd *elem = find_fd(name, id); int fd = elem ? elem->fd : -1; trace_cpr_find_fd(name, id, fd); @@ -XXX,XX +XXX,XX @@ int cpr_find_fd(const char *name, int id) void cpr_resave_fd(const char *name, int id, int fd) { - CprFd *elem = find_fd(&cpr_state.fds, name, id); + CprFd *elem = find_fd(name, id); int old_fd = elem ? elem->fd : -1; if (old_fd < 0) { @@ -XXX,XX +XXX,XX @@ int cpr_open_fd(const char *path, int flags, const char *name, int id, bool cpr_walk_fd(cpr_walk_fd_cb cb) { + GHashTableIter iter; CprFd *elem; - QLIST_FOREACH(elem, &cpr_state.fds, next) { + g_hash_table_iter_init(&iter, get_cpr_fds_hash()); + while (g_hash_table_iter_next(&iter, (gpointer *)&elem, NULL)) { g_assert(elem->fd >= 0); if (!cb(elem->fd)) { return false; @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_cpr_state = { .name = CPR_STATE, .version_id = 1, .minimum_version_id = 1, + .pre_save = cpr_fd_pre_save, + .post_load = cpr_fd_post_load, .fields = (VMStateField[]) { VMSTATE_QLIST_V(fds, CprState, 1, vmstate_cpr_fd, CprFd, next), VMSTATE_END_OF_LIST() -- 2.53.0 From: Hyman Huang <yong.huang@bitdeer.com> I left SmartX two weeks ago. Update my email to stay reachable. Signed-off-by: Hyman Huang <infra.ai.cloud@bitdeer.com> Link: https://lore.kernel.org/r/b3bd81c3d9f425bb750a76d7bad7ad0284e55123.1779178180.git.infra.ai.cloud@bitdeer.com [peterx: fix address, s/biitdeer/bitdeer/] Signed-off-by: Peter Xu <peterx@redhat.com> --- MAINTAINERS | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ F: migration/rdma* F: scripts/rdma-migration-helper.sh Migration dirty limit and dirty page rate -M: Hyman Huang <yong.huang@smartx.com> +M: Hyman Huang <infra.ai.cloud@bitdeer.com> S: Maintained F: system/dirtylimit.c F: include/system/dirtylimit.h @@ -XXX,XX +XXX,XX @@ F: include/system/dirtyrate.h F: docs/devel/migration/dirty-limit.rst Detached LUKS header -M: Hyman Huang <yong.huang@smartx.com> +M: Hyman Huang <infra.ai.cloud@bitdeer.com> S: Maintained F: tests/qemu-iotests/tests/luks-detached-header F: docs/devel/luks-detached-header.rst -- 2.53.0