qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/9] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration
@ 2023-06-06 10:15 Het Gala
  2023-06-06 10:15 ` [PATCH v6 1/9] migration: introduced 'MigrateAddress' in QAPI for migration wire protocol Het Gala
                   ` (8 more replies)
  0 siblings, 9 replies; 18+ messages in thread
From: Het Gala @ 2023-06-06 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran, Het Gala

This is v6 patchset of modified 'migrate' and 'migrate-incoming' QAPI design
for upstream review.

Would like to thank all the maintainers that actively participated in the v5
patchset discussion and gave insightful suggestions to improve the patches.


Link to previous upstream community patchset links:
v1: https://lists.gnu.org/archive/html/qemu-devel/2022-12/msg04339.html
v2: https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg02106.html
v3: https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg02473.html
v4: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg03064.html
v5: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg04845.html

v5 -> v6 changelog:
-------------------
- Most changes on the QAPI side, i.e. Patch 1 and Patch 6.
- Formatting, improvemnt around migration qapi documentation.
- Resolved build issue while building Qemu with compiler as Clang.
- Restructured migration unit tests due to recent change aca0406
  (tests/qtest: replace wait_command() with qtest......) 

Abstract:
---------

Current QAPI 'migrate' command design (for initiating a migration
stream) contains information regarding different migrate transport mechanism
(tcp / unix / exec), dest-host IP address, and binding port number in form of
a string. Thus the design does seem to have some design issues. Some of the
issues, stated below are:

1. Use of string URIs is a data encoding scheme within a data encoding scheme.
   QEMU code should directly be able to work with the results from QAPI,
   without resorting to do a second level of parsing (eg. socket_parse()).
2. For features / parameters related to migration, the migration tunables needs
   to be defined and updated upfront. For example, 'migrate-set-capability'
   and 'migrate-set-parameter' is required to enable multifd capability and
   multifd-number of channels respectively. Instead, 'Multifd-channels' can
   directly be represented as a single additional parameter to 'migrate'
   QAPI. 'migrate-set-capability' and 'migrate-set-parameter' commands could
   be used for runtime tunables that need setting after migration has already
   started.

The current patchset focuses on solving the first problem of multi-level
encoding of URIs. The patch defines 'migrate' command as a QAPI discriminated
union for the various transport backends (like socket, exec and rdma), and on
basis of transport backends, different migration parameters are defined.

(uri) string -->  (channel) Channel-type
                            Transport-type
                            Migration parameters based on transport type
------------------------------------------------------------------------------
Het Gala (9):
  migration: introduced 'MigrateAddress' in QAPI for migration wire
    protocol.
  migration: convert uri parameter into 'MigrateAddress' struct
  migration: convert socket backend to accept MigrateAddress struct
  migration: convert rdma backend to accept MigrateAddress struct
  migration: convert exec backend to accept MigrateAddress struct.
  migration: modified migration QAPIs to accept 'channels' argument for
    migration
  migration: modify migration_channels_and_uri_compatible() to
    incorporate newer migration QAPI syntax
  migration: Introduced MigrateChannelList struct to migration code
    flow.
  migration: adding test case for modified QAPI syntax

 migration/exec.c               |  72 +++++++++----
 migration/exec.h               |   8 +-
 migration/migration-hmp-cmds.c |  16 ++-
 migration/migration.c          | 182 ++++++++++++++++++++++++++-------
 migration/rdma.c               |  34 +++---
 migration/rdma.h               |   6 +-
 migration/socket.c             |  39 ++-----
 migration/socket.h             |   7 +-
 qapi/migration.json            | 154 +++++++++++++++++++++++++++-
 softmmu/vl.c                   |   2 +-
 tests/qtest/migration-test.c   |  45 ++++++++
 11 files changed, 439 insertions(+), 126 deletions(-)

-- 
2.22.3



^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v6 1/9] migration: introduced 'MigrateAddress' in QAPI for migration wire protocol.
  2023-06-06 10:15 [PATCH v6 0/9] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
@ 2023-06-06 10:15 ` Het Gala
  2023-06-13  5:28   ` Het Gala
  2023-07-05 11:21   ` Markus Armbruster
  2023-06-06 10:15 ` [PATCH v6 2/9] migration: convert uri parameter into 'MigrateAddress' struct Het Gala
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 18+ messages in thread
From: Het Gala @ 2023-06-06 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran, Het Gala

This patch introduces well defined MigrateAddress struct and its related
child objects.

The existing argument of 'migrate' and 'migrate-incoming' QAPI - 'uri'
is of string type. The current migration flow follows double encoding
scheme for  fetching migration parameters such as 'uri' and this is
not an ideal design.

Motive for intoducing struct level design is to prevent double encoding
of QAPI arguments, as Qemu should be able to directly use the QAPI
arguments without any level of encoding.

Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qapi/migration.json | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/qapi/migration.json b/qapi/migration.json
index 179af0c4d8..e61d25eba2 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1407,6 +1407,51 @@
 ##
 { 'command': 'migrate-continue', 'data': {'state': 'MigrationStatus'} }
 
+##
+# @MigrationAddressType:
+#
+# The migration stream transport mechanisms.
+#
+# @socket: Migrate via socket.
+#
+# @exec: Direct the migration stream to another process.
+#
+# @rdma: Migrate via RDMA.
+#
+# Since 8.1
+##
+{ 'enum': 'MigrationAddressType',
+  'data': ['socket', 'exec', 'rdma'] }
+
+##
+# @MigrationExecCommand:
+#
+# @args: list of commands for migraton stream execution to a file.
+#
+# Notes:
+#
+# 1. @args[0] needs to be the path to the new program.
+#
+# Since 8.1
+##
+{ 'struct': 'MigrationExecCommand',
+  'data': {'args': [ 'str' ] } }
+
+##
+# @MigrationAddress:
+#
+# Migration endpoint configuration.
+#
+# Since 8.1
+##
+{ 'union': 'MigrationAddress',
+  'base': { 'transport' : 'MigrationAddressType'},
+  'discriminator': 'transport',
+  'data': {
+    'socket': 'SocketAddress',
+    'exec': 'MigrationExecCommand',
+    'rdma': 'InetSocketAddress' } }
+
 ##
 # @migrate:
 #
-- 
2.22.3



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v6 2/9] migration: convert uri parameter into 'MigrateAddress' struct
  2023-06-06 10:15 [PATCH v6 0/9] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
  2023-06-06 10:15 ` [PATCH v6 1/9] migration: introduced 'MigrateAddress' in QAPI for migration wire protocol Het Gala
@ 2023-06-06 10:15 ` Het Gala
  2023-07-05 11:29   ` Markus Armbruster
  2023-06-06 10:15 ` [PATCH v6 3/9] migration: convert socket backend to accept MigrateAddress struct Het Gala
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Het Gala @ 2023-06-06 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran, Het Gala

This patch parses 'migrate' and 'migrate-incoming' QAPI's 'uri' parameter
with all the migration connection related information and stores them
inside well defined 'MigrateAddress' struct.

Misc: limit line width in exec.c to 80 characters recommended by Qemu.

Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
 migration/exec.c      |  4 ++--
 migration/exec.h      |  4 ++++
 migration/migration.c | 54 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/migration/exec.c b/migration/exec.c
index 2bf882bbe1..c4a3293246 100644
--- a/migration/exec.c
+++ b/migration/exec.c
@@ -27,7 +27,6 @@
 #include "qemu/cutils.h"
 
 #ifdef WIN32
-const char *exec_get_cmd_path(void);
 const char *exec_get_cmd_path(void)
 {
     g_autofree char *detected_path = g_new(char, MAX_PATH);
@@ -40,7 +39,8 @@ const char *exec_get_cmd_path(void)
 }
 #endif
 
-void exec_start_outgoing_migration(MigrationState *s, const char *command, Error **errp)
+void exec_start_outgoing_migration(MigrationState *s, const char *command,
+                                   Error **errp)
 {
     QIOChannel *ioc;
 
diff --git a/migration/exec.h b/migration/exec.h
index b210ffde7a..736cd71028 100644
--- a/migration/exec.h
+++ b/migration/exec.h
@@ -19,6 +19,10 @@
 
 #ifndef QEMU_MIGRATION_EXEC_H
 #define QEMU_MIGRATION_EXEC_H
+
+#ifdef WIN32
+const char *exec_get_cmd_path(void);
+#endif
 void exec_start_incoming_migration(const char *host_port, Error **errp);
 
 void exec_start_outgoing_migration(MigrationState *s, const char *host_port,
diff --git a/migration/migration.c b/migration/migration.c
index dc05c6f6ea..0eb25bb5a4 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -64,6 +64,7 @@
 #include "yank_functions.h"
 #include "sysemu/qtest.h"
 #include "options.h"
+#include "qemu/sockets.h"
 
 static NotifierList migration_state_notifiers =
     NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -420,15 +421,63 @@ void migrate_add_address(SocketAddress *address)
                       QAPI_CLONE(SocketAddress, address));
 }
 
