Marc-André Lureau <[email protected]> writes: > Hi Fabiano > > On Mon, May 4, 2026 at 6:32 PM Fabiano Rosas <[email protected]> wrote: >> >> Peter Xu <[email protected]> writes: >> >> > On Wed, Apr 29, 2026 at 04:05:46PM -0300, Fabiano Rosas wrote: >> >> The dbus-vmstate-test has been disabled for years. Here's the things >> >> that have changed in the meantime and how to update the test: >> >> >> >> - Migration tests got new headers. >> >> Update the includes. >> >> >> >> - migrate_qmp got new parameters. >> >> Update the caller. >> >> >> >> - migrate_incoming_qmp is now used instead of -incoming URL. >> >> Use -incoming defer. >> >> >> >> - Tests expecting failure should not check non-zero return code. >> >> Check for failed migration state instead. >> >> >> >> - The test result enum was introduced. >> >> Replace the migration_fail flag with the enum. >> >> >> >> - The DEVICE state was added. >> >> Replace wait_for_migration_complete with migration_event_wait, which >> >> won't trip on intermediary states. >> >> >> >> - Migration completion was reworked. >> >> Explicitly wait for the RESUME event before asserting runstate is >> >> RUNNING to avoid checking too quickly and seeing FINISH_MIGRATE >> >> instead. >> >> >> >> - The FAILING state was added. >> >> Wait for it before waiting for the RESUME event. >> >> >> >> - Sanity checks were added to migration_get_env(). >> >> Start calling that function in main. >> >> >> >> - qtest_add_func now has a wrapper. >> >> Replace qtest_add_func with migration_test_add. Update tests' >> >> signatures to take MigrationCommon, although it's unused. >> >> >> >> - meson now sets up G_TEST_DBUS_DAEMON. >> >> Remove the logic around it. >> >> >> >> Signed-off-by: Fabiano Rosas <[email protected]> >> > >> > I don't think I'm confident reviewing the whole series, but you can take: >> > >> > Acked-by: Peter Xu <[email protected]> >> > >> > One trivial question below, >> > >> >> --- >> >> tests/qtest/dbus-vmstate-test.c | 71 +++++++++++++++++++-------------- >> >> tests/qtest/meson.build | 7 +++- >> >> 2 files changed, 46 insertions(+), 32 deletions(-) >> >> >> >> diff --git a/tests/qtest/dbus-vmstate-test.c >> >> b/tests/qtest/dbus-vmstate-test.c >> >> index 0a82cc9f93..90c050b448 100644 >> >> --- a/tests/qtest/dbus-vmstate-test.c >> >> +++ b/tests/qtest/dbus-vmstate-test.c >> >> @@ -2,8 +2,8 @@ >> >> #include <glib/gstdio.h> >> >> #include <gio/gio.h> >> >> #include "libqtest.h" >> >> +#include "migration/migration-qmp.h" >> >> #include "dbus-vmstate1.h" >> >> -#include "migration-helpers.h" >> >> >> >> static char *workdir; >> >> >> >> @@ -29,7 +29,7 @@ typedef struct TestServer { >> >> >> >> typedef struct Test { >> >> const char *id_list; >> >> - bool migrate_fail; >> >> + int result; >> >> bool without_dst_b; >> >> TestServer srcA; >> >> TestServer dstA; >> >> @@ -190,6 +190,7 @@ test_dbus_vmstate(Test *test) >> >> g_autofree char *uri = NULL; >> >> QTestState *src_qemu = NULL, *dst_qemu = NULL; >> >> guint ownsrcA, ownsrcB, owndstA, owndstB; >> >> + QTestMigrationState src_state = { }; >> >> >> >> uri = g_strdup_printf("unix:%s/migsocket", workdir); >> >> >> >> @@ -224,17 +225,33 @@ test_dbus_vmstate(Test *test) >> >> >> >> src_qemu = qtest_init(src_qemu_args); >> >> dst_qemu = qtest_init(dst_qemu_args); >> >> + >> >> + migrate_set_capability(src_qemu, "events", true); >> >> + qtest_qmp_set_event_callback(src_qemu, migrate_watch_for_events, >> >> + &src_state); >> >> + >> >> set_id_list(test, src_qemu); >> >> set_id_list(test, dst_qemu); >> >> >> >> thread = g_thread_new("dbus-vmstate-thread", dbus_vmstate_thread, >> >> loop); >> >> >> >> migrate_incoming_qmp(dst_qemu, uri, NULL, "{}"); >> >> - migrate_qmp(src_qemu, uri, "{}"); >> >> + migrate_ensure_converge(src_qemu); >> >> + migrate_qmp(src_qemu, NULL, uri, NULL, "{}"); >> >> test->src_qemu = src_qemu; >> >> - if (test->migrate_fail) { >> >> - wait_for_migration_fail(src_qemu, true); >> >> - qtest_set_expected_status(dst_qemu, EXIT_FAILURE); >> >> + >> >> + if (test->result != MIG_TEST_SUCCEED) { >> >> + QDict *rsp; >> >> + >> >> + migration_event_wait(src_qemu, "failing"); >> >> + wait_for_resume(src_qemu, &src_state); >> >> + migration_event_wait(src_qemu, "failed"); >> > >> > Not sure if we need such detailed checks over failing, resume, failed >> > events on this one, but it looks ok. >> > >> >> I actually had a hang when not checking for failing. I didn't give it >> much thought because I know we added a new state. > > Should we keep skipping this test then? should I wait for an updated series? >
No, it's all good. If I decide to change this, I'll send a patch on top.
