Peter Xu <pet...@redhat.com> writes:
> 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 <faro...@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..
>
I'm not sure it is. Having to go follow the hooks is confusing,
specially when hook names start to get similar. Having the test provide
everything it needs right here is clearer. Also, maintenance of the
hooks is a pain when it comes to code conflicts. I'd like to see less
hooks overall.
> 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?
>
Yes the idea is to cover both paths and I chose file-tests for config
arbitrarily.
>
> + 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
>>
