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

Fabiano Rosas posted 24 patches 4 months, 2 weeks ago
Maintainers: Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Laurent Vivier <lvivier@redhat.com>
[PATCH v2 24/24] tests/qtest/migration: Pass the migration config to file tests
Posted by Fabiano Rosas 4 months, 2 weeks ago
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);
+    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
Re: [PATCH v2 24/24] tests/qtest/migration: Pass the migration config to file tests
Posted by Peter Xu 3 months ago
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
Re: [PATCH v2 24/24] tests/qtest/migration: Pass the migration config to file tests
Posted by Fabiano Rosas 3 months ago
Peter Xu <peterx@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 <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..
>

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
>>
Re: [PATCH v2 24/24] tests/qtest/migration: Pass the migration config to file tests
Posted by Peter Xu 3 months ago
On Thu, Aug 14, 2025 at 12:30:00PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@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 <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..
> >
> 
> 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.

IMHO it depends.  If a hook can greatly dedup code, then I'll go for it.

This one only contains a dio setup, definitely not a huge deal.

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

But then file-tests lose the old coverage on using migrate-set-*.

IMHO, we could choose one that will be the officially suggested way, then
let all tests to use it.  With that, we can duplicate one or a few tests to
cover the not-suggested way.

So if "config" is the suggested way, we could make all tests moving over to
"config", then add only one or a few tests to cover migrate-set-parameters?

-- 
Peter Xu