+static bool migrate_uri_parse(const char *uri,
+                              MigrationAddress **channel,
+                              Error **errp)
+{
+    g_autoptr(MigrationAddress) addrs = g_new0(MigrationAddress, 1);
+    SocketAddress *saddr = &addrs->u.socket;
+    InetSocketAddress *isock = &addrs->u.rdma;
+    strList **tail = &addrs->u.exec.args;
+
+    if (strstart(uri, "exec:", NULL)) {
+        addrs->transport = MIGRATION_ADDRESS_TYPE_EXEC;
+#ifdef WIN32
+        QAPI_LIST_APPEND(tail, g_strdup(exec_get_cmd_path()));
+#else
+        QAPI_LIST_APPEND(tail, g_strdup("/bin/sh"));
+#endif
+        QAPI_LIST_APPEND(tail, g_strdup("-c"));
+        QAPI_LIST_APPEND(tail, g_strdup(uri + strlen("exec:")));
+    } else if (strstart(uri, "rdma:", NULL)) {
+        if (inet_parse(isock, uri + strlen("rdma:"), errp)) {
+            qapi_free_InetSocketAddress(isock);
+            return false;
+        }
+        addrs->transport = MIGRATION_ADDRESS_TYPE_RDMA;
+    } else if (strstart(uri, "tcp:", NULL) ||
+                strstart(uri, "unix:", NULL) ||
+                strstart(uri, "vsock:", NULL) ||
+                strstart(uri, "fd:", NULL)) {
+        addrs->transport = MIGRATION_ADDRESS_TYPE_SOCKET;
+        saddr = socket_parse(uri, errp);
+        if (!saddr) {
+            qapi_free_SocketAddress(saddr);
+            return false;
+        }
+    } else {
+        error_setg(errp, "unknown migration protocol: %s", uri);
+        return false;
+    }
+
+    *channel = addrs;
+    return true;
+}
+
 static void qemu_start_incoming_migration(const char *uri, Error **errp)
 {
     const char *p = NULL;
+    g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
 
     /* URI is not suitable for migration? */
     if (!migration_channels_and_uri_compatible(uri, errp)) {
         return;
     }
 
+    if (uri && !migrate_uri_parse(uri, &channel, errp)) {
+        return;
+    }
+
     qapi_event_send_migration(MIGRATION_STATUS_SETUP);
     if (strstart(uri, "tcp:", &p) ||
         strstart(uri, "unix:", NULL) ||
@@ -1632,12 +1681,17 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     Error *local_err = NULL;
     MigrationState *s = migrate_get_current();
     const char *p = NULL;
+    g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
 
     /* URI is not suitable for migration? */
     if (!migration_channels_and_uri_compatible(uri, errp)) {
         return;
     }
 
+    if (!migrate_uri_parse(uri, &channel, &local_err)) {
+        return;
+    }
+
     if (!migrate_prepare(s, has_blk && blk, has_inc && inc,
                          has_resume && resume, errp)) {
         /* Error detected, put into errp */
-- 
2.22.3



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v6 3/9] migration: convert socket backend to accept MigrateAddress struct
  2023-06-06 10:15 [PATCH v6 0/9] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
  2023-06-06 10:15 ` [PATCH v6 1/9] migration: introduced 'MigrateAddress' in QAPI for migration wire protocol Het Gala
  2023-06-06 10:15 ` [PATCH v6 2/9] migration: convert uri parameter into 'MigrateAddress' struct Het Gala
@ 2023-06-06 10:15 ` Het Gala
  2023-06-06 10:15 ` [PATCH v6 4/9] migration: convert rdma " Het Gala
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Het Gala @ 2023-06-06 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran, Het Gala

Socket transport backend for 'migrate'/'migrate-incoming' QAPIs accept
new wire protocol of MigrateAddress struct.

It is achived by parsing 'uri' string and storing migration parameters
required for socket connection into well defined SocketAddress struct.

Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
 migration/migration.c | 32 +++++++++++++++++++-------------
 migration/socket.c    | 34 +++++-----------------------------
 migration/socket.h    |  7 ++++---
 3 files changed, 28 insertions(+), 45 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 0eb25bb5a4..0554536b7f 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -479,18 +479,21 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp)
     }
 
     qapi_event_send_migration(MIGRATION_STATUS_SETUP);
-    if (strstart(uri, "tcp:", &p) ||
-        strstart(uri, "unix:", NULL) ||
-        strstart(uri, "vsock:", NULL)) {
-        socket_start_incoming_migration(p ? p : uri, errp);
+    if (channel->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
+        SocketAddress *saddr = &channel->u.socket;
+        if (saddr->type == SOCKET_ADDRESS_TYPE_INET ||
+            saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
+            saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) {
+            socket_start_incoming_migration(saddr, errp);
+        } else if (saddr->type == SOCKET_ADDRESS_TYPE_FD) {
+            fd_start_incoming_migration(saddr->u.fd.str, errp);
+        }
 #ifdef CONFIG_RDMA
     } else if (strstart(uri, "rdma:", &p)) {
         rdma_start_incoming_migration(p, errp);
 #endif
     } else if (strstart(uri, "exec:", &p)) {
         exec_start_incoming_migration(p, errp);
-    } else if (strstart(uri, "fd:", &p)) {
-        fd_start_incoming_migration(p, errp);
     } else {
         error_setg(errp, "unknown migration protocol: %s", uri);
     }
@@ -1688,7 +1691,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
         return;
     }
 
-    if (!migrate_uri_parse(uri, &channel, &local_err)) {
+    if (!migrate_uri_parse(uri, &channel, errp)) {
         return;
     }
 
@@ -1704,18 +1707,21 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
         }
     }
 
-    if (strstart(uri, "tcp:", &p) ||
-        strstart(uri, "unix:", NULL) ||
-        strstart(uri, "vsock:", NULL)) {
-        socket_start_outgoing_migration(s, p ? p : uri, &local_err);
+    if (channel->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
+        SocketAddress *saddr = &channel->u.socket;
+        if (saddr->type == SOCKET_ADDRESS_TYPE_INET ||
+            saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
+            saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) {
+            socket_start_outgoing_migration(s, saddr, &local_err);
+        } else if (saddr->type == SOCKET_ADDRESS_TYPE_FD) {
+            fd_start_outgoing_migration(s, saddr->u.fd.str, &local_err);
+        }
 #ifdef CONFIG_RDMA
     } else if (strstart(uri, "rdma:", &p)) {
         rdma_start_outgoing_migration(s, p, &local_err);
 #endif
     } else if (strstart(uri, "exec:", &p)) {
         exec_start_outgoing_migration(s, p, &local_err);
-    } else if (strstart(uri, "fd:", &p)) {
-        fd_start_outgoing_migration(s, p, &local_err);
     } else {
         if (!(has_resume && resume)) {
             yank_unregister_instance(MIGRATION_YANK_INSTANCE);
diff --git a/migration/socket.c b/migration/socket.c
index 1b6f5baefb..8e7430b266 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -108,10 +108,9 @@ out:
     object_unref(OBJECT(sioc));
 }
 
-static void
-socket_start_outgoing_migration_internal(MigrationState *s,
-                                         SocketAddress *saddr,
-                                         Error **errp)
+void socket_start_outgoing_migration(MigrationState *s,
+                                     SocketAddress *saddr,
+                                     Error **errp)
 {
     QIOChannelSocket *sioc = qio_channel_socket_new();
     struct SocketConnectData *data = g_new0(struct SocketConnectData, 1);
@@ -135,18 +134,6 @@ socket_start_outgoing_migration_internal(MigrationState *s,
                                      NULL);
 }
 
-void socket_start_outgoing_migration(MigrationState *s,
-                                     const char *str,
-                                     Error **errp)
-{
-    Error *err = NULL;
-    SocketAddress *saddr = socket_parse(str, &err);
-    if (!err) {
-        socket_start_outgoing_migration_internal(s, saddr, &err);
-    }
-    error_propagate(errp, err);
-}
-
 static void socket_accept_incoming_migration(QIONetListener *listener,
                                              QIOChannelSocket *cioc,
                                              gpointer opaque)
@@ -172,9 +159,8 @@ socket_incoming_migration_end(void *opaque)
     object_unref(OBJECT(listener));
 }
 
-static void
-socket_start_incoming_migration_internal(SocketAddress *saddr,
-                                         Error **errp)
+void socket_start_incoming_migration(SocketAddress *saddr,
+                                     Error **errp)
 {
     QIONetListener *listener = qio_net_listener_new();
     MigrationIncomingState *mis = migration_incoming_get_current();
@@ -213,13 +199,3 @@ socket_start_incoming_migration_internal(SocketAddress *saddr,
     }
 }
 
-void socket_start_incoming_migration(const char *str, Error **errp)
-{
-    Error *err = NULL;
-    SocketAddress *saddr = socket_parse(str, &err);
-    if (!err) {
-        socket_start_incoming_migration_internal(saddr, &err);
-    }
-    qapi_free_SocketAddress(saddr);
-    error_propagate(errp, err);
-}
diff --git a/migration/socket.h b/migration/socket.h
index dc54df4e6c..5e4c33b8ea 100644
--- a/migration/socket.h
+++ b/migration/socket.h
@@ -19,13 +19,14 @@
 
 #include "io/channel.h"
 #include "io/task.h"
+#include "qemu/sockets.h"
 
 void socket_send_channel_create(QIOTaskFunc f, void *data);
 QIOChannel *socket_send_channel_create_sync(Error **errp);
 int socket_send_channel_destroy(QIOChannel *send);
 
-void socket_start_incoming_migration(const char *str, Error **errp);
+void socket_start_incoming_migration(SocketAddress *saddr, Error **errp);
 
-void socket_start_outgoing_migration(MigrationState *s, const char *str,
-                                     Error **errp);
+void socket_start_outgoing_migration(MigrationState *s,
+                                     SocketAddress *saddr, Error **errp);
 #endif
-- 
2.22.3



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v6 4/9] migration: convert rdma backend to accept MigrateAddress struct
  2023-06-06 10:15 [PATCH v6 0/9] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
                   ` (2 preceding siblings ...)
  2023-06-06 10:15 ` [PATCH v6 3/9] migration: convert socket backend to accept MigrateAddress struct Het Gala
@ 2023-06-06 10:15 ` Het Gala
  2023-06-06 10:15 ` [PATCH v6 5/9] migration: convert exec " Het Gala
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Het Gala @ 2023-06-06 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran, Het Gala

RDMA based transport backend for 'migrate'/'migrate-incoming' QAPIs
accept new wire protocol of MigrateAddress struct.

It is achived by parsing 'uri' string and storing migration parameters
required for RDMA connection into well defined InetSocketAddress struct.

Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
 migration/migration.c |  8 ++++----
 migration/rdma.c      | 34 ++++++++++++----------------------
 migration/rdma.h      |  6 ++++--
 3 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 0554536b7f..ff020656fb 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -489,8 +489,8 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp)
             fd_start_incoming_migration(saddr->u.fd.str, errp);
         }
 #ifdef CONFIG_RDMA
-    } else if (strstart(uri, "rdma:", &p)) {
-        rdma_start_incoming_migration(p, errp);
+    } else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
+        rdma_start_incoming_migration(&channel->u.rdma, errp);
 #endif
     } else if (strstart(uri, "exec:", &p)) {
         exec_start_incoming_migration(p, errp);
@@ -1717,8 +1717,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
             fd_start_outgoing_migration(s, saddr->u.fd.str, &local_err);
         }
 #ifdef CONFIG_RDMA
