Re: [PATCH v2 24/24] tests/qtest/migration: Pass the migration config to file tests

[Peter Xu <peterx@redhat.com>]

On Mon, Jun 30, 2025 at 04:59:13PM -0300, Fabiano Rosas wrote:
> Use the existing file tests to test the new way of passing parameters
> to the migration via the config argument to qmp_migrate*.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  tests/qtest/migration/file-tests.c    | 68 +++++++++++----------------
>  tests/qtest/migration/framework.c     |  9 ++--
>  tests/qtest/migration/precopy-tests.c |  1 +
>  3 files changed, 34 insertions(+), 44 deletions(-)
> 
> diff --git a/tests/qtest/migration/file-tests.c b/tests/qtest/migration/file-tests.c
> index 4d78ce0855..656d6527e8 100644
> --- a/tests/qtest/migration/file-tests.c
> +++ b/tests/qtest/migration/file-tests.c
> @@ -27,6 +27,7 @@ static void test_precopy_file(void)
>      MigrateCommon args = {
>          .connect_uri = uri,
>          .listen_uri = "defer",
> +        .start.config = qdict_new(),
>      };
>  
>      test_file_common(&args, true);
> @@ -74,6 +75,7 @@ static void test_precopy_file_offset_fdset(void)
>          .connect_uri = uri,
>          .listen_uri = "defer",
>          .start_hook = migrate_hook_start_file_offset_fdset,
> +        .start.config = qdict_new(),
>      };
>  
>      test_file_common(&args, false);
> @@ -88,6 +90,7 @@ static void test_precopy_file_offset(void)
>      MigrateCommon args = {
>          .connect_uri = uri,
>          .listen_uri = "defer",
> +        .start.config = qdict_new(),
>      };
>  
>      test_file_common(&args, false);
> @@ -102,6 +105,7 @@ static void test_precopy_file_offset_bad(void)
>          .connect_uri = uri,
>          .listen_uri = "defer",
>          .result = MIG_TEST_QMP_ERROR,
> +        .start.config = qdict_new(),
>      };
>  
>      test_file_common(&args, false);
> @@ -114,11 +118,10 @@ static void test_precopy_file_mapped_ram_live(void)
>      MigrateCommon args = {
>          .connect_uri = uri,
>          .listen_uri = "defer",
> -        .start = {
> -            .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
> -        },
> +        .start.config = qdict_new(),
>      };
>  
> +    qdict_put_bool(args.start.config, "mapped-ram", true);
>      test_file_common(&args, false);
>  }
>  
> @@ -129,11 +132,9 @@ static void test_precopy_file_mapped_ram(void)
>      MigrateCommon args = {
>          .connect_uri = uri,
>          .listen_uri = "defer",
> -        .start = {
> -            .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
> -        },
> +        .start.config = qdict_new(),
>      };
> -
> +    qdict_put_bool(args.start.config, "mapped-ram", true);
>      test_file_common(&args, true);
>  }
>  
> @@ -144,12 +145,11 @@ static void test_multifd_file_mapped_ram_live(void)
>      MigrateCommon args = {
>          .connect_uri = uri,
>          .listen_uri = "defer",
> -        .start = {
> -            .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
> -            .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
> -        },
> +        .start.config = qdict_new(),
>      };
>  
> +    qdict_put_bool(args.start.config, "mapped-ram", true);
> +    qdict_put_bool(args.start.config, "multifd", true);
>      test_file_common(&args, false);
>  }
>  
> @@ -160,24 +160,13 @@ static void test_multifd_file_mapped_ram(void)
>      MigrateCommon args = {
>          .connect_uri = uri,
>          .listen_uri = "defer",
> -        .start = {
> -            .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
> -            .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
> -        },
> +        .start.config = qdict_new(),
>      };
> -
> +    qdict_put_bool(args.start.config, "mapped-ram", true);
> +    qdict_put_bool(args.start.config, "multifd", true);
>      test_file_common(&args, true);
>  }
>  
> -static void *migrate_hook_start_multifd_mapped_ram_dio(QTestState *from,
> -                                                       QTestState *to)
> -{
> -    migrate_set_parameter_bool(from, "direct-io", true);
> -    migrate_set_parameter_bool(to, "direct-io", true);
> -
> -    return NULL;
> -}
> -
>  static void test_multifd_file_mapped_ram_dio(void)
>  {
>      g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
> @@ -185,13 +174,13 @@ static void test_multifd_file_mapped_ram_dio(void)
>      MigrateCommon args = {
>          .connect_uri = uri,
>          .listen_uri = "defer",
> -        .start_hook = migrate_hook_start_multifd_mapped_ram_dio,
> -        .start = {
> -            .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
> -            .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
> -        },
> +        .start.config = qdict_new(),
>      };
>  
> +    qdict_put_bool(args.start.config, "direct-io", true);

So the start_hook doesn't take args so we need to duplicate all these
direct-io setups in each test.. I assume not a big deal so it's fine, but
this is slightly going backward for sure..

What's your plan in mind on the tests?  Looks like you want to keep both
ways in tests/, only use it in some tests to cover both paths (and you
chose file-tests to start testing config)?  Or is this only an example and
you plan to convert more?

> +    qdict_put_bool(args.start.config, "mapped-ram", true);
> +    qdict_put_bool(args.start.config, "multifd", true);
> +
>      if (!probe_o_direct_support(tmpfs)) {
>          g_test_skip("Filesystem does not support O_DIRECT");
>          return;
> @@ -235,9 +224,6 @@ static void *migrate_hook_start_multifd_mapped_ram_fdset_dio(QTestState *from,
>      fdset_add_fds(from, file, O_WRONLY, 2, true);
>      fdset_add_fds(to, file, O_RDONLY, 2, true);
>  
> -    migrate_set_parameter_bool(from, "direct-io", true);
> -    migrate_set_parameter_bool(to, "direct-io", true);
> -
>      return NULL;
>  }
>  
> @@ -261,12 +247,11 @@ static void test_multifd_file_mapped_ram_fdset(void)
>          .listen_uri = "defer",
>          .start_hook = migrate_hook_start_multifd_mapped_ram_fdset,
>          .end_hook = migrate_hook_end_multifd_mapped_ram_fdset,
> -        .start = {
> -            .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
> -            .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
> -        },
> +        .start.config = qdict_new(),
>      };
>  
> +    qdict_put_bool(args.start.config, "mapped-ram", true);
> +    qdict_put_bool(args.start.config, "multifd", true);
>      test_file_common(&args, true);
>  }
>  
> @@ -279,12 +264,13 @@ static void test_multifd_file_mapped_ram_fdset_dio(void)
>          .listen_uri = "defer",
>          .start_hook = migrate_hook_start_multifd_mapped_ram_fdset_dio,
>          .end_hook = migrate_hook_end_multifd_mapped_ram_fdset,
> -        .start = {
> -            .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
> -            .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
> -        },
> +        .start.config = qdict_new(),
>      };
>  
> +    qdict_put_bool(args.start.config, "direct-io", true);
> +    qdict_put_bool(args.start.config, "mapped-ram", true);
> +    qdict_put_bool(args.start.config, "multifd", true);
> +
>      if (!probe_o_direct_support(tmpfs)) {
>          g_test_skip("Filesystem does not support O_DIRECT");
>          return;
> diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
> index 5025299d6a..37c5c884af 100644
> --- a/tests/qtest/migration/framework.c
> +++ b/tests/qtest/migration/framework.c
> @@ -974,18 +974,21 @@ void test_file_common(MigrateCommon *args, bool stop_src)
>      }
>  
>      if (args->result == MIG_TEST_QMP_ERROR) {
> -        migrate_qmp_fail(from, args->connect_uri, NULL, "{}");
> +        migrate_qmp_fail(from, args->connect_uri, NULL, "{ 'config': %p }",
> +                         args->start.config);
>          goto finish;
>      }
>  
> -    migrate_qmp(from, to, args->connect_uri, NULL, "{}");
> +    migrate_qmp(from, to, args->connect_uri, NULL, "{ 'config': %p }",
> +                args->start.config);
>      wait_for_migration_complete(from);
>  
>      /*
>       * We need to wait for the source to finish before starting the
>       * destination.
>       */
> -    migrate_incoming_qmp(to, args->connect_uri, NULL, "{}");
> +    migrate_incoming_qmp(to, args->connect_uri, NULL, "{ 'config': %p }",
> +                         args->start.config);
>      wait_for_migration_complete(to);
>  
>      if (stop_src) {
> diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c
> index 35d4274c69..9606dc1d02 100644
> --- a/tests/qtest/migration/precopy-tests.c
> +++ b/tests/qtest/migration/precopy-tests.c
> @@ -338,6 +338,7 @@ static void test_precopy_fd_file(void)
>          .connect_uri = "fd:fd-mig",
>          .start_hook = migrate_hook_start_precopy_fd_file,
>          .end_hook = migrate_hook_end_fd,
> +        .start.config = qdict_new(),
>      };
>      test_file_common(&args, true);
>  }
> -- 
> 2.35.3
> 

-- 
Peter Xu