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?
thanks
>
> > Do you plan to remove wait_for_migration_fail() finally? We still have a
> > few other users. IIUC, we could also switch to using events for
> > wait_for_migration_fail().
> >
>
> Seems like a good idea to not need so many query-migrate
> invocations. Except that we could hide some potential races with QMP
> commands. And we lose the timeout as well. I'll take a look at what can
> be done.
>
> >> +
> >> + rsp = qtest_qmp_assert_success_ref(src_qemu,
> >> + "{ 'execute': 'query-status'
> >> }");
> >> + g_assert(qdict_haskey(rsp, "running"));
> >> + g_assert(qdict_get_bool(rsp, "running"));
> >> + qobject_unref(rsp);
> >> } else {
> >> wait_for_migration_complete(src_qemu);
> >> }
> >> @@ -270,7 +287,7 @@ check_migrated(TestServer *s, TestServer *d)
> >> }
> >>
> >> static void
> >> -test_dbus_vmstate_without_list(void)
> >> +test_dbus_vmstate_without_list(char *name, MigrateCommon *args)
> >> {
> >> Test test = { 0, };
> >>
> >> @@ -281,7 +298,7 @@ test_dbus_vmstate_without_list(void)
> >> }
> >>
> >> static void
> >> -test_dbus_vmstate_with_list(void)
> >> +test_dbus_vmstate_with_list(char *name, MigrateCommon *args)
> >> {
> >> Test test = { .id_list = "idA,idB" };
> >>
> >> @@ -292,7 +309,7 @@ test_dbus_vmstate_with_list(void)
> >> }
> >>
> >> static void
> >> -test_dbus_vmstate_only_a(void)
> >> +test_dbus_vmstate_only_a(char *name, MigrateCommon *args)
> >> {
> >> Test test = { .id_list = "idA" };
> >>
> >> @@ -303,9 +320,10 @@ test_dbus_vmstate_only_a(void)
> >> }
> >>
> >> static void
> >> -test_dbus_vmstate_missing_src(void)
> >> +test_dbus_vmstate_missing_src(char *name, MigrateCommon *args)
> >> {
> >> - Test test = { .id_list = "idA,idC", .migrate_fail = true };
> >> + Test test = { .id_list = "idA,idC",
> >> + .result = MIG_TEST_FAIL };
> >>
> >> /* run in subprocess to silence QEMU error reporting */
> >> if (g_test_subprocess()) {
> >> @@ -320,11 +338,11 @@ test_dbus_vmstate_missing_src(void)
> >> }
> >>
> >> static void
> >> -test_dbus_vmstate_missing_dst(void)
> >> +test_dbus_vmstate_missing_dst(char *name, MigrateCommon *args)
> >> {
> >> Test test = { .id_list = "idA,idB",
> >> .without_dst_b = true,
> >> - .migrate_fail = true };
> >> + .result = MIG_TEST_FAIL };
> >>
> >> /* run in subprocess to silence QEMU error reporting */
> >> if (g_test_subprocess()) {
> >> @@ -343,15 +361,8 @@ int
> >> main(int argc, char **argv)
> >> {
> >> GError *err = NULL;
> >> - g_autofree char *dbus_daemon = NULL;
> >> int ret;
> >>
> >> - dbus_daemon = g_build_filename(G_STRINGIFY(SRCDIR),
> >> - "tests",
> >> - "dbus-vmstate-daemon.sh",
> >> - NULL);
> >> - g_setenv("G_TEST_DBUS_DAEMON", dbus_daemon, true);
> >> -
> >> g_test_init(&argc, &argv, NULL);
> >>
> >> workdir = g_dir_make_tmp("dbus-vmstate-test-XXXXXX", &err);
> >> @@ -362,16 +373,16 @@ main(int argc, char **argv)
> >>
> >> g_setenv("DBUS_VMSTATE_TEST_TMPDIR", workdir, true);
> >>
> >> - qtest_add_func("/dbus-vmstate/without-list",
> >> - test_dbus_vmstate_without_list);
> >> - qtest_add_func("/dbus-vmstate/with-list",
> >> - test_dbus_vmstate_with_list);
> >> - qtest_add_func("/dbus-vmstate/only-a",
> >> - test_dbus_vmstate_only_a);
> >> - qtest_add_func("/dbus-vmstate/missing-src",
> >> - test_dbus_vmstate_missing_src);
> >> - qtest_add_func("/dbus-vmstate/missing-dst",
> >> - test_dbus_vmstate_missing_dst);
> >> + migration_test_add("/dbus-vmstate/without-list",
> >> + test_dbus_vmstate_without_list);
> >> + migration_test_add("/dbus-vmstate/with-list",
> >> + test_dbus_vmstate_with_list);
> >> + migration_test_add("/dbus-vmstate/only-a",
> >> + test_dbus_vmstate_only_a);
> >> + migration_test_add("/dbus-vmstate/missing-src",
> >> + test_dbus_vmstate_missing_src);
> >> + migration_test_add("/dbus-vmstate/missing-dst",
> >> + test_dbus_vmstate_missing_dst);
> >>
> >> ret = g_test_run();
> >>
> >> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> >> index b735f55fc4..0d04e2cbaa 100644
> >> --- a/tests/qtest/meson.build
> >> +++ b/tests/qtest/meson.build
> >> @@ -126,10 +126,12 @@ if dbus_daemon.found() and gdbus_codegen.found()
> >> # Temporarily disabled due to Patchew failures:
> >> #qtests_i386 += ['dbus-vmstate-test']
> >> dbus_vmstate1 = custom_target('dbus-vmstate description',
> >> - output: ['dbus-vmstate1.h',
> >> 'dbus-vmstate1.c'],
> >> + build_by_default: true,
> >> + output: [ 'dbus-vmstate1.h',
> >> 'dbus-vmstate1.c'],
> >> input: meson.project_source_root() /
> >> 'backends/dbus-vmstate1.xml',
> >> command: [gdbus_codegen, '@INPUT@',
> >> '--interface-prefix',
> >> 'org.qemu',
> >> + '--output-directory',
> >> meson.current_build_dir(),
> >> '--generate-c-code',
> >> '@BASENAME@']).to_list()
> >> else
> >> dbus_vmstate1 = []
> >> @@ -385,7 +387,8 @@ qtests = {
> >> 'bios-tables-test': [io, 'boot-sector.c', 'acpi-utils.c', 'tpm-emu.c'],
> >> 'cdrom-test': files('boot-sector.c'),
> >> 'dbus-vmstate-test': files('migration/migration-qmp.c',
> >> - 'migration/migration-util.c') +
> >> dbus_vmstate1,
> >> + 'migration/migration-util.c') +
> >> dbus_vmstate1 +
> >> + [gio],
> >> 'erst-test': files('erst-test.c'),
> >> 'ivshmem-test': [rt, '../../contrib/ivshmem-server/ivshmem-server.c'],
> >> 'migration-test': test_migration_files + migration_tls_files +
> >> migration_colo_files,
> >> --
> >> 2.51.0
> >>
>