-    } else if (strstart(uri, "rdma:", &p)) {
-        rdma_start_outgoing_migration(s, p, &local_err);
+    } else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
+        rdma_start_outgoing_migration(s, &channel->u.rdma, &local_err);
 #endif
     } else if (strstart(uri, "exec:", &p)) {
         exec_start_outgoing_migration(s, p, &local_err);
diff --git a/migration/rdma.c b/migration/rdma.c
index dd1c039e6c..4d64fae492 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -319,7 +319,6 @@ typedef struct RDMALocalBlocks {
 typedef struct RDMAContext {
     char *host;
     int port;
-    char *host_port;
 
     RDMAWorkRequestData wr_data[RDMA_WRID_MAX];
 
@@ -2455,9 +2454,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
         rdma->channel = NULL;
     }
     g_free(rdma->host);
-    g_free(rdma->host_port);
     rdma->host = NULL;
-    rdma->host_port = NULL;
 }
 
 
@@ -2739,28 +2736,17 @@ static void qemu_rdma_return_path_dest_init(RDMAContext *rdma_return_path,
     rdma_return_path->is_return_path = true;
 }
 
-static void *qemu_rdma_data_init(const char *host_port, Error **errp)
+static void *qemu_rdma_data_init(InetSocketAddress *saddr, Error **errp)
 {
     RDMAContext *rdma = NULL;
-    InetSocketAddress *addr;
 
-    if (host_port) {
+    if (saddr) {
         rdma = g_new0(RDMAContext, 1);
         rdma->current_index = -1;
         rdma->current_chunk = -1;
 
-        addr = g_new(InetSocketAddress, 1);
-        if (!inet_parse(addr, host_port, NULL)) {
-            rdma->port = atoi(addr->port);
-            rdma->host = g_strdup(addr->host);
-            rdma->host_port = g_strdup(host_port);
-        } else {
-            ERROR(errp, "bad RDMA migration address '%s'", host_port);
-            g_free(rdma);
-            rdma = NULL;
-        }
-
-        qapi_free_InetSocketAddress(addr);
+        rdma->host = g_strdup(saddr->host);
+        rdma->port = atoi(saddr->port);
     }
 
     return rdma;
@@ -3359,6 +3345,7 @@ static int qemu_rdma_accept(RDMAContext *rdma)
                                             .private_data_len = sizeof(cap),
                                          };
     RDMAContext *rdma_return_path = NULL;
+    g_autoptr(InetSocketAddress) isock = g_new0(InetSocketAddress, 1);
     struct rdma_cm_event *cm_event;
     struct ibv_context *verbs;
     int ret = -EINVAL;
@@ -3374,13 +3361,16 @@ static int qemu_rdma_accept(RDMAContext *rdma)
         goto err_rdma_dest_wait;
     }
 
+    isock->host = rdma->host;
+    isock->port = g_strdup_printf("%d", rdma->port);
+
     /*
      * initialize the RDMAContext for return path for postcopy after first
      * connection request reached.
      */
     if ((migrate_postcopy() || migrate_return_path())
         && !rdma->is_return_path) {
-        rdma_return_path = qemu_rdma_data_init(rdma->host_port, NULL);
+        rdma_return_path = qemu_rdma_data_init(isock, NULL);
         if (rdma_return_path == NULL) {
             rdma_ack_cm_event(cm_event);
             goto err_rdma_dest_wait;
@@ -4113,7 +4103,8 @@ static void rdma_accept_incoming_migration(void *opaque)
     }
 }
 
-void rdma_start_incoming_migration(const char *host_port, Error **errp)
+void rdma_start_incoming_migration(InetSocketAddress *host_port,
+                                   Error **errp)
 {
     int ret;
     RDMAContext *rdma;
@@ -4159,13 +4150,12 @@ err:
     error_propagate(errp, local_err);
     if (rdma) {
         g_free(rdma->host);
-        g_free(rdma->host_port);
     }
     g_free(rdma);
 }
 
 void rdma_start_outgoing_migration(void *opaque,
-                            const char *host_port, Error **errp)
+                            InetSocketAddress *host_port, Error **errp)
 {
     MigrationState *s = opaque;
     RDMAContext *rdma_return_path = NULL;
diff --git a/migration/rdma.h b/migration/rdma.h
index de2ba09dc5..ee89296555 100644
--- a/migration/rdma.h
+++ b/migration/rdma.h
@@ -14,12 +14,14 @@
  *
  */
 
+#include "qemu/sockets.h"
+
 #ifndef QEMU_MIGRATION_RDMA_H
 #define QEMU_MIGRATION_RDMA_H
 
-void rdma_start_outgoing_migration(void *opaque, const char *host_port,
+void rdma_start_outgoing_migration(void *opaque, InetSocketAddress *host_port,
                                    Error **errp);
 
-void rdma_start_incoming_migration(const char *host_port, Error **errp);
+void rdma_start_incoming_migration(InetSocketAddress *host_port, Error **errp);
 
 #endif
-- 
2.22.3



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v6 5/9] migration: convert exec backend to accept MigrateAddress struct.
  2023-06-06 10:15 [PATCH v6 0/9] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
                   ` (3 preceding siblings ...)
  2023-06-06 10:15 ` [PATCH v6 4/9] migration: convert rdma " Het Gala
@ 2023-06-06 10:15 ` Het Gala
  2023-06-06 10:15 ` [PATCH v6 6/9] migration: modified migration QAPIs to accept 'channels' argument for migration Het Gala
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Het Gala @ 2023-06-06 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran, Het Gala

Exec transport backend for 'migrate'/'migrate-incoming' QAPIs accept
new wire protocol of MigrateAddress struct.

It is achived by parsing 'uri' string and storing migration parameters
required for exec connection into strList struct.

Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
 migration/exec.c      | 70 ++++++++++++++++++++++++++++++-------------
 migration/exec.h      |  4 +--
 migration/migration.c | 10 +++----
 3 files changed, 56 insertions(+), 28 deletions(-)

diff --git a/migration/exec.c b/migration/exec.c
index c4a3293246..8bc321c66b 100644
--- a/migration/exec.c
+++ b/migration/exec.c
@@ -39,21 +39,50 @@ const char *exec_get_cmd_path(void)
 }
 #endif
 
-void exec_start_outgoing_migration(MigrationState *s, const char *command,
+/* provides the length of strList */
+static int
+str_list_length(strList *list)
+{
+    int len = 0;
+    strList *elem;
+
+    for (elem = list; elem != NULL; elem = elem->next) {
+        len++;
+    }
+
+    return len;
+}
+
+static void
+init_exec_array(strList *command, char **argv, Error **errp)
+{
+    int i = 0;
+    strList *lst;
+
+    for (lst = command; lst; lst = lst->next) {
+        argv[i++] = lst->value;
+    }
+
+    argv[i] = NULL;
+    return;
+}
+
+void exec_start_outgoing_migration(MigrationState *s, strList *command,
                                    Error **errp)
 {
     QIOChannel *ioc;
 
-#ifdef WIN32
-    const char *argv[] = { exec_get_cmd_path(), "/c", command, NULL };
-#else
-    const char *argv[] = { "/bin/sh", "-c", command, NULL };
-#endif
+    int length = str_list_length(command);
+    g_auto(GStrv) argv = (char **) g_new0(const char *, length);
 
-    trace_migration_exec_outgoing(command);
-    ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
-                                                    O_RDWR,
-                                                    errp));
+    init_exec_array(command, argv, errp);
+    g_autofree char *new_command = g_strjoinv(" ", (char **)argv);
+
+    trace_migration_exec_outgoing(new_command);
+    ioc = QIO_CHANNEL(
+        qio_channel_command_new_spawn((const char * const *) argv,
+                                      O_RDWR,
+                                      errp));
     if (!ioc) {
         return;
     }
@@ -72,20 +101,21 @@ static gboolean exec_accept_incoming_migration(QIOChannel *ioc,
     return G_SOURCE_REMOVE;
 }
 
-void exec_start_incoming_migration(const char *command, Error **errp)
+void exec_start_incoming_migration(strList *command, Error **errp)
 {
     QIOChannel *ioc;
 
-#ifdef WIN32
-    const char *argv[] = { exec_get_cmd_path(), "/c", command, NULL };
-#else
-    const char *argv[] = { "/bin/sh", "-c", command, NULL };
-#endif
+    int length = str_list_length(command);
+    g_auto(GStrv) argv = (char **) g_new0(const char *, length);
+
+    init_exec_array(command, argv, errp);
+    g_autofree char *new_command = g_strjoinv(" ", (char **)argv);
 
-    trace_migration_exec_incoming(command);
-    ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
-                                                    O_RDWR,
-                                                    errp));
+    trace_migration_exec_incoming(new_command);
+    ioc = QIO_CHANNEL(
+        qio_channel_command_new_spawn((const char * const *) argv,
+                                      O_RDWR,
+                                      errp));
     if (!ioc) {
         return;
     }
diff --git a/migration/exec.h b/migration/exec.h
index 736cd71028..3107f205e3 100644
--- a/migration/exec.h
+++ b/migration/exec.h
@@ -23,8 +23,8 @@
 #ifdef WIN32
 const char *exec_get_cmd_path(void);
 #endif
-void exec_start_incoming_migration(const char *host_port, Error **errp);
+void exec_start_incoming_migration(strList *host_port, Error **errp);
 
-void exec_start_outgoing_migration(MigrationState *s, const char *host_port,
+void exec_start_outgoing_migration(MigrationState *s, strList *host_port,
                                    Error **errp);
 #endif
diff --git a/migration/migration.c b/migration/migration.c
index ff020656fb..f482de5df3 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -466,7 +466,6 @@ static bool migrate_uri_parse(const char *uri,
 
 static void qemu_start_incoming_migration(const char *uri, Error **errp)
 {
-    const char *p = NULL;
     g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
 
     /* URI is not suitable for migration? */
@@ -492,8 +491,8 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp)
     } else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
         rdma_start_incoming_migration(&channel->u.rdma, errp);
 #endif
-    } else if (strstart(uri, "exec:", &p)) {
-        exec_start_incoming_migration(p, errp);
+    } else if (channel->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
+        exec_start_incoming_migration(channel->u.exec.args, errp);
     } else {
         error_setg(errp, "unknown migration protocol: %s", uri);
     }
@@ -1683,7 +1682,6 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
 {
     Error *local_err = NULL;
     MigrationState *s = migrate_get_current();
-    const char *p = NULL;
     g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
 
     /* URI is not suitable for migration? */
@@ -1720,8 +1718,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     } else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
         rdma_start_outgoing_migration(s, &channel->u.rdma, &local_err);
 #endif
