[PATCH v3 6/7] Add multifd_tcp_plain test using list of channels instead of uri

Het Gala posted 7 patches 8 months, 3 weeks ago
Maintainers: Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>
There is a newer version of this series
[PATCH v3 6/7] Add multifd_tcp_plain test using list of channels instead of uri
Posted by Het Gala 8 months, 3 weeks ago
Add a positive test to check multifd live migration but this time
using list of channels (restricted to 1) as the starting point
instead of simple uri string.

Signed-off-by: Het Gala <het.gala@nutanix.com>
Suggested-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/migration-test.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index f94fe713b2..05e5f3ebe5 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -659,6 +659,12 @@ typedef struct {
      */
     const char *connect_uri;
 
+    /*
+     * Optional: the JSON formatted list of URIs for the src
+     * QEMU to connect to
+     */
+    const char *connect_channels;
+
     /* Optional: callback to run at start to set migration parameters */
     TestMigrateStartHook start_hook;
     /* Optional: callback to run at finish to cleanup */
@@ -2623,7 +2629,7 @@ test_migrate_precopy_tcp_multifd_zstd_start(QTestState *from,
 }
 #endif /* CONFIG_ZSTD */
 
-static void test_multifd_tcp_none(void)
+static void test_multifd_tcp_uri_none(void)
 {
     MigrateCommon args = {
         .listen_uri = "defer",
@@ -2638,6 +2644,21 @@ static void test_multifd_tcp_none(void)
     test_precopy_common(&args);
 }
 
+static void test_multifd_tcp_channels_none(void)
+{
+    MigrateCommon args = {
+        .listen_uri = "defer",
+        .start_hook = test_migrate_precopy_tcp_multifd_start,
+        .live = true,
+        .connect_channels = "[ { 'channel-type': 'main',"
+                            "    'addr': { 'transport': 'socket',"
+                            "              'type': 'inet',"
+                            "              'host': '127.0.0.1',"
+                            "              'port': '0' } } ]",
+    };
+    test_precopy_common(&args);
+}
+
 static void test_multifd_tcp_zlib(void)
 {
     MigrateCommon args = {
@@ -3531,8 +3552,10 @@ int main(int argc, char **argv)
                                test_migrate_dirty_limit);
         }
     }
-    migration_test_add("/migration/multifd/tcp/plain/none",
-                       test_multifd_tcp_none);
+    migration_test_add("/migration/multifd/tcp/uri/plain/none",
+                       test_multifd_tcp_uri_none);
+    migration_test_add("/migration/multifd/tcp/channels/plain/none",
+                       test_multifd_tcp_channels_none);
     migration_test_add("/migration/multifd/tcp/plain/cancel",
                        test_multifd_tcp_cancel);
     migration_test_add("/migration/multifd/tcp/plain/zlib",
-- 
2.22.3
Re: [PATCH v3 6/7] Add multifd_tcp_plain test using list of channels instead of uri
Posted by Fabiano Rosas 8 months, 3 weeks ago
Het Gala <het.gala@nutanix.com> writes:

> Add a positive test to check multifd live migration but this time
> using list of channels (restricted to 1) as the starting point
> instead of simple uri string.
>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> Suggested-by: Fabiano Rosas <farosas@suse.de>
> ---
>  tests/qtest/migration-test.c | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index f94fe713b2..05e5f3ebe5 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -659,6 +659,12 @@ typedef struct {
>       */
>      const char *connect_uri;
>  
> +    /*
> +     * Optional: the JSON formatted list of URIs for the src
> +     * QEMU to connect to
> +     */

You could add some words here mentioning that a port of '0' will be
automatically converted to the correct port that the destination is
using.

> +    const char *connect_channels;
> +
>      /* Optional: callback to run at start to set migration parameters */
>      TestMigrateStartHook start_hook;
>      /* Optional: callback to run at finish to cleanup */
> @@ -2623,7 +2629,7 @@ test_migrate_precopy_tcp_multifd_zstd_start(QTestState *from,
>  }
>  #endif /* CONFIG_ZSTD */
>  
> -static void test_multifd_tcp_none(void)
> +static void test_multifd_tcp_uri_none(void)
>  {
>      MigrateCommon args = {
>          .listen_uri = "defer",
> @@ -2638,6 +2644,21 @@ static void test_multifd_tcp_none(void)
>      test_precopy_common(&args);
>  }
>  
> +static void test_multifd_tcp_channels_none(void)
> +{
> +    MigrateCommon args = {
> +        .listen_uri = "defer",
> +        .start_hook = test_migrate_precopy_tcp_multifd_start,
> +        .live = true,
> +        .connect_channels = "[ { 'channel-type': 'main',"
> +                            "    'addr': { 'transport': 'socket',"
> +                            "              'type': 'inet',"
> +                            "              'host': '127.0.0.1',"
> +                            "              'port': '0' } } ]",
> +    };
> +    test_precopy_common(&args);
> +}
> +
>  static void test_multifd_tcp_zlib(void)
>  {
>      MigrateCommon args = {
> @@ -3531,8 +3552,10 @@ int main(int argc, char **argv)
>                                 test_migrate_dirty_limit);
>          }
>      }
> -    migration_test_add("/migration/multifd/tcp/plain/none",
> -                       test_multifd_tcp_none);
> +    migration_test_add("/migration/multifd/tcp/uri/plain/none",
> +                       test_multifd_tcp_uri_none);
> +    migration_test_add("/migration/multifd/tcp/channels/plain/none",
> +                       test_multifd_tcp_channels_none);

We should eventually make a pass to standardize/simplify these
strings. We could have a little "grammar" defined on how to construct
them.

/<test-type>/<migration-mode>/<transport>/<invocation>/<compression>/<encryption>

test-type      :: migrate | validate
migration-mode :: multifd | precopy | postcopy
transport      :: tcp | fd | unix | file
invocation     :: uri | channels
compression    :: zlib | zstd | none
encryption     :: tls | plain

Anyway, work for the future.
Re: [PATCH v3 6/7] Add multifd_tcp_plain test using list of channels instead of uri
Posted by Het Gala 8 months, 3 weeks ago
On 06/03/24 8:37 pm, Fabiano Rosas wrote:
> Het Gala<het.gala@nutanix.com>  writes:
>
>> Add a positive test to check multifd live migration but this time
>> using list of channels (restricted to 1) as the starting point
>> instead of simple uri string.
>>
>> Signed-off-by: Het Gala<het.gala@nutanix.com>
>> Suggested-by: Fabiano Rosas<farosas@suse.de>
>> ---
>>   tests/qtest/migration-test.c | 29 ++++++++++++++++++++++++++---
>>   1 file changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>> index f94fe713b2..05e5f3ebe5 100644
>> --- a/tests/qtest/migration-test.c
>> +++ b/tests/qtest/migration-test.c
>> @@ -659,6 +659,12 @@ typedef struct {
>>        */
>>       const char *connect_uri;
>>   
>> +    /*
>> +     * Optional: the JSON formatted list of URIs for the src
>> +     * QEMU to connect to
>> +     */
> You could add some words here mentioning that a port of '0' will be
> automatically converted to the correct port that the destination is
> using.
Ack, will add these while defining connect_channels.
>> +    const char *connect_channels;
>> +
>>       /* Optional: callback to run at start to set migration parameters */
>>       TestMigrateStartHook start_hook;
>>       /* Optional: callback to run at finish to cleanup */
>> @@ -2623,7 +2629,7 @@ test_migrate_precopy_tcp_multifd_zstd_start(QTestState *from,
>>   }
>>   #endif /* CONFIG_ZSTD */
>>   
>> -static void test_multifd_tcp_none(void)
>> +static void test_multifd_tcp_uri_none(void)
>>   {
>>       MigrateCommon args = {
>>           .listen_uri = "defer", @@ -2638,6 +2644,21 @@ static void test_multifd_tcp_none(void) 
>> test_precopy_common(&args); } +static void 
>> test_multifd_tcp_channels_none(void) +{ + MigrateCommon args = { + 
>> .listen_uri = "defer",
>> +        .start_hook = test_migrate_precopy_tcp_multifd_start,
>> +        .live = true,
>> +        .connect_channels = "[ { 'channel-type': 'main',"
>> +                            "    'addr': { 'transport': 'socket',"
>> +                            "              'type': 'inet',"
>> +                            "              'host': '127.0.0.1',"
>> +                            "              'port': '0' } } ]",
>> +    };
>> +    test_precopy_common(&args);
>> +}
>> +
>>   static void test_multifd_tcp_zlib(void)
>>   {
>>       MigrateCommon args = {
>> @@ -3531,8 +3552,10 @@ int main(int argc, char **argv)
>>                                  test_migrate_dirty_limit);
>>           }
>>       }
>> -    migration_test_add("/migration/multifd/tcp/plain/none",
>> -                       test_multifd_tcp_none);
>> +    migration_test_add("/migration/multifd/tcp/uri/plain/none",
>> +                       test_multifd_tcp_uri_none);
>> +    migration_test_add("/migration/multifd/tcp/channels/plain/none",
>> +                       test_multifd_tcp_channels_none);
> We should eventually make a pass to standardize/simplify these
> strings. We could have a little "grammar" defined on how to construct
> them.
>
> /<test-type>/<migration-mode>/<transport>/<invocation>/<compression>/<encryption>
>
> test-type      :: migrate | validate
> migration-mode :: multifd | precopy | postcopy
> transport      :: tcp | fd | unix | file
> invocation     :: uri | channels
> compression    :: zlib | zstd | none
> encryption     :: tls | plain
>
> Anyway, work for the future.

Yes, completely agree with you. It makes it much easier for people to 
identify and define every test.

I can take this up as a separate patchset after this one gets merged maybe


Regards,

Het Gala