The following changes since commit deabea6e88f7c4c3c12a36ee30051c6209561165: Merge tag 'for_upstream' of https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging (2023-02-02 10:10:07 +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 5ee6d3d1eeccd85aa2a835e82b8d9e1b4f7441e1: migration: check magic value for deciding the mapping of channels (2023-02-02 17:04:16 +0100) ---------------------------------------------------------------- Migration PULL request, new try Hi It includes: - David Hildenbrand fixes for virtio-men - David Gilbert canary to detect problems - Fix for rdma return values (Fiona) - Peter Xu uffd_open fixes - Peter Xu show right downtime for postcopy - manish.mishra msg fix fixes - my vfio changes. Please apply. Please, apply. ---------------------------------------------------------------- David Hildenbrand (13): migration/ram: Fix populate_read_range() migration/ram: Fix error handling in ram_write_tracking_start() migration/ram: Don't explicitly unprotect when unregistering uffd-wp migration/ram: Rely on used_length for uffd_change_protection() migration/ram: Optimize ram_write_tracking_start() for RamDiscardManager migration/savevm: Move more savevm handling into vmstate_save() migration/savevm: Prepare vmdesc json writer in qemu_savevm_state_setup() migration/savevm: Allow immutable device state to be migrated early (i.e., before RAM) migration/vmstate: Introduce VMSTATE_WITH_TMP_TEST() and VMSTATE_BITMAP_TEST() migration/ram: Factor out check for advised postcopy virtio-mem: Fail if a memory backend with "prealloc=on" is specified virtio-mem: Migrate immutable properties early virtio-mem: Proper support for preallocation with migration Dr. David Alan Gilbert (2): migration: Add canary to VMSTATE_END_OF_LIST migration: Perform vmsd structure check during tests Fiona Ebner (1): migration/rdma: fix return value for qio_channel_rdma_{readv,writev} 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 (3): migration: Fix migration crash when target psize larger than host util/userfaultfd: Add uffd_open() migration: Show downtime during postcopy phase Zhenzhong Duan (1): migration/dirtyrate: Show sample pages only in page-sampling mode manish.mishra (2): io: Add support for MSG_PEEK for socket channel migration: check magic value for deciding the mapping of channels docs/devel/migration.rst | 18 +-- docs/devel/vfio-migration.rst | 4 +- include/hw/virtio/virtio-mem.h | 8 ++ include/io/channel.h | 6 + include/migration/misc.h | 4 +- include/migration/register.h | 17 +-- include/migration/vmstate.h | 35 +++++- include/qemu/userfaultfd.h | 8 ++ migration/channel.h | 5 + migration/migration.h | 4 + migration/multifd.h | 2 +- migration/postcopy-ram.h | 2 +- migration/savevm.h | 10 +- chardev/char-socket.c | 4 +- hw/core/machine.c | 4 +- hw/s390x/s390-stattrib.c | 11 +- hw/vfio/migration.c | 20 +-- hw/virtio/virtio-mem.c | 144 ++++++++++++++++++++- io/channel-buffer.c | 1 + io/channel-command.c | 1 + io/channel-file.c | 1 + io/channel-null.c | 1 + io/channel-socket.c | 19 ++- io/channel-tls.c | 1 + io/channel-websock.c | 1 + io/channel.c | 16 ++- migration/block-dirty-bitmap.c | 14 +-- migration/block.c | 13 +- migration/channel-block.c | 1 + migration/channel.c | 45 +++++++ migration/dirtyrate.c | 10 +- migration/migration.c | 119 ++++++++++++------ migration/multifd.c | 19 +-- migration/postcopy-ram.c | 16 +-- migration/ram.c | 120 +++++++++++++----- migration/rdma.c | 16 ++- migration/savevm.c | 187 ++++++++++++++++++++-------- migration/vmstate.c | 2 + scsi/qemu-pr-helper.c | 2 +- tests/qtest/migration-test.c | 3 +- tests/qtest/tpm-emu.c | 2 +- tests/unit/test-io-channel-socket.c | 1 + util/userfaultfd.c | 13 +- util/vhost-user-server.c | 2 +- hw/vfio/trace-events | 2 +- migration/trace-events | 7 +- 46 files changed, 715 insertions(+), 226 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 | 2 +- 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(+), 10 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, +static void cmma_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/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 | 11 +++++---- 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(+), 76 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(void *opaque, uint64_t max_size, - uint64_t *res_precopy_only, - uint64_t *res_compatible, - uint64_t *res_postcopy_only) +static void cmma_state_pending(void *opaque, uint64_t max_size, + 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/s390x/s390-stattrib.c | 2 +- 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 ++-- 10 files changed, 16 insertions(+), 27 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/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_state_pending(void *opaque, uint64_t max_size, +static void cmma_state_pending(void *opaque, 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) } 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 || 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 From: Peter Xu <peterx@redhat.com> Add a helper to create the uffd handle. Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- include/qemu/userfaultfd.h | 8 ++++++++ migration/postcopy-ram.c | 11 +++++------ tests/qtest/migration-test.c | 3 ++- util/userfaultfd.c | 13 +++++++++++-- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/include/qemu/userfaultfd.h b/include/qemu/userfaultfd.h index XXXXXXX..XXXXXXX 100644 --- a/include/qemu/userfaultfd.h +++ b/include/qemu/userfaultfd.h @@ -XXX,XX +XXX,XX @@ #include "exec/hwaddr.h" #include <linux/userfaultfd.h> +/** + * uffd_open(): Open an userfaultfd handle for current context. + * + * @flags: The flags we want to pass in when creating the handle. + * + * Returns: the uffd handle if >=0, or <0 if error happens. + */ +int uffd_open(int flags); int uffd_query_features(uint64_t *features); int uffd_create_fd(uint64_t features, bool non_blocking); void uffd_close_fd(int uffd_fd); diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index XXXXXXX..XXXXXXX 100644 --- a/migration/postcopy-ram.c +++ b/migration/postcopy-ram.c @@ -XXX,XX +XXX,XX @@ #include "qemu-file.h" #include "yank_functions.h" #include "tls.h" +#include "qemu/userfaultfd.h" /* Arbitrary limit on size of each discard command, * keeps them around ~200 bytes @@ -XXX,XX +XXX,XX @@ static bool receive_ufd_features(uint64_t *features) int ufd; bool ret = true; - /* if we are here __NR_userfaultfd should exists */ - ufd = syscall(__NR_userfaultfd, O_CLOEXEC); + ufd = uffd_open(O_CLOEXEC); if (ufd == -1) { - error_report("%s: syscall __NR_userfaultfd failed: %s", __func__, - strerror(errno)); + error_report("%s: uffd_open() failed: %s", __func__, strerror(errno)); return false; } @@ -XXX,XX +XXX,XX @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis) goto out; } - ufd = syscall(__NR_userfaultfd, O_CLOEXEC); + ufd = uffd_open(O_CLOEXEC); if (ufd == -1) { error_report("%s: userfaultfd not available: %s", __func__, strerror(errno)); @@ -XXX,XX +XXX,XX @@ static int postcopy_temp_pages_setup(MigrationIncomingState *mis) int postcopy_ram_incoming_setup(MigrationIncomingState *mis) { /* Open the fd for the kernel to give us userfaults */ - mis->userfault_fd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK); + mis->userfault_fd = uffd_open(O_CLOEXEC | O_NONBLOCK); if (mis->userfault_fd == -1) { error_report("%s: Failed to open userfault fd: %s", __func__, strerror(errno)); diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -XXX,XX +XXX,XX @@ static bool uffd_feature_thread_id; #include <sys/eventfd.h> #include <sys/ioctl.h> #include <linux/userfaultfd.h> +#include "qemu/userfaultfd.h" static bool ufd_version_check(void) { struct uffdio_api api_struct; uint64_t ioctl_mask; - int ufd = syscall(__NR_userfaultfd, O_CLOEXEC); + int ufd = uffd_open(O_CLOEXEC); if (ufd == -1) { g_test_message("Skipping test: userfaultfd not available"); diff --git a/util/userfaultfd.c b/util/userfaultfd.c index XXXXXXX..XXXXXXX 100644 --- a/util/userfaultfd.c +++ b/util/userfaultfd.c @@ -XXX,XX +XXX,XX @@ #include <sys/syscall.h> #include <sys/ioctl.h> +int uffd_open(int flags) +{ +#if defined(__NR_userfaultfd) + return syscall(__NR_userfaultfd, flags); +#else + return -EINVAL; +#endif +} + /** * uffd_query_features: query UFFD features * @@ -XXX,XX +XXX,XX @@ int uffd_query_features(uint64_t *features) struct uffdio_api api_struct = { 0 }; int ret = -1; - uffd_fd = syscall(__NR_userfaultfd, O_CLOEXEC); + uffd_fd = uffd_open(O_CLOEXEC); if (uffd_fd < 0) { trace_uffd_query_features_nosys(errno); return -1; @@ -XXX,XX +XXX,XX @@ int uffd_create_fd(uint64_t features, bool non_blocking) uint64_t ioctl_mask = BIT(_UFFDIO_REGISTER) | BIT(_UFFDIO_UNREGISTER); flags = O_CLOEXEC | (non_blocking ? O_NONBLOCK : 0); - uffd_fd = syscall(__NR_userfaultfd, flags); + uffd_fd = uffd_open(flags); if (uffd_fd < 0) { trace_uffd_create_fd_nosys(errno); return -1; -- 2.39.1 From: David Hildenbrand <david@redhat.com> Unfortunately, commit f7b9dcfbcf44 broke populate_read_range(): the loop end condition is very wrong, resulting in that function not populating the full range. Lets' fix that. Fixes: f7b9dcfbcf44 ("migration/ram: Factor out populating pages readable in ram_block_populate_pages()") Cc: qemu-stable@nongnu.org Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/ram.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 @@ out: static inline void populate_read_range(RAMBlock *block, ram_addr_t offset, ram_addr_t size) { + const ram_addr_t end = offset + size; + /* * We read one byte of each page; this will preallocate page tables if * required and populate the shared zeropage on MAP_PRIVATE anonymous memory * where no page was populated yet. This might require adaption when * supporting other mappings, like shmem. */ - for (; offset < size; offset += block->page_size) { + for (; offset < end; offset += block->page_size) { char tmp = *((char *)block->host + offset); /* Don't optimize the read out */ -- 2.39.1 From: David Hildenbrand <david@redhat.com> If something goes wrong during uffd_change_protection(), we would miss to unregister uffd-wp and not release our reference. Fix it by performing the uffd_change_protection(true) last. Note that a uffd_change_protection(false) on the recovery path without a prior uffd_change_protection(false) is fine. Fixes: 278e2f551a09 ("migration: support UFFD write fault processing in ram_save_iterate()") Cc: qemu-stable@nongnu.org Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/ram.c | 5 +++-- 1 file changed, 3 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 @@ int ram_write_tracking_start(void) block->max_length, UFFDIO_REGISTER_MODE_WP, NULL)) { goto fail; } + block->flags |= RAM_UF_WRITEPROTECT; + memory_region_ref(block->mr); + /* Apply UFFD write protection to the block memory range */ if (uffd_change_protection(rs->uffdio_fd, block->host, block->max_length, true, false)) { goto fail; } - block->flags |= RAM_UF_WRITEPROTECT; - memory_region_ref(block->mr); trace_ram_write_tracking_ramblock_start(block->idstr, block->page_size, block->host, block->max_length); -- 2.39.1 From: David Hildenbrand <david@redhat.com> When unregistering uffd-wp, older kernels before commit f369b07c86143 ("mm/uffd:reset write protection when unregister with wp-mode") won't clear the uffd-wp PTE bit. When re-registering uffd-wp, the previous uffd-wp PTE bits would trigger again. With above commit, the kernel will clear the uffd-wp PTE bits when unregistering itself. Consequently, we'll clear the uffd-wp PTE bits now twice -- whereby we don't care about clearing them at all: a new background snapshot will re-register uffd-wp and re-protect all memory either way. So let's skip the manual clearing of uffd-wp. If ever relevant, we could clear conditionally in uffd_unregister_memory() -- we just need a way to figure out more recent kernels. Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/ram.c | 9 --------- 1 file changed, 9 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 @@ fail: if ((block->flags & RAM_UF_WRITEPROTECT) == 0) { continue; } - /* - * In case some memory block failed to be write-protected - * remove protection and unregister all succeeded RAM blocks - */ - uffd_change_protection(rs->uffdio_fd, block->host, block->max_length, - false, false); uffd_unregister_memory(rs->uffdio_fd, block->host, block->max_length); /* Cleanup flags and remove reference */ block->flags &= ~RAM_UF_WRITEPROTECT; @@ -XXX,XX +XXX,XX @@ void ram_write_tracking_stop(void) if ((block->flags & RAM_UF_WRITEPROTECT) == 0) { continue; } - /* Remove protection and unregister all affected RAM blocks */ - uffd_change_protection(rs->uffdio_fd, block->host, block->max_length, - false, false); uffd_unregister_memory(rs->uffdio_fd, block->host, block->max_length); trace_ram_write_tracking_ramblock_stop(block->idstr, block->page_size, -- 2.39.1 From: David Hildenbrand <david@redhat.com> ram_mig_ram_block_resized() will abort migration (including background snapshots) when resizing a RAMBlock. ram_block_populate_read() will only populate RAM up to used_length, so at least for anonymous memory protecting everything between used_length and max_length won't actually be protected and is just a NOP. So let's only protect everything up to used_length. Note: it still makes sense to register uffd-wp for max_length, such that RAM_UF_WRITEPROTECT is independent of a changing used_length. Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/ram.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 @@ int ram_write_tracking_start(void) /* Apply UFFD write protection to the block memory range */ if (uffd_change_protection(rs->uffdio_fd, block->host, - block->max_length, true, false)) { + block->used_length, true, false)) { goto fail; } -- 2.39.1 From: David Hildenbrand <david@redhat.com> ram_block_populate_read() already optimizes for RamDiscardManager. However, ram_write_tracking_start() will still try protecting discarded memory ranges. Let's optimize, because discarded ranges don't map any pages and (1) For anonymous memory, trying to protect using uffd-wp without a mapped page is ignored by the kernel and consequently a NOP. (2) For shared/file-backed memory, we will fill present page tables in the range with PTE markers. However, we will even allocate page tables just to fill them with unnecessary PTE markers and effectively waste memory. So let's exclude these ranges, just like ram_block_populate_read() already does. Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/ram.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 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 @@ void ram_write_tracking_prepare(void) } } +static inline int uffd_protect_section(MemoryRegionSection *section, + void *opaque) +{ + const hwaddr size = int128_get64(section->size); + const hwaddr offset = section->offset_within_region; + RAMBlock *rb = section->mr->ram_block; + int uffd_fd = (uintptr_t)opaque; + + return uffd_change_protection(uffd_fd, rb->host + offset, size, true, + false); +} + +static int ram_block_uffd_protect(RAMBlock *rb, int uffd_fd) +{ + assert(rb->flags & RAM_UF_WRITEPROTECT); + + /* See ram_block_populate_read() */ + if (rb->mr && memory_region_has_ram_discard_manager(rb->mr)) { + RamDiscardManager *rdm = memory_region_get_ram_discard_manager(rb->mr); + MemoryRegionSection section = { + .mr = rb->mr, + .offset_within_region = 0, + .size = rb->mr->size, + }; + + return ram_discard_manager_replay_populated(rdm, &section, + uffd_protect_section, + (void *)(uintptr_t)uffd_fd); + } + return uffd_change_protection(uffd_fd, rb->host, + rb->used_length, true, false); +} + /* * ram_write_tracking_start: start UFFD-WP memory tracking * @@ -XXX,XX +XXX,XX @@ int ram_write_tracking_start(void) memory_region_ref(block->mr); /* Apply UFFD write protection to the block memory range */ - if (uffd_change_protection(rs->uffdio_fd, block->host, - block->used_length, true, false)) { + if (ram_block_uffd_protect(block, uffd_fd)) { goto fail; } -- 2.39.1 From: David Hildenbrand <david@redhat.com> Let's move more code into vmstate_save(), reducing code duplication and preparing for reuse of vmstate_save() in qemu_savevm_state_setup(). We have to move vmstate_save() to make the compiler happy. We'll now also trace from qemu_save_device_state(), triggering the same tracepoints as previously called from qemu_savevm_state_complete_precopy_non_iterable() only. Note that qemu_save_device_state() ignores iterable device state, such as RAM, and consequently doesn't trigger some other trace points (e.g., trace_savevm_state_setup()). Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/savevm.c | 79 ++++++++++++++++++++++------------------------ 1 file changed, 37 insertions(+), 42 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 void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, } } -static int vmstate_save(QEMUFile *f, SaveStateEntry *se, - JSONWriter *vmdesc) -{ - trace_vmstate_save(se->idstr, se->vmsd ? se->vmsd->name : "(old)"); - if (!se->vmsd) { - vmstate_save_old_style(f, se, vmdesc); - return 0; - } - return vmstate_save_state(f, se->vmsd, se->opaque, vmdesc); -} - /* * Write the header for device section (QEMU_VM_SECTION START/END/PART/FULL) */ @@ -XXX,XX +XXX,XX @@ static void save_section_footer(QEMUFile *f, SaveStateEntry *se) } } +static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc) +{ + int ret; + + if ((!se->ops || !se->ops->save_state) && !se->vmsd) { + return 0; + } + if (se->vmsd && !vmstate_save_needed(se->vmsd, se->opaque)) { + trace_savevm_section_skip(se->idstr, se->section_id); + return 0; + } + + trace_savevm_section_start(se->idstr, se->section_id); + save_section_header(f, se, QEMU_VM_SECTION_FULL); + if (vmdesc) { + json_writer_start_object(vmdesc, NULL); + json_writer_str(vmdesc, "name", se->idstr); + json_writer_int64(vmdesc, "instance_id", se->instance_id); + } + + trace_vmstate_save(se->idstr, se->vmsd ? se->vmsd->name : "(old)"); + if (!se->vmsd) { + vmstate_save_old_style(f, se, vmdesc); + } else { + ret = vmstate_save_state(f, se->vmsd, se->opaque, vmdesc); + if (ret) { + return ret; + } + } + + trace_savevm_section_end(se->idstr, se->section_id, 0); + save_section_footer(f, se); + if (vmdesc) { + json_writer_end_object(vmdesc); + } + return 0; +} /** * qemu_savevm_command_send: Send a 'QEMU_VM_COMMAND' type element with the * command and associated data. @@ -XXX,XX +XXX,XX @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f, json_writer_int64(vmdesc, "page_size", qemu_target_page_size()); json_writer_start_array(vmdesc, "devices"); QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { - - if ((!se->ops || !se->ops->save_state) && !se->vmsd) { - continue; - } - if (se->vmsd && !vmstate_save_needed(se->vmsd, se->opaque)) { - trace_savevm_section_skip(se->idstr, se->section_id); - continue; - } - - trace_savevm_section_start(se->idstr, se->section_id); - - json_writer_start_object(vmdesc, NULL); - json_writer_str(vmdesc, "name", se->idstr); - json_writer_int64(vmdesc, "instance_id", se->instance_id); - - save_section_header(f, se, QEMU_VM_SECTION_FULL); ret = vmstate_save(f, se, vmdesc); if (ret) { qemu_file_set_error(f, ret); return ret; } - trace_savevm_section_end(se->idstr, se->section_id, 0); - save_section_footer(f, se); - - json_writer_end_object(vmdesc); } if (inactivate_disks) { @@ -XXX,XX +XXX,XX @@ int qemu_save_device_state(QEMUFile *f) if (se->is_ram) { continue; } - if ((!se->ops || !se->ops->save_state) && !se->vmsd) { - continue; - } - if (se->vmsd && !vmstate_save_needed(se->vmsd, se->opaque)) { - continue; - } - - save_section_header(f, se, QEMU_VM_SECTION_FULL); - ret = vmstate_save(f, se, NULL); if (ret) { return ret; } - - save_section_footer(f, se); } qemu_put_byte(f, QEMU_VM_EOF); -- 2.39.1 From: David Hildenbrand <david@redhat.com> ... and store it in the migration state. This is a preparation for storing selected vmds's already in qemu_savevm_state_setup(). Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/migration.h | 4 ++++ migration/migration.c | 2 ++ migration/savevm.c | 18 ++++++++++++------ 3 files changed, 18 insertions(+), 6 deletions(-) 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 @@ #include "exec/cpu-common.h" #include "hw/qdev-core.h" #include "qapi/qapi-types-migration.h" +#include "qapi/qmp/json-writer.h" #include "qemu/thread.h" #include "qemu/coroutine_int.h" #include "io/channel.h" @@ -XXX,XX +XXX,XX @@ struct MigrationState { * This save hostname when out-going migration starts */ char *hostname; + + /* QEMU_VM_VMDESCRIPTION content filled for all non-iterable devices. */ + JSONWriter *vmdesc; }; void migrate_set_state(int *state, int old_state, int new_state); 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 void migrate_fd_cleanup(MigrationState *s) g_free(s->hostname); s->hostname = NULL; + json_writer_free(s->vmdesc); + s->vmdesc = NULL; qemu_savevm_state_cleanup(); 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 @@ #include "postcopy-ram.h" #include "qapi/error.h" #include "qapi/qapi-commands-migration.h" -#include "qapi/qmp/json-writer.h" #include "qapi/clone-visitor.h" #include "qapi/qapi-builtin-visit.h" #include "qapi/qmp/qerror.h" @@ -XXX,XX +XXX,XX @@ bool qemu_savevm_state_guest_unplug_pending(void) void qemu_savevm_state_setup(QEMUFile *f) { + MigrationState *ms = migrate_get_current(); SaveStateEntry *se; Error *local_err = NULL; int ret; + ms->vmdesc = json_writer_new(false); + json_writer_start_object(ms->vmdesc, NULL); + json_writer_int64(ms->vmdesc, "page_size", qemu_target_page_size()); + json_writer_start_array(ms->vmdesc, "devices"); + trace_savevm_state_setup(); QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { if (!se->ops || !se->ops->save_setup) { @@ -XXX,XX +XXX,XX @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f, bool in_postcopy, bool inactivate_disks) { - g_autoptr(JSONWriter) vmdesc = NULL; + MigrationState *ms = migrate_get_current(); + JSONWriter *vmdesc = ms->vmdesc; int vmdesc_len; SaveStateEntry *se; int ret; - vmdesc = json_writer_new(false); - json_writer_start_object(vmdesc, NULL); - json_writer_int64(vmdesc, "page_size", qemu_target_page_size()); - json_writer_start_array(vmdesc, "devices"); QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { ret = vmstate_save(f, se, vmdesc); if (ret) { @@ -XXX,XX +XXX,XX @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f, qemu_put_buffer(f, (uint8_t *)json_writer_get(vmdesc), vmdesc_len); } + /* Free it now to detect any inconsistencies. */ + json_writer_free(vmdesc); + ms->vmdesc = NULL; + return 0; } -- 2.39.1 From: David Hildenbrand <david@redhat.com> For virtio-mem, we want to have the plugged/unplugged state of memory blocks available before migrating any actual RAM content, and perform sanity checks before touching anything on the destination. This information is immutable on the migration source while migration is active, We want to use this information for proper preallocation support with migration: currently, we don't preallocate memory on the migration target, and especially with hugetlb, we can easily run out of hugetlb pages during RAM migration and will crash (SIGBUS) instead of catching this gracefully via preallocation. Migrating device state via a VMSD before we start iterating is currently impossible: the only approach that would be possible is avoiding a VMSD and migrating state manually during save_setup(), to be restored during load_state(). Let's allow for migrating device state via a VMSD early, during the setup phase in qemu_savevm_state_setup(). To keep it simple, we indicate applicable VMSD's using an "early_setup" flag. Note that only very selected devices (i.e., ones seriously messing with RAM setup) are supposed to make use of such early state migration. While at it, also use a bool for the "unmigratable" member. Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com>S Signed-off-by: David Hildenbrand <david@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- include/migration/vmstate.h | 16 +++++++++++++++- migration/savevm.c | 14 ++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) 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 @@ struct VMStateField { struct VMStateDescription { const char *name; - int unmigratable; + bool unmigratable; + /* + * This VMSD describes something that should be sent during setup phase + * of migration. It plays similar role as save_setup() for explicitly + * registered vmstate entries, so it can be seen as a way to describe + * save_setup() in VMSD structures. + * + * Note that for now, a SaveStateEntry cannot have a VMSD and + * operations (e.g., save_setup()) set at the same time. Consequently, + * save_setup() and a VMSD with early_setup set to true are mutually + * exclusive. For this reason, also early_setup VMSDs are migrated in a + * QEMU_VM_SECTION_FULL section, while save_setup() data is migrated in + * a QEMU_VM_SECTION_START section. + */ + bool early_setup; int version_id; int minimum_version_id; MigrationPriority priority; 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 @@ void qemu_savevm_state_setup(QEMUFile *f) trace_savevm_state_setup(); QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { + if (se->vmsd && se->vmsd->early_setup) { + ret = vmstate_save(f, se, ms->vmdesc); + if (ret) { + qemu_file_set_error(f, ret); + break; + } + continue; + } + if (!se->ops || !se->ops->save_setup) { continue; } @@ -XXX,XX +XXX,XX @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f, int ret; QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { + if (se->vmsd && se->vmsd->early_setup) { + /* Already saved during qemu_savevm_state_setup(). */ + continue; + } + ret = vmstate_save(f, se, vmdesc); if (ret) { qemu_file_set_error(f, ret); -- 2.39.1 From: David Hildenbrand <david@redhat.com> We'll make use of both next in the context of virtio-mem. Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com>S Signed-off-by: David Hildenbrand <david@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- include/migration/vmstate.h | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 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 @@ extern const VMStateInfo vmstate_info_qlist; * '_state' type * That the pointer is right at the start of _tmp_type. */ -#define VMSTATE_WITH_TMP(_state, _tmp_type, _vmsd) { \ +#define VMSTATE_WITH_TMP_TEST(_state, _test, _tmp_type, _vmsd) { \ .name = "tmp", \ + .field_exists = (_test), \ .size = sizeof(_tmp_type) + \ QEMU_BUILD_BUG_ON_ZERO(offsetof(_tmp_type, parent) != 0) + \ type_check_pointer(_state, \ @@ -XXX,XX +XXX,XX @@ extern const VMStateInfo vmstate_info_qlist; .info = &vmstate_info_tmp, \ } +#define VMSTATE_WITH_TMP(_state, _tmp_type, _vmsd) \ + VMSTATE_WITH_TMP_TEST(_state, NULL, _tmp_type, _vmsd) + #define VMSTATE_UNUSED_BUFFER(_test, _version, _size) { \ .name = "unused", \ .field_exists = (_test), \ @@ -XXX,XX +XXX,XX @@ extern const VMStateInfo vmstate_info_qlist; /* _field_size should be a int32_t field in the _state struct giving the * size of the bitmap _field in bits. */ -#define VMSTATE_BITMAP(_field, _state, _version, _field_size) { \ +#define VMSTATE_BITMAP_TEST(_field, _state, _test, _version, _field_size) { \ .name = (stringify(_field)), \ + .field_exists = (_test), \ .version_id = (_version), \ .size_offset = vmstate_offset_value(_state, _field_size, int32_t),\ .info = &vmstate_info_bitmap, \ @@ -XXX,XX +XXX,XX @@ extern const VMStateInfo vmstate_info_qlist; .offset = offsetof(_state, _field), \ } +#define VMSTATE_BITMAP(_field, _state, _version, _field_size) \ + VMSTATE_BITMAP_TEST(_field, _state, NULL, _version, _field_size) + /* For migrating a QTAILQ. * Target QTAILQ needs be properly initialized. * _type: type of QTAILQ element -- 2.39.1 From: David Hildenbrand <david@redhat.com> Let's factor out this check, to be used in virtio-mem context next. While at it, fix a spelling error in a related comment. Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com>S Signed-off-by: David Hildenbrand <david@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- include/migration/misc.h | 4 +++- migration/migration.c | 7 +++++++ migration/ram.c | 8 +------- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/include/migration/misc.h b/include/migration/misc.h index XXXXXXX..XXXXXXX 100644 --- a/include/migration/misc.h +++ b/include/migration/misc.h @@ -XXX,XX +XXX,XX @@ bool migration_has_failed(MigrationState *); /* ...and after the device transmission */ bool migration_in_postcopy_after_devices(MigrationState *); void migration_global_dump(Monitor *mon); -/* True if incomming migration entered POSTCOPY_INCOMING_DISCARD */ +/* True if incoming migration entered POSTCOPY_INCOMING_DISCARD */ bool migration_in_incoming_postcopy(void); +/* True if incoming migration entered POSTCOPY_INCOMING_ADVISE */ +bool migration_incoming_postcopy_advised(void); /* True if background snapshot is active */ bool migration_in_bg_snapshot(void); 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_in_incoming_postcopy(void) return ps >= POSTCOPY_INCOMING_DISCARD && ps < POSTCOPY_INCOMING_END; } +bool migration_incoming_postcopy_advised(void) +{ + PostcopyState ps = postcopy_state_get(); + + return ps >= POSTCOPY_INCOMING_ADVISE && ps < POSTCOPY_INCOMING_END; +} + bool migration_in_bg_snapshot(void) { MigrationState *s = migrate_get_current(); 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 @@ int ram_load_postcopy(QEMUFile *f, int channel) return ret; } -static bool postcopy_is_advised(void) -{ - PostcopyState ps = postcopy_state_get(); - return ps >= POSTCOPY_INCOMING_ADVISE && ps < POSTCOPY_INCOMING_END; -} - static bool postcopy_is_running(void) { PostcopyState ps = postcopy_state_get(); @@ -XXX,XX +XXX,XX @@ static int ram_load_precopy(QEMUFile *f) MigrationIncomingState *mis = migration_incoming_get_current(); int flags = 0, ret = 0, invalid_flags = 0, len = 0, i = 0; /* ADVISE is earlier, it shows the source has the postcopy capability on */ - bool postcopy_advised = postcopy_is_advised(); + bool postcopy_advised = migration_incoming_postcopy_advised(); if (!migrate_use_compression()) { invalid_flags |= RAM_SAVE_FLAG_COMPRESS_PAGE; } -- 2.39.1 From: David Hildenbrand <david@redhat.com> "prealloc=on" for the memory backend does not work as expected, as virtio-mem will simply discard all preallocated memory immediately again. In the best case, it's an expensive NOP. In the worst case, it's an unexpected allocation error. Instead, "prealloc=on" should be specified for the virtio-mem device only, such that virtio-mem will try preallocating memory before plugging memory dynamically to the guest. Fail if such a memory backend is provided. Tested-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com>S Signed-off-by: David Hildenbrand <david@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- hw/virtio/virtio-mem.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c index XXXXXXX..XXXXXXX 100644 --- a/hw/virtio/virtio-mem.c +++ b/hw/virtio/virtio-mem.c @@ -XXX,XX +XXX,XX @@ static void virtio_mem_device_realize(DeviceState *dev, Error **errp) error_setg(errp, "'%s' property specifies an unsupported memdev", VIRTIO_MEM_MEMDEV_PROP); return; + } else if (vmem->memdev->prealloc) { + error_setg(errp, "'%s' property specifies a memdev with preallocation" + " enabled: %s. Instead, specify 'prealloc=on' for the" + " virtio-mem device. ", VIRTIO_MEM_MEMDEV_PROP, + object_get_canonical_path_component(OBJECT(vmem->memdev))); + return; } if ((nb_numa_nodes && vmem->node >= nb_numa_nodes) || -- 2.39.1 From: David Hildenbrand <david@redhat.com> The bitmap and the size are immutable while migration is active: see virtio_mem_is_busy(). We can migrate this information early, before migrating any actual RAM content. Further, all information we need for sanity checks is immutable as well. Having this information in place early will, for example, allow for properly preallocating memory before touching these memory locations during RAM migration: this way, we can make sure that all memory was actually preallocated and that any user errors (e.g., insufficient hugetlb pages) can be handled gracefully. In contrast, usable_region_size and requested_size can theoretically still be modified on the source while the VM is running. Keep migrating these properties the usual, late, way. Use a new device property to keep behavior of compat machines unmodified. Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com>S Signed-off-by: David Hildenbrand <david@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- include/hw/virtio/virtio-mem.h | 8 ++++++ hw/core/machine.c | 4 ++- hw/virtio/virtio-mem.c | 51 ++++++++++++++++++++++++++++++++-- 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/include/hw/virtio/virtio-mem.h b/include/hw/virtio/virtio-mem.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/virtio/virtio-mem.h +++ b/include/hw/virtio/virtio-mem.h @@ -XXX,XX +XXX,XX @@ OBJECT_DECLARE_TYPE(VirtIOMEM, VirtIOMEMClass, #define VIRTIO_MEM_BLOCK_SIZE_PROP "block-size" #define VIRTIO_MEM_ADDR_PROP "memaddr" #define VIRTIO_MEM_UNPLUGGED_INACCESSIBLE_PROP "unplugged-inaccessible" +#define VIRTIO_MEM_EARLY_MIGRATION_PROP "x-early-migration" #define VIRTIO_MEM_PREALLOC_PROP "prealloc" struct VirtIOMEM { @@ -XXX,XX +XXX,XX @@ struct VirtIOMEM { /* whether to prealloc memory when plugging new blocks */ bool prealloc; + /* + * Whether we migrate properties that are immutable while migration is + * active early, before state of other devices and especially, before + * migrating any RAM content. + */ + bool early_migration; + /* notifiers to notify when "size" changes */ NotifierList size_change_notifiers; diff --git a/hw/core/machine.c b/hw/core/machine.c index XXXXXXX..XXXXXXX 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -XXX,XX +XXX,XX @@ #include "hw/virtio/virtio-pci.h" #include "qom/object_interfaces.h" -GlobalProperty hw_compat_7_2[] = {}; +GlobalProperty hw_compat_7_2[] = { + { "virtio-mem", "x-early-migration", "false" }, +}; const size_t hw_compat_7_2_len = G_N_ELEMENTS(hw_compat_7_2); GlobalProperty hw_compat_7_1[] = { diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c index XXXXXXX..XXXXXXX 100644 --- a/hw/virtio/virtio-mem.c +++ b/hw/virtio/virtio-mem.c @@ -XXX,XX +XXX,XX @@ #include CONFIG_DEVICES #include "trace.h" +static const VMStateDescription vmstate_virtio_mem_device_early; + /* * We only had legacy x86 guests that did not support * VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE. Other targets don't have legacy guests. @@ -XXX,XX +XXX,XX @@ static void virtio_mem_device_realize(DeviceState *dev, Error **errp) host_memory_backend_set_mapped(vmem->memdev, true); vmstate_register_ram(&vmem->memdev->mr, DEVICE(vmem)); + if (vmem->early_migration) { + vmstate_register(VMSTATE_IF(vmem), VMSTATE_INSTANCE_ID_ANY, + &vmstate_virtio_mem_device_early, vmem); + } qemu_register_reset(virtio_mem_system_reset, vmem); /* @@ -XXX,XX +XXX,XX @@ static void virtio_mem_device_unrealize(DeviceState *dev) */ memory_region_set_ram_discard_manager(&vmem->memdev->mr, NULL); qemu_unregister_reset(virtio_mem_system_reset, vmem); + if (vmem->early_migration) { + vmstate_unregister(VMSTATE_IF(vmem), &vmstate_virtio_mem_device_early, + vmem); + } vmstate_unregister_ram(&vmem->memdev->mr, DEVICE(vmem)); host_memory_backend_set_mapped(vmem->memdev, false); virtio_del_queue(vdev, 0); @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_virtio_mem_sanity_checks = { }, }; +static bool virtio_mem_vmstate_field_exists(void *opaque, int version_id) +{ + const VirtIOMEM *vmem = VIRTIO_MEM(opaque); + + /* With early migration, these fields were already migrated. */ + return !vmem->early_migration; +} + static const VMStateDescription vmstate_virtio_mem_device = { .name = "virtio-mem-device", .minimum_version_id = 1, .version_id = 1, .priority = MIG_PRI_VIRTIO_MEM, .post_load = virtio_mem_post_load, + .fields = (VMStateField[]) { + VMSTATE_WITH_TMP_TEST(VirtIOMEM, virtio_mem_vmstate_field_exists, + VirtIOMEMMigSanityChecks, + vmstate_virtio_mem_sanity_checks), + VMSTATE_UINT64(usable_region_size, VirtIOMEM), + VMSTATE_UINT64_TEST(size, VirtIOMEM, virtio_mem_vmstate_field_exists), + VMSTATE_UINT64(requested_size, VirtIOMEM), + VMSTATE_BITMAP_TEST(bitmap, VirtIOMEM, virtio_mem_vmstate_field_exists, + 0, bitmap_size), + VMSTATE_END_OF_LIST() + }, +}; + +/* + * Transfer properties that are immutable while migration is active early, + * such that we have have this information around before migrating any RAM + * content. + * + * Note that virtio_mem_is_busy() makes sure these properties can no longer + * change on the migration source until migration completed. + * + * With QEMU compat machines, we transmit these properties later, via + * vmstate_virtio_mem_device instead -- see virtio_mem_vmstate_field_exists(). + */ +static const VMStateDescription vmstate_virtio_mem_device_early = { + .name = "virtio-mem-device-early", + .minimum_version_id = 1, + .version_id = 1, + .early_setup = true, .fields = (VMStateField[]) { VMSTATE_WITH_TMP(VirtIOMEM, VirtIOMEMMigSanityChecks, vmstate_virtio_mem_sanity_checks), - VMSTATE_UINT64(usable_region_size, VirtIOMEM), VMSTATE_UINT64(size, VirtIOMEM), - VMSTATE_UINT64(requested_size, VirtIOMEM), VMSTATE_BITMAP(bitmap, VirtIOMEM, 0, bitmap_size), VMSTATE_END_OF_LIST() }, @@ -XXX,XX +XXX,XX @@ static Property virtio_mem_properties[] = { DEFINE_PROP_ON_OFF_AUTO(VIRTIO_MEM_UNPLUGGED_INACCESSIBLE_PROP, VirtIOMEM, unplugged_inaccessible, ON_OFF_AUTO_AUTO), #endif + DEFINE_PROP_BOOL(VIRTIO_MEM_EARLY_MIGRATION_PROP, VirtIOMEM, + early_migration, true), DEFINE_PROP_END_OF_LIST(), }; -- 2.39.1 From: David Hildenbrand <david@redhat.com> Ordinary memory preallocation runs when QEMU starts up and creates the memory backends, before processing the incoming migration stream. With virtio-mem, we don't know which memory blocks to preallocate before migration started. Now that we migrate the virtio-mem bitmap early, before migrating any RAM content, we can safely preallocate memory for all plugged memory blocks before migrating any RAM content. This is especially relevant for the following cases: (1) User errors With hugetlb/files, if we don't have sufficient backend memory available on the migration destination, we'll crash QEMU (SIGBUS) during RAM migration when running out of backend memory. Preallocating memory before actual RAM migration allows for failing gracefully and informing the user about the setup problem. (2) Excluded memory ranges during migration For example, virtio-balloon free page hinting will exclude some pages from getting migrated. In that case, we won't crash during RAM migration, but later, when running the VM on the destination, which is bad. To fix this for new QEMU machines that migrate the bitmap early, preallocate the memory early, before any RAM migration. Warn with old QEMU machines. Getting postcopy right is a bit tricky, but we essentially now implement the same (problematic) preallocation logic as ordinary preallocation: preallocate memory early and discard it again before precopy starts. During ordinary preallocation, discarding of RAM happens when postcopy is advised. As the state (bitmap) is loaded after postcopy was advised but before postcopy starts listening, we have to discard memory we preallocated immediately again ourselves. Note that nothing (not even hugetlb reservations) guarantees for postcopy that backend memory (especially, hugetlb pages) are still free after they were freed ones while discarding RAM. Still, allocating that memory at least once helps catching some basic setup problems. Before this change, trying to restore a VM when insufficient hugetlb pages are around results in the process crashing to to a "Bus error" (SIGBUS). With this change, QEMU fails gracefully: qemu-system-x86_64: qemu_prealloc_mem: preallocating memory failed: Bad address qemu-system-x86_64: error while loading state for instance 0x0 of device '0000:00:03.0/virtio-mem-device-early' qemu-system-x86_64: load of migration failed: Cannot allocate memory And we can even introspect the early migration data, including the bitmap: $ ./scripts/analyze-migration.py -f STATEFILE { "ram (2)": { "section sizes": { "0000:00:03.0/mem0": "0x0000000780000000", "0000:00:04.0/mem1": "0x0000000780000000", "pc.ram": "0x0000000100000000", "/rom@etc/acpi/tables": "0x0000000000020000", "pc.bios": "0x0000000000040000", "0000:00:02.0/e1000.rom": "0x0000000000040000", "pc.rom": "0x0000000000020000", "/rom@etc/table-loader": "0x0000000000001000", "/rom@etc/acpi/rsdp": "0x0000000000001000" } }, "0000:00:03.0/virtio-mem-device-early (51)": { "tmp": "00 00 00 01 40 00 00 00 00 00 00 07 80 00 00 00 00 00 00 00 00 20 00 00 00 00 00 00", "size": "0x0000000040000000", "bitmap": "ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [...] }, "0000:00:04.0/virtio-mem-device-early (53)": { "tmp": "00 00 00 08 c0 00 00 00 00 00 00 07 80 00 00 00 00 00 00 00 00 20 00 00 00 00 00 00", "size": "0x00000001fa400000", "bitmap": "ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [...] }, [...] Reported-by: Jing Qi <jinqi@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com>S Signed-off-by: David Hildenbrand <david@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- hw/virtio/virtio-mem.c | 87 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c index XXXXXXX..XXXXXXX 100644 --- a/hw/virtio/virtio-mem.c +++ b/hw/virtio/virtio-mem.c @@ -XXX,XX +XXX,XX @@ static int virtio_mem_for_each_unplugged_range(const VirtIOMEM *vmem, void *arg, return ret; } +static int virtio_mem_for_each_plugged_range(const VirtIOMEM *vmem, void *arg, + virtio_mem_range_cb cb) +{ + unsigned long first_bit, last_bit; + uint64_t offset, size; + int ret = 0; + + first_bit = find_first_bit(vmem->bitmap, vmem->bitmap_size); + while (first_bit < vmem->bitmap_size) { + offset = first_bit * vmem->block_size; + last_bit = find_next_zero_bit(vmem->bitmap, vmem->bitmap_size, + first_bit + 1) - 1; + size = (last_bit - first_bit + 1) * vmem->block_size; + + ret = cb(vmem, arg, offset, size); + if (ret) { + break; + } + first_bit = find_next_bit(vmem->bitmap, vmem->bitmap_size, + last_bit + 2); + } + return ret; +} + /* * Adjust the memory section to cover the intersection with the given range. * @@ -XXX,XX +XXX,XX @@ static int virtio_mem_post_load(void *opaque, int version_id) RamDiscardListener *rdl; int ret; + if (vmem->prealloc && !vmem->early_migration) { + warn_report("Proper preallocation with migration requires a newer QEMU machine"); + } + /* * We started out with all memory discarded and our memory region is mapped * into an address space. Replay, now that we updated the bitmap. @@ -XXX,XX +XXX,XX @@ static int virtio_mem_post_load(void *opaque, int version_id) return virtio_mem_restore_unplugged(vmem); } +static int virtio_mem_prealloc_range_cb(const VirtIOMEM *vmem, void *arg, + uint64_t offset, uint64_t size) +{ + void *area = memory_region_get_ram_ptr(&vmem->memdev->mr) + offset; + int fd = memory_region_get_fd(&vmem->memdev->mr); + Error *local_err = NULL; + + qemu_prealloc_mem(fd, area, size, 1, NULL, &local_err); + if (local_err) { + error_report_err(local_err); + return -ENOMEM; + } + return 0; +} + +static int virtio_mem_post_load_early(void *opaque, int version_id) +{ + VirtIOMEM *vmem = VIRTIO_MEM(opaque); + RAMBlock *rb = vmem->memdev->mr.ram_block; + int ret; + + if (!vmem->prealloc) { + return 0; + } + + /* + * We restored the bitmap and verified that the basic properties + * match on source and destination, so we can go ahead and preallocate + * memory for all plugged memory blocks, before actual RAM migration starts + * touching this memory. + */ + ret = virtio_mem_for_each_plugged_range(vmem, NULL, + virtio_mem_prealloc_range_cb); + if (ret) { + return ret; + } + + /* + * This is tricky: postcopy wants to start with a clean slate. On + * POSTCOPY_INCOMING_ADVISE, postcopy code discards all (ordinarily + * preallocated) RAM such that postcopy will work as expected later. + * + * However, we run after POSTCOPY_INCOMING_ADVISE -- but before actual + * RAM migration. So let's discard all memory again. This looks like an + * expensive NOP, but actually serves a purpose: we made sure that we + * were able to allocate all required backend memory once. We cannot + * guarantee that the backend memory we will free will remain free + * until we need it during postcopy, but at least we can catch the + * obvious setup issues this way. + */ + if (migration_incoming_postcopy_advised()) { + if (ram_block_discard_range(rb, 0, qemu_ram_get_used_length(rb))) { + return -EBUSY; + } + } + return 0; +} + typedef struct VirtIOMEMMigSanityChecks { VirtIOMEM *parent; uint64_t addr; @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_virtio_mem_device_early = { .minimum_version_id = 1, .version_id = 1, .early_setup = true, + .post_load = virtio_mem_post_load_early, .fields = (VMStateField[]) { VMSTATE_WITH_TMP(VirtIOMEM, VirtIOMEMMigSanityChecks, vmstate_virtio_mem_sanity_checks), -- 2.39.1 From: Peter Xu <peterx@redhat.com> The downtime should be displayed during postcopy phase because the switchover phase is done. OTOH it's weird to show "expected downtime" which can confuse what does that mean if the switchover has already happened anyway. This is a slight ABI change on QMP, but I assume it shouldn't affect anyone. Reviewed-by: Leonardo Bras <leobras@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/migration.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 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(int state) } } +static bool migrate_show_downtime(MigrationState *s) +{ + return (s->state == MIGRATION_STATUS_COMPLETED) || migration_in_postcopy(); +} + static void populate_time_info(MigrationInfo *info, MigrationState *s) { info->has_status = true; info->has_setup_time = true; info->setup_time = s->setup_time; + if (s->state == MIGRATION_STATUS_COMPLETED) { info->has_total_time = true; info->total_time = s->total_time; - info->has_downtime = true; - info->downtime = s->downtime; } else { info->has_total_time = true; info->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - s->start_time; + } + + if (migrate_show_downtime(s)) { + info->has_downtime = true; + info->downtime = s->downtime; + } else { info->has_expected_downtime = true; info->expected_downtime = s->expected_downtime; } -- 2.39.1 From: Fiona Ebner <f.ebner@proxmox.com> upon errors. As the documentation in include/io/channel.h states, only -1 and QIO_CHANNEL_ERR_BLOCK should be returned upon error. Other values have the potential to confuse the call sites. error_setg is used rather than error_setg_errno, because there are certain code paths where -1 (as a non-errno) is propagated up (e.g. starting from qemu_rdma_block_for_wrid or qemu_rdma_post_recv_control) all the way to qio_channel_rdma_{readv,writev}. Similar to a216ec85b7 ("migration/channel-block: fix return value for qio_channel_block_{readv,writev}"). Suggested-by: Zhang Chen <chen.zhang@intel.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/rdma.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/migration/rdma.c b/migration/rdma.c index XXXXXXX..XXXXXXX 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -XXX,XX +XXX,XX @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc, rdma = qatomic_rcu_read(&rioc->rdmaout); if (!rdma) { - return -EIO; + error_setg(errp, "RDMA control channel output is not set"); + return -1; } CHECK_ERROR_STATE(); @@ -XXX,XX +XXX,XX @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc, ret = qemu_rdma_write_flush(f, rdma); if (ret < 0) { rdma->error_state = ret; - return ret; + error_setg(errp, "qemu_rdma_write_flush returned %d", ret); + return -1; } for (i = 0; i < niov; i++) { @@ -XXX,XX +XXX,XX @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc, if (ret < 0) { rdma->error_state = ret; - return ret; + error_setg(errp, "qemu_rdma_exchange_send returned %d", ret); + return -1; } data += len; @@ -XXX,XX +XXX,XX @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc, rdma = qatomic_rcu_read(&rioc->rdmain); if (!rdma) { - return -EIO; + error_setg(errp, "RDMA control channel input is not set"); + return -1; } CHECK_ERROR_STATE(); @@ -XXX,XX +XXX,XX @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc, if (ret < 0) { rdma->error_state = ret; - return ret; + error_setg(errp, "qemu_rdma_exchange_recv returned %d", ret); + return -1; } /* -- 2.39.1 From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> We fairly regularly forget VMSTATE_END_OF_LIST markers off descriptions; given that the current check is only for ->name being NULL, sometimes we get unlucky and the code apparently works and no one spots the error. Explicitly add a flag, VMS_END that should be set, and assert it is set during the traversal. Note: This can't go in until we update the copy of vmstate.h in slirp. Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- include/migration/vmstate.h | 7 ++++++- migration/savevm.c | 1 + migration/vmstate.c | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-) 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 { * VMStateField.struct_version_id to tell which version of the * structure we are referencing to use. */ VMS_VSTRUCT = 0x8000, + + /* Marker for end of list */ + VMS_END = 0x10000 }; typedef enum { @@ -XXX,XX +XXX,XX @@ extern const VMStateInfo vmstate_info_qlist; VMSTATE_UNUSED_BUFFER(_test, 0, _size) #define VMSTATE_END_OF_LIST() \ - {} + { \ + .flags = VMS_END, \ + } int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, void *opaque, int version_id); 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 void dump_vmstate_vmsd(FILE *out_file, field++; first = false; } + assert(field->flags == VMS_END); fprintf(out_file, "\n%*s]", indent, ""); } if (vmsd->subsections != NULL) { 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 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, } field++; } + assert(field->flags == VMS_END); ret = vmstate_subsection_load(f, vmsd, opaque); if (ret != 0) { return ret; @@ -XXX,XX +XXX,XX @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd, } field++; } + assert(field->flags == VMS_END); if (vmdesc) { json_writer_end_array(vmdesc); -- 2.39.1 From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> Perform a check on vmsd structures during test runs in the hope of catching any missing terminators and other simple screwups. Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/savevm.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) 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 @@ #include "net/announce.h" #include "qemu/yank.h" #include "yank_functions.h" +#include "sysemu/qtest.h" const unsigned int postcopy_ram_discard_version; @@ -XXX,XX +XXX,XX @@ void unregister_savevm(VMStateIf *obj, const char *idstr, void *opaque) } } +/* + * Perform some basic checks on vmsd's at registration + * time. + */ +static void vmstate_check(const VMStateDescription *vmsd) +{ + const VMStateField *field = vmsd->fields; + const VMStateDescription **subsection = vmsd->subsections; + + if (field) { + while (field->name) { + if (field->flags & (VMS_STRUCT | VMS_VSTRUCT)) { + /* Recurse to sub structures */ + vmstate_check(field->vmsd); + } + /* Carry on */ + field++; + } + /* Check for the end of field list canary */ + if (field->flags != VMS_END) { + error_report("VMSTATE not ending with VMS_END: %s", vmsd->name); + g_assert_not_reached(); + } + } + + while (subsection && *subsection) { + /* + * The name of a subsection should start with the name of the + * current object. + */ + assert(!strncmp(vmsd->name, (*subsection)->name, strlen(vmsd->name))); + vmstate_check(*subsection); + subsection++; + } +} + int vmstate_register_with_alias_id(VMStateIf *obj, uint32_t instance_id, const VMStateDescription *vmsd, void *opaque, int alias_id, @@ -XXX,XX +XXX,XX @@ int vmstate_register_with_alias_id(VMStateIf *obj, uint32_t instance_id, } else { se->instance_id = instance_id; } + + /* Perform a recursive sanity check during the test runs */ + if (qtest_enabled()) { + vmstate_check(vmsd); + } assert(!se->compat || se->instance_id == 0); savevm_state_handler_insert(se); return 0; -- 2.39.1 From: Zhenzhong Duan <zhenzhong.duan@intel.com> The value of "Sample Pages" is confusing in mode other than page-sampling. See below: (qemu) calc_dirty_rate -b 10 520 (qemu) info dirty_rate Status: measuring Start Time: 11646834 (ms) Sample Pages: 520 (per GB) Period: 10 (sec) Mode: dirty-bitmap Dirty rate: (not ready) (qemu) info dirty_rate Status: measured Start Time: 11646834 (ms) Sample Pages: 0 (per GB) Period: 10 (sec) Mode: dirty-bitmap Dirty rate: 2 (MB/s) While it's totally useless in dirty-ring and dirty-bitmap mode, fix to show it only in page-sampling mode. Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/dirtyrate.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c index XXXXXXX..XXXXXXX 100644 --- a/migration/dirtyrate.c +++ b/migration/dirtyrate.c @@ -XXX,XX +XXX,XX @@ void qmp_calc_dirty_rate(int64_t calc_time, mode = DIRTY_RATE_MEASURE_MODE_PAGE_SAMPLING; } - if (has_sample_pages && mode == DIRTY_RATE_MEASURE_MODE_DIRTY_RING) { - error_setg(errp, "either sample-pages or dirty-ring can be specified."); + if (has_sample_pages && mode != DIRTY_RATE_MEASURE_MODE_PAGE_SAMPLING) { + error_setg(errp, "sample-pages is used only in page-sampling mode"); return; } @@ -XXX,XX +XXX,XX @@ void hmp_info_dirty_rate(Monitor *mon, const QDict *qdict) DirtyRateStatus_str(info->status)); monitor_printf(mon, "Start Time: %"PRIi64" (ms)\n", info->start_time); - monitor_printf(mon, "Sample Pages: %"PRIu64" (per GB)\n", - info->sample_pages); + if (info->mode == DIRTY_RATE_MEASURE_MODE_PAGE_SAMPLING) { + monitor_printf(mon, "Sample Pages: %"PRIu64" (per GB)\n", + info->sample_pages); + } monitor_printf(mon, "Period: %"PRIi64" (sec)\n", info->calc_time); monitor_printf(mon, "Mode: %s\n", -- 2.39.1 From: "manish.mishra" <manish.mishra@nutanix.com> MSG_PEEK peeks at the channel, The data is treated as unread and the next read shall still return this data. This support is currently added only for socket class. Extra parameter 'flags' is added to io_readv calls to pass extra read flags like MSG_PEEK. Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Suggested-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: manish.mishra <manish.mishra@nutanix.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- include/io/channel.h | 6 ++++++ chardev/char-socket.c | 4 ++-- io/channel-buffer.c | 1 + io/channel-command.c | 1 + io/channel-file.c | 1 + io/channel-null.c | 1 + io/channel-socket.c | 19 ++++++++++++++++++- io/channel-tls.c | 1 + io/channel-websock.c | 1 + io/channel.c | 16 ++++++++++++---- migration/channel-block.c | 1 + migration/rdma.c | 1 + scsi/qemu-pr-helper.c | 2 +- tests/qtest/tpm-emu.c | 2 +- tests/unit/test-io-channel-socket.c | 1 + util/vhost-user-server.c | 2 +- 16 files changed, 50 insertions(+), 10 deletions(-) diff --git a/include/io/channel.h b/include/io/channel.h index XXXXXXX..XXXXXXX 100644 --- a/include/io/channel.h +++ b/include/io/channel.h @@ -XXX,XX +XXX,XX @@ OBJECT_DECLARE_TYPE(QIOChannel, QIOChannelClass, #define QIO_CHANNEL_WRITE_FLAG_ZERO_COPY 0x1 +#define QIO_CHANNEL_READ_FLAG_MSG_PEEK 0x1 + typedef enum QIOChannelFeature QIOChannelFeature; enum QIOChannelFeature { @@ -XXX,XX +XXX,XX @@ enum QIOChannelFeature { QIO_CHANNEL_FEATURE_SHUTDOWN, QIO_CHANNEL_FEATURE_LISTEN, QIO_CHANNEL_FEATURE_WRITE_ZERO_COPY, + QIO_CHANNEL_FEATURE_READ_MSG_PEEK, }; @@ -XXX,XX +XXX,XX @@ struct QIOChannelClass { size_t niov, int **fds, size_t *nfds, + int flags, Error **errp); int (*io_close)(QIOChannel *ioc, Error **errp); @@ -XXX,XX +XXX,XX @@ void qio_channel_set_name(QIOChannel *ioc, * @niov: the length of the @iov array * @fds: pointer to an array that will received file handles * @nfds: pointer filled with number of elements in @fds on return + * @flags: read flags (QIO_CHANNEL_READ_FLAG_*) * @errp: pointer to a NULL-initialized error object * * Read data from the IO channel, storing it in the @@ -XXX,XX +XXX,XX @@ ssize_t qio_channel_readv_full(QIOChannel *ioc, size_t niov, int **fds, size_t *nfds, + int flags, Error **errp); diff --git a/chardev/char-socket.c b/chardev/char-socket.c index XXXXXXX..XXXXXXX 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -XXX,XX +XXX,XX @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf, size_t len) if (qio_channel_has_feature(s->ioc, QIO_CHANNEL_FEATURE_FD_PASS)) { ret = qio_channel_readv_full(s->ioc, &iov, 1, &msgfds, &msgfds_num, - NULL); + 0, NULL); } else { ret = qio_channel_readv_full(s->ioc, &iov, 1, NULL, NULL, - NULL); + 0, NULL); } if (msgfds_num) { diff --git a/io/channel-buffer.c b/io/channel-buffer.c index XXXXXXX..XXXXXXX 100644 --- a/io/channel-buffer.c +++ b/io/channel-buffer.c @@ -XXX,XX +XXX,XX @@ static ssize_t qio_channel_buffer_readv(QIOChannel *ioc, size_t niov, int **fds, size_t *nfds, + int flags, Error **errp) { QIOChannelBuffer *bioc = QIO_CHANNEL_BUFFER(ioc); diff --git a/io/channel-command.c b/io/channel-command.c index XXXXXXX..XXXXXXX 100644 --- a/io/channel-command.c +++ b/io/channel-command.c @@ -XXX,XX +XXX,XX @@ static ssize_t qio_channel_command_readv(QIOChannel *ioc, size_t niov, int **fds, size_t *nfds, + int flags, Error **errp) { QIOChannelCommand *cioc = QIO_CHANNEL_COMMAND(ioc); diff --git a/io/channel-file.c b/io/channel-file.c index XXXXXXX..XXXXXXX 100644 --- a/io/channel-file.c +++ b/io/channel-file.c @@ -XXX,XX +XXX,XX @@ static ssize_t qio_channel_file_readv(QIOChannel *ioc, size_t niov, int **fds, size_t *nfds, + int flags, Error **errp) { QIOChannelFile *fioc = QIO_CHANNEL_FILE(ioc); diff --git a/io/channel-null.c b/io/channel-null.c index XXXXXXX..XXXXXXX 100644 --- a/io/channel-null.c +++ b/io/channel-null.c @@ -XXX,XX +XXX,XX @@ qio_channel_null_readv(QIOChannel *ioc, size_t niov, int **fds G_GNUC_UNUSED, size_t *nfds G_GNUC_UNUSED, + int flags, Error **errp) { QIOChannelNull *nioc = QIO_CHANNEL_NULL(ioc); diff --git a/io/channel-socket.c b/io/channel-socket.c index XXXXXXX..XXXXXXX 100644 --- a/io/channel-socket.c +++ b/io/channel-socket.c @@ -XXX,XX +XXX,XX @@ int qio_channel_socket_connect_sync(QIOChannelSocket *ioc, } #endif + qio_channel_set_feature(QIO_CHANNEL(ioc), + QIO_CHANNEL_FEATURE_READ_MSG_PEEK); + return 0; } @@ -XXX,XX +XXX,XX @@ qio_channel_socket_accept(QIOChannelSocket *ioc, } #endif /* WIN32 */ + qio_channel_set_feature(QIO_CHANNEL(cioc), + QIO_CHANNEL_FEATURE_READ_MSG_PEEK); + trace_qio_channel_socket_accept_complete(ioc, cioc, cioc->fd); return cioc; @@ -XXX,XX +XXX,XX @@ static ssize_t qio_channel_socket_readv(QIOChannel *ioc, size_t niov, int **fds, size_t *nfds, + int flags, Error **errp) { QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc); @@ -XXX,XX +XXX,XX @@ static ssize_t qio_channel_socket_readv(QIOChannel *ioc, } + if (flags & QIO_CHANNEL_READ_FLAG_MSG_PEEK) { + sflags |= MSG_PEEK; + } + retry: ret = recvmsg(sioc->fd, &msg, sflags); if (ret < 0) { @@ -XXX,XX +XXX,XX @@ static ssize_t qio_channel_socket_readv(QIOChannel *ioc, size_t niov, int **fds, size_t *nfds, + int flags, Error **errp) { QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc); ssize_t done = 0; ssize_t i; + int sflags = 0; + + if (flags & QIO_CHANNEL_READ_FLAG_MSG_PEEK) { + sflags |= MSG_PEEK; + } for (i = 0; i < niov; i++) { ssize_t ret; @@ -XXX,XX +XXX,XX @@ static ssize_t qio_channel_socket_readv(QIOChannel *ioc, ret = recv(sioc->fd, iov[i].iov_base, iov[i].iov_len, - 0); + sflags); if (ret < 0) { if (errno == EAGAIN) { if (done) { diff --git a/io/channel-tls.c b/io/channel-tls.c index XXXXXXX..XXXXXXX 100644 --- a/io/channel-tls.c +++ b/io/channel-tls.c @@ -XXX,XX +XXX,XX @@ static ssize_t qio_channel_tls_readv(QIOChannel *ioc, size_t niov, int **fds, size_t *nfds, + int flags, Error **errp) { QIOChannelTLS *tioc = QIO_CHANNEL_TLS(ioc); diff --git a/io/channel-websock.c b/io/channel-websock.c index XXXXXXX..XXXXXXX 100644 --- a/io/channel-websock.c +++ b/io/channel-websock.c @@ -XXX,XX +XXX,XX @@ static ssize_t qio_channel_websock_readv(QIOChannel *ioc, size_t niov, int **fds, size_t *nfds, + int flags, Error **errp) { QIOChannelWebsock *wioc = QIO_CHANNEL_WEBSOCK(ioc); diff --git a/io/channel.c b/io/channel.c index XXXXXXX..XXXXXXX 100644 --- a/io/channel.c +++ b/io/channel.c @@ -XXX,XX +XXX,XX @@ ssize_t qio_channel_readv_full(QIOChannel *ioc, size_t niov, int **fds, size_t *nfds, + int flags, Error **errp) { QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc); @@ -XXX,XX +XXX,XX @@ ssize_t qio_channel_readv_full(QIOChannel *ioc, return -1; } - return klass->io_readv(ioc, iov, niov, fds, nfds, errp); + if ((flags & QIO_CHANNEL_READ_FLAG_MSG_PEEK) && + !qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_READ_MSG_PEEK)) { + error_setg_errno(errp, EINVAL, + "Channel does not support peek read"); + return -1; + } + + return klass->io_readv(ioc, iov, niov, fds, nfds, flags, errp); } @@ -XXX,XX +XXX,XX @@ int qio_channel_readv_full_all_eof(QIOChannel *ioc, while ((nlocal_iov > 0) || local_fds) { ssize_t len; len = qio_channel_readv_full(ioc, local_iov, nlocal_iov, local_fds, - local_nfds, errp); + local_nfds, 0, errp); if (len == QIO_CHANNEL_ERR_BLOCK) { if (qemu_in_coroutine()) { qio_channel_yield(ioc, G_IO_IN); @@ -XXX,XX +XXX,XX @@ ssize_t qio_channel_readv(QIOChannel *ioc, size_t niov, Error **errp) { - return qio_channel_readv_full(ioc, iov, niov, NULL, NULL, errp); + return qio_channel_readv_full(ioc, iov, niov, NULL, NULL, 0, errp); } @@ -XXX,XX +XXX,XX @@ ssize_t qio_channel_read(QIOChannel *ioc, Error **errp) { struct iovec iov = { .iov_base = buf, .iov_len = buflen }; - return qio_channel_readv_full(ioc, &iov, 1, NULL, NULL, errp); + return qio_channel_readv_full(ioc, &iov, 1, NULL, NULL, 0, errp); } diff --git a/migration/channel-block.c b/migration/channel-block.c index XXXXXXX..XXXXXXX 100644 --- a/migration/channel-block.c +++ b/migration/channel-block.c @@ -XXX,XX +XXX,XX @@ qio_channel_block_readv(QIOChannel *ioc, size_t niov, int **fds, size_t *nfds, + int flags, Error **errp) { QIOChannelBlock *bioc = QIO_CHANNEL_BLOCK(ioc); diff --git a/migration/rdma.c b/migration/rdma.c index XXXXXXX..XXXXXXX 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -XXX,XX +XXX,XX @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc, size_t niov, int **fds, size_t *nfds, + int flags, Error **errp) { QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc); diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c index XXXXXXX..XXXXXXX 100644 --- a/scsi/qemu-pr-helper.c +++ b/scsi/qemu-pr-helper.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn prh_read(PRHelperClient *client, void *buf, int sz, iov.iov_base = buf; iov.iov_len = sz; n_read = qio_channel_readv_full(QIO_CHANNEL(client->ioc), &iov, 1, - &fds, &nfds, errp); + &fds, &nfds, 0, errp); if (n_read == QIO_CHANNEL_ERR_BLOCK) { qio_channel_yield(QIO_CHANNEL(client->ioc), G_IO_IN); diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/tpm-emu.c +++ b/tests/qtest/tpm-emu.c @@ -XXX,XX +XXX,XX @@ void *tpm_emu_ctrl_thread(void *data) int *pfd = NULL; size_t nfd = 0; - qio_channel_readv_full(ioc, &iov, 1, &pfd, &nfd, &error_abort); + qio_channel_readv_full(ioc, &iov, 1, &pfd, &nfd, 0, &error_abort); cmd = be32_to_cpu(cmd); g_assert_cmpint(cmd, ==, CMD_SET_DATAFD); g_assert_cmpint(nfd, ==, 1); diff --git a/tests/unit/test-io-channel-socket.c b/tests/unit/test-io-channel-socket.c index XXXXXXX..XXXXXXX 100644 --- a/tests/unit/test-io-channel-socket.c +++ b/tests/unit/test-io-channel-socket.c @@ -XXX,XX +XXX,XX @@ static void test_io_channel_unix_fd_pass(void) G_N_ELEMENTS(iorecv), &fdrecv, &nfdrecv, + 0, &error_abort); g_assert(nfdrecv == G_N_ELEMENTS(fdsend)); diff --git a/util/vhost-user-server.c b/util/vhost-user-server.c index XXXXXXX..XXXXXXX 100644 --- a/util/vhost-user-server.c +++ b/util/vhost-user-server.c @@ -XXX,XX +XXX,XX @@ vu_message_read(VuDev *vu_dev, int conn_fd, VhostUserMsg *vmsg) * qio_channel_readv_full may have short reads, keeping calling it * until getting VHOST_USER_HDR_SIZE or 0 bytes in total */ - rc = qio_channel_readv_full(ioc, &iov, 1, &fds, &nfds, &local_err); + rc = qio_channel_readv_full(ioc, &iov, 1, &fds, &nfds, 0, &local_err); if (rc < 0) { if (rc == QIO_CHANNEL_ERR_BLOCK) { assert(local_err == NULL); -- 2.39.1 From: "manish.mishra" <manish.mishra@nutanix.com> Current logic assumes that channel connections on the destination side are always established in the same order as the source and the first one will always be the main channel followed by the multifid or post-copy preemption channel. This may not be always true, as even if a channel has a connection established on the source side it can be in the pending state on the destination side and a newer connection can be established first. Basically causing out of order mapping of channels on the destination side. Currently, all channels except post-copy preempt send a magic number, this patch uses that magic number to decide the type of channel. This logic is applicable only for precopy(multifd) live migration, as mentioned, the post-copy preempt channel does not send any magic number. Also, tls live migrations already does tls handshake before creating other channels, so this issue is not possible with tls, hence this logic is avoided for tls live migrations. This patch uses read peek to check the magic number of channels so that current data/control stream management remains un-effected. Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Suggested-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: manish.mishra <manish.mishra@nutanix.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/channel.h | 5 ++++ migration/multifd.h | 2 +- migration/postcopy-ram.h | 2 +- migration/channel.c | 45 +++++++++++++++++++++++++++++++++ migration/migration.c | 54 ++++++++++++++++++++++++++++------------ migration/multifd.c | 19 +++++++------- migration/postcopy-ram.c | 5 +--- 7 files changed, 101 insertions(+), 31 deletions(-) diff --git a/migration/channel.h b/migration/channel.h index XXXXXXX..XXXXXXX 100644 --- a/migration/channel.h +++ b/migration/channel.h @@ -XXX,XX +XXX,XX @@ void migration_channel_connect(MigrationState *s, QIOChannel *ioc, const char *hostname, Error *error_in); + +int migration_channel_read_peek(QIOChannel *ioc, + const char *buf, + const size_t buflen, + Error **errp); #endif diff --git a/migration/multifd.h b/migration/multifd.h index XXXXXXX..XXXXXXX 100644 --- a/migration/multifd.h +++ b/migration/multifd.h @@ -XXX,XX +XXX,XX @@ void multifd_save_cleanup(void); int multifd_load_setup(Error **errp); int multifd_load_cleanup(Error **errp); bool multifd_recv_all_channels_created(void); -bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp); +void multifd_recv_new_channel(QIOChannel *ioc, Error **errp); void multifd_recv_sync_main(void); int multifd_send_sync_main(QEMUFile *f); int multifd_queue_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset); diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h index XXXXXXX..XXXXXXX 100644 --- a/migration/postcopy-ram.h +++ b/migration/postcopy-ram.h @@ -XXX,XX +XXX,XX @@ enum PostcopyChannels { RAM_CHANNEL_MAX, }; -bool postcopy_preempt_new_channel(MigrationIncomingState *mis, QEMUFile *file); +void postcopy_preempt_new_channel(MigrationIncomingState *mis, QEMUFile *file); int postcopy_preempt_setup(MigrationState *s, Error **errp); int postcopy_preempt_wait_channel(MigrationState *s); diff --git a/migration/channel.c b/migration/channel.c index XXXXXXX..XXXXXXX 100644 --- a/migration/channel.c +++ b/migration/channel.c @@ -XXX,XX +XXX,XX @@ void migration_channel_connect(MigrationState *s, migrate_fd_connect(s, error); error_free(error); } + + +/** + * @migration_channel_read_peek - Peek at migration channel, without + * actually removing it from channel buffer. + * + * @ioc: the channel object + * @buf: the memory region to read data into + * @buflen: the number of bytes to read in @buf + * @errp: pointer to a NULL-initialized error object + * + * Returns 0 if successful, returns -1 and sets @errp if fails. + */ +int migration_channel_read_peek(QIOChannel *ioc, + const char *buf, + const size_t buflen, + Error **errp) +{ + ssize_t len = 0; + struct iovec iov = { .iov_base = (char *)buf, .iov_len = buflen }; + + while (true) { + len = qio_channel_readv_full(ioc, &iov, 1, NULL, NULL, + QIO_CHANNEL_READ_FLAG_MSG_PEEK, errp); + + if (len <= 0 && len != QIO_CHANNEL_ERR_BLOCK) { + error_setg(errp, + "Failed to peek at channel"); + return -1; + } + + if (len == buflen) { + break; + } + + /* 1ms sleep. */ + if (qemu_in_coroutine()) { + qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 1000000); + } else { + g_usleep(1000); + } + } + + return 0; +} 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 "migration.h" #include "savevm.h" #include "qemu-file.h" +#include "channel.h" #include "migration/vmstate.h" #include "block/block.h" #include "qapi/error.h" @@ -XXX,XX +XXX,XX @@ static bool migration_incoming_setup(QEMUFile *f, Error **errp) { MigrationIncomingState *mis = migration_incoming_get_current(); - if (multifd_load_setup(errp) != 0) { - return false; - } - if (!mis->from_src_file) { mis->from_src_file = f; } @@ -XXX,XX +XXX,XX @@ void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp) { MigrationIncomingState *mis = migration_incoming_get_current(); Error *local_err = NULL; - bool start_migration; QEMUFile *f; + bool default_channel = true; + uint32_t channel_magic = 0; + int ret = 0; - if (!mis->from_src_file) { - /* The first connection (multifd may have multiple) */ + if (migrate_use_multifd() && !migrate_postcopy_ram() && + qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_READ_MSG_PEEK)) { + /* + * With multiple channels, it is possible that we receive channels + * out of order on destination side, causing incorrect mapping of + * source channels on destination side. Check channel MAGIC to + * decide type of channel. Please note this is best effort, postcopy + * preempt channel does not send any magic number so avoid it for + * postcopy live migration. Also tls live migration already does + * tls handshake while initializing main channel so with tls this + * issue is not possible. + */ + ret = migration_channel_read_peek(ioc, (void *)&channel_magic, + sizeof(channel_magic), &local_err); + + if (ret != 0) { + error_propagate(errp, local_err); + return; + } + + default_channel = (channel_magic == cpu_to_be32(QEMU_VM_FILE_MAGIC)); + } else { + default_channel = !mis->from_src_file; + } + + if (multifd_load_setup(errp) != 0) { + error_setg(errp, "Failed to setup multifd channels"); + return; + } + + if (default_channel) { f = qemu_file_new_input(ioc); if (!migration_incoming_setup(f, errp)) { return; } - - /* - * Common migration only needs one channel, so we can start - * right now. Some features need more than one channel, we wait. - */ - start_migration = !migration_needs_multiple_sockets(); } else { /* Multiple connections */ assert(migration_needs_multiple_sockets()); if (migrate_use_multifd()) { - start_migration = multifd_recv_new_channel(ioc, &local_err); + multifd_recv_new_channel(ioc, &local_err); } else { assert(migrate_postcopy_preempt()); f = qemu_file_new_input(ioc); - start_migration = postcopy_preempt_new_channel(mis, f); + postcopy_preempt_new_channel(mis, f); } if (local_err) { error_propagate(errp, local_err); @@ -XXX,XX +XXX,XX @@ void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp) } } - if (start_migration) { + if (migration_has_all_channels()) { /* If it's a recovery, we're done */ if (postcopy_try_recover()) { return; 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 @@ int multifd_load_setup(Error **errp) uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size(); uint8_t i; - if (!migrate_use_multifd()) { + /* + * Return successfully if multiFD recv state is already initialised + * or multiFD is not enabled. + */ + if (multifd_recv_state || !migrate_use_multifd()) { return 0; } + if (!migrate_multi_channels_is_allowed()) { error_setg(errp, "multifd is not supported by current protocol"); return -1; @@ -XXX,XX +XXX,XX @@ bool multifd_recv_all_channels_created(void) /* * Try to receive all multifd channels to get ready for the migration. - * - Return true and do not set @errp when correctly receiving all channels; - * - Return false and do not set @errp when correctly receiving the current one; - * - Return false and set @errp when failing to receive the current channel. + * Sets @errp when failing to receive the current channel. */ -bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp) +void multifd_recv_new_channel(QIOChannel *ioc, Error **errp) { MultiFDRecvParams *p; Error *local_err = NULL; @@ -XXX,XX +XXX,XX @@ bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp) "failed to receive packet" " via multifd channel %d: ", qatomic_read(&multifd_recv_state->count)); - return false; + return; } trace_multifd_recv_new_channel(id); @@ -XXX,XX +XXX,XX @@ bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp) id); multifd_recv_terminate_threads(local_err); error_propagate(errp, local_err); - return false; + return; } p->c = ioc; object_ref(OBJECT(ioc)); @@ -XXX,XX +XXX,XX @@ bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp) qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p, QEMU_THREAD_JOINABLE); qatomic_inc(&multifd_recv_state->count); - return qatomic_read(&multifd_recv_state->count) == - migrate_multifd_channels(); } diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index XXXXXXX..XXXXXXX 100644 --- a/migration/postcopy-ram.c +++ b/migration/postcopy-ram.c @@ -XXX,XX +XXX,XX @@ void postcopy_unregister_shared_ufd(struct PostCopyFD *pcfd) } } -bool postcopy_preempt_new_channel(MigrationIncomingState *mis, QEMUFile *file) +void postcopy_preempt_new_channel(MigrationIncomingState *mis, QEMUFile *file) { /* * The new loading channel has its own threads, so it needs to be @@ -XXX,XX +XXX,XX @@ bool postcopy_preempt_new_channel(MigrationIncomingState *mis, QEMUFile *file) qemu_file_set_blocking(file, true); mis->postcopy_qemufile_dst = file; trace_postcopy_preempt_new_channel(); - - /* Start the migration immediately */ - return true; } /* -- 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