All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration
@ 2023-10-09 14:36 Het Gala
  2023-10-09 14:36 ` [PATCH v12 01/10] migration: New QAPI type 'MigrateAddress' Het Gala
                   ` (10 more replies)
  0 siblings, 11 replies; 19+ messages in thread
From: Het Gala @ 2023-10-09 14:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, farosas, manish.mishra, aravind.retnakaran, Het Gala

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

Would like to thank all the maintainers that actively participated in the v11
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
v6: https://lists.gnu.org/archive/html/qemu-devel/2023-06/msg01251.html
v7: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg02027.html
v8: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg02770.html
v9: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg04216.html
v10: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg05022.html
v11: https://lists.gnu.org/archive/html/qemu-devel/2023-10/msg00740.html

v11 -> v12 changelog:
-------------------
- Resolved double-freeing when using g_autoptr in structures that are
  nested into another.
- Overwriting of pointers to an existing allocated memory is solved at
  various places.
- Use of g_autoptr caused make check errors in non-error path while going
  out of scope inside function. Added g_steal_pointer() in the non-error
  paths wherever required.

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 (10):
  migration: New QAPI type 'MigrateAddress'
  migration: convert migration 'uri' into 'MigrateAddress'
  migration: convert socket backend to accept MigrateAddress
  migration: convert rdma backend to accept MigrateAddress
  migration: convert exec backend to accept MigrateAddress.
  migration: New migrate and migrate-incoming argument 'channels'
  migration: modify migration_channels_and_uri_compatible() for new QAPI
    syntax
  migration: Implement MigrateChannelList to qmp migration flow.
  migration: Implement MigrateChannelList to hmp migration flow.
  migration: modify test_multifd_tcp_none() to use new QAPI syntax.

 migration/exec.c               |  74 +++++++++----
 migration/exec.h               |   8 +-
 migration/migration-hmp-cmds.c |  17 ++-
 migration/migration.c          | 189 +++++++++++++++++++++++++++------
 migration/migration.h          |   3 +-
 migration/rdma.c               |  34 +++---
 migration/rdma.h               |   6 +-
 migration/socket.c             |  39 ++-----
 migration/socket.h             |   7 +-
 qapi/migration.json            | 150 +++++++++++++++++++++++++-
 softmmu/vl.c                   |   2 +-
 tests/qtest/migration-test.c   |   7 +-
 12 files changed, 412 insertions(+), 124 deletions(-)

-- 
2.22.3



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

