* [PATCH v1 0/5] tests/qtest: Re-enable dbus-vmstate
@ 2026-04-29 19:05 Fabiano Rosas
2026-04-29 19:05 ` [PATCH v1 1/5] tests/qtest/dbus-vmstate: Bring the test up-to-date Fabiano Rosas
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Fabiano Rosas @ 2026-04-29 19:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, marcandre.lureau
The dbus-vmstate test has been disabled for years. By now, a lot has
changed around it. Do some cleanup and re-enable the test.
Top commit tested locally for 700 iterations plus another 100 for the
rest.
CI run: https://gitlab.com/farosas/qemu/-/pipelines/2488909078
Fabiano Rosas (5):
tests/qtest/dbus-vmstate: Bring the test up-to-date
tests/qtest/dbus-vmstate: Mute Glib complaints about g_unsetenv
thread-safety
tests/qtest/dbus-vmstate: Honor QTEST_LOG env variable
tests/qtest/dbus-vmstate: Stop the daemons explicitly
tests/qtest/dbus-vmstate: Re-enable the test
tests/qtest/dbus-vmstate-test.c | 121 ++++++++++++++++++++------------
tests/qtest/meson.build | 10 +--
2 files changed, 84 insertions(+), 47 deletions(-)
--
2.51.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v1 1/5] tests/qtest/dbus-vmstate: Bring the test up-to-date 2026-04-29 19:05 [PATCH v1 0/5] tests/qtest: Re-enable dbus-vmstate Fabiano Rosas @ 2026-04-29 19:05 ` Fabiano Rosas 2026-04-30 20:35 ` Peter Xu 2026-04-29 19:05 ` [PATCH v1 2/5] tests/qtest/dbus-vmstate: Mute Glib complaints about g_unsetenv thread-safety Fabiano Rosas ` (4 subsequent siblings) 5 siblings, 1 reply; 11+ messages in thread From: Fabiano Rosas @ 2026-04-29 19:05 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Xu, marcandre.lureau, Laurent Vivier, Paolo Bonzini 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 <farosas@suse.de> --- 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"); + + 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/5] tests/qtest/dbus-vmstate: Bring the test up-to-date 2026-04-29 19:05 ` [PATCH v1 1/5] tests/qtest/dbus-vmstate: Bring the test up-to-date Fabiano Rosas @ 2026-04-30 20:35 ` Peter Xu 2026-05-04 14:30 ` Fabiano Rosas 0 siblings, 1 reply; 11+ messages in thread From: Peter Xu @ 2026-04-30 20:35 UTC (permalink / raw) To: Fabiano Rosas; +Cc: qemu-devel, marcandre.lureau, Laurent Vivier, Paolo Bonzini 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 <farosas@suse.de> I don't think I'm confident reviewing the whole series, but you can take: Acked-by: Peter Xu <peterx@redhat.com> 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. 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(). > + > + 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 > -- Peter Xu ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/5] tests/qtest/dbus-vmstate: Bring the test up-to-date 2026-04-30 20:35 ` Peter Xu @ 2026-05-04 14:30 ` Fabiano Rosas 2026-05-04 14:59 ` Marc-André Lureau 0 siblings, 1 reply; 11+ messages in thread From: Fabiano Rosas @ 2026-05-04 14:30 UTC (permalink / raw) To: Peter Xu; +Cc: qemu-devel, marcandre.lureau, Laurent Vivier, Paolo Bonzini Peter Xu <peterx@redhat.com> 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 <farosas@suse.de> > > I don't think I'm confident reviewing the whole series, but you can take: > > Acked-by: Peter Xu <peterx@redhat.com> > > 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. > 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 >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/5] tests/qtest/dbus-vmstate: Bring the test up-to-date 2026-05-04 14:30 ` Fabiano Rosas @ 2026-05-04 14:59 ` Marc-André Lureau 2026-05-04 16:36 ` Fabiano Rosas 0 siblings, 1 reply; 11+ messages in thread From: Marc-André Lureau @ 2026-05-04 14:59 UTC (permalink / raw) To: Fabiano Rosas; +Cc: Peter Xu, qemu-devel, Laurent Vivier, Paolo Bonzini Hi Fabiano On Mon, May 4, 2026 at 6:32 PM Fabiano Rosas <farosas@suse.de> wrote: > > Peter Xu <peterx@redhat.com> 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 <farosas@suse.de> > > > > I don't think I'm confident reviewing the whole series, but you can take: > > > > Acked-by: Peter Xu <peterx@redhat.com> > > > > 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 > >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/5] tests/qtest/dbus-vmstate: Bring the test up-to-date 2026-05-04 14:59 ` Marc-André Lureau @ 2026-05-04 16:36 ` Fabiano Rosas 0 siblings, 0 replies; 11+ messages in thread From: Fabiano Rosas @ 2026-05-04 16:36 UTC (permalink / raw) To: Marc-André Lureau Cc: Peter Xu, qemu-devel, Laurent Vivier, Paolo Bonzini Marc-André Lureau <marcandre.lureau@redhat.com> writes: > Hi Fabiano > > On Mon, May 4, 2026 at 6:32 PM Fabiano Rosas <farosas@suse.de> wrote: >> >> Peter Xu <peterx@redhat.com> 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 <farosas@suse.de> >> > >> > I don't think I'm confident reviewing the whole series, but you can take: >> > >> > Acked-by: Peter Xu <peterx@redhat.com> >> > >> > 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v1 2/5] tests/qtest/dbus-vmstate: Mute Glib complaints about g_unsetenv thread-safety 2026-04-29 19:05 [PATCH v1 0/5] tests/qtest: Re-enable dbus-vmstate Fabiano Rosas 2026-04-29 19:05 ` [PATCH v1 1/5] tests/qtest/dbus-vmstate: Bring the test up-to-date Fabiano Rosas @ 2026-04-29 19:05 ` Fabiano Rosas 2026-04-29 19:05 ` [PATCH v1 3/5] tests/qtest/dbus-vmstate: Honor QTEST_LOG env variable Fabiano Rosas ` (3 subsequent siblings) 5 siblings, 0 replies; 11+ messages in thread From: Fabiano Rosas @ 2026-04-29 19:05 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Xu, marcandre.lureau, Laurent Vivier, Paolo Bonzini TLDR: GLib is bugged, the dbus-vmstate-test spams debug messages unconditionally. Mute them. GLib is trying to protect against the lack of thread-safety of setenv/unsetsenv functions by warning when those functions were invoked after a thread has been started. https://gitlab.gnome.org/GNOME/glib/issues/715 Unfortunately: 1) GLib itself starts a thread pool via _g_dbus_initialize when working around a bug in its type dependency chain. This happens in many places, but the test triggers it via g_dbus_address_get_for_bus_sync. https://bugzilla.gnome.org/show_bug.cgi?id=627724 2) GLib itself calls g_unsetenv after the test calls g_test_dbus_up. 3) The debug message at g_unsetenv is issued to the G_LOG_DOMAIN defined while compiling the library, i.e "GLib", but this domain is never initialized, so the log functions go into a fallback chain that results in ignoring the G_MESSAGES_DEBUG environment variable, causing the debug messages to not be suppressed when they should. Mute the messages by implementing a handler for G_LOG_LEVEL_DEBUG in the "GLib" log domain and honoring G_MESSAGES_DEBUG. Signed-off-by: Fabiano Rosas <farosas@suse.de> --- tests/qtest/dbus-vmstate-test.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/qtest/dbus-vmstate-test.c b/tests/qtest/dbus-vmstate-test.c index 90c050b448..51c5fdb995 100644 --- a/tests/qtest/dbus-vmstate-test.c +++ b/tests/qtest/dbus-vmstate-test.c @@ -357,12 +357,33 @@ test_dbus_vmstate_missing_dst(char *name, MigrateCommon *args) g_test_trap_assert_passed(); } +static void log_func(const gchar *log_domain, GLogLevelFlags log_level, + const gchar *message, gpointer user_data) +{ + const gchar *domains; + + assert(log_level & G_LOG_LEVEL_DEBUG); + + domains = getenv("G_MESSAGES_DEBUG"); + if (!domains || (!strstr(domains, "GLib") && !strstr(domains, "all"))) { + return; + } + g_log_default_handler("GLib", G_LOG_LEVEL_DEBUG, message, NULL); +} + int main(int argc, char **argv) { GError *err = NULL; int ret; + /* + * GLib currently emits debug messages that ignore + * G_MESSAGES_DEBUG. Set a custom log handler to work around the + * issue. + */ + g_log_set_handler("GLib", G_LOG_LEVEL_DEBUG, log_func, NULL); + g_test_init(&argc, &argv, NULL); workdir = g_dir_make_tmp("dbus-vmstate-test-XXXXXX", &err); -- 2.51.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 3/5] tests/qtest/dbus-vmstate: Honor QTEST_LOG env variable 2026-04-29 19:05 [PATCH v1 0/5] tests/qtest: Re-enable dbus-vmstate Fabiano Rosas 2026-04-29 19:05 ` [PATCH v1 1/5] tests/qtest/dbus-vmstate: Bring the test up-to-date Fabiano Rosas 2026-04-29 19:05 ` [PATCH v1 2/5] tests/qtest/dbus-vmstate: Mute Glib complaints about g_unsetenv thread-safety Fabiano Rosas @ 2026-04-29 19:05 ` Fabiano Rosas 2026-04-29 19:05 ` [PATCH v1 4/5] tests/qtest/dbus-vmstate: Stop the daemons explicitly Fabiano Rosas ` (2 subsequent siblings) 5 siblings, 0 replies; 11+ messages in thread From: Fabiano Rosas @ 2026-04-29 19:05 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Xu, marcandre.lureau, Laurent Vivier, Paolo Bonzini Don't hide QEMU error messages unconditionally, make the tests that expect to fail honor QTEST_LOG and show every output if the variable is set. Signed-off-by: Fabiano Rosas <farosas@suse.de> --- tests/qtest/dbus-vmstate-test.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/tests/qtest/dbus-vmstate-test.c b/tests/qtest/dbus-vmstate-test.c index 51c5fdb995..05e598a3e3 100644 --- a/tests/qtest/dbus-vmstate-test.c +++ b/tests/qtest/dbus-vmstate-test.c @@ -324,17 +324,18 @@ test_dbus_vmstate_missing_src(char *name, MigrateCommon *args) { Test test = { .id_list = "idA,idC", .result = MIG_TEST_FAIL }; + bool silent = !getenv("QTEST_LOG"); /* run in subprocess to silence QEMU error reporting */ - if (g_test_subprocess()) { - test_dbus_vmstate(&test); - check_not_migrated(&test.srcA, &test.dstA); - check_not_migrated(&test.srcB, &test.dstB); + if (silent && !g_test_subprocess()) { + g_test_trap_subprocess(NULL, 0, 0); + g_test_trap_assert_passed(); return; } - g_test_trap_subprocess(NULL, 0, 0); - g_test_trap_assert_passed(); + test_dbus_vmstate(&test); + check_not_migrated(&test.srcA, &test.dstA); + check_not_migrated(&test.srcB, &test.dstB); } static void @@ -343,18 +344,19 @@ test_dbus_vmstate_missing_dst(char *name, MigrateCommon *args) Test test = { .id_list = "idA,idB", .without_dst_b = true, .result = MIG_TEST_FAIL }; + bool silent = !getenv("QTEST_LOG"); /* run in subprocess to silence QEMU error reporting */ - if (g_test_subprocess()) { - test_dbus_vmstate(&test); - assert(test.srcA.save_called); - assert(test.srcB.save_called); - assert(!test.dstB.save_called); + if (silent && !g_test_subprocess()) { + g_test_trap_subprocess(NULL, 0, 0); + g_test_trap_assert_passed(); return; } - g_test_trap_subprocess(NULL, 0, 0); - g_test_trap_assert_passed(); + test_dbus_vmstate(&test); + assert(test.srcA.save_called); + assert(test.srcB.save_called); + assert(!test.dstB.save_called); } static void log_func(const gchar *log_domain, GLogLevelFlags log_level, -- 2.51.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 4/5] tests/qtest/dbus-vmstate: Stop the daemons explicitly 2026-04-29 19:05 [PATCH v1 0/5] tests/qtest: Re-enable dbus-vmstate Fabiano Rosas ` (2 preceding siblings ...) 2026-04-29 19:05 ` [PATCH v1 3/5] tests/qtest/dbus-vmstate: Honor QTEST_LOG env variable Fabiano Rosas @ 2026-04-29 19:05 ` Fabiano Rosas 2026-04-29 19:05 ` [PATCH v1 5/5] tests/qtest/dbus-vmstate: Re-enable the test Fabiano Rosas 2026-04-29 19:36 ` [PATCH v1 0/5] tests/qtest: Re-enable dbus-vmstate marcandre.lureau 5 siblings, 0 replies; 11+ messages in thread From: Fabiano Rosas @ 2026-04-29 19:05 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Xu, marcandre.lureau, Laurent Vivier, Paolo Bonzini The dbus-vmstate test is currently non-deterministically emitting a "cleaning up pid" message, followed by the PID of one of the dbus-daemon processes used during the test. This is due to a race between the GTestDBus g_autoptr destructor and a child process that does cleanup when the program ends. Add calls to g_test_dbus_down() to make the issuance of the SIGTERM to the dbus-daemon deterministic. Signed-off-by: Fabiano Rosas <farosas@suse.de> --- tests/qtest/dbus-vmstate-test.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qtest/dbus-vmstate-test.c b/tests/qtest/dbus-vmstate-test.c index 05e598a3e3..15c35e7c0f 100644 --- a/tests/qtest/dbus-vmstate-test.c +++ b/tests/qtest/dbus-vmstate-test.c @@ -260,11 +260,12 @@ test_dbus_vmstate(Test *test) qtest_quit(src_qemu); g_bus_unown_name(ownsrcA); g_bus_unown_name(ownsrcB); + g_test_dbus_down(srcbus); g_bus_unown_name(owndstA); if (!test->without_dst_b) { g_bus_unown_name(owndstB); } - + g_test_dbus_down(dstbus); g_main_loop_quit(test->loop); } -- 2.51.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 5/5] tests/qtest/dbus-vmstate: Re-enable the test 2026-04-29 19:05 [PATCH v1 0/5] tests/qtest: Re-enable dbus-vmstate Fabiano Rosas ` (3 preceding siblings ...) 2026-04-29 19:05 ` [PATCH v1 4/5] tests/qtest/dbus-vmstate: Stop the daemons explicitly Fabiano Rosas @ 2026-04-29 19:05 ` Fabiano Rosas 2026-04-29 19:36 ` [PATCH v1 0/5] tests/qtest: Re-enable dbus-vmstate marcandre.lureau 5 siblings, 0 replies; 11+ messages in thread From: Fabiano Rosas @ 2026-04-29 19:05 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Xu, marcandre.lureau, Laurent Vivier, Paolo Bonzini Back in 2020, the dbus-vmstate test was disabled by commit d46f81cb74 ("tests: Disable dbus-vmstate-test") due to Patchew failures. We don't use Patchew anymore for CI, so re-enable the test. G_TEST_DBUS_DAEMON=../tests/dbus-vmstate-daemon.sh \ QTEST_QEMU_BINARY=./qemu-system-x86_64 \ ./tests/qtest/dbus-vmstate-test Signed-off-by: Fabiano Rosas <farosas@suse.de> --- tests/qtest/meson.build | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index 0d04e2cbaa..b3f62e8cd5 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -123,8 +123,7 @@ endif dbus_daemon = find_program('dbus-daemon', required: false) if dbus_daemon.found() and gdbus_codegen.found() - # Temporarily disabled due to Patchew failures: - #qtests_i386 += ['dbus-vmstate-test'] + qtests_i386 += ['dbus-vmstate-test'] dbus_vmstate1 = custom_target('dbus-vmstate description', build_by_default: true, output: [ 'dbus-vmstate1.h', 'dbus-vmstate1.c'], -- 2.51.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 0/5] tests/qtest: Re-enable dbus-vmstate 2026-04-29 19:05 [PATCH v1 0/5] tests/qtest: Re-enable dbus-vmstate Fabiano Rosas ` (4 preceding siblings ...) 2026-04-29 19:05 ` [PATCH v1 5/5] tests/qtest/dbus-vmstate: Re-enable the test Fabiano Rosas @ 2026-04-29 19:36 ` marcandre.lureau 5 siblings, 0 replies; 11+ messages in thread From: marcandre.lureau @ 2026-04-29 19:36 UTC (permalink / raw) To: Fabiano Rosas; +Cc: qemu-devel, Peter Xu, marcandre.lureau On Wed, 29 Apr 2026 16:05:45 -0300, Fabiano Rosas <farosas@suse.de> wrote: > [...] > tests/qtest/dbus-vmstate-test.c | 121 ++++++++++++++++++++------------ > tests/qtest/meson.build | 10 +-- > 2 files changed, 84 insertions(+), 47 deletions(-) > > -- > 2.51.0 Awesome, many thanks! Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> -- Marc-André Lureau <marcandre.lureau@redhat.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-05-04 16:37 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-29 19:05 [PATCH v1 0/5] tests/qtest: Re-enable dbus-vmstate Fabiano Rosas 2026-04-29 19:05 ` [PATCH v1 1/5] tests/qtest/dbus-vmstate: Bring the test up-to-date Fabiano Rosas 2026-04-30 20:35 ` Peter Xu 2026-05-04 14:30 ` Fabiano Rosas 2026-05-04 14:59 ` Marc-André Lureau 2026-05-04 16:36 ` Fabiano Rosas 2026-04-29 19:05 ` [PATCH v1 2/5] tests/qtest/dbus-vmstate: Mute Glib complaints about g_unsetenv thread-safety Fabiano Rosas 2026-04-29 19:05 ` [PATCH v1 3/5] tests/qtest/dbus-vmstate: Honor QTEST_LOG env variable Fabiano Rosas 2026-04-29 19:05 ` [PATCH v1 4/5] tests/qtest/dbus-vmstate: Stop the daemons explicitly Fabiano Rosas 2026-04-29 19:05 ` [PATCH v1 5/5] tests/qtest/dbus-vmstate: Re-enable the test Fabiano Rosas 2026-04-29 19:36 ` [PATCH v1 0/5] tests/qtest: Re-enable dbus-vmstate marcandre.lureau
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.