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 The following changes since commit 84f07211cc5b4fc6a371559bf8a5de4fb068e648: Update version for v11.1.0 release (2026-08-11 10:04:46 -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 2375e9b1239eb6ad794eaae29245b94f429c87c5: migration: Fix rare hang of migration_channel_read_peek() (2026-08-12 10:42:09 -0400) ---------------------------------------------------------------- migration/mem pull for 11.2 next 11.2: - Dongli's patch to add cpr-transfer support for HMP - Fabiano's doc update for migration on security issues - Gavin's fix for MMIO access support for memory APIs, reverting ram_device ops - Sam's migration test build fix for !ASN1 - Peter's a few migration hardening fixes ---------------------------------------------------------------- Dongli Zhang (1): migration/cpr: Add HMP support for cpr-transfer Fabiano Rosas (1): docs: Add security considerations for migration Gavin Shan (3): system/memory: Use memmove() for directly accessible regions system/memory: Use qemu_ram_move() for directly accessible regions system/memory: Make ram device region directly accessible Peter Xu (4): migration/multifd: Validate next_packet_size in zlib/zstd recv migration/multifd: Replace assert() with error_setg() in recv paths migration/ram: Check for RAMBlock size mismatch when parsing migration: Fix rare hang of migration_channel_read_peek() Sam Heney (1): tests/qtest/migration: Only build tls_no_hostname test with TASN1 docs/system/security.rst | 22 +++++++++++--- include/system/memory.h | 46 ++++++++++++++++++++++------- hw/remote/vfio-user-obj.c | 4 +-- migration/channel.c | 11 +++++-- migration/migration-hmp-cmds.c | 18 ++++++++++++ migration/multifd-qatzip.c | 5 +++- migration/multifd-qpl.c | 24 +++++++++++++--- migration/multifd-uadk.c | 24 +++++++++++++--- migration/multifd-zlib.c | 11 ++++++- migration/multifd-zstd.c | 11 ++++++- migration/ram.c | 11 +++++-- system/memory.c | 41 +------------------------- system/physmem.c | 48 +++++++++++++++++++++++++++++-- tests/qtest/migration/tls-tests.c | 4 ++- hmp-commands.hx | 12 +++++--- system/trace-events | 2 -- 16 files changed, 214 insertions(+), 80 deletions(-) -- 2.54.0 From: Fabiano Rosas <farosas@suse.de> Add the security considerations that are unique to migration and that do not already fall into one of the other categories. Some aspects are better framed as security architecture considerations, so extend that section to mention TLS and clarify that disk images and guest network also need to be isolated from other processes, not just other guests. Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20260721131457.3062767-1-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> --- docs/system/security.rst | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/docs/system/security.rst b/docs/system/security.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/security.rst +++ b/docs/system/security.rst @@ -XXX,XX +XXX,XX @@ an issue as a normal bug. that affect the level 0 QEMU process. While these bugs should be fixed, they will not be triaged as security flaws at this time. +* **migration/snapshots**. Migration failures and snapshot load + failures are considered part of normal operation as long as the + source virtual machine and savevm file, respectively, are still + functional. Aborting the QEMU process at the migration/snapshot + destination is similarly not considered a security issue. The + migration stream is assumed to be secure as long as the design + principles described in the Architecture section are held, in + which case plain manipulation of the stream is not considered as + an attack vector. + * **low severity impact**. As a catch all rule, issues which are judged to have a "low" severity impact on the system will usually not justify handling as security bugs, nor assignment @@ -XXX,XX +XXX,XX @@ could allow malicious guests to gain code execution in QEMU. At this point the guest has escaped the virtual machine and is able to act in the context of the QEMU process on the host. -Guests often interact with other guests and share resources with them. A -malicious guest must not gain control of other guests or access their data. -Disk image files and network traffic must be protected from other guests unless -explicitly shared between them by the user. +Guests often interact with other guests and share resources with them. +A malicious guest must not gain control of other guests or access +their data. Disk image files and network traffic must be protected +from other guests, users and processes unless explicitly shared with +them by the user. Principle of Least Privilege '''''''''''''''''''''''''''' @@ -XXX,XX +XXX,XX @@ Some Linux distros already ship with UNIX groups for these devices by default. system calls that are not needed by QEMU, thereby reducing the host kernel attack surface. +- Transport Layer Security (TLS) protocol can be used to ensure authenticity and + encryption of the live migration connection where the network is untrusted. + Sensitive configurations ------------------------ -- 2.54.0 From: Dongli Zhang <dongli.zhang@oracle.com> Currently the cpr-transfer source QEMU instance cannot be driven entirely via HMP. The source must use QMP in order to specify both the main migration channel and the CPR channel. Extend the HMP migrate command with an optional CPR channel URI. When the migration mode is cpr-transfer, HMP uses this URI to build a CPR MigrationChannel in addition to the main migration channel. The new option is rejected unless the migration mode is cpr-transfer, so existing HMP migrate usage is unchanged. For example, source QEMU HMP commands can be something like below. The "-c unix:/tmp/cpr.sock" is for CPR URI. (qemu) migrate_set_parameter mode cpr-transfer (qemu) migrate -c unix:/tmp/cpr.sock tcp:0:50002 Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com> Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org> Acked-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> Link: https://lore.kernel.org/r/20260728085903.173265-1-dongli.zhang@oracle.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/migration-hmp-cmds.c | 18 ++++++++++++++++++ hmp-commands.hx | 12 ++++++++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c index XXXXXXX..XXXXXXX 100644 --- a/migration/migration-hmp-cmds.c +++ b/migration/migration-hmp-cmds.c @@ -XXX,XX +XXX,XX @@ void hmp_migrate(Monitor *mon, const QDict *qdict) bool detach = qdict_get_try_bool(qdict, "detach", false); bool resume = qdict_get_try_bool(qdict, "resume", false); const char *uri = qdict_get_str(qdict, "uri"); + const char *uri_cpr = qdict_get_try_str(qdict, "uri-cpr"); Error *err = NULL; g_autoptr(MigrationChannelList) caps = NULL; g_autoptr(MigrationChannel) channel = NULL; + g_autoptr(MigrationChannel) channel_cpr = NULL; if (!migrate_uri_parse(uri, &channel, &err)) { hmp_handle_error(mon, err); @@ -XXX,XX +XXX,XX @@ void hmp_migrate(Monitor *mon, const QDict *qdict) } QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel)); + if (uri_cpr) { + if (migrate_mode() != MIG_MODE_CPR_TRANSFER) { + error_setg(&err, "-c can only be used in cpr-transfer mode"); + hmp_handle_error(mon, err); + return; + } + + if (!migrate_uri_parse(uri_cpr, &channel_cpr, &err)) { + hmp_handle_error(mon, err); + return; + } + + channel_cpr->channel_type = MIGRATION_CHANNEL_TYPE_CPR; + QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel_cpr)); + } + qmp_migrate(NULL, true, caps, true, resume, &err); if (hmp_handle_error(mon, err)) { return; diff --git a/hmp-commands.hx b/hmp-commands.hx index XXXXXXX..XXXXXXX 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -XXX,XX +XXX,XX @@ ERST { .name = "migrate", - .args_type = "detach:-d,resume:-r,uri:s", - .params = "[-d] [-r] uri", + .args_type = "detach:-d,resume:-r,uri-cpr:-cs,uri:s", + .params = "[-d] [-r] [-c uri-cpr] uri", .help = "migrate to URI (using -d to not wait for completion)" - "\n\t\t\t -r to resume a paused postcopy migration", + "\n\t\t\t -r to resume a paused postcopy migration" + "\n\t\t\t -c to specify a CPR URI for cpr-transfer mode", .cmd = hmp_migrate, }, SRST -``migrate [-d] [-r]`` *uri* +``migrate [-d] [-r] [-c uri-cpr]`` *uri* Migrate the VM to *uri*. ``-d`` @@ -XXX,XX +XXX,XX @@ SRST query an ongoing migration process, use "info migrate". ``-r`` Resume a paused postcopy migration. + ``-c`` *uri-cpr* + Specify the CPR URI for cpr-transfer mode. It must be a UNIX domain + socket. ERST { -- 2.54.0 From: Gavin Shan <gshan@redhat.com> Similar to what's done in commit 4a73aee88140 ("softmmu: Use memmove in flatview_write_continue"), there are more sites where the overlapping source and destination buffer are allowed for the directly accessible regions. Use memmove() in those sites, listed as below. hw/remote/vfio-user-obj.c::vfu_object_mr_rw include/system/memory.h::address_space_read system/physmem.c::flatview_read_continue_step Signed-off-by: Gavin Shan <gshan@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20260728031731.286666-2-gshan@redhat.com Signed-off-by: Peter Xu <peterx@redhat.com> --- include/system/memory.h | 2 +- hw/remote/vfio-user-obj.c | 4 ++-- system/physmem.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/system/memory.h b/include/system/memory.h index XXXXXXX..XXXXXXX 100644 --- a/include/system/memory.h +++ b/include/system/memory.h @@ -XXX,XX +XXX,XX @@ MemTxResult address_space_read(const AddressSpace *as, hwaddr addr, mr = flatview_translate(fv, addr, &addr1, &l, false, attrs); if (len == l && memory_access_is_direct(mr, false, attrs)) { ptr = qemu_map_ram_ptr(mr->ram_block, addr1); - memcpy(buf, ptr, len); + memmove(buf, ptr, len); } else { result = flatview_read_continue(fv, addr, attrs, buf, len, addr1, l, mr); diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c index XXXXXXX..XXXXXXX 100644 --- a/hw/remote/vfio-user-obj.c +++ b/hw/remote/vfio-user-obj.c @@ -XXX,XX +XXX,XX @@ static int vfu_object_mr_rw(MemoryRegion *mr, uint8_t *buf, hwaddr offset, ram_ptr = memory_region_get_ram_ptr(mr); if (is_write) { - memcpy((ram_ptr + offset), buf, size); + memmove((ram_ptr + offset), buf, size); } else { - memcpy(buf, (ram_ptr + offset), size); + memmove(buf, (ram_ptr + offset), size); } return 0; diff --git a/system/physmem.c b/system/physmem.c index XXXXXXX..XXXXXXX 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -XXX,XX +XXX,XX @@ static MemTxResult flatview_read_continue_step(MemTxAttrs attrs, uint8_t *buf, uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, l, false, false); - memcpy(buf, ram_ptr, *l); + memmove(buf, ram_ptr, *l); return MEMTX_OK; } -- 2.54.0 From: Gavin Shan <gshan@redhat.com> All ram device regions were turned to be indirectly accessible by commit 4a2e242bbb ("memory: Don't use memcpy for ram_device regions"). This leads to guest hang on attempt to build 'cuda-samples' as reported by Julia. The guest is started by the following command lines, with GH100 GPU card passed from the host. host$ lspci | grep GH100 0009:01:00.0 3D controller: NVIDIA Corporation GH100 [GH200 120GB / 480GB] (rev a1) host$ /home/sandbox/gavin/qemu.main/build/qemu-system-aarch64 \ -machine virt,gic-version=host,ras=on,highmem-mmio-size=4T \ -accel kvm -cpu host -smp cpus=48 -m size=8G \ -drive file=/home/gavin/sandbox/images/disk.qcow2,if=none,id=d0 \ -device virtio-blk-pci,id=vb0,bus=pcie.0,drive=d0,num-queues=4 \ -device vfio-pci-nohotplug,host=0009:01:00.0,bus=pcie.1.0 : guest$ cd cuda-samples/build guest$ make -j 20 clean guest$ make -j 20 : [ 54%] Linking CUDA executable graphMemoryNodes [ 54%] Built target graphMemoryNodes <no more output afterwards, guest becomes frozen here> guest$ qemu-system-aarch64: virtio: bogus descriptor or out of resources [ 555.814025] virtio_blk virtio0: [vda] new size: 268435456 512-byte logical blocks (137 GB/128 GiB) When the GPU's driver (NVidia open driver) is loaded on guest bootup, the memory blocks residing in the PCI BAR#4 of the GH100 GPU card can be presented to the guest through memory hot-add. The page cache can then be allocated from the hot added memory blocks when cuda-samples is being built. Afterwards, the page cache is sent to QEMU's virtio-blk device as part of the DMA request, the bounce buffer has to be used to accomodate the request as the corresponding memory region (MemoryRegion) is an indirectly accessible ram device region in qemu. However, the max bounce bufer size is only 4096 bytes by default and that is exhausted quickly, leading to a reset on the virtio-blk device and frozen guest eventually. QEMU ==== virtio_blk_handle_output virtio_blk_handle_vq virtio_blk_get_request virtqueue_pop virtqueue_split_pop virtqueue_map_desc address_space_map memory_access_is_direct # Return false memory_region_supports_direct_access (qemu) info mtree memory-region: pci_bridge_pci 0000000000000000-ffffffffffffffff (prio 0, container): pci_bridge_pci 0000042000000000-0000043fffffffff (prio 1, i/o): 0009:01:00.0 base BAR 4 0000042000000000-0000043fffffffff (prio 0, i/o): 0009:01:00.0 BAR 4 0000042000000000-000004379fffffff (prio 0, ramd): 0009:01:00.0 BAR 4 mmaps[0] This adds qemu_ram_move() where the aligned and small-sized accesses are handled by qatomics, and fall back to memmove() otherwise. The memove() for the directly accessible regions is replaced by qemu_ram_move() so that the issue covered by commit 4a2e242bbb (MMIO access instructions were optimized to SSE instructions) is fixed. This makes 'ram_device_mem_ops' redundant, paving the way to revert that commit to make the ram device region directly accessible again in the next patch. Besides, this also fixes the issue of the unexpected frozen reception on e1000 NIC in the scenario of DPDK due to the wrong Rx queue full indication caused by the following memcpy(), which is turned to 3 consective 'strb' instructions to the same location by glibc-2.24+ for aarch64. With this applied, the syntax of one-byte store is strictly ensured by a one-byte qatomic set. QEMU ==== e1000_receive_iov pci_dma_write pci_dma_rw dma_memory_rw dma_memory_rw_relaxed address_space_rw address_space_write flatview_write flatview_write_continue flatview_write_continue_step memcpy # 3 consective 'strb' instructions Reported-by: Julia Graham <jugraham@redhat.com> Reported-by: Liu Gang <liugang24219@sangfor.com.cn> Reported-by: Ding Hui <dinghui@sangfor.com.cn> Suggested-by: Michael S. Tsirkin <mst@redhat.com> Suggested-by: Peter Xu <peterx@redhat.com> Suggested-by: Richard Henderson <richard.henderson@linaro.org> Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Gavin Shan <gshan@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Link: https://lore.kernel.org/r/20260728031731.286666-3-gshan@redhat.com [peterx: remove src==dst check, fix doc, enhance comments, per PeterM, add R-b] Signed-off-by: Peter Xu <peterx@redhat.com> --- include/system/memory.h | 35 +++++++++++++++++++++++++++- hw/remote/vfio-user-obj.c | 4 ++-- system/physmem.c | 48 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 82 insertions(+), 5 deletions(-) diff --git a/include/system/memory.h b/include/system/memory.h index XXXXXXX..XXXXXXX 100644 --- a/include/system/memory.h +++ b/include/system/memory.h @@ -XXX,XX +XXX,XX @@ void address_space_register_map_client(AddressSpace *as, QEMUBH *bh); void address_space_unregister_map_client(AddressSpace *as, QEMUBH *bh); /* Internal functions, part of the implementation of address_space_read. */ + +/** + * qemu_ram_move: move data from or to ramblock + * + * @dst: destination where the data is moved to + * @src: source where the data is moved from + * @n: length of data to be moved + * + * Move @n bytes from @src to @dst, the memory areas may overlap. This + * provides the same semantics as memmove(), plus an additional stronger + * guarantee: if @n is 1, 2 or 4 or 8 bytes, and @src and @dst are both + * naturally aligned for that access size, then both the load and the store + * will be done as a single atomic access (with the semantics of + * qatomic_read() and qatomic_set()). + * + * This is the underlying function that we use to implement accesses by + * a guest vCPU or a device DMA operation to a ram block. The atomic + * guarantee is needed for two major cases: (A) When the ram block is + * backed by a PCI BAR passed through from a host device (and so it might + * be hardware registers that must be accessed exactly once at the right + * width); (B) When an emulated device updates a data structure shared in + * guest memory with guest software (e.g. a network device's set of tx and + * rx descriptor blocks), if a write to memory is accidentally performed + * multiple times then it can break the guest code when it busy polls the + * guest memory. + * + * We don't attempt to perform the exact access when it would be unaligned + * because this can't be done on all host architectures. Although this is + * strictly speaking not doing what would happen on real hardware, we don't + * think there are going to be situations where that matters in practice. + */ +void qemu_ram_move(void *dst, const void *src, size_t n); + MemTxResult address_space_read_full(const AddressSpace *as, hwaddr addr, MemTxAttrs attrs, void *buf, hwaddr len); MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr, @@ -XXX,XX +XXX,XX @@ MemTxResult address_space_read(const AddressSpace *as, hwaddr addr, mr = flatview_translate(fv, addr, &addr1, &l, false, attrs); if (len == l && memory_access_is_direct(mr, false, attrs)) { ptr = qemu_map_ram_ptr(mr->ram_block, addr1); - memmove(buf, ptr, len); + qemu_ram_move(buf, ptr, len); } else { result = flatview_read_continue(fv, addr, attrs, buf, len, addr1, l, mr); diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c index XXXXXXX..XXXXXXX 100644 --- a/hw/remote/vfio-user-obj.c +++ b/hw/remote/vfio-user-obj.c @@ -XXX,XX +XXX,XX @@ static int vfu_object_mr_rw(MemoryRegion *mr, uint8_t *buf, hwaddr offset, ram_ptr = memory_region_get_ram_ptr(mr); if (is_write) { - memmove((ram_ptr + offset), buf, size); + qemu_ram_move((ram_ptr + offset), buf, size); } else { - memmove(buf, (ram_ptr + offset), size); + qemu_ram_move(buf, (ram_ptr + offset), size); } return 0; diff --git a/system/physmem.c b/system/physmem.c index XXXXXXX..XXXXXXX 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -XXX,XX +XXX,XX @@ void memory_region_flush_rom_device(MemoryRegion *mr, hwaddr addr, hwaddr size) invalidate_and_set_dirty(mr, addr, size); } +void qemu_ram_move(void *dst, const void *src, size_t n) +{ + uintptr_t test, len; + + if (n == 0) { + return; + } + + /* + * Calculate "the lowest set bit" over @src, @dst and @n, result put + * into @len (which guarantees a power-of-two). With that and the + * later check (len!=n), it makes sure that we will only do the atomic + * ops when: + * + * (1) @n is a power-of-two + * (2) @src and @dst addresses are both aligned to @n + */ + test = (uintptr_t)src | (uintptr_t)dst | n; + len = test & -test; + + /* Overlapping buffers, unaligned or oversized access */ + if (n > 8 || len != n) { + memmove(dst, src, n); + return; + } + + switch (len) { + case 1: + qatomic_set((uint8_t *)dst, qatomic_read((uint8_t *)src)); + break; + case 2: + qatomic_set((uint16_t *)dst, qatomic_read((uint16_t *)src)); + break; + case 4: + qatomic_set((uint32_t *)dst, qatomic_read((uint32_t *)src)); + break; + case 8: + qatomic_set((uint64_t *)dst, qatomic_read((uint64_t *)src)); + break; + default: + g_assert_not_reached(); + } +} + int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) { unsigned access_size_max = mr->ops->valid.max_access_size; @@ -XXX,XX +XXX,XX @@ static MemTxResult flatview_write_continue_step(MemTxAttrs attrs, uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, l, false, true); - memmove(ram_ptr, buf, *l); + qemu_ram_move(ram_ptr, buf, *l); invalidate_and_set_dirty(mr, mr_addr, *l); return MEMTX_OK; @@ -XXX,XX +XXX,XX @@ static MemTxResult flatview_read_continue_step(MemTxAttrs attrs, uint8_t *buf, uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, l, false, false); - memmove(buf, ram_ptr, *l); + qemu_ram_move(buf, ram_ptr, *l); return MEMTX_OK; } -- 2.54.0 From: Gavin Shan <gshan@redhat.com> This basically reverts 4a2e242bbb30 ("memory: Don't use memcpy for ram_device regions") to make ram device region directly accessible again. With this, the bounce buffer is bypassed in address_space_map() when a ram device region is involved, potentially avoid to overrun the (small) bounce buffer. Reported-by: Julia Graham <jugraham@redhat.com> Suggested-by: Michael S. Tsirkin <mst@redhat.com> Suggested-by: Peter Xu <peterx@redhat.com> Suggested-by: Richard Henderson <richard.henderson@linaro.org> Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Gavin Shan <gshan@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Link: https://lore.kernel.org/r/20260728031731.286666-4-gshan@redhat.com Signed-off-by: Peter Xu <peterx@redhat.com> --- include/system/memory.h | 11 ++--------- system/memory.c | 41 +---------------------------------------- system/trace-events | 2 -- 3 files changed, 3 insertions(+), 51 deletions(-) diff --git a/include/system/memory.h b/include/system/memory.h index XXXXXXX..XXXXXXX 100644 --- a/include/system/memory.h +++ b/include/system/memory.h @@ -XXX,XX +XXX,XX @@ static inline bool memory_region_supports_direct_access(const MemoryRegion *mr) if (memory_region_is_romd(mr)) { return true; } - if (!memory_region_is_ram(mr)) { - return false; - } - /* - * RAM DEVICE regions can be accessed directly using memcpy, but it might - * be MMIO and access using mempy can be wrong (e.g., using instructions not - * intended for MMIO access). So we treat this as IO. - */ - return !memory_region_is_ram_device(mr); + + return memory_region_is_ram(mr); } static inline bool memory_access_is_direct(const MemoryRegion *mr, diff --git a/system/memory.c b/system/memory.c index XXXXXXX..XXXXXXX 100644 --- a/system/memory.c +++ b/system/memory.c @@ -XXX,XX +XXX,XX @@ const MemoryRegionOps unassigned_mem_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static uint64_t memory_region_ram_device_read(void *opaque, - hwaddr addr, unsigned size) -{ - MemoryRegion *mr = opaque; - uint64_t data = ldn_he_p(mr->ram_block->host + addr, size); - - trace_memory_region_ram_device_read(get_cpu_index(), mr, addr, data, size); - - return data; -} - -static void memory_region_ram_device_write(void *opaque, hwaddr addr, - uint64_t data, unsigned size) -{ - MemoryRegion *mr = opaque; - - trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data, size); - - stn_he_p(mr->ram_block->host + addr, size, data); -} - -static const MemoryRegionOps ram_device_mem_ops = { - .read = memory_region_ram_device_read, - .write = memory_region_ram_device_write, - .endianness = HOST_BIG_ENDIAN ? DEVICE_BIG_ENDIAN : DEVICE_LITTLE_ENDIAN, - .valid = { - .min_access_size = 1, - .max_access_size = 8, - .unaligned = true, - }, - .impl = { - .min_access_size = 1, - .max_access_size = 8, - .unaligned = true, - }, -}; - bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr, unsigned size, @@ -XXX,XX +XXX,XX @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr, Object *owner, const char *name, uint64_t size, void *ptr) { - memory_region_init_io(mr, owner, &ram_device_mem_ops, mr, name, size); - mr->ram = true; + memory_region_init_ram_ptr(mr, owner, name, size, ptr); mr->ram_device = true; - memory_region_set_ram_ptr(mr, size, ptr); } void memory_region_init_alias(MemoryRegion *mr, Object *owner, diff --git a/system/trace-events b/system/trace-events index XXXXXXX..XXXXXXX 100644 --- a/system/trace-events +++ b/system/trace-events @@ -XXX,XX +XXX,XX @@ memory_region_ops_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, u memory_region_ops_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size, const char *name) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u name '%s'" memory_region_subpage_read(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u" memory_region_subpage_write(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u" -memory_region_ram_device_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u" -memory_region_ram_device_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u" memory_region_sync_dirty(const char *mr, const char *listener, int global) "mr '%s' listener '%s' synced (global=%d)" flatview_new(void *view, void *root) "%p (root %p)" flatview_destroy(void *view, void *root) "%p (root %p)" -- 2.54.0 From: Sam Heney <github@me.samiser.xyz> The test_precopy_tcp_tls_no_hostname test and its start hook use TestMigrateTLSX509 and migrate_hook_start_tls_x509_common(), which are only defined when CONFIG_TASN1 is set. This means building with gnutls enabled but libtasn1 unavailable fails: ../tests/qtest/migration/tls-tests.c: In function 'migrate_hook_start_tls_x509_no_host': ../tests/qtest/migration/tls-tests.c:510:5: error: unknown type name 'TestMigrateTLSX509' Guard the test with CONFIG_TASN1 like the other x509 tests. Fixes: df9c38b19af8 ("tests/qtest/migration: Add a NULL parameters test for TLS") Signed-off-by: Sam Heney <github@me.samiser.xyz> Link: https://lore.kernel.org/r/5f24de0e-49af-45a7-927f-f79b203bb335@app.fastmail.com Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration/tls-tests.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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_no_tls(char *name, MigrateCommon *args) test_precopy_common(args); } +#ifdef CONFIG_TASN1 static void * migrate_hook_start_tls_x509_no_host(QTestState *from, QTestState *to) { @@ -XXX,XX +XXX,XX @@ static void test_precopy_tcp_tls_no_hostname(char *name, MigrateCommon *args) test_precopy_common(args); } -#ifdef CONFIG_TASN1 static void test_precopy_tcp_tls_x509_default_host(char *name, MigrateCommon *args) { @@ -XXX,XX +XXX,XX @@ void migration_test_add_tls(MigrationTestEnv *env) migration_test_add("/migration/precopy/tcp/no-tls", test_precopy_tcp_no_tls); +#ifdef CONFIG_TASN1 migration_test_add("/migration/precopy/tcp/tls/no-hostname", test_precopy_tcp_tls_no_hostname); +#endif /* CONFIG_TASN1 */ migration_test_add("/migration/precopy/unix/tls/psk", test_precopy_unix_tls_psk); -- 2.54.0 The zlib and zstd multifd compression backends read next_packet_size from the incoming migration stream and use it directly as the read length into a fixed-size buffer (MULTIFD_PACKET_SIZE * 2 = 1MB). A malicious migration source can set next_packet_size bigger than allocated, causing a heap buffer overflow write on the destination. Add a check against zbuff_len before reading, matching what the qatzip backend already does. Also replace the assert(in_size == 0) for empty packets with proper error reporting, since the value is wire-controlled, meanwhile assert() stops working with -DNDEBUG builds. Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3737 Reported-by: xlabai <xlabai@tencent.com> Reported-by: Jules Denardou <jules.denardou@datadoghq.com> Reported-by: Tristan Madani <tristan@talencesecurity.com> Reported-by: david korczynski (@david1766) Reported-by: huntr bubble (@bubblehuntr) Cc: qemu-stable <qemu-stable@nongnu.org> Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20260728210417.1925078-3-peterx@redhat.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/multifd-zlib.c | 11 ++++++++++- migration/multifd-zstd.c | 11 ++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c index XXXXXXX..XXXXXXX 100644 --- a/migration/multifd-zlib.c +++ b/migration/multifd-zlib.c @@ -XXX,XX +XXX,XX @@ static int multifd_zlib_recv(MultiFDRecvParams *p, Error **errp) return -1; } + if (in_size > z->zbuff_len) { + error_setg(errp, "multifd %u: next_packet_size %"PRIu32 + " exceeds allocated %"PRIu32, p->id, in_size, z->zbuff_len); + return -1; + } + multifd_recv_zero_page_process(p); if (!p->normal_num) { - assert(in_size == 0); + if (in_size != 0) { + error_setg(errp, "multifd %u: expected empty packet", p->id); + return -1; + } return 0; } diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c index XXXXXXX..XXXXXXX 100644 --- a/migration/multifd-zstd.c +++ b/migration/multifd-zstd.c @@ -XXX,XX +XXX,XX @@ static int multifd_zstd_recv(MultiFDRecvParams *p, Error **errp) return -1; } + if (in_size > z->zbuff_len) { + error_setg(errp, "multifd %u: next_packet_size %"PRIu32 + " exceeds allocated %"PRIu32, p->id, in_size, z->zbuff_len); + return -1; + } + multifd_recv_zero_page_process(p); if (!p->normal_num) { - assert(in_size == 0); + if (in_size != 0) { + error_setg(errp, "multifd %u: expected empty packet", p->id); + return -1; + } return 0; } -- 2.54.0 QPL and UADK multifd backends use assert() to validate wire-controlled fields like per-page compressed lengths and packet size consistency. These asserts will stop working with -DNDEBUG builds, so may stop working. Replace all assert() calls in the receive path with proper error_setg() so validation failures are reported gracefully rather than crashing or silently ignored. While at it, touch up an assert() in qatzip recv path too. Cc: qemu-stable <qemu-stable@nongnu.org> Cc: Yuan Liu <yuan1.liu@intel.com> Cc: Yichen Wang <yichen.wang@bytedance.com> Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20260728210417.1925078-4-peterx@redhat.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/multifd-qatzip.c | 5 ++++- migration/multifd-qpl.c | 24 ++++++++++++++++++++---- migration/multifd-uadk.c | 24 ++++++++++++++++++++---- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/migration/multifd-qatzip.c b/migration/multifd-qatzip.c index XXXXXXX..XXXXXXX 100644 --- a/migration/multifd-qatzip.c +++ b/migration/multifd-qatzip.c @@ -XXX,XX +XXX,XX @@ static int qatzip_recv(MultiFDRecvParams *p, Error **errp) multifd_recv_zero_page_process(p); if (!p->normal_num) { - assert(in_size == 0); + if (in_size != 0) { + error_setg(errp, "multifd %u: expected empty packet", p->id); + return -1; + } return 0; } diff --git a/migration/multifd-qpl.c b/migration/multifd-qpl.c index XXXXXXX..XXXXXXX 100644 --- a/migration/multifd-qpl.c +++ b/migration/multifd-qpl.c @@ -XXX,XX +XXX,XX @@ static int multifd_qpl_recv(MultiFDRecvParams *p, Error **errp) } multifd_recv_zero_page_process(p); if (!p->normal_num) { - assert(in_size == 0); + if (in_size != 0) { + error_setg(errp, "multifd %u: expected empty packet", p->id); + return -1; + } return 0; } /* read compressed page lengths */ len = p->normal_num * sizeof(uint32_t); - assert(len < in_size); + if (len >= in_size) { + error_setg(errp, "multifd %u: header len %"PRIu32 + " >= packet size %"PRIu32, p->id, len, in_size); + return -1; + } ret = qio_channel_read_all(p->c, (void *) qpl->zlen, len, errp); if (ret != 0) { return ret; } for (int i = 0; i < p->normal_num; i++) { qpl->zlen[i] = be32_to_cpu(qpl->zlen[i]); - assert(qpl->zlen[i] <= multifd_ram_page_size()); + if (qpl->zlen[i] > multifd_ram_page_size()) { + error_setg(errp, "multifd %u: page %d compressed len %" + PRIu32" too large", p->id, i, qpl->zlen[i]); + return -1; + } zbuf_len += qpl->zlen[i]; ramblock_recv_bitmap_set_offset(p->block, p->normal[i]); } /* read compressed pages */ - assert(in_size == len + zbuf_len); + if (in_size != len + zbuf_len) { + error_setg(errp, "multifd %u: packet size %"PRIu32 + " != header %"PRIu32" + data %"PRIu32, + p->id, in_size, len, zbuf_len); + return -1; + } ret = qio_channel_read_all(p->c, (void *) qpl->zbuf, zbuf_len, errp); if (ret != 0) { return ret; diff --git a/migration/multifd-uadk.c b/migration/multifd-uadk.c index XXXXXXX..XXXXXXX 100644 --- a/migration/multifd-uadk.c +++ b/migration/multifd-uadk.c @@ -XXX,XX +XXX,XX @@ static int multifd_uadk_recv(MultiFDRecvParams *p, Error **errp) multifd_recv_zero_page_process(p); if (!p->normal_num) { - assert(in_size == 0); + if (in_size != 0) { + error_setg(errp, "multifd %u: expected empty packet", p->id); + return -1; + } return 0; } /* read compressed data lengths */ - assert(hdr_len < in_size); + if (hdr_len >= in_size) { + error_setg(errp, "multifd %u: header len %"PRIu32 + " >= packet size %"PRIu32, p->id, hdr_len, in_size); + return -1; + } ret = qio_channel_read_all(p->c, (void *) uadk_data->buf_hdr, hdr_len, errp); if (ret != 0) { @@ -XXX,XX +XXX,XX @@ static int multifd_uadk_recv(MultiFDRecvParams *p, Error **errp) for (int i = 0; i < p->normal_num; i++) { uadk_data->buf_hdr[i] = be32_to_cpu(uadk_data->buf_hdr[i]); + if (uadk_data->buf_hdr[i] > page_size) { + error_setg(errp, "multifd %u: page %d compressed len %"PRIu32 + " too large", p->id, i, uadk_data->buf_hdr[i]); + return -1; + } data_len += uadk_data->buf_hdr[i]; - assert(uadk_data->buf_hdr[i] <= page_size); } /* read compressed data */ - assert(in_size == hdr_len + data_len); + if (in_size != hdr_len + data_len) { + error_setg(errp, "multifd %u: packet size %"PRIu32 + " != header %"PRIu32" + data %"PRIu32, + p->id, in_size, hdr_len, data_len); + return -1; + } ret = qio_channel_read_all(p->c, (void *)buf, data_len, errp); if (ret != 0) { return ret; -- 2.54.0 Add an underflow check for the subtract of total RAMBlock size to make sure it won't underflow. It should not happen in production systems but only if the migration stream was hijacked, which is not a real concern since migration channel is trusted. Still protect against it. Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4013 Reported-by: Tristan Madani <tristan@talencesecurity.com> Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20260728210417.1925078-6-peterx@redhat.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/ram.c | 11 +++++++++-- 1 file changed, 9 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 int parse_ramblocks(QEMUFile *f, ram_addr_t total_ram_bytes) int ret = 0; /* Synchronize RAM block list */ - while (!ret && total_ram_bytes) { + while (total_ram_bytes) { RAMBlock *block; char id[256]; ram_addr_t length; @@ -XXX,XX +XXX,XX @@ static int parse_ramblocks(QEMUFile *f, ram_addr_t total_ram_bytes) error_report("Unknown ramblock \"%s\", cannot accept " "migration", id); ret = -EINVAL; + break; + } + + if (usub64_overflow(total_ram_bytes, length, &total_ram_bytes)) { + error_report("%s: RAMBlock '%s' size underflow total RAM size", + __func__, block->idstr); + ret = -EFAULT; + break; } - total_ram_bytes -= length; } return ret; -- 2.54.0 In an unlikely case, when a migration stream is attached to the destination QEMU and only send <4 bytes to the channel as magic, it's possible that migration_channel_read_peek() may spin forever. Fix it by adding a manual sleep for partial read. Since the path isn't attached to a coroutine, it means when partial read happens, there's yet not much we can do but hang the main thread, it will happen even for len==0 case. It means monitors can hang due to this, either partial read or no data arrived (but connection established). Leave this for later, the hope is this is extremely rare in production. Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3889 Reported-by: Feifan Qian <bea1e@proton.me> Cc: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Link: https://lore.kernel.org/r/20260812124327.2572363-1-peterx@redhat.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/channel.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) 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 @@ int migration_channel_read_peek(QIOChannel *ioc, if (len == buflen) { break; + } else if (len == QIO_CHANNEL_ERR_BLOCK) { + qio_channel_wait_cond(ioc, G_IO_IN); + } else { + /* + * When partially ready, we can't use qio_channel_wait_cond() + * because it will return immediately. Apply a manual wait. + */ + assert(!qemu_in_coroutine()); + g_usleep(1000); } - - qio_channel_wait_cond(ioc, G_IO_IN); } return 0; -- 2.54.0