* [PATCH v12 01/10] migration: New QAPI type 'MigrateAddress'
  2023-10-09 14:36 [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
@ 2023-10-09 14:36 ` Het Gala
  2023-10-09 14:36 ` [PATCH v12 02/10] migration: convert migration 'uri' into 'MigrateAddress' Het Gala
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Het Gala @ 2023-10-09 14:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, farosas, 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 type string. The current implementation follows
double encoding scheme for fetching migration parameters like
'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.

Note: this commit only adds the type, and actual uses comes
in later commits.

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>
Acked-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/migration.json | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/qapi/migration.json b/qapi/migration.json
index 8843e74b59..4e4c39a9bd 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1478,6 +1478,47 @@
 ##
 { '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.2
+##
+{ 'enum': 'MigrationAddressType',
+  'data': ['socket', 'exec', 'rdma'] }
+
+##
+# @MigrationExecCommand:
+#
+# @args: command (list head) and arguments to execute.
+#
+# Since 8.2
+##
+{ 'struct': 'MigrationExecCommand',
+  'data': {'args': [ 'str' ] } }
+
+##
+# @MigrationAddress:
+#
+# Migration endpoint configuration.
+#
+# Since 8.2
+##
+{ 'union': 'MigrationAddress',
+  'base': { 'transport' : 'MigrationAddressType'},
+  'discriminator': 'transport',
+  'data': {
+    'socket': 'SocketAddress',
+    'exec': 'MigrationExecCommand',
+    'rdma': 'InetSocketAddress' } }
+
 ##
 # @migrate:
 #
-- 
2.22.3



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

* [PATCH v12 02/10] migration: convert migration 'uri' into 'MigrateAddress'
  2023-10-09 14:36 [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
  2023-10-09 14:36 ` [PATCH v12 01/10] migration: New QAPI type 'MigrateAddress' Het Gala
@ 2023-10-09 14:36 ` Het Gala
  2023-10-09 14:36 ` [PATCH v12 03/10] migration: convert socket backend to accept MigrateAddress Het Gala
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Het Gala @ 2023-10-09 14:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, farosas, manish.mishra, aravind.retnakaran, Het Gala

This patch parses 'migrate' and 'migrate-incoming' QAPI's 'uri'
string containing migration connection related information
and stores them inside well defined 'MigrateAddress' struct.

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

diff --git a/migration/exec.c b/migration/exec.c
index 2bf882bbe1..32f5143dfd 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);
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 585d3c8f55..89fa074e8c 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -66,6 +66,7 @@
 #include "sysemu/qtest.h"
 #include "options.h"
 #include "sysemu/dirtylimit.h"
+#include "qemu/sockets.h"
 
 static NotifierList migration_state_notifiers =
     NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -428,15 +429,65 @@ 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) addr = g_new0(MigrationAddress, 1);
+    SocketAddress *saddr = NULL;
+    InetSocketAddress *isock = &addr->u.rdma;
+    strList **tail = &addr->u.exec.args;
+
+    if (strstart(uri, "exec:", NULL)) {
+        addr->transport = MIGRATION_ADDRESS_TYPE_EXEC;
+#ifdef WIN32
+        QAPI_LIST_APPEND(tail, g_strdup(exec_get_cmd_path()));
+        QAPI_LIST_APPEND(tail, g_strdup("/c"));
+#else
+        QAPI_LIST_APPEND(tail, g_strdup("/bin/sh"));
+        QAPI_LIST_APPEND(tail, g_strdup("-c"));
+#endif
+        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;
+        }
+        addr->transport = MIGRATION_ADDRESS_TYPE_RDMA;
+    } else if (strstart(uri, "tcp:", NULL) ||
+                strstart(uri, "unix:", NULL) ||
+                strstart(uri, "vsock:", NULL) ||
+                strstart(uri, "fd:", NULL)) {
+        addr->transport = MIGRATION_ADDRESS_TYPE_SOCKET;
+        saddr = socket_parse(uri, errp);
+        if (!saddr) {
+            return false;
+        }
+        addr->u.socket.type = saddr->type;
+        addr->u.socket.u = saddr->u;
+    } else {
+        error_setg(errp, "unknown migration protocol: %s", uri);
+        return false;
+    }
+
+    *channel = g_steal_pointer(&addr);
+    return true;
+}
+
 static void qemu_start_incoming_migration(const char *uri, Error **errp)
 {
     const char *p = NULL;
+    g_autoptr(MigrationAddress) channel = NULL;
 
     /* 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) ||
@@ -1674,12 +1725,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 = NULL;
 
     /* URI is not suitable for migration? */
     if (!migration_channels_and_uri_compatible(uri, errp)) {
         return;
     }
 
+    if (!migrate_uri_parse(uri, &channel, errp)) {
+        return;
+    }
+
     resume_requested = has_resume && resume;
     if (!migrate_prepare(s, has_blk && blk, has_inc && inc,
                          resume_requested, errp)) {
-- 
2.22.3



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

* [PATCH v12 03/10] migration: convert socket backend to accept MigrateAddress
  2023-10-09 14:36 [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
  2023-10-09 14:36 ` [PATCH v12 01/10] migration: New QAPI type 'MigrateAddress' Het Gala
  2023-10-09 14:36 ` [PATCH v12 02/10] migration: convert migration 'uri' into 'MigrateAddress' Het Gala
@ 2023-10-09 14:36 ` Het Gala
  2023-10-09 14:36 ` [PATCH v12 04/10] migration: convert rdma " Het Gala
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Het Gala @ 2023-10-09 14:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, farosas, 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>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 migration/migration.c | 30 ++++++++++++++++++------------
 migration/socket.c    | 39 +++++++++------------------------------
 migration/socket.h    |  7 ++++---
 3 files changed, 31 insertions(+), 45 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 89fa074e8c..c9961db1c9 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -489,18 +489,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 if (strstart(uri, "file:", &p)) {
         file_start_incoming_migration(p, errp);
     } else {
@@ -1749,18 +1752,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 (strstart(uri, "file:", &p)) {
         file_start_outgoing_migration(s, p, &local_err);
     } else {
diff --git a/migration/socket.c b/migration/socket.c
index 1b6f5baefb..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;
@@ -108,19 +110,19 @@ 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);
+    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);
@@ -135,18 +137,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 +162,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 +202,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] 19+ messages in thread

* [PATCH v12 04/10] migration: convert rdma backend to accept MigrateAddress
  2023-10-09 14:36 [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
                   ` (2 preceding siblings ...)
  2023-10-09 14:36 ` [PATCH v12 03/10] migration: convert socket backend to accept MigrateAddress Het Gala
@ 2023-10-09 14:36 ` Het Gala
  2023-10-09 14:36 ` [PATCH v12 05/10] migration: convert exec " Het Gala
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Het Gala @ 2023-10-09 14:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, farosas, 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>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.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 c9961db1c9..ac58b35d44 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -499,8 +499,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);
@@ -1762,8 +1762,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 cd5e1afe60..ab1b86532b 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];
 
@@ -2475,9 +2474,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;
 }
 
 
@@ -2759,28 +2756,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;
@@ -3368,6 +3354,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;
@@ -3383,13 +3370,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;
@@ -4121,7 +4111,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;
@@ -4167,13 +4158,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] 19+ messages in thread

* [PATCH v12 05/10] migration: convert exec backend to accept MigrateAddress.
  2023-10-09 14:36 [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
                   ` (3 preceding siblings ...)
  2023-10-09 14:36 ` [PATCH v12 04/10] migration: convert rdma " Het Gala
@ 2023-10-09 14:36 ` Het Gala
  2023-10-09 14:36 ` [PATCH v12 06/10] migration: New migrate and migrate-incoming argument 'channels' Het Gala
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Het Gala @ 2023-10-09 14:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, farosas, 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      | 73 +++++++++++++++++++++++++++++++------------
 migration/exec.h      |  4 +--
 migration/migration.c |  8 ++---
 3 files changed, 59 insertions(+), 26 deletions(-)

diff --git a/migration/exec.c b/migration/exec.c
index 32f5143dfd..b5361abc75 100644
--- a/migration/exec.c
+++ b/migration/exec.c
@@ -39,20 +39,51 @@ const char *exec_get_cmd_path(void)
 }
 #endif
 
-void exec_start_outgoing_migration(MigrationState *s, const char *command, Error **errp)
+/* 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 + 1);
 
-    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 *) g_steal_pointer (&argv),
+                            O_RDWR,
+                            errp));
     if (!ioc) {
         return;
     }
@@ -71,20 +102,22 @@ 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 + 1);
+
+    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 *) g_steal_pointer (&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 ac58b35d44..732fdadb11 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -502,8 +502,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 if (strstart(uri, "file:", &p)) {
         file_start_incoming_migration(p, errp);
     } else {
@@ -1765,8 +1765,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 (strstart(uri, "file:", &p)) {
         file_start_outgoing_migration(s, p, &local_err);
     } else {
-- 
2.22.3



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

* [PATCH v12 06/10] migration: New migrate and migrate-incoming argument 'channels'
  2023-10-09 14:36 [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
                   ` (4 preceding siblings ...)
  2023-10-09 14:36 ` [PATCH v12 05/10] migration: convert exec " Het Gala
@ 2023-10-09 14:36 ` Het Gala
  2023-10-09 14:36 ` [PATCH v12 07/10] migration: modify migration_channels_and_uri_compatible() for new QAPI syntax Het Gala
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Het Gala @ 2023-10-09 14:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, farosas, manish.mishra, aravind.retnakaran, Het Gala

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

We plan to include multiple channels in future, to connnect
multiple interfaces. Hence, we choose 'MigrateChannelList'
as the new argument over 'MigrateChannel' to make migration
QAPIs future proof.

Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 migration/migration-hmp-cmds.c |   6 +-
 migration/migration.c          |  56 +++++++++++++++--
 qapi/migration.json            | 109 ++++++++++++++++++++++++++++++++-
 softmmu/vl.c                   |   2 +-
 4 files changed, 161 insertions(+), 12 deletions(-)

diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index c115ef2d23..a2e6a5c51e 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -442,7 +442,7 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
     const char *uri = qdict_get_str(qdict, "uri");
 
-    qmp_migrate_incoming(uri, &err);
+    qmp_migrate_incoming(uri, false, NULL, &err);
 
     hmp_handle_error(mon, err);
 }
@@ -731,8 +731,8 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
     const char *uri = qdict_get_str(qdict, "uri");
     Error *err = NULL;
 
-    qmp_migrate(uri, !!blk, blk, !!inc, inc,
-                false, false, true, resume, &err);
+    qmp_migrate(uri, false, NULL, !!blk, blk, !!inc, inc,
+                 false, false, true, resume, &err);
     if (hmp_handle_error(mon, err)) {
         return;
     }
diff --git a/migration/migration.c b/migration/migration.c
index 732fdadb11..f3e7c338ec 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -474,11 +474,34 @@ 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)
 {
     const char *p = NULL;
     g_autoptr(MigrationAddress) channel = NULL;
 
+    /*
+     * Having preliminary checks for uri and channel
+     */
+    if (has_channels) {
+        error_setg(errp, "'channels' argument should not be set yet.");
+        return;
+    }
+
+    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;
+    }
+
+    if (!uri && !has_channels) {
+        error_setg(errp, "neither 'uri' or 'channels' argument are "
+                   "specified in 'migrate-incoming' qmp command ");
+        return;
+    }
+
     /* URI is not suitable for migration? */
     if (!migration_channels_and_uri_compatible(uri, errp)) {
         return;
@@ -1535,7 +1558,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;
@@ -1553,7 +1577,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);
@@ -1589,7 +1613,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)
@@ -1720,7 +1744,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)
 {
@@ -1730,6 +1755,27 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     const char *p = NULL;
     g_autoptr(MigrationAddress) channel = NULL;
 
+    /*
+     * Having preliminary checks for uri and channel
+     */
+    if (has_channels) {
+        error_setg(errp, "'channels' argument should not be set yet.");
+        return;
+    }
+
+    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;
+    }
+
+    if (!uri && !has_channels) {
+        error_setg(errp, "neither 'uri' or 'channels' argument are "
+                   "specified 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 4e4c39a9bd..7b84c04617 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1519,6 +1519,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:
 #
@@ -1526,6 +1554,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
@@ -1550,14 +1581,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:
@@ -1568,6 +1635,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
@@ -1583,13 +1653,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 98e071e63b..99ab0d3a51 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2690,7 +2690,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] 19+ messages in thread

* [PATCH v12 07/10] migration: modify migration_channels_and_uri_compatible() for new QAPI syntax
  2023-10-09 14:36 [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
                   ` (5 preceding siblings ...)
  2023-10-09 14:36 ` [PATCH v12 06/10] migration: New migrate and migrate-incoming argument 'channels' Het Gala
@ 2023-10-09 14:36 ` Het Gala
  2023-10-09 14:36 ` [PATCH v12 08/10] migration: Implement MigrateChannelList to qmp migration flow Het Gala
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Het Gala @ 2023-10-09 14:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, farosas, 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 modern MigrateChannel
QAPI syntax too.

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

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

diff --git a/migration/migration.c b/migration/migration.c
index f3e7c338ec..23c3a1079f 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -107,17 +107,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 *addr,
+                                            Error **errp)
 {
     if (migration_needs_multiple_sockets() &&
-        !uri_supports_multi_channels(uri)) {
+        (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET) &&
+        !transport_supports_multi_channels(&addr->u.socket)) {
         error_setg(errp, "Migration requires multi-channel URIs (e.g. tcp)");
         return false;
     }
@@ -502,12 +505,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;
     }
 
@@ -1776,12 +1779,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] 19+ messages in thread

* [PATCH v12 08/10] migration: Implement MigrateChannelList to qmp migration flow.
  2023-10-09 14:36 [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
                   ` (6 preceding siblings ...)
  2023-10-09 14:36 ` [PATCH v12 07/10] migration: modify migration_channels_and_uri_compatible() for new QAPI syntax Het Gala
@ 2023-10-09 14:36 ` Het Gala
  2023-10-09 14:36 ` [PATCH v12 09/10] migration: Implement MigrateChannelList to hmp " Het Gala
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Het Gala @ 2023-10-09 14:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, farosas, manish.mishra, aravind.retnakaran, Het Gala

Integrate MigrateChannelList with all transport backends
(socket, exec and rdma) for both src and dest migration
endpoints for qmp migration.

For current series, limit the size of MigrateChannelList
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.c | 95 +++++++++++++++++++++++--------------------
 1 file changed, 52 insertions(+), 43 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 23c3a1079f..ff04728b33 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -433,9 +433,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) addr = g_new0(MigrationAddress, 1);
     SocketAddress *saddr = NULL;
     InetSocketAddress *isock = &addr->u.rdma;
@@ -473,7 +474,9 @@ static bool migrate_uri_parse(const char *uri,
         return false;
     }
 
-    *channel = g_steal_pointer(&addr);
+    val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;
+    val->addr = g_steal_pointer (&addr);
+    *channel = g_steal_pointer (&val);
     return true;
 }
 
@@ -482,41 +485,44 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
                                           Error **errp)
 {
     const char *p = NULL;
-    g_autoptr(MigrationAddress) channel = NULL;
+    MigrationChannel *channel = NULL;
+    MigrationAddress *addr = NULL;
 
     /*
      * Having preliminary checks for uri and channel
      */
-    if (has_channels) {
-        error_setg(errp, "'channels' argument should not be set yet.");
-        return;
-    }
-
     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;
-    }
-
-    if (!uri && !has_channels) {
+    } 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;
+    } else if (uri) {
+        /* caller uses the old URI syntax */
+        if (!migrate_uri_parse(uri, &channel, errp)) {
+            return;
+        }
+    } else {
         error_setg(errp, "neither 'uri' or 'channels' argument are "
                    "specified in 'migrate-incoming' qmp command ");
         return;
     }
-
-    if (uri && !migrate_uri_parse(uri, &channel, errp)) {
-        return;
-    }
+    addr = channel->addr;
 
     /* transport mechanism not suitable for migration? */
-    if (!migration_channels_and_transport_compatible(channel, errp)) {
+    if (!migration_channels_and_transport_compatible(addr, errp)) {
         return;
     }
 
     qapi_event_send_migration(MIGRATION_STATUS_SETUP);
-    if (channel->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
-        SocketAddress *saddr = &channel->u.socket;
+    if (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
+        SocketAddress *saddr = &addr->u.socket;
         if (saddr->type == SOCKET_ADDRESS_TYPE_INET ||
             saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
             saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) {
@@ -525,11 +531,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 (addr->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
+        rdma_start_incoming_migration(&addr->u.rdma, errp);
+ #endif
+    } else if (addr->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
+        exec_start_incoming_migration(addr->u.exec.args, errp);
     } else if (strstart(uri, "file:", &p)) {
         file_start_incoming_migration(p, errp);
     } else {
@@ -1756,35 +1762,38 @@ void qmp_migrate(const char *uri, bool has_channels,
     Error *local_err = NULL;
     MigrationState *s = migrate_get_current();
     const char *p = NULL;
-    g_autoptr(MigrationAddress) channel = NULL;
+    MigrationChannel *channel = NULL;
+    MigrationAddress *addr = NULL;
 
     /*
      * Having preliminary checks for uri and channel
      */
-    if (has_channels) {
-        error_setg(errp, "'channels' argument should not be set yet.");
-        return;
-    }
-
     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;
-    }
-
-    if (!uri && !has_channels) {
+    } 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;
+    } else if (uri) {
+        /* caller uses the old URI syntax */
+        if (!migrate_uri_parse(uri, &channel, errp)) {
+            return;
+        }
+    } else {
         error_setg(errp, "neither 'uri' or 'channels' argument are "
                    "specified in 'migrate' qmp command ");
         return;
     }
-
-    if (!migrate_uri_parse(uri, &channel, errp)) {
-        return;
-    }
+    addr = channel->addr;
 
     /* transport mechanism not suitable for migration? */
-    if (!migration_channels_and_transport_compatible(channel, errp)) {
+    if (!migration_channels_and_transport_compatible(addr, errp)) {
         return;
     }
 
@@ -1801,8 +1810,8 @@ void qmp_migrate(const char *uri, bool has_channels,
         }
     }
 
-    if (channel->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
-        SocketAddress *saddr = &channel->u.socket;
+    if (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
+        SocketAddress *saddr = &addr->u.socket;
         if (saddr->type == SOCKET_ADDRESS_TYPE_INET ||
             saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
             saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) {
@@ -1811,11 +1820,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 (addr->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
+        rdma_start_outgoing_migration(s, &addr->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 (addr->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
+        exec_start_outgoing_migration(s, addr->u.exec.args, &local_err);
     } else if (strstart(uri, "file:", &p)) {
         file_start_outgoing_migration(s, p, &local_err);
     } else {
-- 
2.22.3



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

* [PATCH v12 09/10] migration: Implement MigrateChannelList to hmp migration flow.
  2023-10-09 14:36 [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
                   ` (7 preceding siblings ...)
  2023-10-09 14:36 ` [PATCH v12 08/10] migration: Implement MigrateChannelList to qmp migration flow Het Gala
@ 2023-10-09 14:36 ` Het Gala
  2023-10-09 21:08   ` Fabiano Rosas
  2023-10-09 14:36 ` [PATCH v12 10/10] migration: modify test_multifd_tcp_none() to use new QAPI syntax Het Gala
  2023-10-09 21:04 ` [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Fabiano Rosas
  10 siblings, 1 reply; 19+ messages in thread
From: Het Gala @ 2023-10-09 14:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, farosas, manish.mishra, aravind.retnakaran, Het Gala

Integrate MigrateChannelList with all transport backends
(socket, exec and rdma) for both src and dest migration
endpoints for hmp migration.

Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
 migration/migration-hmp-cmds.c | 15 +++++++++++++--
 migration/migration.c          |  5 ++---
 migration/migration.h          |  3 ++-
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index a2e6a5c51e..21b57f7ed8 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -441,9 +441,14 @@ 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 = NULL;
 
-    qmp_migrate_incoming(uri, false, NULL, &err);
+    migrate_uri_parse(uri, &channel, &err);
+    QAPI_LIST_PREPEND(caps, channel);
 
+    qmp_migrate_incoming(NULL, true, caps, &err);
+    qapi_free_MigrationChannelList(caps);
     hmp_handle_error(mon, err);
 }
 
@@ -730,9 +735,15 @@ 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 = NULL;
 
-    qmp_migrate(uri, false, NULL, !!blk, blk, !!inc, inc,
+    migrate_uri_parse(uri, &channel, &err);
+    QAPI_LIST_PREPEND(caps, channel);
+
+    qmp_migrate(NULL, true, 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 ff04728b33..a651106bff 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -432,9 +432,8 @@ void migrate_add_address(SocketAddress *address)
                       QAPI_CLONE(SocketAddress, address));
 }
 
-static bool migrate_uri_parse(const char *uri,
-                              MigrationChannel **channel,
-                              Error **errp)
+bool migrate_uri_parse(const char *uri, MigrationChannel **channel,
+                       Error **errp)
 {
     g_autoptr(MigrationChannel) val = g_new0(MigrationChannel, 1);
     g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
diff --git a/migration/migration.h b/migration/migration.h
index 972597f4de..f9127707f5 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -511,7 +511,8 @@ bool check_dirty_bitmap_mig_alias_map(const BitmapMigrationNodeAliasList *bbm,
                                       Error **errp);
 
 void migrate_add_address(SocketAddress *address);
-
+bool migrate_uri_parse(const char *uri, MigrationChannel **channel,
+                       Error **errp);
 int foreach_not_ignored_block(RAMBlockIterFunc func, void *opaque);
 
 #define qemu_ram_foreach_block \
-- 
2.22.3



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

* [PATCH v12 10/10] migration: modify test_multifd_tcp_none() to use new QAPI syntax.
  2023-10-09 14:36 [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
                   ` (8 preceding siblings ...)
  2023-10-09 14:36 ` [PATCH v12 09/10] migration: Implement MigrateChannelList to hmp " Het Gala
@ 2023-10-09 14:36 ` Het Gala
  2023-10-09 21:04 ` [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Fabiano Rosas
  10 siblings, 0 replies; 19+ messages in thread
From: Het Gala @ 2023-10-09 14:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, farosas, manish.mishra, aravind.retnakaran, Het Gala

modify multifd tcp common test to incorporate the new 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 | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 46f1c275a2..7a72e1ef03 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2205,7 +2205,12 @@ test_migrate_precopy_tcp_multifd_start_common(QTestState *from,
 
     /* Start incoming migration from the 1st socket */
     qtest_qmp_assert_success(to, "{ 'execute': 'migrate-incoming',"
-                             "  'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
+                             "  'arguments': { "
+                             "      'channels': [ { 'channel-type': 'main',"
+                             "      'addr': { 'transport': 'socket',"
+                             "                'type': 'inet',"
+                             "                'host': '127.0.0.1',"
+                             "                'port': '0' } } ] } }");
 
     return NULL;
 }
-- 
2.22.3



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

* Re: [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration
  2023-10-09 14:36 [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
                   ` (9 preceding siblings ...)
  2023-10-09 14:36 ` [PATCH v12 10/10] migration: modify test_multifd_tcp_none() to use new QAPI syntax Het Gala
@ 2023-10-09 21:04 ` Fabiano Rosas
  2023-10-10  5:55   ` Het Gala
  10 siblings, 1 reply; 19+ messages in thread
From: Fabiano Rosas @ 2023-10-09 21:04 UTC (permalink / raw)
  To: Het Gala, qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran, Het Gala

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

> This is v12 patchset of modified 'migrate' and 'migrate-incoming' QAPI design
> for upstream review.
>
> Would like to thank all the maintainers that actively participated in the v11
> 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
> v6: https://lists.gnu.org/archive/html/qemu-devel/2023-06/msg01251.html
> v7: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg02027.html
> v8: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg02770.html
> v9: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg04216.html
> v10: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg05022.html
> v11: https://lists.gnu.org/archive/html/qemu-devel/2023-10/msg00740.html
>
> v11 -> v12 changelog:
> -------------------
> - Resolved double-freeing when using g_autoptr in structures that are
>   nested into another.
> - Overwriting of pointers to an existing allocated memory is solved at
>   various places.
> - Use of g_autoptr caused make check errors in non-error path while going
>   out of scope inside function. Added g_steal_pointer() in the non-error
>   paths wherever required.

Please run make check before sending:
▶ 242/355 qcow2 181  FAIL

You can also push your code to your gitlab and run a pipeline with the
branch.


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

* Re: [PATCH v12 09/10] migration: Implement MigrateChannelList to hmp migration flow.
  2023-10-09 14:36 ` [PATCH v12 09/10] migration: Implement MigrateChannelList to hmp " Het Gala
@ 2023-10-09 21:08   ` Fabiano Rosas
  2023-10-11 13:33     ` Het Gala
  0 siblings, 1 reply; 19+ messages in thread
From: Fabiano Rosas @ 2023-10-09 21:08 UTC (permalink / raw)
  To: Het Gala, qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran, Het Gala

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

> Integrate MigrateChannelList with all transport backends
> (socket, exec and rdma) for both src and dest migration
> endpoints for hmp migration.
>
> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> ---
>  migration/migration-hmp-cmds.c | 15 +++++++++++++--
>  migration/migration.c          |  5 ++---
>  migration/migration.h          |  3 ++-
>  3 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index a2e6a5c51e..21b57f7ed8 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -441,9 +441,14 @@ 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 = NULL;

No g_autoptr here because the list code will already free the channel.

>  
> -    qmp_migrate_incoming(uri, false, NULL, &err);
> +    migrate_uri_parse(uri, &channel, &err);

Need to check the return value of this function.

$ (echo "migrate -d unix:") | ./qemu-system-x86_64 -monitor stdio -display none
QEMU 8.1.50 monitor - type 'help' for more information
(qemu) migrate -d unix:
Segmentation fault (core dumped)

> +    QAPI_LIST_PREPEND(caps, channel);
>  
> +    qmp_migrate_incoming(NULL, true, caps, &err);
> +    qapi_free_MigrationChannelList(caps);
>      hmp_handle_error(mon, err);
>  }
>  
> @@ -730,9 +735,15 @@ 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 = NULL;
>  
> -    qmp_migrate(uri, false, NULL, !!blk, blk, !!inc, inc,
> +    migrate_uri_parse(uri, &channel, &err);
> +    QAPI_LIST_PREPEND(caps, channel);
> +
> +    qmp_migrate(NULL, true, caps, !!blk, blk, !!inc, inc,
>                   false, false, true, resume, &err);
> +    qapi_free_MigrationChannelList(caps);
>      if (hmp_handle_error(mon, err)) {
>          return;
>      }


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

* Re: [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration
  2023-10-09 21:04 ` [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Fabiano Rosas
@ 2023-10-10  5:55   ` Het Gala
  2023-10-10 13:30     ` Fabiano Rosas
  0 siblings, 1 reply; 19+ messages in thread
From: Het Gala @ 2023-10-10  5:55 UTC (permalink / raw)
  To: Fabiano Rosas, qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran

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


On 10/10/2023 2:34 AM, Fabiano Rosas wrote:
> Het Gala<het.gala@nutanix.com>  writes:
>
>> This is v12 patchset of modified 'migrate' and 'migrate-incoming' QAPI design
>> for upstream review.
>>
>> Would like to thank all the maintainers that actively participated in the v11
>> patchset discussion and gave insightful suggestions to improve the patches.
>>
>> Link to previous upstream community patchset links:
>> v1:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2022-2D12_msg04339.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=OXhp-cq93AZ1ZRIwKL5wXhx5-8ei7RfBdmmbU9KNDfg&e=  
>> v2:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02106.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=r7SYaB2fxLcEP2DiHslsbEyaY7ZPrXxageSBRtRZ7TA&e=  
>> v3:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02473.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=fnGhJw56ypUavnslnUL6JeK4OLi7xwh7TGsafaSSZvw&e=  
>> v4:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg03064.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=SA4q18GEE4q3Eh7sy5nhQ8OZO5KM8kfapiBkSPZYDxE&e=  
>> v5:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg04845.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=QRW_aAGHmTBajBnu1a4jcxQFZ1lf1N3RCyLgOt82Ji4&e=  
>> v6:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D06_msg01251.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=7Dmgm47UdQ0h0Y9-XffsUW_ZdeQ-eCCVzhUkigTMKbc&e=  
>> v7:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02027.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=8a3tAfIJ-6st1tlYkbjsRWEv-JvEFxokXzanf0WCqzw&e=  
>> v8:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02770.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=4q_F05ZPdhWsPJ0fa850gHS90AsVX7MbsaIHi-3OsMI&e=  
>> v9:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg04216.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=1wNhJSfSvAoadG06F2JKFHZ2mA4QWSgqvYpt1zRX9qw&e=  
>> v10:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg05022.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=guEm3FuFn7jutT4ZB4RlgwttD4IMSBJy1MNh2zp3tYY&e=  
>> v11:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D10_msg00740.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=W7rbQhebtuWtT2ydMuG21OOkbqlh9KxMi1ZM5yZP6Ig&e=  
>>
>> v11 -> v12 changelog:
>> -------------------
>> - Resolved double-freeing when using g_autoptr in structures that are
>>    nested into another.
>> - Overwriting of pointers to an existing allocated memory is solved at
>>    various places.
>> - Use of g_autoptr caused make check errors in non-error path while going
>>    out of scope inside function. Added g_steal_pointer() in the non-error
>>    paths wherever required.
> Please run make check before sending:
> ▶ 242/355 qcow2 181  FAIL
>
> You can also push your code to your gitlab and run a pipeline with the
> branch.

I tested on my setup, and it passed all the checks.

[root@06f1b38a5d6a build]# make check
/rpmbuild/SOURCES/qemu/build/pyvenv/bin/meson test  --no-rebuild -t 0  
--num-processes 1 --print-errorlogs
   1/348 qemu:qtest+qtest-x86_64 / 
qtest-x86_64/qom-test                        OK 14.00s   8 subtests passed
   2/348 qemu:qtest+qtest-x86_64 / 
qtest-x86_64/migration-test                  OK 82.28s   13 subtests passed
   3/348 qemu:qtest+qtest-x86_64 / 
qtest-x86_64/bios-tables-test                OK 79.38s   51 subtests passed
   4/348 qemu:qtest+qtest-x86_64 / 
qtest-x86_64/test-hmp                        OK 5.67s   9 subtests passed
   5/348 qemu:qtest+qtest-x86_64 / 
qtest-x86_64/ahci-test                       OK 21.16s   74 subtests passed
   6/348 qemu:qtest+qtest-x86_64 / 
qtest-x86_64/qos-test                        OK 34.48s   119 subtests passed
   7/348 qemu:unit / check-block-qdict OK              0.01s   10 
subtests passed
   8/348 qemu:unit / check-qdict OK              0.01s   15 subtests passed
   9/348 qemu:unit / check-qnum OK              0.01s   8 subtests passed
  10/348 qemu:unit / check-qstring OK              0.01s   4 subtests passed
  11/348 qemu:unit / check-qlist OK              0.01s   4 subtests passed
[...]
340/348 qemu:softfloat+softfloat-compare / 
fp-test-lt_quiet                    OK              0.10s
341/348 qemu:softfloat+softfloat-ops / 
fp-test-add                             OK              1.61s
342/348 qemu:softfloat+softfloat-ops / 
fp-test-sub                             OK              1.55s
343/348 qemu:softfloat+softfloat-ops / 
fp-test-mul                             OK              7.81s
344/348 qemu:softfloat+softfloat-ops / 
fp-test-div                             OK              6.84s
345/348 qemu:softfloat+softfloat-ops / 
fp-test-rem                             OK              4.24s
346/348 qemu:softfloat+softfloat-ops / 
fp-test-sqrt                            OK              0.10s
347/348 qemu:softfloat+softfloat-ops / 
fp-test-log2                            OK              0.25s
348/348 qemu:qapi-schema+qapi-frontend / QAPI schema regression 
tests          OK              0.20s

Ok:                 339
Expected Fail:      0
Fail:               0
Unexpected Pass:    0
Skipped:            9
Timeout:            0

I am not sure if something is different in the configuration. I have 
never run a pipeline on gitlab though.  Can you point me out to the 
documentation of gitlab once again

Regards,
Het Gala

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

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

* Re: [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration
  2023-10-10  5:55   ` Het Gala
@ 2023-10-10 13:30     ` Fabiano Rosas
  2023-10-10 14:26       ` Fabiano Rosas
  0 siblings, 1 reply; 19+ messages in thread
From: Fabiano Rosas @ 2023-10-10 13:30 UTC (permalink / raw)
  To: Het Gala, qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran

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

> On 10/10/2023 2:34 AM, Fabiano Rosas wrote:
>> Het Gala<het.gala@nutanix.com>  writes:
>>
>>> This is v12 patchset of modified 'migrate' and 'migrate-incoming' QAPI design
>>> for upstream review.
>>>
>>> Would like to thank all the maintainers that actively participated in the v11
>>> patchset discussion and gave insightful suggestions to improve the patches.
>>>
>>> Link to previous upstream community patchset links:
>>> v1:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2022-2D12_msg04339.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=OXhp-cq93AZ1ZRIwKL5wXhx5-8ei7RfBdmmbU9KNDfg&e=  
>>> v2:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02106.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=r7SYaB2fxLcEP2DiHslsbEyaY7ZPrXxageSBRtRZ7TA&e=  
>>> v3:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02473.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=fnGhJw56ypUavnslnUL6JeK4OLi7xwh7TGsafaSSZvw&e=  
>>> v4:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg03064.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=SA4q18GEE4q3Eh7sy5nhQ8OZO5KM8kfapiBkSPZYDxE&e=  
>>> v5:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg04845.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=QRW_aAGHmTBajBnu1a4jcxQFZ1lf1N3RCyLgOt82Ji4&e=  
>>> v6:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D06_msg01251.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=7Dmgm47UdQ0h0Y9-XffsUW_ZdeQ-eCCVzhUkigTMKbc&e=  
>>> v7:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02027.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=8a3tAfIJ-6st1tlYkbjsRWEv-JvEFxokXzanf0WCqzw&e=  
>>> v8:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02770.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=4q_F05ZPdhWsPJ0fa850gHS90AsVX7MbsaIHi-3OsMI&e=  
>>> v9:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg04216.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=1wNhJSfSvAoadG06F2JKFHZ2mA4QWSgqvYpt1zRX9qw&e=  
>>> v10:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg05022.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=guEm3FuFn7jutT4ZB4RlgwttD4IMSBJy1MNh2zp3tYY&e=  
>>> v11:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D10_msg00740.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=W7rbQhebtuWtT2ydMuG21OOkbqlh9KxMi1ZM5yZP6Ig&e=  
>>>
>>> v11 -> v12 changelog:
>>> -------------------
>>> - Resolved double-freeing when using g_autoptr in structures that are
>>>    nested into another.
>>> - Overwriting of pointers to an existing allocated memory is solved at
>>>    various places.
>>> - Use of g_autoptr caused make check errors in non-error path while going
>>>    out of scope inside function. Added g_steal_pointer() in the non-error
>>>    paths wherever required.
>> Please run make check before sending:
>> ▶ 242/355 qcow2 181  FAIL
>>
>> You can also push your code to your gitlab and run a pipeline with the
>> branch.
>
> I tested on my setup, and it passed all the checks.

Ok, so this particular test must have been skipped. It's not possible
that the test ran and passed since the issue is a very abrupt abort().

Try to run 'make check-block' and watch for the test 181. Or run from the
build directory: ./tests/qemu-iotests/check -qcow2 181

I started a gitlab pipeline with this code to rule out anything in my
own setup. Let's see:
https://gitlab.com/farosas/qemu/-/pipelines/1032002487

> [root@06f1b38a5d6a build]# make check
> /rpmbuild/SOURCES/qemu/build/pyvenv/bin/meson test  --no-rebuild -t 0  
> --num-processes 1 --print-errorlogs

hmm, hopefuly you're not using an outdated tree that you fetched from
the rpm source package.

...

> I am not sure if something is different in the configuration. I have 
> never run a pipeline on gitlab though.  Can you point me out to the 
> documentation of gitlab once again

To run a pipeline on Gitlab you need to push a branch to your QEMU fork
and trigger the pipeline via the Build->Pipelines->Run Pipeline
option. Choose the branch from the dropdown and input an environment
variable with key: QEMU_CI and value: 2 in the Variables field.

We have documentation on how to do that directly from the command line
here: https://www.qemu.org/docs/master/devel/ci.html


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

* Re: [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration
  2023-10-10 13:30     ` Fabiano Rosas
@ 2023-10-10 14:26       ` Fabiano Rosas
  2023-10-10 14:38         ` Het Gala
  0 siblings, 1 reply; 19+ messages in thread
From: Fabiano Rosas @ 2023-10-10 14:26 UTC (permalink / raw)
  To: Het Gala, qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran

Fabiano Rosas <farosas@suse.de> writes:

> Het Gala <het.gala@nutanix.com> writes:
>
>> On 10/10/2023 2:34 AM, Fabiano Rosas wrote:
>>> Het Gala<het.gala@nutanix.com>  writes:
>>>
>>>> This is v12 patchset of modified 'migrate' and 'migrate-incoming' QAPI design
>>>> for upstream review.
>>>>
>>>> Would like to thank all the maintainers that actively participated in the v11
>>>> patchset discussion and gave insightful suggestions to improve the patches.
>>>>
>>>> Link to previous upstream community patchset links:
>>>> v1:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2022-2D12_msg04339.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=OXhp-cq93AZ1ZRIwKL5wXhx5-8ei7RfBdmmbU9KNDfg&e=  
>>>> v2:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02106.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=r7SYaB2fxLcEP2DiHslsbEyaY7ZPrXxageSBRtRZ7TA&e=  
>>>> v3:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02473.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=fnGhJw56ypUavnslnUL6JeK4OLi7xwh7TGsafaSSZvw&e=  
>>>> v4:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg03064.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=SA4q18GEE4q3Eh7sy5nhQ8OZO5KM8kfapiBkSPZYDxE&e=  
>>>> v5:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg04845.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=QRW_aAGHmTBajBnu1a4jcxQFZ1lf1N3RCyLgOt82Ji4&e=  
>>>> v6:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D06_msg01251.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=7Dmgm47UdQ0h0Y9-XffsUW_ZdeQ-eCCVzhUkigTMKbc&e=  
>>>> v7:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02027.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=8a3tAfIJ-6st1tlYkbjsRWEv-JvEFxokXzanf0WCqzw&e=  
>>>> v8:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02770.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=4q_F05ZPdhWsPJ0fa850gHS90AsVX7MbsaIHi-3OsMI&e=  
>>>> v9:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg04216.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=1wNhJSfSvAoadG06F2JKFHZ2mA4QWSgqvYpt1zRX9qw&e=  
>>>> v10:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg05022.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=guEm3FuFn7jutT4ZB4RlgwttD4IMSBJy1MNh2zp3tYY&e=  
>>>> v11:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D10_msg00740.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=W7rbQhebtuWtT2ydMuG21OOkbqlh9KxMi1ZM5yZP6Ig&e=  
>>>>
>>>> v11 -> v12 changelog:
>>>> -------------------
>>>> - Resolved double-freeing when using g_autoptr in structures that are
>>>>    nested into another.
>>>> - Overwriting of pointers to an existing allocated memory is solved at
>>>>    various places.
>>>> - Use of g_autoptr caused make check errors in non-error path while going
>>>>    out of scope inside function. Added g_steal_pointer() in the non-error
>>>>    paths wherever required.
>>> Please run make check before sending:
>>> ▶ 242/355 qcow2 181  FAIL
>>>
>>> You can also push your code to your gitlab and run a pipeline with the
>>> branch.
>>
>> I tested on my setup, and it passed all the checks.
>
> Ok, so this particular test must have been skipped. It's not possible
> that the test ran and passed since the issue is a very abrupt abort().
>

Ok, so I think what is going on is that both your system and CI don't
support postcopy_ram so this test is skipped.

> Try to run 'make check-block' and watch for the test 181. Or run from the
> build directory: ./tests/qemu-iotests/check -qcow2 181
>
> I started a gitlab pipeline with this code to rule out anything in my
> own setup. Let's see:
> https://gitlab.com/farosas/qemu/-/pipelines/1032002487
>

The issue manifested in the disable-tcg build in the qcow2 091
test. It's the same issue as 181.

There's also checkpatch issues. We have ./scripts/checkpatch.pl to help
with those. Just give it .patch files and it will tell you if
something's wrong with the patch.


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

* Re: [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration
  2023-10-10 14:26       ` Fabiano Rosas
@ 2023-10-10 14:38         ` Het Gala
  2023-10-10 14:49           ` Fabiano Rosas
  0 siblings, 1 reply; 19+ messages in thread
From: Het Gala @ 2023-10-10 14:38 UTC (permalink / raw)
  To: Fabiano Rosas, qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran

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

On 10/10/2023 7:56 PM, Fabiano Rosas wrote:
> Fabiano Rosas<farosas@suse.de>  writes:
>
>> Het Gala<het.gala@nutanix.com>  writes:
>>
>>> On 10/10/2023 2:34 AM, Fabiano Rosas wrote:
>>>> Het Gala<het.gala@nutanix.com>   writes:
>>>>
>>>>> This is v12 patchset of modified 'migrate' and 'migrate-incoming' QAPI design
>>>>> for upstream review.
>>>>>
>>>>> Would like to thank all the maintainers that actively participated in the v11
>>>>> patchset discussion and gave insightful suggestions to improve the patches.
>>>>>
>>>>> Link to previous upstream community patchset links:
>>>>> v1:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2022-2D12_msg04339.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=OXhp-cq93AZ1ZRIwKL5wXhx5-8ei7RfBdmmbU9KNDfg&e=   
>>>>> v2:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02106.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=r7SYaB2fxLcEP2DiHslsbEyaY7ZPrXxageSBRtRZ7TA&e=   
>>>>> v3:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02473.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=fnGhJw56ypUavnslnUL6JeK4OLi7xwh7TGsafaSSZvw&e=   
>>>>> v4:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg03064.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=SA4q18GEE4q3Eh7sy5nhQ8OZO5KM8kfapiBkSPZYDxE&e=   
>>>>> v5:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg04845.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=QRW_aAGHmTBajBnu1a4jcxQFZ1lf1N3RCyLgOt82Ji4&e=   
>>>>> v6:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D06_msg01251.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=7Dmgm47UdQ0h0Y9-XffsUW_ZdeQ-eCCVzhUkigTMKbc&e=   
>>>>> v7:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02027.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=8a3tAfIJ-6st1tlYkbjsRWEv-JvEFxokXzanf0WCqzw&e=   
>>>>> v8:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02770.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=4q_F05ZPdhWsPJ0fa850gHS90AsVX7MbsaIHi-3OsMI&e=   
>>>>> v9:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg04216.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=1wNhJSfSvAoadG06F2JKFHZ2mA4QWSgqvYpt1zRX9qw&e=   
>>>>> v10:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg05022.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=guEm3FuFn7jutT4ZB4RlgwttD4IMSBJy1MNh2zp3tYY&e=   
>>>>> v11:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D10_msg00740.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=W7rbQhebtuWtT2ydMuG21OOkbqlh9KxMi1ZM5yZP6Ig&e=   
>>>>>
>>>>> v11 -> v12 changelog:
>>>>> -------------------
>>>>> - Resolved double-freeing when using g_autoptr in structures that are
>>>>>     nested into another.
>>>>> - Overwriting of pointers to an existing allocated memory is solved at
>>>>>     various places.
>>>>> - Use of g_autoptr caused make check errors in non-error path while going
>>>>>     out of scope inside function. Added g_steal_pointer() in the non-error
>>>>>     paths wherever required.
>>>> Please run make check before sending:
>>>> ▶ 242/355 qcow2 181  FAIL
>>>>
>>>> You can also push your code to your gitlab and run a pipeline with the
>>>> branch.
>>> I tested on my setup, and it passed all the checks.
>> Ok, so this particular test must have been skipped. It's not possible
>> that the test ran and passed since the issue is a very abrupt abort().
>>
> Ok, so I think what is going on is that both your system and CI don't
> support postcopy_ram so this test is skipped.

Yes, on my setup, postcopy_ram is not supported / enabled for now. So 
the test failing is related to post copy, is that a concern for our 
patches ?

I looked at the test logs, and the error does not belong to migration 
failure imo.

>> Try to run 'make check-block' and watch for the test 181. Or run from the
>> build directory: ./tests/qemu-iotests/check -qcow2 181
>>
>> I started a gitlab pipeline with this code to rule out anything in my
>> own setup. Let's see:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_farosas_qemu_-2D_pipelines_1032002487&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=gGMQaHYTLaVYV5pnYVZXGJfYst4XCAmfGs9Ke84e99EZOorbubM8ZScBQq5Jn-Yw&s=kCkY_z43mLb7OTgH_L4cdz8OcmCwo0r_hojI7aCOqZ8&e=  
>>
> The issue manifested in the disable-tcg build in the qcow2 091
> test. It's the same issue as 181.
Ack.
> There's also checkpatch issues. We have ./scripts/checkpatch.pl to help
> with those. Just give it .patch files and it will tell you if
> something's wrong with the patch.

Yes, sorry about that. I am aware of checkpath.pl scripts. I will go 
through the patches and make sure, it follows all the guidelines in the 
next version.

Regards,
Het Gala

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

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

* Re: [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration
  2023-10-10 14:38         ` Het Gala
@ 2023-10-10 14:49           ` Fabiano Rosas
  0 siblings, 0 replies; 19+ messages in thread
From: Fabiano Rosas @ 2023-10-10 14:49 UTC (permalink / raw)
  To: Het Gala, qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran

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

> On 10/10/2023 7:56 PM, Fabiano Rosas wrote:
>> Fabiano Rosas<farosas@suse.de>  writes:
>>
>>> Het Gala<het.gala@nutanix.com>  writes:
>>>
>>>> On 10/10/2023 2:34 AM, Fabiano Rosas wrote:
>>>>> Het Gala<het.gala@nutanix.com>   writes:
>>>>>
>>>>>> This is v12 patchset of modified 'migrate' and 'migrate-incoming' QAPI design
>>>>>> for upstream review.
>>>>>>
>>>>>> Would like to thank all the maintainers that actively participated in the v11
>>>>>> patchset discussion and gave insightful suggestions to improve the patches.
>>>>>>
>>>>>> Link to previous upstream community patchset links:
>>>>>> v1:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2022-2D12_msg04339.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=OXhp-cq93AZ1ZRIwKL5wXhx5-8ei7RfBdmmbU9KNDfg&e=   
>>>>>> v2:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02106.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=r7SYaB2fxLcEP2DiHslsbEyaY7ZPrXxageSBRtRZ7TA&e=   
>>>>>> v3:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02473.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=fnGhJw56ypUavnslnUL6JeK4OLi7xwh7TGsafaSSZvw&e=   
>>>>>> v4:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg03064.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=SA4q18GEE4q3Eh7sy5nhQ8OZO5KM8kfapiBkSPZYDxE&e=   
>>>>>> v5:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg04845.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=QRW_aAGHmTBajBnu1a4jcxQFZ1lf1N3RCyLgOt82Ji4&e=   
>>>>>> v6:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D06_msg01251.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=7Dmgm47UdQ0h0Y9-XffsUW_ZdeQ-eCCVzhUkigTMKbc&e=   
>>>>>> v7:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02027.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=8a3tAfIJ-6st1tlYkbjsRWEv-JvEFxokXzanf0WCqzw&e=   
>>>>>> v8:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02770.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=4q_F05ZPdhWsPJ0fa850gHS90AsVX7MbsaIHi-3OsMI&e=   
>>>>>> v9:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg04216.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=1wNhJSfSvAoadG06F2JKFHZ2mA4QWSgqvYpt1zRX9qw&e=   
>>>>>> v10:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg05022.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=guEm3FuFn7jutT4ZB4RlgwttD4IMSBJy1MNh2zp3tYY&e=   
>>>>>> v11:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D10_msg00740.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=s4FkORPOMqBTxCrkaYO6frUTsbcGM6c3K4FCdGaNAKeCIZ2GS9VaQxJGytQv0mGv&s=W7rbQhebtuWtT2ydMuG21OOkbqlh9KxMi1ZM5yZP6Ig&e=   
>>>>>>
>>>>>> v11 -> v12 changelog:
>>>>>> -------------------
>>>>>> - Resolved double-freeing when using g_autoptr in structures that are
>>>>>>     nested into another.
>>>>>> - Overwriting of pointers to an existing allocated memory is solved at
>>>>>>     various places.
>>>>>> - Use of g_autoptr caused make check errors in non-error path while going
>>>>>>     out of scope inside function. Added g_steal_pointer() in the non-error
>>>>>>     paths wherever required.
>>>>> Please run make check before sending:
>>>>> ▶ 242/355 qcow2 181  FAIL
>>>>>
>>>>> You can also push your code to your gitlab and run a pipeline with the
>>>>> branch.
>>>> I tested on my setup, and it passed all the checks.
>>> Ok, so this particular test must have been skipped. It's not possible
>>> that the test ran and passed since the issue is a very abrupt abort().
>>>
>> Ok, so I think what is going on is that both your system and CI don't
>> support postcopy_ram so this test is skipped.
>
> Yes, on my setup, postcopy_ram is not supported / enabled for now. So 
> the test failing is related to post copy, is that a concern for our 
> patches ?
>
> I looked at the test logs, and the error does not belong to migration 
> failure imo.
>

The tests (091, 181) are failing due to the hmp_migrate changes from
this series.


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

* Re: [PATCH v12 09/10] migration: Implement MigrateChannelList to hmp migration flow.
  2023-10-09 21:08   ` Fabiano Rosas
@ 2023-10-11 13:33     ` Het Gala
  0 siblings, 0 replies; 19+ messages in thread
From: Het Gala @ 2023-10-11 13:33 UTC (permalink / raw)
  To: Fabiano Rosas, qemu-devel
  Cc: prerna.saxena, quintela, dgilbert, pbonzini, berrange, armbru,
	eblake, manish.mishra, aravind.retnakaran

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

On 10/10/2023 2:38 AM, Fabiano Rosas wrote:
> Het Gala<het.gala@nutanix.com>  writes:
>
>> Integrate MigrateChannelList with all transport backends
>> (socket, exec and rdma) for both src and dest migration
>> endpoints for hmp migration.
>>
>> Suggested-by: Aravind Retnakaran<aravind.retnakaran@nutanix.com>
>> Signed-off-by: Het Gala<het.gala@nutanix.com>
>> ---
>>   migration/migration-hmp-cmds.c | 15 +++++++++++++--
>>   migration/migration.c          |  5 ++---
>>   migration/migration.h          |  3 ++-
>>   3 files changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
>> index a2e6a5c51e..21b57f7ed8 100644
>> --- a/migration/migration-hmp-cmds.c
>> +++ b/migration/migration-hmp-cmds.c
>> @@ -441,9 +441,14 @@ 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 = NULL;
> No g_autoptr here because the list code will already free the channel.
Yes, g_autoptr is not needed here. Removing g_autoptr passes all the 
make checks (even -qcow2 181 test)but the return value is not right of 
the hmp_migrate() function.
>>   
>> -    qmp_migrate_incoming(uri, false, NULL, &err);
>> +    migrate_uri_parse(uri, &channel, &err);
> Need to check the return value of this function.
>
> $ (echo "migrate -d unix:") | ./qemu-system-x86_64 -monitor stdio -display none
> QEMU 8.1.50 monitor - type 'help' for more information
> (qemu) migrate -d unix:
> Segmentation fault (core dumped)

Yes, there is something wrong here, expected result should have been :

(qemu) migrate -d unix:
Error: invalid Unix socket address

or

(qemu) migrate -d tcp:12.11.34.142
Error: error parsing address '12.11.34.142'

I am investigating it right now. Will update on it and try to resolve it 
soon.

>> +    QAPI_LIST_PREPEND(caps, channel);
>>   
>> +    qmp_migrate_incoming(NULL, true, caps, &err);
>> +    qapi_free_MigrationChannelList(caps);
>>       hmp_handle_error(mon, err);
>>   }
>>   
>> @@ -730,9 +735,15 @@ 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 = NULL;
>>   
>> -    qmp_migrate(uri, false, NULL, !!blk, blk, !!inc, inc,
>> +    migrate_uri_parse(uri, &channel, &err);
>> +    QAPI_LIST_PREPEND(caps, channel);
>> +
>> +    qmp_migrate(NULL, true, caps, !!blk, blk, !!inc, inc,
>>                    false, false, true, resume, &err);
>> +    qapi_free_MigrationChannelList(caps);
>>       if (hmp_handle_error(mon, err)) {
>>           return;
>>       }
Regards,
Het Gala

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

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

end of thread, other threads:[~2023-10-11 13:34 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-09 14:36 [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
2023-10-09 14:36 ` [PATCH v12 01/10] migration: New QAPI type 'MigrateAddress' Het Gala
2023-10-09 14:36 ` [PATCH v12 02/10] migration: convert migration 'uri' into 'MigrateAddress' Het Gala
2023-10-09 14:36 ` [PATCH v12 03/10] migration: convert socket backend to accept MigrateAddress Het Gala
2023-10-09 14:36 ` [PATCH v12 04/10] migration: convert rdma " Het Gala
2023-10-09 14:36 ` [PATCH v12 05/10] migration: convert exec " Het Gala
2023-10-09 14:36 ` [PATCH v12 06/10] migration: New migrate and migrate-incoming argument 'channels' Het Gala
2023-10-09 14:36 ` [PATCH v12 07/10] migration: modify migration_channels_and_uri_compatible() for new QAPI syntax Het Gala
2023-10-09 14:36 ` [PATCH v12 08/10] migration: Implement MigrateChannelList to qmp migration flow Het Gala
2023-10-09 14:36 ` [PATCH v12 09/10] migration: Implement MigrateChannelList to hmp " Het Gala
2023-10-09 21:08   ` Fabiano Rosas
2023-10-11 13:33     ` Het Gala
2023-10-09 14:36 ` [PATCH v12 10/10] migration: modify test_multifd_tcp_none() to use new QAPI syntax Het Gala
2023-10-09 21:04 ` [PATCH v12 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Fabiano Rosas
2023-10-10  5:55   ` Het Gala
2023-10-10 13:30     ` Fabiano Rosas
2023-10-10 14:26       ` Fabiano Rosas
2023-10-10 14:38         ` Het Gala
2023-10-10 14:49           ` Fabiano Rosas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.