All of lore.kernel.org
 help / color / mirror / Atom feed
* [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

* [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

* 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

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.