On Wed, Apr 29, 2026 at 04:05:46PM -0300, Fabiano Rosas wrote:
> The dbus-vmstate-test has been disabled for years. Here's the things
> that have changed in the meantime and how to update the test:
> 
> - Migration tests got new headers.
> Update the includes.
> 
> - migrate_qmp got new parameters.
> Update the caller.
> 
> - migrate_incoming_qmp is now used instead of -incoming URL.
> Use -incoming defer.
> 
> - Tests expecting failure should not check non-zero return code.
> Check for failed migration state instead.
> 
> - The test result enum was introduced.
> Replace the migration_fail flag with the enum.
> 
> - The DEVICE state was added.
> Replace wait_for_migration_complete with migration_event_wait, which
> won't trip on intermediary states.
> 
> - Migration completion was reworked.
> Explicitly wait for the RESUME event before asserting runstate is
> RUNNING to avoid checking too quickly and seeing FINISH_MIGRATE
> instead.
> 
> - The FAILING state was added.
> Wait for it before waiting for the RESUME event.
> 
> - Sanity checks were added to migration_get_env().
> Start calling that function in main.
> 
> - qtest_add_func now has a wrapper.
> Replace qtest_add_func with migration_test_add. Update tests'
> signatures to take MigrationCommon, although it's unused.
> 
> - meson now sets up G_TEST_DBUS_DAEMON.
> Remove the logic around it.
> 
> Signed-off-by: Fabiano Rosas <[email protected]>

I don't think I'm confident reviewing the whole series, but you can take:

Acked-by: Peter Xu <[email protected]>

One trivial question below,

> ---
>  tests/qtest/dbus-vmstate-test.c | 71 +++++++++++++++++++--------------
>  tests/qtest/meson.build         |  7 +++-
>  2 files changed, 46 insertions(+), 32 deletions(-)
> 
> diff --git a/tests/qtest/dbus-vmstate-test.c b/tests/qtest/dbus-vmstate-test.c
> index 0a82cc9f93..90c050b448 100644
> --- a/tests/qtest/dbus-vmstate-test.c
> +++ b/tests/qtest/dbus-vmstate-test.c
> @@ -2,8 +2,8 @@
>  #include <glib/gstdio.h>
>  #include <gio/gio.h>
>  #include "libqtest.h"
> +#include "migration/migration-qmp.h"
>  #include "dbus-vmstate1.h"
> -#include "migration-helpers.h"
>  
>  static char *workdir;
>  
> @@ -29,7 +29,7 @@ typedef struct TestServer {
>  
>  typedef struct Test {
>      const char *id_list;
> -    bool migrate_fail;
> +    int result;
>      bool without_dst_b;
>      TestServer srcA;
>      TestServer dstA;
> @@ -190,6 +190,7 @@ test_dbus_vmstate(Test *test)
>      g_autofree char *uri = NULL;
>      QTestState *src_qemu = NULL, *dst_qemu = NULL;
>      guint ownsrcA, ownsrcB, owndstA, owndstB;
> +    QTestMigrationState src_state = { };
>  
>      uri = g_strdup_printf("unix:%s/migsocket", workdir);
>  
> @@ -224,17 +225,33 @@ test_dbus_vmstate(Test *test)
>  
>      src_qemu = qtest_init(src_qemu_args);
>      dst_qemu = qtest_init(dst_qemu_args);
> +
> +    migrate_set_capability(src_qemu, "events", true);
> +    qtest_qmp_set_event_callback(src_qemu, migrate_watch_for_events,
> +                                 &src_state);
> +
>      set_id_list(test, src_qemu);
>      set_id_list(test, dst_qemu);
>  
>      thread = g_thread_new("dbus-vmstate-thread", dbus_vmstate_thread, loop);
>  
>      migrate_incoming_qmp(dst_qemu, uri, NULL, "{}");
> -    migrate_qmp(src_qemu, uri, "{}");
> +    migrate_ensure_converge(src_qemu);
> +    migrate_qmp(src_qemu, NULL, uri, NULL, "{}");
>      test->src_qemu = src_qemu;
> -    if (test->migrate_fail) {
> -        wait_for_migration_fail(src_qemu, true);
> -        qtest_set_expected_status(dst_qemu, EXIT_FAILURE);
> +
> +    if (test->result != MIG_TEST_SUCCEED) {
> +        QDict *rsp;
> +
> +        migration_event_wait(src_qemu, "failing");
> +        wait_for_resume(src_qemu, &src_state);
> +        migration_event_wait(src_qemu, "failed");

Not sure if we need such detailed checks over failing, resume, failed
events on this one, but it looks ok.

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


Reply via email to