-    } else if (strstart(uri, "exec:", &p)) {
-        exec_start_outgoing_migration(s, p, &local_err);
+    } else if (channel->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
+        exec_start_outgoing_migration(s, channel->u.exec.args, &local_err);
     } else {
         if (!(has_resume && resume)) {
             yank_unregister_instance(MIGRATION_YANK_INSTANCE);
-- 
2.22.3



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v6 6/9] migration: modified migration QAPIs to accept 'channels' argument for migration
  2023-06-06 10:15 [PATCH v6 0/9] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
                   ` (4 preceding siblings ...)
  2023-06-06 10:15 ` [PATCH v6 5/9] migration: convert exec " Het Gala
@ 2023-06-06 10:15 ` Het Gala
  2023-06-08 11:52   ` Het Gala
  2023-06-06 10:15 ` [PATCH v6 7/9] migration: modify migration_channels_and_uri_compatible() to incorporate newer migration QAPI syntax Het Gala
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Het Gala @ 2023-06-06 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran, Het Gala

MigrateChannelList allows to connect accross multiple interfaces. Added
MigrateChannelList struct as argument to migration QAPIs.

Future patchset series plans to include multiple MigrateChannels
for multiple interfaces to be connected. That is the reason,
'MigrateChannelList'
is the preferred choice of argument over 'MigrateChannel' and making
migration QAPIs future proof.

For current patchset series, have limited the size of the list to single
element (single interface) as runtime check.

Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
 migration/migration-hmp-cmds.c |  16 +++--
 migration/migration.c          |  34 ++++++++--
 qapi/migration.json            | 109 ++++++++++++++++++++++++++++++++-
 softmmu/vl.c                   |   2 +-
 4 files changed, 147 insertions(+), 14 deletions(-)

diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 9885d7c9f7..5f04598202 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -423,10 +423,12 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
     const char *uri = qdict_get_str(qdict, "uri");
+    MigrationChannelList *caps = NULL;
+    g_autoptr(MigrationChannel) channel = g_new0(MigrationChannel, 1);
 
-    qmp_migrate_incoming(uri, &err);
-
-    hmp_handle_error(mon, err);
+    QAPI_LIST_PREPEND(caps, channel);
+    qmp_migrate_incoming(uri, false, caps, &err);
+    qapi_free_MigrationChannelList(caps);
 }
 
 void hmp_migrate_recover(Monitor *mon, const QDict *qdict)
@@ -704,9 +706,13 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
     bool resume = qdict_get_try_bool(qdict, "resume", false);
     const char *uri = qdict_get_str(qdict, "uri");
     Error *err = NULL;
+    MigrationChannelList *caps = NULL;
+    g_autoptr(MigrationChannel) channel = g_new0(MigrationChannel, 1);
 
-    qmp_migrate(uri, !!blk, blk, !!inc, inc,
-                false, false, true, resume, &err);
+    QAPI_LIST_PREPEND(caps, channel);
+    qmp_migrate(uri, false, caps, !!blk, blk, !!inc, inc,
+                 false, false, true, resume, &err);
+    qapi_free_MigrationChannelList(caps);
     if (hmp_handle_error(mon, err)) {
         return;
     }
diff --git a/migration/migration.c b/migration/migration.c
index f482de5df3..a5ca9e0e27 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -464,10 +464,22 @@ static bool migrate_uri_parse(const char *uri,
     return true;
 }
 
-static void qemu_start_incoming_migration(const char *uri, Error **errp)
+static void qemu_start_incoming_migration(const char *uri, bool has_channels,
+                                          MigrationChannelList *channels,
+                                          Error **errp)
 {
     g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
 
+    /*
+     * Having preliminary checks for uri and channel
+     */
+    if (uri && has_channels) {
+        error_setg(errp, "'uri' and 'channels' arguments are mutually "
+                   "exclusive; exactly one of the two should be present in "
+                   "'migrate-incoming' qmp command ");
+        return;
+    }
+
     /* URI is not suitable for migration? */
     if (!migration_channels_and_uri_compatible(uri, errp)) {
         return;
@@ -1489,7 +1501,8 @@ void migrate_del_blocker(Error *reason)
     migration_blockers = g_slist_remove(migration_blockers, reason);
 }
 
-void qmp_migrate_incoming(const char *uri, Error **errp)
+void qmp_migrate_incoming(const char *uri, bool has_channels,
+                          MigrationChannelList *channels, Error **errp)
 {
     Error *local_err = NULL;
     static bool once = true;
@@ -1507,7 +1520,7 @@ void qmp_migrate_incoming(const char *uri, Error **errp)
         return;
     }
 
-    qemu_start_incoming_migration(uri, &local_err);
+    qemu_start_incoming_migration(uri, has_channels, channels, &local_err);
 
     if (local_err) {
         yank_unregister_instance(MIGRATION_YANK_INSTANCE);
@@ -1543,7 +1556,7 @@ void qmp_migrate_recover(const char *uri, Error **errp)
      * only re-setup the migration stream and poke existing migration
      * to continue using that newly established channel.
      */
-    qemu_start_incoming_migration(uri, errp);
+    qemu_start_incoming_migration(uri, false, NULL, errp);
 }
 
 void qmp_migrate_pause(Error **errp)
@@ -1676,7 +1689,8 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
     return true;
 }
 
-void qmp_migrate(const char *uri, bool has_blk, bool blk,
+void qmp_migrate(const char *uri, bool has_channels,
+                 MigrationChannelList *channels, bool has_blk, bool blk,
                  bool has_inc, bool inc, bool has_detach, bool detach,
                  bool has_resume, bool resume, Error **errp)
 {
@@ -1684,6 +1698,16 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     MigrationState *s = migrate_get_current();
     g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
 
+    /*
+     * Having preliminary checks for uri and channel
+     */
+    if (uri && has_channels) {
+        error_setg(errp, "'uri' and 'channels' arguments are mutually "
+                   "exclusive; exactly one of the two should be present in "
+                   "'migrate' qmp command ");
+        return;
+    }
+
     /* URI is not suitable for migration? */
     if (!migration_channels_and_uri_compatible(uri, errp)) {
         return;
diff --git a/qapi/migration.json b/qapi/migration.json
index e61d25eba2..7d4160e130 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1452,6 +1452,34 @@
     'exec': 'MigrationExecCommand',
     'rdma': 'InetSocketAddress' } }
 
+##
+# @MigrationChannelType:
+#
+# The migration channel-type request options.
+#
+# @main: Main outbound migration channel.
+#
+# Since 8.1
+##
+{ 'enum': 'MigrationChannelType',
+  'data': [ 'main' ] }
+
+##
+# @MigrationChannel:
+#
+# Migration stream channel parameters.
+#
+# @channel-type: Channel type for transfering packet information.
+#
+# @addr: Migration endpoint configuration on destination interface.
+#
+# Since 8.1
+##
+{ 'struct': 'MigrationChannel',
+  'data': {
+      'channel-type': 'MigrationChannelType',
+      'addr': 'MigrationAddress' } }
+
 ##
 # @migrate:
 #
@@ -1459,6 +1487,9 @@
 #
 # @uri: the Uniform Resource Identifier of the destination VM
 #
+# @channels: list of migration stream channels with each stream
+#     in the list connected to a destination interface endpoint.
+#
 # @blk: do block migration (full disk copy)
 #
 # @inc: incremental disk copy migration
@@ -1483,14 +1514,50 @@
 # 3. The user Monitor's "detach" argument is invalid in QMP and should
 #    not be used
 #
+# 4. The uri argument should have the Uniform Resource Identifier of
+#    default destination VM. This connection will be bound to default
+#    network.
+#
+# 5. For now, number of migration streams is restricted to one, i.e
+#    number of items in 'channels' list is just 1.
+#
+# 6. The 'uri' and 'channels' arguments are mutually exclusive;
+#    exactly one of the two should be present.
+#
 # Example:
 #
 # -> { "execute": "migrate", "arguments": { "uri": "tcp:0:4446" } }
 # <- { "return": {} }
+# -> { "execute": "migrate",
+#      "arguments": {
+#          "channels": [ { "channel-type": "main",
+#                          "addr": { "transport": "socket",
+#                                    "type": "inet",
+#                                    "host": "10.12.34.9",
+#                                    "port": "1050" } } ] } }
+# <- { "return": {} }
+#
+# -> { "execute": "migrate",
+#      "arguments": {
+#          "channels": [ { "channel-type": "main",
+#                          "addr": { "transport": "exec",
+#                                    "args": [ "/bin/nc", "-p", "6000",
+#                                              "/some/sock" ] } } ] } }
+# <- { "return": {} }
+#
+# -> { "execute": "migrate",
+#      "arguments": {
+#          "channels": [ { "channel-type": "main",
+#                          "addr": { "transport": "rdma",
+#                                    "host": "10.12.34.9",
+#                                    "port": "1050" } } ] } }
+# <- { "return": {} }
+#
 ##
 { 'command': 'migrate',
-  'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool',
-           '*detach': 'bool', '*resume': 'bool' } }
+  'data': {'*uri': 'str', '*channels': [ 'MigrationChannel' ],
+           '*blk': 'bool', '*inc': 'bool', '*detach': 'bool',
+           '*resume': 'bool' } }
 
 ##
 # @migrate-incoming:
@@ -1501,6 +1568,9 @@
 # @uri: The Uniform Resource Identifier identifying the source or
 #     address to listen on
 #
+# @channels: list of migration stream channels with each stream
+#     in the list connected to a destination interface endpoint.
+#
 # Returns: nothing on success
 #
 # Since: 2.3
@@ -1516,13 +1586,46 @@
 #
 # 3. The uri format is the same as for -incoming
 #
+# 5. For now, number of migration streams is restricted to one, i.e
+#    number of items in 'channels' list is just 1.
+#
+# 4. The 'uri' and 'channels' arguments are mutually exclusive;
+#    exactly one of the two should be present.
+#
 # Example:
 #
 # -> { "execute": "migrate-incoming",
 #      "arguments": { "uri": "tcp::4446" } }
 # <- { "return": {} }
+#
+# -> { "execute": "migrate",
+#      "arguments": {
+#          "channels": [ { "channel-type": "main",
+#                          "addr": { "transport": "socket",
+#                                    "type": "inet",
+#                                    "host": "10.12.34.9",
+#                                    "port": "1050" } } ] } }
+# <- { "return": {} }
+#
+# -> { "execute": "migrate",
+#      "arguments": {
+#          "channels": [ { "channel-type": "main",
+#                          "addr": { "transport": "exec",
+#                                    "args": [ "/bin/nc", "-p", "6000",
+#                                              "/some/sock" ] } } ] } }
+# <- { "return": {} }
+#
+# -> { "execute": "migrate",
+#      "arguments": {
+#          "channels": [ { "channel-type": "main",
+#                          "addr": { "transport": "rdma",
+#                                    "host": "10.12.34.9",
+#                                    "port": "1050" } } ] } }
+# <- { "return": {} }
 ##
-{ 'command': 'migrate-incoming', 'data': {'uri': 'str' } }
+{ 'command': 'migrate-incoming',
+             'data': {'*uri': 'str',
+                      '*channels': [ 'MigrationChannel' ] } }
 
 ##
 # @xen-save-devices-state:
diff --git a/softmmu/vl.c b/softmmu/vl.c
index b0b96f67fa..d811f3f878 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2651,7 +2651,7 @@ void qmp_x_exit_preconfig(Error **errp)
     if (incoming) {
         Error *local_err = NULL;
         if (strcmp(incoming, "defer") != 0) {
-            qmp_migrate_incoming(incoming, &local_err);
+            qmp_migrate_incoming(incoming, false, NULL, &local_err);
             if (local_err) {
                 error_reportf_err(local_err, "-incoming %s: ", incoming);
                 exit(1);
-- 
2.22.3



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v6 7/9] migration: modify migration_channels_and_uri_compatible() to incorporate newer migration QAPI syntax
  2023-06-06 10:15 [PATCH v6 0/9] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
                   ` (5 preceding siblings ...)
  2023-06-06 10:15 ` [PATCH v6 6/9] migration: modified migration QAPIs to accept 'channels' argument for migration Het Gala
@ 2023-06-06 10:15 ` Het Gala
  2023-06-06 10:15 ` [PATCH v6 8/9] migration: Introduced MigrateChannelList struct to migration code flow Het Gala
  2023-06-06 10:15 ` [PATCH v6 9/9] migration: adding test case for modified QAPI syntax Het Gala
  8 siblings, 0 replies; 18+ messages in thread
From: Het Gala @ 2023-06-06 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran, Het Gala

migration_channels_and_uri_compatible() check for transport mechanism
suitable for multifd migration gets executed when the caller calls old
uri syntax. It needs it to be run when using the mordern MigrateCHannel
QAPI syntax too.

After URI -> 'MigrateChannel' object conversion,
migration_channels_and_uri_compatible() ->
migration_channels_and_transport_compatible() passes object as argument
and checks for valid transport mechanism.

Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
 migration/migration.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index a5ca9e0e27..98b797679b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -103,17 +103,20 @@ static bool migration_needs_multiple_sockets(void)
     return migrate_multifd() || migrate_postcopy_preempt();
 }
 
