[PATCH v4 0/8] qtest: migration: Add tests for introducing 'channels' argument in migrate QAPIs

[PATCH v4 0/8] qtest: migration: Add tests for introducing 'channels' argument in migrate QAPIs

[Het Gala]

With recent migrate QAPI changes, enabling the direct use of the
'channels' argument to avoid redundant URI string parsing is achieved.

To ensure backward compatibility, both 'uri' and 'channels' are kept as
optional parameters in migration QMP commands. However, they are mutually
exhaustive, requiring at least one for a successful migration connection.
This patchset adds qtests to validate 'uri' and 'channels' arguments'
mututally exhaustive behaviour.

Additionally, all migration qtests fail to employ 'channel' as the primary
method for validating migration QAPIs. This patchset also adds test to
enforce only use of 'channel' argument as the initial entry point for
migration QAPIs.

Patch Summary:
-------------
Patch 1-2:
---------
Introduce 'to' object inside migrate_qmp() so and move the calls to
migrate_get_socket_address() inside migrate_qmp. Also, replace connect_uri
with args->connect_uri everywhere.

Patch 3-6:
---------
Add channels argument to allow both migration QAPI arguments independently
into migrate_qmp and migrate_qmp_fail. migrate_qmp requires the port value to
be changed from 0 to port value coming from migrate_get_socket_address. Add
migrate_set_ports to address this change of port value.

Patch 7-8:
---------
Add 2 negative tests to validate mutually exhaustive behaviour of migration
QAPIs. Add a positive multifd_tcp_plain qtest with only channels as the
initial entry point for migration QAPIs.

v3->v4 Changelog:
----------------
1. introduced migrate_get_connect_uri and migrate_get_connect_qdict to
   both used migrate_get_socket_address to get dest uri in socket-
   address, and then use SokcketAddress_to_qdict to convert it into qdict.
2. Misc code changes.

v2->v3 Changelog:
-----------------
1. 'channels' introduction is not required now for migrate_qmp_incoming
2. Refactor the code into 7 different patches
3. 'channels' introduction is not required now for migrate_qmp_incoming
4. Remove custom function for converting string to MigrationChannelList
5. move calls for migrate_get_socket_address inside migrate_qmp so that
   migrate_set_ports can replace the QAPI's port with correct value.

Het Gala (8):
  Add 'to' object into migrate_qmp()
  Replace connect_uri and move migrate_get_socket_address inside
    migrate_qmp
  Replace migrate_get_connect_uri inplace of migrate_get_socket_address
  Add channels parameter in migrate_qmp_fail
  Add migrate_set_ports into migrate_qmp to update migration port value
  Add channels parameter in migrate_qmp
  Add multifd_tcp_plain test using list of channels instead of uri
  Add negative tests to validate migration QAPIs

 tests/qtest/migration-helpers.c | 158 +++++++++++++++++++++++++++-
 tests/qtest/migration-helpers.h |  10 +-
 tests/qtest/migration-test.c    | 177 ++++++++++++++++++--------------
 3 files changed, 258 insertions(+), 87 deletions(-)

-- 
2.22.3

[PATCH v4 1/8] Add 'to' object into migrate_qmp()

[Het Gala]

Add the 'to' object into migrate_qmp(), so we can use
migrate_get_socket_address() inside migrate_qmp() to get
the port value. This is not applied to other migrate_qmp*
because they don't need the port.

Signed-off-by: Het Gala <het.gala@nutanix.com>
Suggested-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/migration-helpers.c |  3 ++-
 tests/qtest/migration-helpers.h |  5 +++--
 tests/qtest/migration-test.c    | 28 ++++++++++++++--------------
 3 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index e451dbdbed..b6206a04fb 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -68,7 +68,8 @@ void migrate_qmp_fail(QTestState *who, const char *uri, const char *fmt, ...)
  * Arguments are built from @fmt... (formatted like
  * qobject_from_jsonf_nofail()) with "uri": @uri spliced in.
  */
-void migrate_qmp(QTestState *who, const char *uri, const char *fmt, ...)
+void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
+                 const char *fmt, ...)
 {
     va_list ap;
     QDict *args;
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index 3bf7ded1b9..e16a34c796 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -25,8 +25,9 @@ typedef struct QTestMigrationState {
 bool migrate_watch_for_events(QTestState *who, const char *name,
                               QDict *event, void *opaque);
 
-G_GNUC_PRINTF(3, 4)
-void migrate_qmp(QTestState *who, const char *uri, const char *fmt, ...);
+G_GNUC_PRINTF(4, 5)
+void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
+                 const char *fmt, ...);
 
 G_GNUC_PRINTF(3, 4)
 void migrate_incoming_qmp(QTestState *who, const char *uri,
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 4023d808f9..d9b4e28c12 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1350,7 +1350,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
     wait_for_suspend(from, &src_state);
 
     g_autofree char *uri = migrate_get_socket_address(to, "socket-address");
-    migrate_qmp(from, uri, "{}");
+    migrate_qmp(from, to, uri, "{}");
 
     migrate_wait_for_dirty_mem(from, to);
 
@@ -1500,7 +1500,7 @@ static void postcopy_recover_fail(QTestState *from, QTestState *to)
     g_assert_cmpint(ret, ==, 1);
 
     migrate_recover(to, "fd:fd-mig");
-    migrate_qmp(from, "fd:fd-mig", "{'resume': true}");
+    migrate_qmp(from, to, "fd:fd-mig", "{'resume': true}");
 
     /*
      * Make sure both QEMU instances will go into RECOVER stage, then test
@@ -1588,7 +1588,7 @@ static void test_postcopy_recovery_common(MigrateCommon *args)
      * Try to rebuild the migration channel using the resume flag and
      * the newly created channel
      */
-    migrate_qmp(from, uri, "{'resume': true}");
+    migrate_qmp(from, to, uri, "{'resume': true}");
 
     /* Restore the postcopy bandwidth to unlimited */
     migrate_set_parameter_int(from, "max-postcopy-bandwidth", 0);
@@ -1669,7 +1669,7 @@ static void test_baddest(void)
     if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", &args)) {
         return;
     }
-    migrate_qmp(from, "tcp:127.0.0.1:0", "{}");
+    migrate_qmp(from, to, "tcp:127.0.0.1:0", "{}");
     wait_for_migration_fail(from, false);
     test_migrate_end(from, to, false);
 }
@@ -1708,7 +1708,7 @@ static void test_analyze_script(void)
     uri = g_strdup_printf("exec:cat > %s", file);
 
     migrate_ensure_converge(from);
-    migrate_qmp(from, uri, "{}");
+    migrate_qmp(from, to, uri, "{}");
     wait_for_migration_complete(from);
 
     pid = fork();
@@ -1777,7 +1777,7 @@ static void test_precopy_common(MigrateCommon *args)
         goto finish;
     }
 
-    migrate_qmp(from, connect_uri, "{}");
+    migrate_qmp(from, to, connect_uri, "{}");
 
     if (args->result != MIG_TEST_SUCCEED) {
         bool allow_active = args->result == MIG_TEST_FAIL;
@@ -1873,7 +1873,7 @@ static void test_file_common(MigrateCommon *args, bool stop_src)
         goto finish;
     }
 
-    migrate_qmp(from, connect_uri, "{}");
+    migrate_qmp(from, to, connect_uri, "{}");
     wait_for_migration_complete(from);
 
     /*
@@ -2029,7 +2029,7 @@ static void test_ignore_shared(void)
     /* Wait for the first serial output from the source */
     wait_for_serial("src_serial");
 
-    migrate_qmp(from, uri, "{}");
+    migrate_qmp(from, to, uri, "{}");
 
     migrate_wait_for_dirty_mem(from, to);
 
@@ -2605,7 +2605,7 @@ static void do_test_validate_uuid(MigrateStart *args, bool should_fail)
     /* Wait for the first serial output from the source */
     wait_for_serial("src_serial");
 
-    migrate_qmp(from, uri, "{}");
+    migrate_qmp(from, to, uri, "{}");
 
     if (should_fail) {
         qtest_set_expected_status(to, EXIT_FAILURE);
@@ -2708,7 +2708,7 @@ static void test_migrate_auto_converge(void)
     /* Wait for the first serial output from the source */
     wait_for_serial("src_serial");
 
-    migrate_qmp(from, uri, "{}");
+    migrate_qmp(from, to, uri, "{}");
 
     /* Wait for throttling begins */
     percentage = 0;
@@ -3029,7 +3029,7 @@ static void test_multifd_tcp_cancel(void)
 
     uri = migrate_get_socket_address(to, "socket-address");
 
-    migrate_qmp(from, uri, "{}");
+    migrate_qmp(from, to, uri, "{}");
 
     migrate_wait_for_dirty_mem(from, to);
 
@@ -3061,7 +3061,7 @@ static void test_multifd_tcp_cancel(void)
 
     migrate_ensure_non_converge(from);
 
-    migrate_qmp(from, uri, "{}");
+    migrate_qmp(from, to2, uri, "{}");
 
     migrate_wait_for_dirty_mem(from, to2);
 
@@ -3394,7 +3394,7 @@ static void test_migrate_dirty_limit(void)
     migrate_dirty_limit_wait_showup(from, dirtylimit_period, dirtylimit_value);
 
     /* Start migrate */
-    migrate_qmp(from, uri, "{}");
+    migrate_qmp(from, to, uri, "{}");
 
     /* Wait for dirty limit throttle begin */
     throttle_us_per_full = 0;
@@ -3435,7 +3435,7 @@ static void test_migrate_dirty_limit(void)
     }
 
     /* Start migrate */
