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