-static bool uri_supports_multi_channels(const char *uri)
+static bool transport_supports_multi_channels(SocketAddress *saddr)
 {
-    return strstart(uri, "tcp:", NULL) || strstart(uri, "unix:", NULL) ||
-           strstart(uri, "vsock:", NULL);
+    return saddr->type == SOCKET_ADDRESS_TYPE_INET ||
+           saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
+           saddr->type == SOCKET_ADDRESS_TYPE_VSOCK;
 }
 
 static bool
-migration_channels_and_uri_compatible(const char *uri, Error **errp)
+migration_channels_and_transport_compatible(MigrationAddress *addrs,
+                                            Error **errp)
 {
     if (migration_needs_multiple_sockets() &&
-        !uri_supports_multi_channels(uri)) {
+        (addrs->transport == MIGRATION_ADDRESS_TYPE_SOCKET) &&
+        !transport_supports_multi_channels(&addrs->u.socket)) {
         error_setg(errp, "Migration requires multi-channel URIs (e.g. tcp)");
         return false;
     }
@@ -480,12 +483,12 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
         return;
     }
 
-    /* URI is not suitable for migration? */
-    if (!migration_channels_and_uri_compatible(uri, errp)) {
+    if (uri && !migrate_uri_parse(uri, &channel, errp)) {
         return;
     }
 
-    if (uri && !migrate_uri_parse(uri, &channel, errp)) {
+    /* transport mechanism not suitable for migration? */
+    if (!migration_channels_and_transport_compatible(channel, errp)) {
         return;
     }
 
@@ -1708,12 +1711,12 @@ void qmp_migrate(const char *uri, bool has_channels,
         return;
     }
 
-    /* URI is not suitable for migration? */
-    if (!migration_channels_and_uri_compatible(uri, errp)) {
+    if (!migrate_uri_parse(uri, &channel, errp)) {
         return;
     }
 
-    if (!migrate_uri_parse(uri, &channel, errp)) {
+    /* transport mechanism not suitable for migration? */
+    if (!migration_channels_and_transport_compatible(channel, errp)) {
         return;
     }
 
-- 
2.22.3



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v6 8/9] migration: Introduced MigrateChannelList struct to migration code flow.
  2023-06-06 10:15 [PATCH v6 0/9] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
                   ` (6 preceding siblings ...)
  2023-06-06 10:15 ` [PATCH v6 7/9] migration: modify migration_channels_and_uri_compatible() to incorporate newer migration QAPI syntax Het Gala
@ 2023-06-06 10:15 ` Het Gala
  2023-06-06 10:15 ` [PATCH v6 9/9] migration: adding test case for modified QAPI syntax Het Gala
  8 siblings, 0 replies; 18+ messages in thread
From: Het Gala @ 2023-06-06 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran, Het Gala

Integrated MigrateChannelList with all transport backends (socket, exec
and rdma) for both source and destination migration code flow.

Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
 migration/migration.c | 77 ++++++++++++++++++++++++++++---------------
 migration/socket.c    |  5 ++-
 2 files changed, 54 insertions(+), 28 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 98b797679b..9a0aec3ecb 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -425,9 +425,10 @@ void migrate_add_address(SocketAddress *address)
 }
 
 static bool migrate_uri_parse(const char *uri,
-                              MigrationAddress **channel,
+                              MigrationChannel **channel,
                               Error **errp)
 {
+    g_autoptr(MigrationChannel) val = g_new0(MigrationChannel, 1);
     g_autoptr(MigrationAddress) addrs = g_new0(MigrationAddress, 1);
     SocketAddress *saddr = &addrs->u.socket;
     InetSocketAddress *isock = &addrs->u.rdma;
@@ -463,7 +464,9 @@ static bool migrate_uri_parse(const char *uri,
         return false;
     }
 
-    *channel = addrs;
+    val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;
+    val->addr = addrs;
+    *channel = val;
     return true;
 }
 
@@ -471,7 +474,8 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
                                           MigrationChannelList *channels,
                                           Error **errp)
 {
-    g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
+    g_autoptr(MigrationChannel) channel = g_new0(MigrationChannel, 1);
+    g_autoptr(MigrationAddress) addrs = g_new0(MigrationAddress, 1);
 
     /*
      * Having preliminary checks for uri and channel
@@ -481,20 +485,29 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
                    "exclusive; exactly one of the two should be present in "
                    "'migrate-incoming' qmp command ");
         return;
-    }
-
-    if (uri && !migrate_uri_parse(uri, &channel, errp)) {
-        return;
+    } else if (channels) {
+        /* To verify that Migrate channel list has only item */
+        if (channels->next) {
+            error_setg(errp, "Channel list has more than one entries");
+            return;
+        }
+        channel = channels->value;
+        addrs = channel->addr;
+    } else {
+        /* caller uses the old URI syntax */
+        if (uri && !migrate_uri_parse(uri, &channel, errp)) {
+            return;
+        }
     }
 
     /* transport mechanism not suitable for migration? */
-    if (!migration_channels_and_transport_compatible(channel, errp)) {
+    if (!migration_channels_and_transport_compatible(addrs, errp)) {
         return;
     }
 
     qapi_event_send_migration(MIGRATION_STATUS_SETUP);
-    if (channel->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
-        SocketAddress *saddr = &channel->u.socket;
+    if (addrs->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
+        SocketAddress *saddr = &addrs->u.socket;
         if (saddr->type == SOCKET_ADDRESS_TYPE_INET ||
             saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
             saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) {
@@ -503,11 +516,11 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
             fd_start_incoming_migration(saddr->u.fd.str, errp);
         }
 #ifdef CONFIG_RDMA
-    } else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
-        rdma_start_incoming_migration(&channel->u.rdma, errp);
-#endif
-    } else if (channel->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
-        exec_start_incoming_migration(channel->u.exec.args, errp);
+    } else if (addrs->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
+        rdma_start_incoming_migration(&addrs->u.rdma, errp);
+ #endif
+    } else if (addrs->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
+        exec_start_incoming_migration(addrs->u.exec.args, errp);
     } else {
         error_setg(errp, "unknown migration protocol: %s", uri);
     }
@@ -1699,7 +1712,8 @@ void qmp_migrate(const char *uri, bool has_channels,
 {
     Error *local_err = NULL;
     MigrationState *s = migrate_get_current();
-    g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
+    g_autoptr(MigrationChannel) channel = g_new0(MigrationChannel, 1);
+    g_autoptr(MigrationAddress) addrs = g_new0(MigrationAddress, 1);
 
     /*
      * Having preliminary checks for uri and channel
@@ -1709,14 +1723,23 @@ void qmp_migrate(const char *uri, bool has_channels,
                    "exclusive; exactly one of the two should be present in "
                    "'migrate' qmp command ");
         return;
-    }
-
-    if (!migrate_uri_parse(uri, &channel, errp)) {
-        return;
+    } else if (channels) {
+        /* To verify that Migrate channel list has only item */
+        if (channels->next) {
+            error_setg(errp, "Channel list has more than one entries");
+            return;
+        }
+        channel = channels->value;
+        addrs = channel->addr;
+    } else {
+        /* caller uses the old URI syntax */
+        if (uri && !migrate_uri_parse(uri, &channel, errp)) {
+            return;
+        }
     }
 
     /* transport mechanism not suitable for migration? */
-    if (!migration_channels_and_transport_compatible(channel, errp)) {
+    if (!migration_channels_and_transport_compatible(addrs, errp)) {
         return;
     }
 
@@ -1732,8 +1755,8 @@ void qmp_migrate(const char *uri, bool has_channels,
         }
     }
 
-    if (channel->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
-        SocketAddress *saddr = &channel->u.socket;
+    if (addrs->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
+        SocketAddress *saddr = &addrs->u.socket;
         if (saddr->type == SOCKET_ADDRESS_TYPE_INET ||
             saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
             saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) {
@@ -1742,11 +1765,11 @@ void qmp_migrate(const char *uri, bool has_channels,
             fd_start_outgoing_migration(s, saddr->u.fd.str, &local_err);
         }
 #ifdef CONFIG_RDMA
-    } else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
-        rdma_start_outgoing_migration(s, &channel->u.rdma, &local_err);
+    } else if (addrs->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
+        rdma_start_outgoing_migration(s, &addrs->u.rdma, &local_err);
 #endif
-    } else if (channel->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
-        exec_start_outgoing_migration(s, channel->u.exec.args, &local_err);
+    } else if (addrs->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
+        exec_start_outgoing_migration(s, addrs->u.exec.args, &local_err);
     } else {
         if (!(has_resume && resume)) {
             yank_unregister_instance(MIGRATION_YANK_INSTANCE);
diff --git a/migration/socket.c b/migration/socket.c
index 8e7430b266..98e3ea1514 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -28,6 +28,8 @@
 #include "trace.h"
 #include "postcopy-ram.h"
 #include "options.h"
+#include "qapi/clone-visitor.h"
+#include "qapi/qapi-visit-sockets.h"
 
 struct SocketOutgoingArgs {
     SocketAddress *saddr;
@@ -114,12 +116,13 @@ void socket_start_outgoing_migration(MigrationState *s,
 {
     QIOChannelSocket *sioc = qio_channel_socket_new();
     struct SocketConnectData *data = g_new0(struct SocketConnectData, 1);
+    SocketAddress *addr = QAPI_CLONE(SocketAddress, saddr);
 
     data->s = s;
 
     /* in case previous migration leaked it */
     qapi_free_SocketAddress(outgoing_args.saddr);
-    outgoing_args.saddr = saddr;
+    outgoing_args.saddr = addr;
 
     if (saddr->type == SOCKET_ADDRESS_TYPE_INET) {
         data->hostname = g_strdup(saddr->u.inet.host);
-- 
2.22.3



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v6 9/9] migration: adding test case for modified QAPI syntax
  2023-06-06 10:15 [PATCH v6 0/9] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
                   ` (7 preceding siblings ...)
  2023-06-06 10:15 ` [PATCH v6 8/9] migration: Introduced MigrateChannelList struct to migration code flow Het Gala
@ 2023-06-06 10:15 ` Het Gala
  8 siblings, 0 replies; 18+ messages in thread
From: Het Gala @ 2023-06-06 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran, Het Gala

Adding multifd tcp common test case for modified QAPI syntax defined.

Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
 tests/qtest/migration-test.c | 45 ++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index b0c355bbd9..310484f6d5 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2059,6 +2059,32 @@ test_migrate_precopy_tcp_multifd_start_common(QTestState *from,
     return NULL;
 }
 
+static void *
+test_migrate_precopy_tcp_multifd_start_new_syntax_common(QTestState *from,
+                                                         QTestState *to,
+                                                         const char *method)
+{
+    migrate_set_parameter_int(from, "multifd-channels", 16);
+    migrate_set_parameter_int(to, "multifd-channels", 16);
+
+    migrate_set_parameter_str(from, "multifd-compression", method);
+    migrate_set_parameter_str(to, "multifd-compression", method);
+
+    migrate_set_capability(from, "multifd", true);
+    migrate_set_capability(to, "multifd", true);
+
+    /* Start incoming migration from the 1st socket */
+    qtest_qmp_assert_success(to, "{ 'execute': 'migrate-incoming',"
+                             "  'arguments': { "
+                             "      'channels': [ { 'channeltype': 'main',"
+                             "      'addr': { 'transport': 'socket',"
+                             "                'type': 'inet',"
+                             "                'host': '127.0.0.1',"
+                             "                'port': '0' } } ] } }");
+
+    return NULL;
+}
+
 static void *
 test_migrate_precopy_tcp_multifd_start(QTestState *from,
                                        QTestState *to)
@@ -2066,6 +2092,14 @@ test_migrate_precopy_tcp_multifd_start(QTestState *from,
     return test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
 }
 
+static void *
+test_migrate_precopy_tcp_multifd_new_syntax_start(QTestState *from,
+                                                  QTestState *to)
+{
+    return test_migrate_precopy_tcp_multifd_start_new_syntax_common(from,
+                                                              to, "none");
+}
+
 static void *
 test_migrate_precopy_tcp_multifd_zlib_start(QTestState *from,
                                             QTestState *to)
@@ -2097,6 +2131,15 @@ static void test_multifd_tcp_none(void)
     test_precopy_common(&args);
 }
 
+static void test_multifd_tcp_new_syntax_none(void)
+{
+    MigrateCommon args = {
+        .listen_uri = "defer",
+        .start_hook = test_migrate_precopy_tcp_multifd_new_syntax_start,
+    };
+    test_precopy_common(&args);
+}
+
 static void test_multifd_tcp_zlib(void)
 {
     MigrateCommon args = {
@@ -2778,6 +2821,8 @@ int main(int argc, char **argv)
     }
     qtest_add_func("/migration/multifd/tcp/plain/none",
                    test_multifd_tcp_none);
+    qtest_add_func("/migration/multifd/tcp/plain/none",
+                   test_multifd_tcp_new_syntax_none);
     /*
      * This test is flaky and sometimes fails in CI and otherwise:
      * don't run unless user opts in via environment variable.
-- 
2.22.3



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v6 6/9] migration: modified migration QAPIs to accept 'channels' argument for migration
  2023-06-06 10:15 ` [PATCH v6 6/9] migration: modified migration QAPIs to accept 'channels' argument for migration Het Gala
@ 2023-06-08 11:52   ` Het Gala
  0 siblings, 0 replies; 18+ messages in thread
From: Het Gala @ 2023-06-08 11:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran


On 06/06/23 3:45 pm, Het Gala wrote:
> MigrateChannelList allows to connect accross multiple interfaces. Added
> MigrateChannelList struct as argument to migration QAPIs.
>
> Future patchset series plans to include multiple MigrateChannels
> for multiple interfaces to be connected. That is the reason,
> 'MigrateChannelList'
> is the preferred choice of argument over 'MigrateChannel' and making
> migration QAPIs future proof.
>
> For current patchset series, have limited the size of the list to single
> element (single interface) as runtime check.
>
> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> ---
>   migration/migration-hmp-cmds.c |  16 +++--
>   migration/migration.c          |  34 ++++++++--
>   qapi/migration.json            | 109 ++++++++++++++++++++++++++++++++-
>   softmmu/vl.c                   |   2 +-
>   4 files changed, 147 insertions(+), 14 deletions(-)
>
> diff --git a/qapi/migration.json b/qapi/migration.json
> index e61d25eba2..7d4160e130 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1452,6 +1452,34 @@
>       'exec': 'MigrationExecCommand',
>       'rdma': 'InetSocketAddress' } }
>   
> +##
> +# @MigrationChannelType:
> +#
> +# The migration channel-type request options.
> +#
> +# @main: Main outbound migration channel.
> +#
> +# Since 8.1
> +##
> +{ 'enum': 'MigrationChannelType',
> +  'data': [ 'main' ] }
> +
> +##
> +# @MigrationChannel:
> +#
> +# Migration stream channel parameters.
> +#
> +# @channel-type: Channel type for transfering packet information.
> +#
> +# @addr: Migration endpoint configuration on destination interface.
> +#
> +# Since 8.1
> +##
> +{ 'struct': 'MigrationChannel',
> +  'data': {
> +      'channel-type': 'MigrationChannelType',
> +      'addr': 'MigrationAddress' } }
> +
>   ##
>   # @migrate:
>   #
> @@ -1459,6 +1487,9 @@
>   #
>   # @uri: the Uniform Resource Identifier of the destination VM
>   #
> +# @channels: list of migration stream channels with each stream
> +#     in the list connected to a destination interface endpoint.
> +#
>   # @blk: do block migration (full disk copy)
>   #
>   # @inc: incremental disk copy migration
> @@ -1483,14 +1514,50 @@
>   # 3. The user Monitor's "detach" argument is invalid in QMP and should
>   #    not be used
>   #
> +# 4. The uri argument should have the Uniform Resource Identifier of
> +#    default destination VM. This connection will be bound to default
> +#    network.
> +#
> +# 5. For now, number of migration streams is restricted to one, i.e
> +#    number of items in 'channels' list is just 1.
> +#
> +# 6. The 'uri' and 'channels' arguments are mutually exclusive;
> +#    exactly one of the two should be present.
> +#
>   # Example:
>   #
>   # -> { "execute": "migrate", "arguments": { "uri": "tcp:0:4446" } }
>   # <- { "return": {} }
> +# -> { "execute": "migrate",
> +#      "arguments": {
> +#          "channels": [ { "channel-type": "main",
> +#                          "addr": { "transport": "socket",
> +#                                    "type": "inet",
> +#                                    "host": "10.12.34.9",
> +#                                    "port": "1050" } } ] } }
> +# <- { "return": {} }
> +#
> +# -> { "execute": "migrate",
> +#      "arguments": {
> +#          "channels": [ { "channel-type": "main",
> +#                          "addr": { "transport": "exec",
> +#                                    "args": [ "/bin/nc", "-p", "6000",
> +#                                              "/some/sock" ] } } ] } }
> +# <- { "return": {} }
> +#
> +# -> { "execute": "migrate",
> +#      "arguments": {
> +#          "channels": [ { "channel-type": "main",
> +#                          "addr": { "transport": "rdma",
> +#                                    "host": "10.12.34.9",
> +#                                    "port": "1050" } } ] } }
> +# <- { "return": {} }
> +#
>   ##
>   { 'command': 'migrate',
> -  'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool',
> -           '*detach': 'bool', '*resume': 'bool' } }
> +  'data': {'*uri': 'str', '*channels': [ 'MigrationChannel' ],
> +           '*blk': 'bool', '*inc': 'bool', '*detach': 'bool',
> +           '*resume': 'bool' } }
>   
>   ##
>   # @migrate-incoming:
> @@ -1501,6 +1568,9 @@
>   # @uri: The Uniform Resource Identifier identifying the source or
>   #     address to listen on
>   #
> +# @channels: list of migration stream channels with each stream
> +#     in the list connected to a destination interface endpoint.
> +#
>   # Returns: nothing on success
>   #
>   # Since: 2.3
> @@ -1516,13 +1586,46 @@
>   #
>   # 3. The uri format is the same as for -incoming
>   #
> +# 5. For now, number of migration streams is restricted to one, i.e
> +#    number of items in 'channels' list is just 1.
> +#
> +# 4. The 'uri' and 'channels' arguments are mutually exclusive;
> +#    exactly one of the two should be present.
> +#
>   # Example:
>   #
>   # -> { "execute": "migrate-incoming",
>   #      "arguments": { "uri": "tcp::4446" } }
>   # <- { "return": {} }
> +#
> +# -> { "execute": "migrate",
> +#      "arguments": {
> +#          "channels": [ { "channel-type": "main",
> +#                          "addr": { "transport": "socket",
> +#                                    "type": "inet",
> +#                                    "host": "10.12.34.9",
> +#                                    "port": "1050" } } ] } }
> +# <- { "return": {} }
> +#
> +# -> { "execute": "migrate",
> +#      "arguments": {
> +#          "channels": [ { "channel-type": "main",
> +#                          "addr": { "transport": "exec",
> +#                                    "args": [ "/bin/nc", "-p", "6000",
> +#                                              "/some/sock" ] } } ] } }
> +# <- { "return": {} }
> +#
> +# -> { "execute": "migrate",
> +#      "arguments": {
> +#          "channels": [ { "channel-type": "main",
> +#                          "addr": { "transport": "rdma",
> +#                                    "host": "10.12.34.9",
> +#                                    "port": "1050" } } ] } }
> +# <- { "return": {} }
>   ##
> -{ 'command': 'migrate-incoming', 'data': {'uri': 'str' } }
> +{ 'command': 'migrate-incoming',
> +             'data': {'*uri': 'str',
> +                      '*channels': [ 'MigrationChannel' ] } }
>   

Markus, I have tried to make the definitons short, compact and 
meaningful. Please have a look at it once again, and add your 
suggestions on what looks wrong. TIA!

Regards,
Het Gala


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v6 1/9] migration: introduced 'MigrateAddress' in QAPI for migration wire protocol.
  2023-06-06 10:15 ` [PATCH v6 1/9] migration: introduced 'MigrateAddress' in QAPI for migration wire protocol Het Gala
@ 2023-06-13  5:28   ` Het Gala
  2023-06-28 11:15     ` Het Gala
  2023-07-05 11:21   ` Markus Armbruster
  1 sibling, 1 reply; 18+ messages in thread
From: Het Gala @ 2023-06-13  5:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran


On 06/06/23 3:45 pm, Het Gala wrote:
> This patch introduces well defined MigrateAddress struct and its related
> child objects.
>
> The existing argument of 'migrate' and 'migrate-incoming' QAPI - 'uri'
> is of string type. The current migration flow follows double encoding
> scheme for  fetching migration parameters such as 'uri' and this is
> not an ideal design.
>
> Motive for intoducing struct level design is to prevent double encoding
> of QAPI arguments, as Qemu should be able to directly use the QAPI
> arguments without any level of encoding.
>
> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   qapi/migration.json | 45 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 45 insertions(+)
>
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 179af0c4d8..e61d25eba2 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1407,6 +1407,51 @@
>   ##
>   { 'command': 'migrate-continue', 'data': {'state': 'MigrationStatus'} }
>   
> +##
> +# @MigrationAddressType:
> +#
> +# The migration stream transport mechanisms.
> +#
> +# @socket: Migrate via socket.
> +#
> +# @exec: Direct the migration stream to another process.
> +#
> +# @rdma: Migrate via RDMA.
> +#
> +# Since 8.1
> +##
> +{ 'enum': 'MigrationAddressType',
> +  'data': ['socket', 'exec', 'rdma'] }
> +
> +##
> +# @MigrationExecCommand:
> +#
> +# @args: list of commands for migraton stream execution to a file.
> +#
> +# Notes:
> +#
> +# 1. @args[0] needs to be the path to the new program.
> +#
> +# Since 8.1
> +##
> +{ 'struct': 'MigrationExecCommand',
> +  'data': {'args': [ 'str' ] } }
> +
> +##
> +# @MigrationAddress:
> +#
> +# Migration endpoint configuration.
> +#
> +# Since 8.1
> +##
> +{ 'union': 'MigrationAddress',
> +  'base': { 'transport' : 'MigrationAddressType'},
> +  'discriminator': 'transport',
> +  'data': {
> +    'socket': 'SocketAddress',
> +    'exec': 'MigrationExecCommand',
> +    'rdma': 'InetSocketAddress' } }
> +
>   ##
>   # @migrate:
>   #

Hi maintainers, this is just a reminder mail for v6 patchset for 
modification the QAPI design for migration qapis - 'migrate' and 
'incoming-migrate'. From the last discussion, I have modified 
definitions specifically around QAPIs in patch 1 and 6, and have tried 
to make it short and consice and meaningful. Please have a look at it 
and suggest if any changes required. Tagging maintainers who have 
actively participated in the discussion in last few iterations : Markus, 
Daniel, Eric, Juan - but regardless other maintainers are also very well 
welcomed to share their opinions.

Regards,
Het Gala


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v6 1/9] migration: introduced 'MigrateAddress' in QAPI for migration wire protocol.
  2023-06-13  5:28   ` Het Gala
@ 2023-06-28 11:15     ` Het Gala
  0 siblings, 0 replies; 18+ messages in thread
From: Het Gala @ 2023-06-28 11:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran


On 13/06/23 10:58 am, Het Gala wrote:
>
> On 06/06/23 3:45 pm, Het Gala wrote:
>> This patch introduces well defined MigrateAddress struct and its related
>> child objects.
>>
>> The existing argument of 'migrate' and 'migrate-incoming' QAPI - 'uri'
>> is of string type. The current migration flow follows double encoding
>> scheme for  fetching migration parameters such as 'uri' and this is
>> not an ideal design.
>>
>> Motive for intoducing struct level design is to prevent double encoding
>> of QAPI arguments, as Qemu should be able to directly use the QAPI
>> arguments without any level of encoding.
>>
>> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
>> Signed-off-by: Het Gala <het.gala@nutanix.com>
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>   qapi/migration.json | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 45 insertions(+)
>>
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 179af0c4d8..e61d25eba2 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -1407,6 +1407,51 @@
>>   ##
>>   { 'command': 'migrate-continue', 'data': {'state': 
>> 'MigrationStatus'} }
>>   +##
>> +##
>> +# @MigrationAddress:
>> +#
>> +# Migration endpoint configuration.
>> +#
>> +# Since 8.1
>> +##
>> +{ 'union': 'MigrationAddress',
>> +  'base': { 'transport' : 'MigrationAddressType'},
>> +  'discriminator': 'transport',
>> +  'data': {
>> +    'socket': 'SocketAddress',
>> +    'exec': 'MigrationExecCommand',
>> +    'rdma': 'InetSocketAddress' } }
>> +
>>   ##
>>   # @migrate:
>>   #
>
> Hi maintainers, this is just a reminder mail for v6 patchset for 
> modification the QAPI design for migration qapis - 'migrate' and 
> 'incoming-migrate'. From the last discussion, I have modified 
> definitions specifically around QAPIs in patch 1 and 6, and have tried 
> to make it short and consice and meaningful. Please have a look at it 
> and suggest if any changes required. Tagging maintainers who have 
> actively participated in the discussion in last few iterations : 
> Markus, Daniel, Eric, Juan - but regardless other maintainers are also 
> very well welcomed to share their opinions.
>
This is just a friendly reminder mail for v6 patchset discusiion for 
modification of QAPI design for migration qapis. I know the maintainers 
were quite busy with KVM forum during the time I had sent out first 
reminder. Hope to get some positive reviews on the v6 patches soon. 
Thanks in advance !!

Regards,
Het Gala


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v6 1/9] migration: introduced 'MigrateAddress' in QAPI for migration wire protocol.
  2023-06-06 10:15 ` [PATCH v6 1/9] migration: introduced 'MigrateAddress' in QAPI for migration wire protocol Het Gala
  2023-06-13  5:28   ` Het Gala
@ 2023-07-05 11:21   ` Markus Armbruster
  2023-07-05 12:49     ` Het Gala
  1 sibling, 1 reply; 18+ messages in thread
From: Markus Armbruster @ 2023-07-05 11:21 UTC (permalink / raw)
  To: Het Gala
  Cc: qemu-devel, prerna.saxena, quintela, dgilbert, pbonzini, berrange,
	eblake, manish.mishra, aravind.retnakaran

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

> This patch introduces well defined MigrateAddress struct and its related
> child objects.
>
> The existing argument of 'migrate' and 'migrate-incoming' QAPI - 'uri'
> is of string type. The current migration flow follows double encoding
> scheme for  fetching migration parameters such as 'uri' and this is
> not an ideal design.
>
> Motive for intoducing struct level design is to prevent double encoding
> of QAPI arguments, as Qemu should be able to directly use the QAPI
> arguments without any level of encoding.
>
> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  qapi/migration.json | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 179af0c4d8..e61d25eba2 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1407,6 +1407,51 @@
>  ##
>  { 'command': 'migrate-continue', 'data': {'state': 'MigrationStatus'} }
>  
> +##
> +# @MigrationAddressType:
> +#
> +# The migration stream transport mechanisms.
> +#
> +# @socket: Migrate via socket.
> +#
> +# @exec: Direct the migration stream to another process.
> +#
> +# @rdma: Migrate via RDMA.
> +#
> +# Since 8.1
> +##
> +{ 'enum': 'MigrationAddressType',
> +  'data': ['socket', 'exec', 'rdma'] }
> +
> +##
> +# @MigrationExecCommand:
> +#
> +# @args: list of commands for migraton stream execution to a file.

Typo: migration

> +#
> +# Notes:
> +#
> +# 1. @args[0] needs to be the path to the new program.

@args can't be a "list of commands", as we're spawning just one process.
So what is it?

Digging through the code with the entire series applied...  Member @args
is used in two places:

1. qemu_start_incoming_migration() passes it to
   exec_start_incoming_migration(), which translates it into an array
   and passes it to qio_channel_command_new_spawn().

2. qmp_migrate() passes it to exec_start_outgoing_migration(), which
   does the same.

qio_channel_command_new_spawn() passes it to
g_spawn_async_with_pipes().  A close read of the latter's documentation
leads me to:

* args[0] is the excutable's file name.  As usual, a relative name is
  relative to the QEMU process's current working directory.

* args[1..] are the arguments.

Unlike POSIX interfaces like execv() and posix_spawn(), this doesn't
separate the executable's file name and 0-th argument.

In short, the head of @args is the executable's filename, and the
remainder are the arguments.  The fact that the the executable's
filename is passed as 0-th argument to the child process is detail.

Perhaps this could do:

   ##
   # @MigrationExecCommand:
   #
   # @args: command and arguments to execute.

If we want more detail, perhaps:

   # @args: command (list head) and arguments (list tail) to execute.

Not sure we need it.  Thoughts?

> +#
> +# Since 8.1
> +##
> +{ 'struct': 'MigrationExecCommand',
> +  'data': {'args': [ 'str' ] } }
> +
> +##
> +# @MigrationAddress:
> +#
> +# Migration endpoint configuration.
> +#
> +# Since 8.1
> +##
> +{ 'union': 'MigrationAddress',
> +  'base': { 'transport' : 'MigrationAddressType'},
> +  'discriminator': 'transport',
> +  'data': {
> +    'socket': 'SocketAddress',
> +    'exec': 'MigrationExecCommand',
> +    'rdma': 'InetSocketAddress' } }
> +
>  ##
>  # @migrate:
>  #



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v6 2/9] migration: convert uri parameter into 'MigrateAddress' struct
  2023-06-06 10:15 ` [PATCH v6 2/9] migration: convert uri parameter into 'MigrateAddress' struct Het Gala
@ 2023-07-05 11:29   ` Markus Armbruster
  2023-07-05 13:18     ` Het Gala
  0 siblings, 1 reply; 18+ messages in thread
From: Markus Armbruster @ 2023-07-05 11:29 UTC (permalink / raw)
  To: Het Gala
  Cc: qemu-devel, prerna.saxena, quintela, dgilbert, pbonzini, berrange,
	armbru, eblake, manish.mishra, aravind.retnakaran

Drive-by comment...

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

> This patch parses 'migrate' and 'migrate-incoming' QAPI's 'uri' parameter
> with all the migration connection related information and stores them
> inside well defined 'MigrateAddress' struct.
>
> Misc: limit line width in exec.c to 80 characters recommended by Qemu.
>
> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
> Signed-off-by: Het Gala <het.gala@nutanix.com>

[...]

> diff --git a/migration/migration.c b/migration/migration.c
> index dc05c6f6ea..0eb25bb5a4 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -64,6 +64,7 @@
>  #include "yank_functions.h"
>  #include "sysemu/qtest.h"
>  #include "options.h"
> +#include "qemu/sockets.h"
>  
>  static NotifierList migration_state_notifiers =
>      NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
> @@ -420,15 +421,63 @@ void migrate_add_address(SocketAddress *address)
>                        QAPI_CLONE(SocketAddress, address));
>  }
>  
> +static bool migrate_uri_parse(const char *uri,
> +                              MigrationAddress **channel,
> +                              Error **errp)
> +{
> +    g_autoptr(MigrationAddress) addrs = g_new0(MigrationAddress, 1);

@addrs suggests plural, yet it points to a single MigrationAddress.
Recommend @addr.

Same elsewhere.

[...]



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v6 1/9] migration: introduced 'MigrateAddress' in QAPI for migration wire protocol.
  2023-07-05 11:21   ` Markus Armbruster
@ 2023-07-05 12:49     ` Het Gala
  2023-07-05 12:58       ` Markus Armbruster
  0 siblings, 1 reply; 18+ messages in thread
From: Het Gala @ 2023-07-05 12:49 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: qemu-devel, prerna.saxena, quintela, dgilbert, pbonzini, berrange,
	eblake, manish.mishra, aravind.retnakaran


On 05/07/23 4:51 pm, Markus Armbruster wrote:
> Het Gala <het.gala@nutanix.com> writes:
>
>> This patch introduces well defined MigrateAddress struct and its related
>> child objects.
>>
>> The existing argument of 'migrate' and 'migrate-incoming' QAPI - 'uri'
>> is of string type. The current migration flow follows double encoding
>> scheme for  fetching migration parameters such as 'uri' and this is
>> not an ideal design.
>>
>> Motive for intoducing struct level design is to prevent double encoding
>> of QAPI arguments, as Qemu should be able to directly use the QAPI
>> arguments without any level of encoding.
>>
>> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
>> Signed-off-by: Het Gala <het.gala@nutanix.com>
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>   qapi/migration.json | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 45 insertions(+)
>>
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 179af0c4d8..e61d25eba2 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -1407,6 +1407,51 @@
>>   ##
>>   { 'command': 'migrate-continue', 'data': {'state': 'MigrationStatus'} }
>>   
>> +##
>> +# @MigrationAddressType:
>> +#
>> +# The migration stream transport mechanisms.
>> +#
>> +# @socket: Migrate via socket.
>> +#
>> +# @exec: Direct the migration stream to another process.
>> +#
>> +# @rdma: Migrate via RDMA.
>> +#
>> +# Since 8.1
>> +##
>> +{ 'enum': 'MigrationAddressType',
>> +  'data': ['socket', 'exec', 'rdma'] }
>> +
>> +##
>> +# @MigrationExecCommand:
>> +#
>> +# @args: list of commands for migraton stream execution to a file.
> Typo: migration
>
>> +#
>> +# Notes:
>> +#
>> +# 1. @args[0] needs to be the path to the new program.
> @args can't be a "list of commands", as we're spawning just one process.
> So what is it?
>
> Digging through the code with the entire series applied...  Member @args
> is used in two places:
>
> 1. qemu_start_incoming_migration() passes it to
>     exec_start_incoming_migration(), which translates it into an array
>     and passes it to qio_channel_command_new_spawn().
>
> 2. qmp_migrate() passes it to exec_start_outgoing_migration(), which
>     does the same.
>
> qio_channel_command_new_spawn() passes it to
> g_spawn_async_with_pipes().  A close read of the latter's documentation
> leads me to:
>
> * args[0] is the excutable's file name.  As usual, a relative name is
>    relative to the QEMU process's current working directory.
>
> * args[1..] are the arguments.
>
> Unlike POSIX interfaces like execv() and posix_spawn(), this doesn't
> separate the executable's file name and 0-th argument.
>
> In short, the head of @args is the executable's filename, and the
> remainder are the arguments.  The fact that the the executable's
> filename is passed as 0-th argument to the child process is detail.
>
> Perhaps this could do:
>
>     ##
>     # @MigrationExecCommand:
>     #
>     # @args: command and arguments to execute.
>
> If we want more detail, perhaps:
>
>     # @args: command (list head) and arguments (list tail) to execute.
>
> Not sure we need it.  Thoughts?
 From a user prespective, I think defining that command / executable's 
filename is the list head would be good ? something like

@args: command (list head) and arguments to execute.

>> +#
>> +# Since 8.1
>> +##
>> +{ 'struct': 'MigrationExecCommand',
>> +  'data': {'args': [ 'str' ] } }
>> +
>> +##
>> +# @MigrationAddress:
>> +#
>> +# Migration endpoint configuration.
>> +#
>> +# Since 8.1
>> +##
>> +{ 'union': 'MigrationAddress',
>> +  'base': { 'transport' : 'MigrationAddressType'},
>> +  'discriminator': 'transport',
>> +  'data': {
>> +    'socket': 'SocketAddress',
>> +    'exec': 'MigrationExecCommand',
>> +    'rdma': 'InetSocketAddress' } }
>> +
>>   ##
>>   # @migrate:
>>   #
Regards,
Het Gala


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v6 1/9] migration: introduced 'MigrateAddress' in QAPI for migration wire protocol.
  2023-07-05 12:49     ` Het Gala
@ 2023-07-05 12:58       ` Markus Armbruster
  0 siblings, 0 replies; 18+ messages in thread
From: Markus Armbruster @ 2023-07-05 12:58 UTC (permalink / raw)
  To: Het Gala
  Cc: qemu-devel, prerna.saxena, quintela, dgilbert, pbonzini, berrange,
	eblake, manish.mishra, aravind.retnakaran

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

> On 05/07/23 4:51 pm, Markus Armbruster wrote:
>> Het Gala <het.gala@nutanix.com> writes:
>>
>>> This patch introduces well defined MigrateAddress struct and its related
>>> child objects.
>>>
>>> The existing argument of 'migrate' and 'migrate-incoming' QAPI - 'uri'
>>> is of string type. The current migration flow follows double encoding
>>> scheme for  fetching migration parameters such as 'uri' and this is
>>> not an ideal design.
>>>
>>> Motive for intoducing struct level design is to prevent double encoding
>>> of QAPI arguments, as Qemu should be able to directly use the QAPI
>>> arguments without any level of encoding.
>>>
>>> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
>>> Signed-off-by: Het Gala <het.gala@nutanix.com>
>>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>> ---
>>>   qapi/migration.json | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 45 insertions(+)
>>>
>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>> index 179af0c4d8..e61d25eba2 100644
>>> --- a/qapi/migration.json
>>> +++ b/qapi/migration.json
>>> @@ -1407,6 +1407,51 @@
>>> ##
>>> { 'command': 'migrate-continue', 'data': {'state': 'MigrationStatus'} }
>>> +##
>>> +# @MigrationAddressType:
>>> +#
>>> +# The migration stream transport mechanisms.
>>> +#
>>> +# @socket: Migrate via socket.
>>> +#
>>> +# @exec: Direct the migration stream to another process.
>>> +#
>>> +# @rdma: Migrate via RDMA.
>>> +#
>>> +# Since 8.1
>>> +##
>>> +{ 'enum': 'MigrationAddressType',
>>> +  'data': ['socket', 'exec', 'rdma'] }
>>> +
>>> +##
>>> +# @MigrationExecCommand:
>>> +#
>>> +# @args: list of commands for migraton stream execution to a file.
>>
>> Typo: migration
>>
>>> +#
>>> +# Notes:
>>> +#
>>> +# 1. @args[0] needs to be the path to the new program.
>>
>> @args can't be a "list of commands", as we're spawning just one process.
>> So what is it?
>>
>> Digging through the code with the entire series applied...  Member @args
>> is used in two places:
>>
>> 1. qemu_start_incoming_migration() passes it to
>>     exec_start_incoming_migration(), which translates it into an array
>>     and passes it to qio_channel_command_new_spawn().
>>
>> 2. qmp_migrate() passes it to exec_start_outgoing_migration(), which
>>     does the same.
>>
>> qio_channel_command_new_spawn() passes it to
>> g_spawn_async_with_pipes().  A close read of the latter's documentation
>> leads me to:
>>
>> * args[0] is the excutable's file name.  As usual, a relative name is
>>    relative to the QEMU process's current working directory.
>>
>> * args[1..] are the arguments.
>>
>> Unlike POSIX interfaces like execv() and posix_spawn(), this doesn't
>> separate the executable's file name and 0-th argument.
>>
>> In short, the head of @args is the executable's filename, and the
>> remainder are the arguments.  The fact that the the executable's
>> filename is passed as 0-th argument to the child process is detail.
>>
>> Perhaps this could do:
>>
>>     ##
>>     # @MigrationExecCommand:
>>     #
>>     # @args: command and arguments to execute.
>>
>> If we want more detail, perhaps:
>>
>>     # @args: command (list head) and arguments (list tail) to execute.
>>
>> Not sure we need it.  Thoughts?
>
> From a user prespective, I think defining that command / executable's filename is the list head would be good ? something like
>
> @args: command (list head) and arguments to execute.

Works for me.

[...]



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v6 2/9] migration: convert uri parameter into 'MigrateAddress' struct
  2023-07-05 11:29   ` Markus Armbruster
@ 2023-07-05 13:18     ` Het Gala
  0 siblings, 0 replies; 18+ messages in thread
From: Het Gala @ 2023-07-05 13:18 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: qemu-devel, prerna.saxena, quintela, dgilbert, pbonzini, berrange,
	eblake, manish.mishra, aravind.retnakaran


On 05/07/23 4:59 pm, Markus Armbruster wrote:
> Drive-by comment...
>
> Het Gala <het.gala@nutanix.com> writes:
>
>> This patch parses 'migrate' and 'migrate-incoming' QAPI's 'uri' parameter
>> with all the migration connection related information and stores them
>> inside well defined 'MigrateAddress' struct.
>>
>> Misc: limit line width in exec.c to 80 characters recommended by Qemu.
>>
>> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
>> Signed-off-by: Het Gala <het.gala@nutanix.com>
> [...]
>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index dc05c6f6ea..0eb25bb5a4 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -64,6 +64,7 @@
>>   #include "yank_functions.h"
>>   #include "sysemu/qtest.h"
>>   #include "options.h"
>> +#include "qemu/sockets.h"
>>   
>>   static NotifierList migration_state_notifiers =
>>       NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
>> @@ -420,15 +421,63 @@ void migrate_add_address(SocketAddress *address)
>>                         QAPI_CLONE(SocketAddress, address));
>>   }
>>   
>> +static bool migrate_uri_parse(const char *uri,
>> +                              MigrationAddress **channel,
>> +                              Error **errp)
>> +{
>> +    g_autoptr(MigrationAddress) addrs = g_new0(MigrationAddress, 1);
> @addrs suggests plural, yet it points to a single MigrationAddress.
> Recommend @addr.
>
> Same elsewhere.
Ack.
> [...]
Regards,
Het Gala


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-07-05 13:19 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-06 10:15 [PATCH v6 0/9] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
2023-06-06 10:15 ` [PATCH v6 1/9] migration: introduced 'MigrateAddress' in QAPI for migration wire protocol Het Gala
2023-06-13  5:28   ` Het Gala
2023-06-28 11:15     ` Het Gala
2023-07-05 11:21   ` Markus Armbruster
2023-07-05 12:49     ` Het Gala
2023-07-05 12:58       ` Markus Armbruster
2023-06-06 10:15 ` [PATCH v6 2/9] migration: convert uri parameter into 'MigrateAddress' struct Het Gala
2023-07-05 11:29   ` Markus Armbruster
2023-07-05 13:18     ` Het Gala
2023-06-06 10:15 ` [PATCH v6 3/9] migration: convert socket backend to accept MigrateAddress struct Het Gala
2023-06-06 10:15 ` [PATCH v6 4/9] migration: convert rdma " Het Gala
2023-06-06 10:15 ` [PATCH v6 5/9] migration: convert exec " Het Gala
2023-06-06 10:15 ` [PATCH v6 6/9] migration: modified migration QAPIs to accept 'channels' argument for migration Het Gala
2023-06-08 11:52   ` Het Gala
2023-06-06 10:15 ` [PATCH v6 7/9] migration: modify migration_channels_and_uri_compatible() to incorporate newer migration QAPI syntax Het Gala
2023-06-06 10:15 ` [PATCH v6 8/9] migration: Introduced MigrateChannelList struct to migration code flow Het Gala
2023-06-06 10:15 ` [PATCH v6 9/9] migration: adding test case for modified QAPI syntax Het Gala

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).