-    migrate_qmp(from, uri, "{}");
+    migrate_qmp(from, to, uri, "{}");
 
     /* Wait for dirty limit throttle begin */
     throttle_us_per_full = 0;
-- 
2.22.3

[PATCH v4 2/8] Replace connect_uri and move migrate_get_socket_address inside migrate_qmp

[Het Gala]

Move the calls to migrate_get_socket_address() into migrate_qmp().
Get rid of connect_uri and replace it with args->connect_uri only
because 'to' object will help to generate connect_uri with the
correct port number.

Signed-off-by: Het Gala <het.gala@nutanix.com>
Suggested-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/migration-helpers.c | 55 ++++++++++++++++++++++-
 tests/qtest/migration-test.c    | 79 +++++----------------------------
 2 files changed, 64 insertions(+), 70 deletions(-)

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index b6206a04fb..9af3c7d4d5 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -13,6 +13,10 @@
 #include "qemu/osdep.h"
 #include "qemu/ctype.h"
 #include "qapi/qmp/qjson.h"
+#include "qemu/sockets.h"
+#include "qapi/qapi-visit-sockets.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/error.h"
 
 #include "migration-helpers.h"
 
@@ -24,6 +28,51 @@
  */
 #define MIGRATION_STATUS_WAIT_TIMEOUT 120
 
+static char *SocketAddress_to_str(SocketAddress *addr)
+{
+    switch (addr->type) {
+    case SOCKET_ADDRESS_TYPE_INET:
+        return g_strdup_printf("tcp:%s:%s",
+                               addr->u.inet.host,
+                               addr->u.inet.port);
+    case SOCKET_ADDRESS_TYPE_UNIX:
+        return g_strdup_printf("unix:%s",
+                               addr->u.q_unix.path);
+    case SOCKET_ADDRESS_TYPE_FD:
+        return g_strdup_printf("fd:%s", addr->u.fd.str);
+    case SOCKET_ADDRESS_TYPE_VSOCK:
+        return g_strdup_printf("tcp:%s:%s",
+                               addr->u.vsock.cid,
+                               addr->u.vsock.port);
+    default:
+        return g_strdup("unknown address type");
+    }
+}
+
+static char *
+migrate_get_socket_address(QTestState *who, const char *parameter)
+{
+    QDict *rsp;
+    char *result;
+    SocketAddressList *addrs;
+    Visitor *iv = NULL;
+    QObject *object;
+
+    rsp = migrate_query(who);
+    object = qdict_get(rsp, parameter);
+
+    iv = qobject_input_visitor_new(object);
+    visit_type_SocketAddressList(iv, NULL, &addrs, &error_abort);
+    visit_free(iv);
+
+    /* we are only using a single address */
+    result = SocketAddress_to_str(addrs->value);
+
+    qapi_free_SocketAddressList(addrs);
+    qobject_unref(rsp);
+    return result;
+}
+
 bool migrate_watch_for_events(QTestState *who, const char *name,
                               QDict *event, void *opaque)
 {
@@ -73,13 +122,17 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
 {
     va_list ap;
     QDict *args;
+    g_autofree char *connect_uri = NULL;
 
     va_start(ap, fmt);
     args = qdict_from_vjsonf_nofail(fmt, ap);
     va_end(ap);
 
     g_assert(!qdict_haskey(args, "uri"));
-    qdict_put_str(args, "uri", uri);
+    if (!uri) {
+        connect_uri = migrate_get_socket_address(to, "socket-address");
+    }
+    qdict_put_str(args, "uri", uri ? uri : connect_uri);
 
     qtest_qmp_assert_success(who,
                              "{ 'execute': 'migrate', 'arguments': %p}", args);
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index d9b4e28c12..01255e7e7e 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -369,50 +369,6 @@ static void cleanup(const char *filename)
     unlink(path);
 }
 
-static char *SocketAddress_to_str(SocketAddress *addr)
-{
-    switch (addr->type) {
-    case SOCKET_ADDRESS_TYPE_INET:
-        return g_strdup_printf("tcp:%s:%s",
-                               addr->u.inet.host,
-                               addr->u.inet.port);
-    case SOCKET_ADDRESS_TYPE_UNIX:
-        return g_strdup_printf("unix:%s",
-                               addr->u.q_unix.path);
-    case SOCKET_ADDRESS_TYPE_FD:
-        return g_strdup_printf("fd:%s", addr->u.fd.str);
-    case SOCKET_ADDRESS_TYPE_VSOCK:
-        return g_strdup_printf("tcp:%s:%s",
-                               addr->u.vsock.cid,
-                               addr->u.vsock.port);
-    default:
-        return g_strdup("unknown address type");
-    }
-}
-
-static char *migrate_get_socket_address(QTestState *who, const char *parameter)
-{
-    QDict *rsp;
-    char *result;
-    SocketAddressList *addrs;
-    Visitor *iv = NULL;
-    QObject *object;
-
-    rsp = migrate_query(who);
-    object = qdict_get(rsp, parameter);
-
-    iv = qobject_input_visitor_new(object);
-    visit_type_SocketAddressList(iv, NULL, &addrs, &error_abort);
-    visit_free(iv);
-
-    /* we are only using a single address */
-    result = SocketAddress_to_str(addrs->value);
-
-    qapi_free_SocketAddressList(addrs);
-    qobject_unref(rsp);
-    return result;
-}
-
 static long long migrate_get_parameter_int(QTestState *who,
                                            const char *parameter)
 {
@@ -1349,8 +1305,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
     wait_for_serial("src_serial");
     wait_for_suspend(from, &src_state);
 
-    g_autofree char *uri = migrate_get_socket_address(to, "socket-address");
-    migrate_qmp(from, to, uri, "{}");
+    migrate_qmp(from, to, NULL, "{}");
 
     migrate_wait_for_dirty_mem(from, to);
 
@@ -1733,7 +1688,6 @@ static void test_precopy_common(MigrateCommon *args)
 {
     QTestState *from, *to;
     void *data_hook = NULL;
-    g_autofree char *connect_uri = NULL;
 
     if (test_migrate_start(&from, &to, args->listen_uri, &args->start)) {
         return;
@@ -1766,18 +1720,12 @@ static void test_precopy_common(MigrateCommon *args)
         }
     }
 
-    if (!args->connect_uri) {
-        connect_uri = migrate_get_socket_address(to, "socket-address");
-    } else {
-        connect_uri = g_strdup(args->connect_uri);
-    }
-
     if (args->result == MIG_TEST_QMP_ERROR) {
-        migrate_qmp_fail(from, connect_uri, "{}");
+        migrate_qmp_fail(from, args->connect_uri, "{}");
         goto finish;
     }
 
-    migrate_qmp(from, to, connect_uri, "{}");
+    migrate_qmp(from, to, args->connect_uri, "{}");
 
     if (args->result != MIG_TEST_SUCCEED) {
         bool allow_active = args->result == MIG_TEST_FAIL;
@@ -1843,7 +1791,6 @@ static void test_file_common(MigrateCommon *args, bool stop_src)
 {
     QTestState *from, *to;
     void *data_hook = NULL;
-    g_autofree char *connect_uri = g_strdup(args->connect_uri);
 
     if (test_migrate_start(&from, &to, args->listen_uri, &args->start)) {
         return;
@@ -1869,18 +1816,18 @@ static void test_file_common(MigrateCommon *args, bool stop_src)
     }
 
     if (args->result == MIG_TEST_QMP_ERROR) {
-        migrate_qmp_fail(from, connect_uri, "{}");
+        migrate_qmp_fail(from, args->connect_uri, "{}");
         goto finish;
     }
 
-    migrate_qmp(from, to, connect_uri, "{}");
+    migrate_qmp(from, to, args->connect_uri, "{}");
     wait_for_migration_complete(from);
 
     /*
      * We need to wait for the source to finish before starting the
      * destination.
      */
-    migrate_incoming_qmp(to, connect_uri, "{}");
+    migrate_incoming_qmp(to, args->connect_uri, "{}");
     wait_for_migration_complete(to);
 
     if (stop_src) {
@@ -3006,7 +2953,6 @@ static void test_multifd_tcp_cancel(void)
         .hide_stderr = true,
     };
     QTestState *from, *to, *to2;
-    g_autofree char *uri = NULL;
 
     if (test_migrate_start(&from, &to, "defer", &args)) {
         return;
@@ -3027,9 +2973,7 @@ static void test_multifd_tcp_cancel(void)
     /* Wait for the first serial output from the source */
     wait_for_serial("src_serial");
 
-    uri = migrate_get_socket_address(to, "socket-address");
-
-    migrate_qmp(from, to, uri, "{}");
+    migrate_qmp(from, to, NULL, "{}");
 
     migrate_wait_for_dirty_mem(from, to);
 
@@ -3054,14 +2998,11 @@ static void test_multifd_tcp_cancel(void)
     /* Start incoming migration from the 1st socket */
     migrate_incoming_qmp(to2, "tcp:127.0.0.1:0", "{}");
 
-    g_free(uri);
-    uri = migrate_get_socket_address(to2, "socket-address");
-
     wait_for_migration_status(from, "cancelled", NULL);
 
     migrate_ensure_non_converge(from);
 
-    migrate_qmp(from, to2, uri, "{}");
+    migrate_qmp(from, to2, NULL, "{}");
 
     migrate_wait_for_dirty_mem(from, to2);
 
@@ -3394,7 +3335,7 @@ static void test_migrate_dirty_limit(void)
     migrate_dirty_limit_wait_showup(from, dirtylimit_period, dirtylimit_value);
 
     /* Start migrate */
-    migrate_qmp(from, to, uri, "{}");
+    migrate_qmp(from, to, args.connect_uri, "{}");
 
     /* Wait for dirty limit throttle begin */
     throttle_us_per_full = 0;
@@ -3435,7 +3376,7 @@ static void test_migrate_dirty_limit(void)
     }
 
     /* Start migrate */
-    migrate_qmp(from, to, uri, "{}");
+    migrate_qmp(from, to, args.connect_uri, "{}");
 
     /* Wait for dirty limit throttle begin */
     throttle_us_per_full = 0;
-- 
2.22.3

Re: [PATCH v4 2/8] Replace connect_uri and move migrate_get_socket_address inside migrate_qmp

[Fabiano Rosas]

Het Gala <het.gala@nutanix.com> writes:

> Move the calls to migrate_get_socket_address() into migrate_qmp().
> Get rid of connect_uri and replace it with args->connect_uri only
> because 'to' object will help to generate connect_uri with the
> correct port number.
>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> Suggested-by: Fabiano Rosas <farosas@suse.de>
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> ---
>  tests/qtest/migration-helpers.c | 55 ++++++++++++++++++++++-
>  tests/qtest/migration-test.c    | 79 +++++----------------------------
>  2 files changed, 64 insertions(+), 70 deletions(-)
>
> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
> index b6206a04fb..9af3c7d4d5 100644
> --- a/tests/qtest/migration-helpers.c
> +++ b/tests/qtest/migration-helpers.c
> @@ -13,6 +13,10 @@
>  #include "qemu/osdep.h"
>  #include "qemu/ctype.h"
>  #include "qapi/qmp/qjson.h"
> +#include "qemu/sockets.h"
> +#include "qapi/qapi-visit-sockets.h"
> +#include "qapi/qobject-input-visitor.h"
> +#include "qapi/error.h"

Are any of these now superfluous at migration-test.c?

Re: [PATCH v4 2/8] Replace connect_uri and move migrate_get_socket_address inside migrate_qmp

[Het Gala]

On 11/03/24 11:46 pm, Fabiano Rosas wrote:
> Het Gala <het.gala@nutanix.com> writes:
>
>> Move the calls to migrate_get_socket_address() into migrate_qmp().
>> Get rid of connect_uri and replace it with args->connect_uri only
>> because 'to' object will help to generate connect_uri with the
>> correct port number.
>>
>> Signed-off-by: Het Gala <het.gala@nutanix.com>
>> Suggested-by: Fabiano Rosas <farosas@suse.de>
>> Reviewed-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>   tests/qtest/migration-helpers.c | 55 ++++++++++++++++++++++-
>>   tests/qtest/migration-test.c    | 79 +++++----------------------------
>>   2 files changed, 64 insertions(+), 70 deletions(-)
>>
>> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
>> index b6206a04fb..9af3c7d4d5 100644
>> --- a/tests/qtest/migration-helpers.c
>> +++ b/tests/qtest/migration-helpers.c
>> @@ -13,6 +13,10 @@
>>   #include "qemu/osdep.h"
>>   #include "qemu/ctype.h"
>>   #include "qapi/qmp/qjson.h"
>> +#include "qemu/sockets.h"
>> +#include "qapi/qapi-visit-sockets.h"
>> +#include "qapi/qobject-input-visitor.h"
>> +#include "qapi/error.h"
> Are any of these now superfluous at migration-test.c?

Yess, actually all of them are now redundant or not required from the 
migration-test.c. Will remove all imports from there


Regards,

Het Gala

>

[PATCH v4 3/8] Replace migrate_get_connect_uri inplace of migrate_get_socket_address

[Het Gala]

migrate_get_socket_address implicitly converts SocketAddress into str.
Move migrate_get_socket_address inside migrate_get_connect_uri which
should return the uri string instead.

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

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 9af3c7d4d5..3c3fe9d8aa 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -49,12 +49,12 @@ static char *SocketAddress_to_str(SocketAddress *addr)
     }
 }
 
-static char *
+static SocketAddress *
 migrate_get_socket_address(QTestState *who, const char *parameter)
 {
     QDict *rsp;
-    char *result;
     SocketAddressList *addrs;
+    SocketAddress *addr;
     Visitor *iv = NULL;
     QObject *object;
 
@@ -63,14 +63,24 @@ migrate_get_socket_address(QTestState *who, const char *parameter)
 
     iv = qobject_input_visitor_new(object);
     visit_type_SocketAddressList(iv, NULL, &addrs, &error_abort);
+    addr = addrs->value;
     visit_free(iv);
 
-    /* we are only using a single address */
-    result = SocketAddress_to_str(addrs->value);
-
-    qapi_free_SocketAddressList(addrs);
     qobject_unref(rsp);
-    return result;
+    return addr;
+}
+
+static char *
+migrate_get_connect_uri(QTestState *who, const char *parameter)
+{
+    SocketAddress *addrs;
+    char *connect_uri;
+
+    addrs = migrate_get_socket_address(who, parameter);
+    connect_uri = SocketAddress_to_str(addrs);
+
+    qapi_free_SocketAddress(addrs);
+    return connect_uri;
 }
 
 bool migrate_watch_for_events(QTestState *who, const char *name,
@@ -130,7 +140,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
 
     g_assert(!qdict_haskey(args, "uri"));
     if (!uri) {
-        connect_uri = migrate_get_socket_address(to, "socket-address");
+        connect_uri = migrate_get_connect_uri(to, "socket-address");
     }
     qdict_put_str(args, "uri", uri ? uri : connect_uri);
 
-- 
2.22.3

Re: [PATCH v4 3/8] Replace migrate_get_connect_uri inplace of migrate_get_socket_address

[Fabiano Rosas]

Het Gala <het.gala@nutanix.com> writes:

> migrate_get_socket_address implicitly converts SocketAddress into str.
> Move migrate_get_socket_address inside migrate_get_connect_uri which
> should return the uri string instead.
>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> Suggested-by: Fabiano Rosas <farosas@suse.de>
> ---
>  tests/qtest/migration-helpers.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
> index 9af3c7d4d5..3c3fe9d8aa 100644
> --- a/tests/qtest/migration-helpers.c
> +++ b/tests/qtest/migration-helpers.c
> @@ -49,12 +49,12 @@ static char *SocketAddress_to_str(SocketAddress *addr)
>      }
>  }
>  
> -static char *
> +static SocketAddress *
>  migrate_get_socket_address(QTestState *who, const char *parameter)
>  {
>      QDict *rsp;
> -    char *result;
>      SocketAddressList *addrs;
> +    SocketAddress *addr;
>      Visitor *iv = NULL;
>      QObject *object;
>  
> @@ -63,14 +63,24 @@ migrate_get_socket_address(QTestState *who, const char *parameter)
>  
>      iv = qobject_input_visitor_new(object);
>      visit_type_SocketAddressList(iv, NULL, &addrs, &error_abort);
> +    addr = addrs->value;
>      visit_free(iv);
>  
> -    /* we are only using a single address */
> -    result = SocketAddress_to_str(addrs->value);
> -
> -    qapi_free_SocketAddressList(addrs);
>      qobject_unref(rsp);
> -    return result;
> +    return addr;
> +}
> +
> +static char *
> +migrate_get_connect_uri(QTestState *who, const char *parameter)
> +{
> +    SocketAddress *addrs;
> +    char *connect_uri;
> +
> +    addrs = migrate_get_socket_address(who, parameter);
> +    connect_uri = SocketAddress_to_str(addrs);
> +
> +    qapi_free_SocketAddress(addrs);
> +    return connect_uri;
>  }
>  
>  bool migrate_watch_for_events(QTestState *who, const char *name,
> @@ -130,7 +140,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
>  
>      g_assert(!qdict_haskey(args, "uri"));
>      if (!uri) {
> -        connect_uri = migrate_get_socket_address(to, "socket-address");
> +        connect_uri = migrate_get_connect_uri(to, "socket-address");

What's the point of the "socket-address" argument here? Seems a bit
nonsensical to me to call: migrate_get_socket_address(..., "socket-address").

What about we just suppress this throughout the stack and directly call:

    object = qdict_get(rsp, "socket-address");

Re: [PATCH v4 3/8] Replace migrate_get_connect_uri inplace of migrate_get_socket_address

[Het Gala]

[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]


On 11/03/24 11:49 pm, Fabiano Rosas wrote:
> Het Gala<het.gala@nutanix.com>  writes:
>
>>
>>   bool migrate_watch_for_events(QTestState *who, const char *name,
>> @@ -130,7 +140,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
>>   
>>       g_assert(!qdict_haskey(args, "uri"));
>>       if (!uri) {
>> -        connect_uri = migrate_get_socket_address(to, "socket-address");
>> +        connect_uri = migrate_get_connect_uri(to, "socket-address");
> What's the point of the "socket-address" argument here? Seems a bit
> nonsensical to me to call: migrate_get_socket_address(..., "socket-address").
>
> What about we just suppress this throughout the stack and directly call:
>
>      object = qdict_get(rsp, "socket-address");

Fabiano, I didn't get clearly understand your point here. From what I 
understand,
you want to call just
1. migrate_get_connect_uri(to) and migrate_get_connect_qdict(to)
2. delete migrate_get_socket_address(..., "socket-address") altogether 
3. Just call qdict_get(rsp, "socket-address") which will return an 
object 4. Then convert this object into qdict and uri string respectively ?

Hmm, If that's the case, converting to qdict shouldn't be a problem. But 
for uri string is there a simpler method or writing a parsing function 
would be needed ?


Regards,

Het Gala

[-- Attachment #2: Type: text/html, Size: 2321 bytes --]

Re: [PATCH v4 3/8] Replace migrate_get_connect_uri inplace of migrate_get_socket_address

[Fabiano Rosas]

Het Gala <het.gala@nutanix.com> writes:

> On 11/03/24 11:49 pm, Fabiano Rosas wrote:
>> Het Gala<het.gala@nutanix.com>  writes:
>>
>>>
>>>   bool migrate_watch_for_events(QTestState *who, const char *name,
>>> @@ -130,7 +140,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
>>>   
>>>       g_assert(!qdict_haskey(args, "uri"));
>>>       if (!uri) {
>>> -        connect_uri = migrate_get_socket_address(to, "socket-address");
>>> +        connect_uri = migrate_get_connect_uri(to, "socket-address");
>> What's the point of the "socket-address" argument here? Seems a bit
>> nonsensical to me to call: migrate_get_socket_address(..., "socket-address").
>>
>> What about we just suppress this throughout the stack and directly call:
>>
>>      object = qdict_get(rsp, "socket-address");
>
> Fabiano, I didn't get clearly understand your point here. From what I 
> understand,
> you want to call just
> 1. migrate_get_connect_uri(to) and migrate_get_connect_qdict(to)

Yes.

> 2. delete migrate_get_socket_address(..., "socket-address") altogether 

No, just the string argument, not the whole function:

static char *migrate_get_socket_address(QTestState *who) <----
{
    QDict *rsp;
    char *result;
    SocketAddressList *addrs;
    Visitor *iv = NULL;
    QObject *object;

    rsp = migrate_query(who);
    object = qdict_get(rsp, "socket-address"); <-----
    ...
}

If the thing is called migrate_get_SOCKET_ADDRESS(), it's obvious that
the "socket-address" is the parameter we want. We even call
SocketAddress_to_str, so there's no point in having that argument
there. We will never call the function with something else in
'parameter'.

Re: [PATCH v4 3/8] Replace migrate_get_connect_uri inplace of migrate_get_socket_address

[Het Gala]

[-- Attachment #1: Type: text/plain, Size: 1962 bytes --]


On 12/03/24 2:21 am, Fabiano Rosas wrote:
> Het Gala<het.gala@nutanix.com>  writes:
>
>> On 11/03/24 11:49 pm, Fabiano Rosas wrote:
>>> Het Gala<het.gala@nutanix.com>   writes:
>>>
>>>>    bool migrate_watch_for_events(QTestState *who, const char *name,
>>>> @@ -130,7 +140,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
>>>>    
>>>>        g_assert(!qdict_haskey(args, "uri"));
>>>>        if (!uri) {
>>>> -        connect_uri = migrate_get_socket_address(to, "socket-address");
>>>> +        connect_uri = migrate_get_connect_uri(to, "socket-address");
>>> What's the point of the "socket-address" argument here? Seems a bit
>>> nonsensical to me to call: migrate_get_socket_address(..., "socket-address").
>>>
>>> What about we just suppress this throughout the stack and directly call:
>>>
>>>       object = qdict_get(rsp, "socket-address");
>> Fabiano, I didn't get clearly understand your point here. From what I
>> understand,
>> you want to call just
>> 1. migrate_get_connect_uri(to) and migrate_get_connect_qdict(to)
> Yes.
Ack
>> 2. delete migrate_get_socket_address(..., "socket-address") altogether
> No, just the string argument, not the whole function:
>
> static char *migrate_get_socket_address(QTestState *who) <----
> {
>      QDict *rsp;
>      char *result;
>      SocketAddressList *addrs;
>      Visitor *iv = NULL;
>      QObject *object;
>
>      rsp = migrate_query(who);
>      object = qdict_get(rsp, "socket-address"); <-----
>      ...
> }
>
> If the thing is called migrate_get_SOCKET_ADDRESS(), it's obvious that
> the "socket-address" is the parameter we want. We even call
> SocketAddress_to_str, so there's no point in having that argument
> there. We will never call the function with something else in
> 'parameter'.
Ahh, okay. I got your point, and yes, it makes sense. Will just call
migrate_get_socket_address(to) and let the qdict_get() call 
"socket-address" internally.

Regards,

Het Gala

[-- Attachment #2: Type: text/html, Size: 3599 bytes --]

[PATCH v4 4/8] Add channels parameter in migrate_qmp_fail

[Het Gala]

Alter migrate_qmp_fail() to allow both uri and channels
independently. For channels, convert string to a Dict.
No dealing with migrate_get_socket_address() here because
we will fail before starting the migration anyway.

Signed-off-by: Het Gala <het.gala@nutanix.com>
Suggested-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/migration-helpers.c | 13 +++++++++++--
 tests/qtest/migration-helpers.h |  5 +++--
 tests/qtest/migration-test.c    |  4 ++--
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 3c3fe9d8aa..91c8a817d2 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -102,7 +102,8 @@ bool migrate_watch_for_events(QTestState *who, const char *name,
     return false;
 }
 
-void migrate_qmp_fail(QTestState *who, const char *uri, const char *fmt, ...)
+void migrate_qmp_fail(QTestState *who, const char *uri,
+                      const char *channels, const char *fmt, ...)
 {
     va_list ap;
     QDict *args, *err;
@@ -112,7 +113,15 @@ void migrate_qmp_fail(QTestState *who, const char *uri, const char *fmt, ...)
     va_end(ap);
 
     g_assert(!qdict_haskey(args, "uri"));
-    qdict_put_str(args, "uri", uri);
+    if (uri) {
+        qdict_put_str(args, "uri", uri);
+    }
+
+    g_assert(!qdict_haskey(args, "channels"));
+    if (channels) {
+        QObject *channels_obj = qobject_from_json(channels, &error_abort);
+        qdict_put_obj(args, "channels", channels_obj);
+    }
 
     err = qtest_qmp_assert_failure_ref(
         who, "{ 'execute': 'migrate', 'arguments': %p}", args);
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index e16a34c796..4e664148a5 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -33,8 +33,9 @@ G_GNUC_PRINTF(3, 4)
 void migrate_incoming_qmp(QTestState *who, const char *uri,
                           const char *fmt, ...);
 
-G_GNUC_PRINTF(3, 4)
-void migrate_qmp_fail(QTestState *who, const char *uri, const char *fmt, ...);
+G_GNUC_PRINTF(4, 5)
+void migrate_qmp_fail(QTestState *who, const char *uri,
+                      const char *channels, const char *fmt, ...);
 
 void migrate_set_capability(QTestState *who, const char *capability,
                             bool value);
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 01255e7e7e..61aa53c3f7 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1721,7 +1721,7 @@ static void test_precopy_common(MigrateCommon *args)
     }
 
     if (args->result == MIG_TEST_QMP_ERROR) {
-        migrate_qmp_fail(from, args->connect_uri, "{}");
+        migrate_qmp_fail(from, args->connect_uri, NULL, "{}");
         goto finish;
     }
 
@@ -1816,7 +1816,7 @@ static void test_file_common(MigrateCommon *args, bool stop_src)
     }
 
     if (args->result == MIG_TEST_QMP_ERROR) {
-        migrate_qmp_fail(from, args->connect_uri, "{}");
+        migrate_qmp_fail(from, args->connect_uri, NULL, "{}");
         goto finish;
     }
 
-- 
2.22.3

Re: [PATCH v4 4/8] Add channels parameter in migrate_qmp_fail

[Fabiano Rosas]

Het Gala <het.gala@nutanix.com> writes:

> Alter migrate_qmp_fail() to allow both uri and channels
> independently. For channels, convert string to a Dict.
> No dealing with migrate_get_socket_address() here because
> we will fail before starting the migration anyway.
>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> Suggested-by: Fabiano Rosas <farosas@suse.de>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

[PATCH v4 5/8] Add migrate_set_ports into migrate_qmp to update migration port value

[Het Gala]

migrate_set_get_qdict gets qdict with the dst QEMU parameters
migrate_set_ports() from list of channels reads each QDict for port,
and fills the port with correct value in case it was 0 in the test.

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

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 91c8a817d2..7c17d78d6b 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -17,6 +17,8 @@
 #include "qapi/qapi-visit-sockets.h"
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/error.h"
+#include "qapi/qmp/qlist.h"
+#include "include/qemu/cutils.h"
 
 #include "migration-helpers.h"
 
@@ -49,6 +51,37 @@ static char *SocketAddress_to_str(SocketAddress *addr)
     }
 }
 
+static QDict *SocketAddress_to_qdict(SocketAddress *addr)
+{
+    QDict *dict = qdict_new();
+
+    switch (addr->type) {
+    case SOCKET_ADDRESS_TYPE_INET:
+        qdict_put_str(dict, "type", "inet");
+        qdict_put_str(dict, "host", addr->u.inet.host);
+        qdict_put_str(dict, "port", addr->u.inet.port);
+        break;
+    case SOCKET_ADDRESS_TYPE_UNIX:
+        qdict_put_str(dict, "type", "unix");
+        qdict_put_str(dict, "path", addr->u.q_unix.path);
+        break;
+    case SOCKET_ADDRESS_TYPE_FD:
+        qdict_put_str(dict, "type", "fd");
+        qdict_put_str(dict, "str", addr->u.fd.str);
+        break;
+    case SOCKET_ADDRESS_TYPE_VSOCK:
+        qdict_put_str(dict, "type", "vsock");
+        qdict_put_str(dict, "cid", addr->u.vsock.cid);
+        qdict_put_str(dict, "port", addr->u.vsock.port);
+        break;
+    default:
+        g_assert_not_reached();
+        break;
+    }
+
+    return dict;
+}
+
 static SocketAddress *
 migrate_get_socket_address(QTestState *who, const char *parameter)
 {
@@ -83,6 +116,44 @@ migrate_get_connect_uri(QTestState *who, const char *parameter)
     return connect_uri;
 }
 
+static QDict *
+migrate_get_connect_qdict(QTestState *who, const char *parameter)
+{
+    SocketAddress *addrs;
+    QDict *connect_qdict;
+
+    addrs = migrate_get_socket_address(who, parameter);
+    connect_qdict = SocketAddress_to_qdict(addrs);
+
+    qapi_free_SocketAddress(addrs);
+    return connect_qdict;
+}
+
+static void migrate_set_ports(QTestState *to, QList *channel_list)
+{
+    QDict *addr;
+    QListEntry *entry;
+    g_autofree const char *addr_port = NULL;
+
+    if (channel_list == NULL) {
+        return;
+    }
+
+    addr = migrate_get_connect_qdict(to, "socket-address");
+
+    QLIST_FOREACH_ENTRY(channel_list, entry) {
+        QDict *channel = qobject_to(QDict, qlist_entry_obj(entry));
+        QDict *addrdict = qdict_get_qdict(channel, "addr");
+
+        if (qdict_haskey(addrdict, "port") &&
+            qdict_haskey(addr, "port") &&
+            (strcmp(qdict_get_str(addrdict, "port"), "0") == 0)) {
+                addr_port = qdict_get_str(addr, "port");
+                qdict_put_str(addrdict, "port", addr_port);
+        }
+    }
+}
+
 bool migrate_watch_for_events(QTestState *who, const char *name,
                               QDict *event, void *opaque)
 {
@@ -141,6 +212,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
 {
     va_list ap;
     QDict *args;
+    QList *channel_list = NULL;
     g_autofree char *connect_uri = NULL;
 
     va_start(ap, fmt);
@@ -151,6 +223,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
     if (!uri) {
         connect_uri = migrate_get_connect_uri(to, "socket-address");
     }
+    migrate_set_ports(to, channel_list);
     qdict_put_str(args, "uri", uri ? uri : connect_uri);
 
     qtest_qmp_assert_success(who,
-- 
2.22.3

Re: [PATCH v4 5/8] Add migrate_set_ports into migrate_qmp to update migration port value

[Fabiano Rosas]

Het Gala <het.gala@nutanix.com> writes:

> migrate_set_get_qdict gets qdict with the dst QEMU parameters

s/set_//

> migrate_set_ports() from list of channels reads each QDict for port,
> and fills the port with correct value in case it was 0 in the test.
>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> Suggested-by: Fabiano Rosas <farosas@suse.de>
> ---
>  tests/qtest/migration-helpers.c | 73 +++++++++++++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
>
> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
> index 91c8a817d2..7c17d78d6b 100644
> --- a/tests/qtest/migration-helpers.c
> +++ b/tests/qtest/migration-helpers.c
> @@ -17,6 +17,8 @@
>  #include "qapi/qapi-visit-sockets.h"
>  #include "qapi/qobject-input-visitor.h"
>  #include "qapi/error.h"
> +#include "qapi/qmp/qlist.h"
> +#include "include/qemu/cutils.h"

Extra "include/" here?

>  
>  #include "migration-helpers.h"
>  
> @@ -49,6 +51,37 @@ static char *SocketAddress_to_str(SocketAddress *addr)
>      }
>  }
>  
> +static QDict *SocketAddress_to_qdict(SocketAddress *addr)
> +{
> +    QDict *dict = qdict_new();
> +
> +    switch (addr->type) {
> +    case SOCKET_ADDRESS_TYPE_INET:
> +        qdict_put_str(dict, "type", "inet");
> +        qdict_put_str(dict, "host", addr->u.inet.host);
> +        qdict_put_str(dict, "port", addr->u.inet.port);
> +        break;
> +    case SOCKET_ADDRESS_TYPE_UNIX:
> +        qdict_put_str(dict, "type", "unix");
> +        qdict_put_str(dict, "path", addr->u.q_unix.path);
> +        break;
> +    case SOCKET_ADDRESS_TYPE_FD:
> +        qdict_put_str(dict, "type", "fd");
> +        qdict_put_str(dict, "str", addr->u.fd.str);
> +        break;
> +    case SOCKET_ADDRESS_TYPE_VSOCK:
> +        qdict_put_str(dict, "type", "vsock");
> +        qdict_put_str(dict, "cid", addr->u.vsock.cid);
> +        qdict_put_str(dict, "port", addr->u.vsock.port);
> +        break;
> +    default:
> +        g_assert_not_reached();
> +        break;
> +    }
> +
> +    return dict;
> +}
> +
>  static SocketAddress *
>  migrate_get_socket_address(QTestState *who, const char *parameter)
>  {
> @@ -83,6 +116,44 @@ migrate_get_connect_uri(QTestState *who, const char *parameter)
>      return connect_uri;
>  }
>  
> +static QDict *
> +migrate_get_connect_qdict(QTestState *who, const char *parameter)
> +{
> +    SocketAddress *addrs;
> +    QDict *connect_qdict;
> +
> +    addrs = migrate_get_socket_address(who, parameter);
> +    connect_qdict = SocketAddress_to_qdict(addrs);
> +
> +    qapi_free_SocketAddress(addrs);
> +    return connect_qdict;
> +}
> +
> +static void migrate_set_ports(QTestState *to, QList *channel_list)
> +{
> +    QDict *addr;
> +    QListEntry *entry;
> +    g_autofree const char *addr_port = NULL;
> +
> +    if (channel_list == NULL) {
> +        return;
> +    }
> +
> +    addr = migrate_get_connect_qdict(to, "socket-address");

addr needs to be freed.

> +
> +    QLIST_FOREACH_ENTRY(channel_list, entry) {
> +        QDict *channel = qobject_to(QDict, qlist_entry_obj(entry));
> +        QDict *addrdict = qdict_get_qdict(channel, "addr");
> +
> +        if (qdict_haskey(addrdict, "port") &&
> +            qdict_haskey(addr, "port") &&
> +            (strcmp(qdict_get_str(addrdict, "port"), "0") == 0)) {
> +                addr_port = qdict_get_str(addr, "port");
> +                qdict_put_str(addrdict, "port", addr_port);
> +        }
> +    }
> +}
> +
>  bool migrate_watch_for_events(QTestState *who, const char *name,
>                                QDict *event, void *opaque)
>  {
> @@ -141,6 +212,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
>  {
>      va_list ap;
>      QDict *args;
> +    QList *channel_list = NULL;
>      g_autofree char *connect_uri = NULL;
>  
>      va_start(ap, fmt);
> @@ -151,6 +223,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
>      if (!uri) {
>          connect_uri = migrate_get_connect_uri(to, "socket-address");
>      }
> +    migrate_set_ports(to, channel_list);
>      qdict_put_str(args, "uri", uri ? uri : connect_uri);
>  
>      qtest_qmp_assert_success(who,

Re: [PATCH v4 5/8] Add migrate_set_ports into migrate_qmp to update migration port value

[Het Gala]

[-- Attachment #1: Type: text/plain, Size: 4419 bytes --]


On 12/03/24 12:12 am, Fabiano Rosas wrote:
> Het Gala<het.gala@nutanix.com>  writes:
>
>> migrate_set_get_qdict gets qdict with the dst QEMU parameters
> s/set_//
Ack
>> migrate_set_ports() from list of channels reads each QDict for port,
>> and fills the port with correct value in case it was 0 in the test.
>>
>> Signed-off-by: Het Gala<het.gala@nutanix.com>
>> Suggested-by: Fabiano Rosas<farosas@suse.de>
>> ---
>>   tests/qtest/migration-helpers.c | 73 +++++++++++++++++++++++++++++++++
>>   1 file changed, 73 insertions(+)
>>
>> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
>> index 91c8a817d2..7c17d78d6b 100644
>> --- a/tests/qtest/migration-helpers.c
>> +++ b/tests/qtest/migration-helpers.c
>> @@ -17,6 +17,8 @@
>>   #include "qapi/qapi-visit-sockets.h"
>>   #include "qapi/qobject-input-visitor.h"
>>   #include "qapi/error.h"
>> +#include "qapi/qmp/qlist.h"
>> +#include "include/qemu/cutils.h"
> Extra "include/" here?
Ack
>>   
>>   #include "migration-helpers.h"
>>   
>> @@ -49,6 +51,37 @@ static char *SocketAddress_to_str(SocketAddress *addr)
>>       }
>>   }
>>   
>> +static QDict *SocketAddress_to_qdict(SocketAddress *addr)
>> +{
>> +    QDict *dict = qdict_new();
>> +
>> +    switch (addr->type) {
>> +    case SOCKET_ADDRESS_TYPE_INET:
>> +        qdict_put_str(dict, "type", "inet");
>> +        qdict_put_str(dict, "host", addr->u.inet.host);
>> +        qdict_put_str(dict, "port", addr->u.inet.port);
>> +        break;
>> +    case SOCKET_ADDRESS_TYPE_UNIX:
>> +        qdict_put_str(dict, "type", "unix");
>> +        qdict_put_str(dict, "path", addr->u.q_unix.path);
>> +        break;
>> +    case SOCKET_ADDRESS_TYPE_FD:
>> +        qdict_put_str(dict, "type", "fd");
>> +        qdict_put_str(dict, "str", addr->u.fd.str);
>> +        break;
>> +    case SOCKET_ADDRESS_TYPE_VSOCK:
>> +        qdict_put_str(dict, "type", "vsock");
>> +        qdict_put_str(dict, "cid", addr->u.vsock.cid);
>> +        qdict_put_str(dict, "port", addr->u.vsock.port);
>> +        break;
>> +    default:
>> +        g_assert_not_reached();
>> +        break;
>> +    }
>> +
>> +    return dict;
>> +}
>> +
>>   static SocketAddress *
>>   migrate_get_socket_address(QTestState *who, const char *parameter)
>>   {
>> @@ -83,6 +116,44 @@ migrate_get_connect_uri(QTestState *who, const char *parameter)
>>       return connect_uri;
>>   }
>>   
>> +static QDict *
>> +migrate_get_connect_qdict(QTestState *who, const char *parameter)
>> +{
>> +    SocketAddress *addrs;
>> +    QDict *connect_qdict;
>> +
>> +    addrs = migrate_get_socket_address(who, parameter);
>> +    connect_qdict = SocketAddress_to_qdict(addrs);
>> +
>> +    qapi_free_SocketAddress(addrs);
>> +    return connect_qdict;
>> +}
>> +
>> +static void migrate_set_ports(QTestState *to, QList *channel_list)
>> +{
>> +    QDict *addr;
>> +    QListEntry *entry;
>> +    g_autofree const char *addr_port = NULL;
>> +
>> +    if (channel_list == NULL) {
>> +        return;
>> +    }
>> +
>> +    addr = migrate_get_connect_qdict(to, "socket-address");
> addr needs to be freed.
Ack. Thanks for pointing this out
>> +
>> +    QLIST_FOREACH_ENTRY(channel_list, entry) {
>> +        QDict *channel = qobject_to(QDict, qlist_entry_obj(entry));
>> +        QDict *addrdict = qdict_get_qdict(channel, "addr");
>> +
>> +        if (qdict_haskey(addrdict, "port") &&
>> +            qdict_haskey(addr, "port") &&
>> +            (strcmp(qdict_get_str(addrdict, "port"), "0") == 0)) {
>> +                addr_port = qdict_get_str(addr, "port");
>> +                qdict_put_str(addrdict, "port", addr_port);
>> +        }
>> +    }
>> +}
>> +
>>   bool migrate_watch_for_events(QTestState *who, const char *name,
>>                                 QDict *event, void *opaque)
>>   {
>> @@ -141,6 +212,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
>>   {
>>       va_list ap;
>>       QDict *args;
>> +    QList *channel_list = NULL;
>>       g_autofree char *connect_uri = NULL;
>>   
>>       va_start(ap, fmt);
>> @@ -151,6 +223,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
>>       if (!uri) {
>>           connect_uri = migrate_get_connect_uri(to, "socket-address");
>>       }
>> +    migrate_set_ports(to, channel_list);
>>       qdict_put_str(args, "uri", uri ? uri : connect_uri);
>>   
>>       qtest_qmp_assert_success(who,
Regards,
Het Gala

[-- Attachment #2: Type: text/html, Size: 6002 bytes --]

[PATCH v4 6/8] Add channels parameter in migrate_qmp

[Het Gala]

Alter migrate_qmp() to allow use of channels parameter, but only
fill the uri with correct port number if there are no channels.
Here we don't want to allow the wrong cases of having both or
none (ex: migrate_qmp_fail).

Signed-off-by: Het Gala <het.gala@nutanix.com>
Suggested-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/migration-helpers.c | 22 +++++++++++++---------
 tests/qtest/migration-helpers.h |  4 ++--
 tests/qtest/migration-test.c    | 28 ++++++++++++++--------------
 3 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 7c17d78d6b..bf9fd61035 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -135,10 +135,6 @@ static void migrate_set_ports(QTestState *to, QList *channel_list)
     QListEntry *entry;
     g_autofree const char *addr_port = NULL;
 
-    if (channel_list == NULL) {
-        return;
-    }
-
     addr = migrate_get_connect_qdict(to, "socket-address");
 
     QLIST_FOREACH_ENTRY(channel_list, entry) {
@@ -208,11 +204,10 @@ void migrate_qmp_fail(QTestState *who, const char *uri,
  * qobject_from_jsonf_nofail()) with "uri": @uri spliced in.
  */
 void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
-                 const char *fmt, ...)
+                 const char *channels, const char *fmt, ...)
 {
     va_list ap;
     QDict *args;
-    QList *channel_list = NULL;
     g_autofree char *connect_uri = NULL;
 
     va_start(ap, fmt);
@@ -220,11 +215,20 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
     va_end(ap);
 
     g_assert(!qdict_haskey(args, "uri"));
-    if (!uri) {
+    if (uri) {
+        qdict_put_str(args, "uri", uri);
+    } else if (!channels) {
         connect_uri = migrate_get_connect_uri(to, "socket-address");
+        qdict_put_str(args, "uri", connect_uri);
+    }
+
+    g_assert(!qdict_haskey(args, "channels"));
+    if (channels) {
+        QObject *channels_obj = qobject_from_json(channels, &error_abort);
+        QList *channel_list = qobject_to(QList, channels_obj);
+        migrate_set_ports(to, channel_list);
+        qdict_put_obj(args, "channels", channels_obj);
     }
-    migrate_set_ports(to, channel_list);
-    qdict_put_str(args, "uri", uri ? uri : connect_uri);
 
     qtest_qmp_assert_success(who,
                              "{ 'execute': 'migrate', 'arguments': %p}", args);
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index 4e664148a5..1339835698 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -25,9 +25,9 @@ typedef struct QTestMigrationState {
 bool migrate_watch_for_events(QTestState *who, const char *name,
                               QDict *event, void *opaque);
 
-G_GNUC_PRINTF(4, 5)
+G_GNUC_PRINTF(5, 6)
 void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
-                 const char *fmt, ...);
+                 const char *channels, const char *fmt, ...);
 
 G_GNUC_PRINTF(3, 4)
 void migrate_incoming_qmp(QTestState *who, const char *uri,
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 61aa53c3f7..b1e5660dbf 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1305,7 +1305,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
     wait_for_serial("src_serial");
     wait_for_suspend(from, &src_state);
 
-    migrate_qmp(from, to, NULL, "{}");
+    migrate_qmp(from, to, NULL, NULL, "{}");
 
     migrate_wait_for_dirty_mem(from, to);
 
@@ -1455,7 +1455,7 @@ static void postcopy_recover_fail(QTestState *from, QTestState *to)
     g_assert_cmpint(ret, ==, 1);
 
     migrate_recover(to, "fd:fd-mig");
-    migrate_qmp(from, to, "fd:fd-mig", "{'resume': true}");
+    migrate_qmp(from, to, "fd:fd-mig", NULL, "{'resume': true}");
 
     /*
      * Make sure both QEMU instances will go into RECOVER stage, then test
@@ -1543,7 +1543,7 @@ static void test_postcopy_recovery_common(MigrateCommon *args)
      * Try to rebuild the migration channel using the resume flag and
      * the newly created channel
      */
-    migrate_qmp(from, to, uri, "{'resume': true}");
+    migrate_qmp(from, to, uri, NULL, "{'resume': true}");
 
     /* Restore the postcopy bandwidth to unlimited */
     migrate_set_parameter_int(from, "max-postcopy-bandwidth", 0);
@@ -1624,7 +1624,7 @@ static void test_baddest(void)
     if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", &args)) {
         return;
     }
-    migrate_qmp(from, to, "tcp:127.0.0.1:0", "{}");
+    migrate_qmp(from, to, "tcp:127.0.0.1:0", NULL, "{}");
     wait_for_migration_fail(from, false);
     test_migrate_end(from, to, false);
 }
@@ -1663,7 +1663,7 @@ static void test_analyze_script(void)
     uri = g_strdup_printf("exec:cat > %s", file);
 
     migrate_ensure_converge(from);
-    migrate_qmp(from, to, uri, "{}");
+    migrate_qmp(from, to, uri, NULL, "{}");
     wait_for_migration_complete(from);
 
     pid = fork();
@@ -1725,7 +1725,7 @@ static void test_precopy_common(MigrateCommon *args)
         goto finish;
     }
 
-    migrate_qmp(from, to, args->connect_uri, "{}");
+    migrate_qmp(from, to, args->connect_uri, NULL, "{}");
 
     if (args->result != MIG_TEST_SUCCEED) {
         bool allow_active = args->result == MIG_TEST_FAIL;
@@ -1820,7 +1820,7 @@ static void test_file_common(MigrateCommon *args, bool stop_src)
         goto finish;
     }
 
-    migrate_qmp(from, to, args->connect_uri, "{}");
+    migrate_qmp(from, to, args->connect_uri, NULL, "{}");
     wait_for_migration_complete(from);
 
     /*
@@ -1976,7 +1976,7 @@ static void test_ignore_shared(void)
     /* Wait for the first serial output from the source */
     wait_for_serial("src_serial");
 
-    migrate_qmp(from, to, uri, "{}");
+    migrate_qmp(from, to, uri, NULL, "{}");
 
     migrate_wait_for_dirty_mem(from, to);
 
@@ -2552,7 +2552,7 @@ static void do_test_validate_uuid(MigrateStart *args, bool should_fail)
     /* Wait for the first serial output from the source */
     wait_for_serial("src_serial");
 
-    migrate_qmp(from, to, uri, "{}");
+    migrate_qmp(from, to, uri, NULL, "{}");
 
     if (should_fail) {
         qtest_set_expected_status(to, EXIT_FAILURE);
@@ -2655,7 +2655,7 @@ static void test_migrate_auto_converge(void)
     /* Wait for the first serial output from the source */
     wait_for_serial("src_serial");
 
-    migrate_qmp(from, to, uri, "{}");
+    migrate_qmp(from, to, uri, NULL, "{}");
 
     /* Wait for throttling begins */
     percentage = 0;
@@ -2973,7 +2973,7 @@ static void test_multifd_tcp_cancel(void)
     /* Wait for the first serial output from the source */
     wait_for_serial("src_serial");
 
-    migrate_qmp(from, to, NULL, "{}");
+    migrate_qmp(from, to, NULL, NULL, "{}");
 
     migrate_wait_for_dirty_mem(from, to);
 
@@ -3002,7 +3002,7 @@ static void test_multifd_tcp_cancel(void)
 
     migrate_ensure_non_converge(from);
 
-    migrate_qmp(from, to2, NULL, "{}");
+    migrate_qmp(from, to2, NULL, NULL, "{}");
 
     migrate_wait_for_dirty_mem(from, to2);
 
@@ -3335,7 +3335,7 @@ static void test_migrate_dirty_limit(void)
     migrate_dirty_limit_wait_showup(from, dirtylimit_period, dirtylimit_value);
 
     /* Start migrate */
-    migrate_qmp(from, to, args.connect_uri, "{}");
+    migrate_qmp(from, to, args.connect_uri, NULL, "{}");
 
     /* Wait for dirty limit throttle begin */
     throttle_us_per_full = 0;
@@ -3376,7 +3376,7 @@ static void test_migrate_dirty_limit(void)
     }
 
     /* Start migrate */
-    migrate_qmp(from, to, args.connect_uri, "{}");
+    migrate_qmp(from, to, args.connect_uri, NULL, "{}");
 
     /* Wait for dirty limit throttle begin */
     throttle_us_per_full = 0;
-- 
2.22.3

Re: [PATCH v4 6/8] Add channels parameter in migrate_qmp

[Fabiano Rosas]

Het Gala <het.gala@nutanix.com> writes:

> Alter migrate_qmp() to allow use of channels parameter, but only
> fill the uri with correct port number if there are no channels.
> Here we don't want to allow the wrong cases of having both or
> none (ex: migrate_qmp_fail).
>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> Suggested-by: Fabiano Rosas <farosas@suse.de>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

[PATCH v4 7/8] Add multifd_tcp_plain test using list of channels instead of uri

[Het Gala]

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 | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index b1e5660dbf..6ba3cfd1e4 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -659,6 +659,13 @@ typedef struct {
      */
     const char *connect_uri;
 
+    /*
+     * Optional: JSON-formatted list of src QEMU URIs. If a port is
+     * defined as '0' in any QDict key a value of '0' will be
+     * automatically converted to the correct destination port.
+     */
+    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 */
@@ -2744,7 +2751,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",
@@ -2759,6 +2766,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 = {
@@ -3668,8 +3690,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 v4 7/8] Add multifd_tcp_plain test using list of channels instead of uri

[Fabiano Rosas]

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>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

[PATCH v4 8/8] Add negative tests to validate migration QAPIs

[Het Gala]

Migration QAPI arguments - uri and channels are mutually exhaustive.
Add negative validation tests, one with both arguments present and
one with none present.

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

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 6ba3cfd1e4..385f696a3d 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2612,6 +2612,56 @@ static void test_validate_uuid_dst_not_set(void)
     do_test_validate_uuid(&args, false);
 }
 
+static void do_test_validate_uri_channel(MigrateCommon *args)
+{
+    QTestState *from, *to;
+    g_autofree char *connect_uri = NULL;
+
+    if (test_migrate_start(&from, &to, args->listen_uri, &args->start)) {
+        return;
+    }
+
+    /* Wait for the first serial output from the source */
+    wait_for_serial("src_serial");
+
+    /*
+     * 'uri' and 'channels' validation is checked even before the migration
+     * starts.
+     */
+    migrate_qmp_fail(from, args->connect_uri, args->connect_channels, "{}");
+    test_migrate_end(from, to, false);
+}
+
+static void test_validate_uri_channels_both_set(void)
+{
+    MigrateCommon args = {
+        .start = {
+            .hide_stderr = true,
+        },
+        .listen_uri = "defer",
+        .connect_uri = "tcp:127.0.0.1:0",
+        .connect_channels = "[ { 'channel-type': 'main',"
+                            "    'addr': { 'transport': 'socket',"
+                            "              'type': 'inet',"
+                            "              'host': '127.0.0.1',"
+                            "              'port': '0' } } ]",
+    };
+
+    do_test_validate_uri_channel(&args);
+}
+
+static void test_validate_uri_channels_none_set(void)
+{
+    MigrateCommon args = {
+        .start = {
+            .hide_stderr = true,
+        },
+        .listen_uri = "defer",
+    };
+
+    do_test_validate_uri_channel(&args);
+}
+
 /*
  * The way auto_converge works, we need to do too many passes to
  * run this test.  Auto_converge logic is only run once every
@@ -3678,6 +3728,10 @@ int main(int argc, char **argv)
                        test_validate_uuid_src_not_set);
     migration_test_add("/migration/validate_uuid_dst_not_set",
                        test_validate_uuid_dst_not_set);
+    migration_test_add("/migration/validate_uri/channels/both_set",
+                       test_validate_uri_channels_both_set);
+    migration_test_add("/migration/validate_uri/channels/none_set",
+                       test_validate_uri_channels_none_set);
     /*
      * See explanation why this test is slow on function definition
      */
-- 
2.22.3

Re: [PATCH v4 0/8] qtest: migration: Add tests for introducing 'channels' argument in migrate QAPIs

[Het Gala]

Can find the reference to the githab pipeline (before patchset) : 
https://gitlab.com/galahet/Qemu/-/pipelines/1207185095

Can find the reference to the githab pipeline (after patchset) : 
https://gitlab.com/galahet/Qemu/-/pipelines/1207183673

On 09/03/24 2:29 am, Het Gala wrote:
> With recent migrate QAPI changes, enabling the direct use of the
> 'channels' argument to avoid redundant URI string parsing is achieved.
>
> To ensure backward compatibility, both 'uri' and 'channels' are kept as
> optional parameters in migration QMP commands. However, they are mutually
> exhaustive, requiring at least one for a successful migration connection.
> This patchset adds qtests to validate 'uri' and 'channels' arguments'
> mututally exhaustive behaviour.
>
> Additionally, all migration qtests fail to employ 'channel' as the primary
> method for validating migration QAPIs. This patchset also adds test to
> enforce only use of 'channel' argument as the initial entry point for
> migration QAPIs.
>
> Patch Summary:
> -------------
> Patch 1-2:
> ---------
> Introduce 'to' object inside migrate_qmp() so and move the calls to
> migrate_get_socket_address() inside migrate_qmp. Also, replace connect_uri
> with args->connect_uri everywhere.
>
> Patch 3-6:
> ---------
> Add channels argument to allow both migration QAPI arguments independently
> into migrate_qmp and migrate_qmp_fail. migrate_qmp requires the port value to
> be changed from 0 to port value coming from migrate_get_socket_address. Add
> migrate_set_ports to address this change of port value.
>
> Patch 7-8:
> ---------
> Add 2 negative tests to validate mutually exhaustive behaviour of migration
> QAPIs. Add a positive multifd_tcp_plain qtest with only channels as the
> initial entry point for migration QAPIs.
>
> v3->v4 Changelog:
> ----------------
> 1. introduced migrate_get_connect_uri and migrate_get_connect_qdict to
>     both used migrate_get_socket_address to get dest uri in socket-
>     address, and then use SokcketAddress_to_qdict to convert it into qdict.
> 2. Misc code changes.
>
> v2->v3 Changelog:
> -----------------
> 1. 'channels' introduction is not required now for migrate_qmp_incoming
> 2. Refactor the code into 7 different patches
> 3. 'channels' introduction is not required now for migrate_qmp_incoming
> 4. Remove custom function for converting string to MigrationChannelList
> 5. move calls for migrate_get_socket_address inside migrate_qmp so that
>     migrate_set_ports can replace the QAPI's port with correct value.
>
> Het Gala (8):
>    Add 'to' object into migrate_qmp()
>    Replace connect_uri and move migrate_get_socket_address inside
>      migrate_qmp
>    Replace migrate_get_connect_uri inplace of migrate_get_socket_address
>    Add channels parameter in migrate_qmp_fail
>    Add migrate_set_ports into migrate_qmp to update migration port value
>    Add channels parameter in migrate_qmp
>    Add multifd_tcp_plain test using list of channels instead of uri
>    Add negative tests to validate migration QAPIs
>
>   tests/qtest/migration-helpers.c | 158 +++++++++++++++++++++++++++-
>   tests/qtest/migration-helpers.h |  10 +-
>   tests/qtest/migration-test.c    | 177 ++++++++++++++++++--------------
>   3 files changed, 258 insertions(+), 87 deletions(-)
>

Re: [PATCH v4 0/8] qtest: migration: Add tests for introducing 'channels' argument in migrate QAPIs

[Peter Xu]

On Sat, Mar 09, 2024 at 01:11:45PM +0530, Het Gala wrote:
> Can find the reference to the githab pipeline (before patchset) :
> https://gitlab.com/galahet/Qemu/-/pipelines/1207185095
> 
> Can find the reference to the githab pipeline (after patchset) :
> https://gitlab.com/galahet/Qemu/-/pipelines/1207183673

Het,

Please still copy me for any migration patches.  In this case Fabiano is
looking it'll be all fine, but it will still help me on marking the emails.

Thanks,

-- 
Peter Xu

Re: [PATCH v4 0/8] qtest: migration: Add tests for introducing 'channels' argument in migrate QAPIs

[Het Gala]

On 12/03/24 2:55 am, Peter Xu wrote:
> On Sat, Mar 09, 2024 at 01:11:45PM +0530, Het Gala wrote:
>> Can find the reference to the githab pipeline (before patchset) :
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_galahet_Qemu_-2D_pipelines_1207185095&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=y2xUaOwvRVC5eTpFNEdxb37JYDdxN61W406HlCyx3CWIVyBRgLwjJhAYALZLinoi&s=vZRNX33_DuLO1TsfTpYR_s9bf_EMFm3oHHH_eg57zE0&e=
>>
>> Can find the reference to the githab pipeline (after patchset) :
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_galahet_Qemu_-2D_pipelines_1207183673&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=y2xUaOwvRVC5eTpFNEdxb37JYDdxN61W406HlCyx3CWIVyBRgLwjJhAYALZLinoi&s=C73ka3k3ouAuRJYNVLPIBQiWx3jDFDDvVYDiEYqfE04&e=
> Het,
>
> Please still copy me for any migration patches.  In this case Fabiano is
> looking it'll be all fine, but it will still help me on marking the emails.
>
> Thanks,
So sorry about that Peter. I am aware that you and Fabiano are the go to 
migration
maintainers. I thought I emailed or cc'd all the stakeholders that 
should be involved
for this patchset series. Even in earlier series of this patchset, you 
were cc'ed,
but somehow I just forgot to cc you for this patchset. Sure, will take 
care from next
time. Again apologies for the mixup :)

Regards,
Het Gala

Re: [PATCH v4 0/8] qtest: migration: Add tests for introducing 'channels' argument in migrate QAPIs

[Peter Xu]

On Tue, Mar 12, 2024 at 03:01:51AM +0530, Het Gala wrote:
> 
> On 12/03/24 2:55 am, Peter Xu wrote:
> > On Sat, Mar 09, 2024 at 01:11:45PM +0530, Het Gala wrote:
> > > Can find the reference to the githab pipeline (before patchset) :
> > > https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_galahet_Qemu_-2D_pipelines_1207185095&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=y2xUaOwvRVC5eTpFNEdxb37JYDdxN61W406HlCyx3CWIVyBRgLwjJhAYALZLinoi&s=vZRNX33_DuLO1TsfTpYR_s9bf_EMFm3oHHH_eg57zE0&e=
> > > 
> > > Can find the reference to the githab pipeline (after patchset) :
> > > https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_galahet_Qemu_-2D_pipelines_1207183673&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=y2xUaOwvRVC5eTpFNEdxb37JYDdxN61W406HlCyx3CWIVyBRgLwjJhAYALZLinoi&s=C73ka3k3ouAuRJYNVLPIBQiWx3jDFDDvVYDiEYqfE04&e=
> > Het,
> > 
> > Please still copy me for any migration patches.  In this case Fabiano is
> > looking it'll be all fine, but it will still help me on marking the emails.
> > 
> > Thanks,
> So sorry about that Peter. I am aware that you and Fabiano are the go to
> migration
> maintainers. I thought I emailed or cc'd all the stakeholders that should be
> involved
> for this patchset series. Even in earlier series of this patchset, you were
> cc'ed,
> but somehow I just forgot to cc you for this patchset. Sure, will take care
> from next
> time. Again apologies for the mixup :)

No problem at all.  As long as you have at least 1 maintainers copied,
logically nothing will get lost.  It's just that it helps me in the routines.

Are you managing cc list manually for each version?  In that case I suggest
you have a look at Stefan's tool:

https://github.com/stefanha/git-publish

It might help a great deal in patch managements at least to me, and it
definitely covers more than maintaining the cc list for a patchset.

-- 
Peter Xu

Re: [PATCH v4 0/8] qtest: migration: Add tests for introducing 'channels' argument in migrate QAPIs

[Het Gala]

[-- Attachment #1: Type: text/plain, Size: 2660 bytes --]


On 12/03/24 3:08 am, Peter Xu wrote:
> On Tue, Mar 12, 2024 at 03:01:51AM +0530, Het Gala wrote:
>> On 12/03/24 2:55 am, Peter Xu wrote:
>>> On Sat, Mar 09, 2024 at 01:11:45PM +0530, Het Gala wrote:
>>>> Can find the reference to the githab pipeline (before patchset) :
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_galahet_Qemu_-2D_pipelines_1207185095&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=y2xUaOwvRVC5eTpFNEdxb37JYDdxN61W406HlCyx3CWIVyBRgLwjJhAYALZLinoi&s=vZRNX33_DuLO1TsfTpYR_s9bf_EMFm3oHHH_eg57zE0&e=
>>>>
>>>> Can find the reference to the githab pipeline (after patchset) :
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_galahet_Qemu_-2D_pipelines_1207183673&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=y2xUaOwvRVC5eTpFNEdxb37JYDdxN61W406HlCyx3CWIVyBRgLwjJhAYALZLinoi&s=C73ka3k3ouAuRJYNVLPIBQiWx3jDFDDvVYDiEYqfE04&e=
>>> Het,
>>>
>>> Please still copy me for any migration patches.  In this case Fabiano is
>>> looking it'll be all fine, but it will still help me on marking the emails.
>>>
>>> Thanks,
>> So sorry about that Peter. I am aware that you and Fabiano are the go to
>> migration
>> maintainers. I thought I emailed or cc'd all the stakeholders that should be
>> involved
>> for this patchset series. Even in earlier series of this patchset, you were
>> cc'ed,
>> but somehow I just forgot to cc you for this patchset. Sure, will take care
>> from next
>> time. Again apologies for the mixup :)
> No problem at all.  As long as you have at least 1 maintainers copied,
> logically nothing will get lost.  It's just that it helps me in the routines.
>
> Are you managing cc list manually for each version?  In that case I suggest
> you have a look at Stefan's tool:
I used to earlier. But lately markus introduced me to
scripts/get_maintainers.pl -f <file>
It gives list of all the maintainers handling that particular file.
So that helped me for this patchset.
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_stefanha_git-2Dpublish&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=ydJfb02Wuk_NnlYl8-RkRkYXzWNpzlEht7yj5kakeAlz_WPoD6yvC7b-fVCeLzom&s=8KSe9MiMzmHda3uZ_uaGCIEjub4tSzpeDTpZZwq5knc&e=  
Thanks a lot Peter, looks cool. Will try to explore and use git-publish
and its different methods for next patchset.
> It might help a great deal in patch managements at least to me, and it
> definitely covers more than maintaining the cc list for a patchset.
>
Yes, it looks like there are a lot of useful methods that I can leverage
in future :)

Regards,
Het Gala

[-- Attachment #2: Type: text/html, Size: 5076 bytes --]