* [PATCH v9 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration
@ 2023-07-21 14:49 Het Gala
2023-07-21 14:49 ` [PATCH v9 01/10] migration: New QAPI type 'MigrateAddress' Het Gala
` (9 more replies)
0 siblings, 10 replies; 22+ messages in thread
From: Het Gala @ 2023-07-21 14:49 UTC (permalink / raw)
To: qemu-devel
Cc: Prerna Saxena, quintela, dgilbert, pbonzini, berrange, armbru,
eblake, Manish Mishra, Aravind Retnakaran, Het Gala
This is v9 patchset of modified 'migrate' and 'migrate-incoming' QAPI design
for upstream review.
Would like to thank all the maintainers that actively participated in the v8
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
v8 -> v9 changelog:
-------------------
- Patch1 : changed the qemu version from v8.1 -> v8.2.
- Patch2 : changes for WIN32 regarding exec command.
- Patch6 : Split qmp changes from hmp changes (Patch 9).
- Patch8 : QAPI_CLONE of SocketAddress defined eariler in Patch3.
- Patch9 : introducing migrate 'channels' in HMP, and change in
implementation from earlier patch.
- Patch10 : modify existing migration test case with new QAPI syntax
instead of adding new test case all together.
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 | 72 +++++++++----
migration/exec.h | 8 +-
migration/migration-hmp-cmds.c | 18 +++-
migration/migration.c | 182 ++++++++++++++++++++++++++-------
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, 401 insertions(+), 127 deletions(-)
--
2.22.3
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v9 01/10] migration: New QAPI type 'MigrateAddress'
2023-07-21 14:49 [PATCH v9 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
@ 2023-07-21 14:49 ` Het Gala
2023-07-21 14:49 ` [PATCH v9 02/10] migration: convert migration 'uri' into 'MigrateAddress' Het Gala
` (8 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Het Gala @ 2023-07-21 14:49 UTC (permalink / raw)
To: qemu-devel
Cc: Prerna Saxena, quintela, dgilbert, pbonzini, berrange, armbru,
eblake, Manish Mishra, Aravind Retnakaran, Het Gala
This patch introduces well defined MigrateAddress struct
and its related child objects.
The existing argument of 'migrate' and 'migrate-incoming' QAPI
- 'uri' is of 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 47dfef0278..0f4a54a7ed 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1417,6 +1417,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] 22+ messages in thread
* [PATCH v9 02/10] migration: convert migration 'uri' into 'MigrateAddress'
2023-07-21 14:49 [PATCH v9 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
2023-07-21 14:49 ` [PATCH v9 01/10] migration: New QAPI type 'MigrateAddress' Het Gala
@ 2023-07-21 14:49 ` Het Gala
2023-07-21 14:49 ` [PATCH v9 03/10] migration: convert socket backend to accept MigrateAddress Het Gala
` (7 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Het Gala @ 2023-07-21 14:49 UTC (permalink / raw)
To: qemu-devel
Cc: Prerna Saxena, quintela, dgilbert, pbonzini, berrange, armbru,
eblake, Manish Mishra, Aravind Retnakaran, Het Gala
This patch parses 'migrate' and 'migrate-incoming' QAPI's 'uri'
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>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/exec.c | 1 -
migration/exec.h | 4 ++++
migration/migration.c | 55 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 59 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 91bba630a8..5b3be767f0 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -64,6 +64,7 @@
#include "yank_functions.h"
#include "sysemu/qtest.h"
#include "options.h"
+#include "qemu/sockets.h"
static NotifierList migration_state_notifiers =
NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -421,15 +422,64 @@ 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 = &addr->u.socket;
+ 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) {
+ qapi_free_SocketAddress(saddr);
+ return false;
+ }
+ } else {
+ error_setg(errp, "unknown migration protocol: %s", uri);
+ return false;
+ }
+
+ *channel = addr;
+ return true;
+}
+
static void qemu_start_incoming_migration(const char *uri, Error **errp)
{
const char *p = NULL;
+ g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
/* URI is not suitable for migration? */
if (!migration_channels_and_uri_compatible(uri, errp)) {
return;
}
+ if (uri && !migrate_uri_parse(uri, &channel, errp)) {
+ return;
+ }
+
qapi_event_send_migration(MIGRATION_STATUS_SETUP);
if (strstart(uri, "tcp:", &p) ||
strstart(uri, "unix:", NULL) ||
@@ -1641,12 +1691,17 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
Error *local_err = NULL;
MigrationState *s = migrate_get_current();
const char *p = NULL;
+ g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
/* URI is not suitable for migration? */
if (!migration_channels_and_uri_compatible(uri, errp)) {
return;
}
+ if (!migrate_uri_parse(uri, &channel, 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] 22+ messages in thread
* [PATCH v9 03/10] migration: convert socket backend to accept MigrateAddress
2023-07-21 14:49 [PATCH v9 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
2023-07-21 14:49 ` [PATCH v9 01/10] migration: New QAPI type 'MigrateAddress' Het Gala
2023-07-21 14:49 ` [PATCH v9 02/10] migration: convert migration 'uri' into 'MigrateAddress' Het Gala
@ 2023-07-21 14:49 ` Het Gala
2023-07-21 14:49 ` [PATCH v9 04/10] migration: convert rdma " Het Gala
` (6 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Het Gala @ 2023-07-21 14:49 UTC (permalink / raw)
To: qemu-devel
Cc: Prerna Saxena, quintela, dgilbert, pbonzini, berrange, armbru,
eblake, Manish Mishra, Aravind Retnakaran, Het Gala
Socket transport backend for 'migrate'/'migrate-incoming' QAPIs accept
new wire protocol of MigrateAddress struct.
It is achived by parsing 'uri' string and storing migration parameters
required for socket connection into well defined SocketAddress struct.
Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
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 5b3be767f0..c4bcf8bbd7 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -481,18 +481,21 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp)
}
qapi_event_send_migration(MIGRATION_STATUS_SETUP);
- if (strstart(uri, "tcp:", &p) ||
- strstart(uri, "unix:", NULL) ||
- strstart(uri, "vsock:", NULL)) {
- socket_start_incoming_migration(p ? p : uri, errp);
+ if (channel->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
+ SocketAddress *saddr = &channel->u.socket;
+ if (saddr->type == SOCKET_ADDRESS_TYPE_INET ||
+ saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
+ saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) {
+ socket_start_incoming_migration(saddr, errp);
+ } else if (saddr->type == SOCKET_ADDRESS_TYPE_FD) {
+ fd_start_incoming_migration(saddr->u.fd.str, errp);
+ }
#ifdef CONFIG_RDMA
} else if (strstart(uri, "rdma:", &p)) {
rdma_start_incoming_migration(p, errp);
#endif
} else if (strstart(uri, "exec:", &p)) {
exec_start_incoming_migration(p, errp);
- } else if (strstart(uri, "fd:", &p)) {
- fd_start_incoming_migration(p, errp);
} else {
error_setg(errp, "unknown migration protocol: %s", uri);
}
@@ -1715,18 +1718,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 (!resume_requested) {
yank_unregister_instance(MIGRATION_YANK_INSTANCE);
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] 22+ messages in thread
* [PATCH v9 04/10] migration: convert rdma backend to accept MigrateAddress
2023-07-21 14:49 [PATCH v9 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
` (2 preceding siblings ...)
2023-07-21 14:49 ` [PATCH v9 03/10] migration: convert socket backend to accept MigrateAddress Het Gala
@ 2023-07-21 14:49 ` Het Gala
2023-07-21 14:49 ` [PATCH v9 05/10] migration: convert exec " Het Gala
` (5 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Het Gala @ 2023-07-21 14:49 UTC (permalink / raw)
To: qemu-devel
Cc: Prerna Saxena, quintela, dgilbert, pbonzini, berrange, armbru,
eblake, Manish Mishra, Aravind Retnakaran, Het Gala
RDMA based transport backend for 'migrate'/'migrate-incoming' QAPIs
accept new wire protocol of MigrateAddress struct.
It is achived by parsing 'uri' string and storing migration parameters
required for RDMA connection into well defined InetSocketAddress struct.
Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
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 c4bcf8bbd7..8012f93f1b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -491,8 +491,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);
@@ -1728,8 +1728,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
fd_start_outgoing_migration(s, saddr->u.fd.str, &local_err);
}
#ifdef CONFIG_RDMA
- } else if (strstart(uri, "rdma:", &p)) {
- rdma_start_outgoing_migration(s, p, &local_err);
+ } else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
+ rdma_start_outgoing_migration(s, &channel->u.rdma, &local_err);
#endif
} else if (strstart(uri, "exec:", &p)) {
exec_start_outgoing_migration(s, p, &local_err);
diff --git a/migration/rdma.c b/migration/rdma.c
index dd1c039e6c..4d64fae492 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -319,7 +319,6 @@ typedef struct RDMALocalBlocks {
typedef struct RDMAContext {
char *host;
int port;
- char *host_port;
RDMAWorkRequestData wr_data[RDMA_WRID_MAX];
@@ -2455,9 +2454,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
rdma->channel = NULL;
}
g_free(rdma->host);
- g_free(rdma->host_port);
rdma->host = NULL;
- rdma->host_port = NULL;
}
@@ -2739,28 +2736,17 @@ static void qemu_rdma_return_path_dest_init(RDMAContext *rdma_return_path,
rdma_return_path->is_return_path = true;
}
-static void *qemu_rdma_data_init(const char *host_port, Error **errp)
+static void *qemu_rdma_data_init(InetSocketAddress *saddr, Error **errp)
{
RDMAContext *rdma = NULL;
- InetSocketAddress *addr;
- if (host_port) {
+ if (saddr) {
rdma = g_new0(RDMAContext, 1);
rdma->current_index = -1;
rdma->current_chunk = -1;
- addr = g_new(InetSocketAddress, 1);
- if (!inet_parse(addr, host_port, NULL)) {
- rdma->port = atoi(addr->port);
- rdma->host = g_strdup(addr->host);
- rdma->host_port = g_strdup(host_port);
- } else {
- ERROR(errp, "bad RDMA migration address '%s'", host_port);
- g_free(rdma);
- rdma = NULL;
- }
-
- qapi_free_InetSocketAddress(addr);
+ rdma->host = g_strdup(saddr->host);
+ rdma->port = atoi(saddr->port);
}
return rdma;
@@ -3359,6 +3345,7 @@ static int qemu_rdma_accept(RDMAContext *rdma)
.private_data_len = sizeof(cap),
};
RDMAContext *rdma_return_path = NULL;
+ g_autoptr(InetSocketAddress) isock = g_new0(InetSocketAddress, 1);
struct rdma_cm_event *cm_event;
struct ibv_context *verbs;
int ret = -EINVAL;
@@ -3374,13 +3361,16 @@ static int qemu_rdma_accept(RDMAContext *rdma)
goto err_rdma_dest_wait;
}
+ isock->host = rdma->host;
+ isock->port = g_strdup_printf("%d", rdma->port);
+
/*
* initialize the RDMAContext for return path for postcopy after first
* connection request reached.
*/
if ((migrate_postcopy() || migrate_return_path())
&& !rdma->is_return_path) {
- rdma_return_path = qemu_rdma_data_init(rdma->host_port, NULL);
+ rdma_return_path = qemu_rdma_data_init(isock, NULL);
if (rdma_return_path == NULL) {
rdma_ack_cm_event(cm_event);
goto err_rdma_dest_wait;
@@ -4113,7 +4103,8 @@ static void rdma_accept_incoming_migration(void *opaque)
}
}
-void rdma_start_incoming_migration(const char *host_port, Error **errp)
+void rdma_start_incoming_migration(InetSocketAddress *host_port,
+ Error **errp)
{
int ret;
RDMAContext *rdma;
@@ -4159,13 +4150,12 @@ err:
error_propagate(errp, local_err);
if (rdma) {
g_free(rdma->host);
- g_free(rdma->host_port);
}
g_free(rdma);
}
void rdma_start_outgoing_migration(void *opaque,
- const char *host_port, Error **errp)
+ InetSocketAddress *host_port, Error **errp)
{
MigrationState *s = opaque;
RDMAContext *rdma_return_path = NULL;
diff --git a/migration/rdma.h b/migration/rdma.h
index de2ba09dc5..ee89296555 100644
--- a/migration/rdma.h
+++ b/migration/rdma.h
@@ -14,12 +14,14 @@
*
*/
+#include "qemu/sockets.h"
+
#ifndef QEMU_MIGRATION_RDMA_H
#define QEMU_MIGRATION_RDMA_H
-void rdma_start_outgoing_migration(void *opaque, const char *host_port,
+void rdma_start_outgoing_migration(void *opaque, InetSocketAddress *host_port,
Error **errp);
-void rdma_start_incoming_migration(const char *host_port, Error **errp);
+void rdma_start_incoming_migration(InetSocketAddress *host_port, Error **errp);
#endif
--
2.22.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 05/10] migration: convert exec backend to accept MigrateAddress.
2023-07-21 14:49 [PATCH v9 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
` (3 preceding siblings ...)
2023-07-21 14:49 ` [PATCH v9 04/10] migration: convert rdma " Het Gala
@ 2023-07-21 14:49 ` Het Gala
2023-07-21 14:49 ` [PATCH v9 06/10] migration: New migrate and migrate-incoming argument 'channels' Het Gala
` (4 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Het Gala @ 2023-07-21 14:49 UTC (permalink / raw)
To: qemu-devel
Cc: Prerna Saxena, quintela, dgilbert, pbonzini, berrange, armbru,
eblake, Manish Mishra, Aravind Retnakaran, Het Gala
Exec transport backend for 'migrate'/'migrate-incoming' QAPIs accept
new wire protocol of MigrateAddress struct.
It is achived by parsing 'uri' string and storing migration parameters
required for exec connection into strList struct.
Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/exec.c | 71 +++++++++++++++++++++++++++++++------------
migration/exec.h | 4 +--
migration/migration.c | 10 +++---
3 files changed, 57 insertions(+), 28 deletions(-)
diff --git a/migration/exec.c b/migration/exec.c
index 32f5143dfd..8bc321c66b 100644
--- a/migration/exec.c
+++ b/migration/exec.c
@@ -39,20 +39,50 @@ 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);
- trace_migration_exec_outgoing(command);
- ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
- O_RDWR,
- errp));
+ init_exec_array(command, argv, errp);
+ g_autofree char *new_command = g_strjoinv(" ", (char **)argv);
+
+ trace_migration_exec_outgoing(new_command);
+ ioc = QIO_CHANNEL(
+ qio_channel_command_new_spawn((const char * const *) argv,
+ O_RDWR,
+ errp));
if (!ioc) {
return;
}
@@ -71,20 +101,21 @@ static gboolean exec_accept_incoming_migration(QIOChannel *ioc,
return G_SOURCE_REMOVE;
}
-void exec_start_incoming_migration(const char *command, Error **errp)
+void exec_start_incoming_migration(strList *command, Error **errp)
{
QIOChannel *ioc;
-#ifdef WIN32
- const char *argv[] = { exec_get_cmd_path(), "/c", command, NULL };
-#else
- const char *argv[] = { "/bin/sh", "-c", command, NULL };
-#endif
+ int length = str_list_length(command);
+ g_auto(GStrv) argv = (char **) g_new0(const char *, length);
+
+ init_exec_array(command, argv, errp);
+ g_autofree char *new_command = g_strjoinv(" ", (char **)argv);
- trace_migration_exec_incoming(command);
- ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
- O_RDWR,
- errp));
+ trace_migration_exec_incoming(new_command);
+ ioc = QIO_CHANNEL(
+ qio_channel_command_new_spawn((const char * const *) argv,
+ O_RDWR,
+ errp));
if (!ioc) {
return;
}
diff --git a/migration/exec.h b/migration/exec.h
index 736cd71028..3107f205e3 100644
--- a/migration/exec.h
+++ b/migration/exec.h
@@ -23,8 +23,8 @@
#ifdef WIN32
const char *exec_get_cmd_path(void);
#endif
-void exec_start_incoming_migration(const char *host_port, Error **errp);
+void exec_start_incoming_migration(strList *host_port, Error **errp);
-void exec_start_outgoing_migration(MigrationState *s, const char *host_port,
+void exec_start_outgoing_migration(MigrationState *s, strList *host_port,
Error **errp);
#endif
diff --git a/migration/migration.c b/migration/migration.c
index 8012f93f1b..f37b388876 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -468,7 +468,6 @@ static bool migrate_uri_parse(const char *uri,
static void qemu_start_incoming_migration(const char *uri, Error **errp)
{
- const char *p = NULL;
g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
/* URI is not suitable for migration? */
@@ -494,8 +493,8 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp)
} else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
rdma_start_incoming_migration(&channel->u.rdma, errp);
#endif
- } else if (strstart(uri, "exec:", &p)) {
- exec_start_incoming_migration(p, errp);
+ } else if (channel->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
+ exec_start_incoming_migration(channel->u.exec.args, errp);
} else {
error_setg(errp, "unknown migration protocol: %s", uri);
}
@@ -1693,7 +1692,6 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
bool resume_requested;
Error *local_err = NULL;
MigrationState *s = migrate_get_current();
- const char *p = NULL;
g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
/* URI is not suitable for migration? */
@@ -1731,8 +1729,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 (!resume_requested) {
yank_unregister_instance(MIGRATION_YANK_INSTANCE);
--
2.22.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 06/10] migration: New migrate and migrate-incoming argument 'channels'
2023-07-21 14:49 [PATCH v9 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
` (4 preceding siblings ...)
2023-07-21 14:49 ` [PATCH v9 05/10] migration: convert exec " Het Gala
@ 2023-07-21 14:49 ` Het Gala
2023-07-25 18:34 ` Daniel P. Berrangé
2023-07-21 14:49 ` [PATCH v9 07/10] migration: modify migration_channels_and_uri_compatible() for new QAPI syntax Het Gala
` (3 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Het Gala @ 2023-07-21 14:49 UTC (permalink / raw)
To: qemu-devel
Cc: Prerna Saxena, quintela, dgilbert, pbonzini, berrange, armbru,
eblake, Manish Mishra, Aravind Retnakaran, Het Gala
MigrateChannelList allows to connect accross multiple interfaces.
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>
---
migration/migration-hmp-cmds.c | 6 +-
migration/migration.c | 34 ++++++++--
qapi/migration.json | 109 ++++++++++++++++++++++++++++++++-
softmmu/vl.c | 2 +-
4 files changed, 139 insertions(+), 12 deletions(-)
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 9885d7c9f7..49b150f33f 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -424,7 +424,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);
}
@@ -705,8 +705,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 f37b388876..bd3a93fc8c 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -466,10 +466,22 @@ static bool migrate_uri_parse(const char *uri,
return true;
}
-static void qemu_start_incoming_migration(const char *uri, Error **errp)
+static void qemu_start_incoming_migration(const char *uri, bool has_channels,
+ MigrationChannelList *channels,
+ Error **errp)
{
g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
+ /*
+ * Having preliminary checks for uri and channel
+ */
+ if (uri && has_channels) {
+ error_setg(errp, "'uri' and 'channels' arguments are mutually "
+ "exclusive; exactly one of the two should be present in "
+ "'migrate-incoming' qmp command ");
+ return;
+ }
+
/* URI is not suitable for migration? */
if (!migration_channels_and_uri_compatible(uri, errp)) {
return;
@@ -1497,7 +1509,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;
@@ -1515,7 +1528,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);
@@ -1551,7 +1564,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)
@@ -1685,7 +1698,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)
{
@@ -1694,6 +1708,16 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
MigrationState *s = migrate_get_current();
g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
+ /*
+ * Having preliminary checks for uri and channel
+ */
+ if (uri && has_channels) {
+ error_setg(errp, "'uri' and 'channels' arguments are mutually "
+ "exclusive; exactly one of the two should be present in "
+ "'migrate' qmp command ");
+ return;
+ }
+
/* URI is not suitable for migration? */
if (!migration_channels_and_uri_compatible(uri, errp)) {
return;
diff --git a/qapi/migration.json b/qapi/migration.json
index 0f4a54a7ed..9a365ac774 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1458,6 +1458,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:
#
@@ -1465,6 +1493,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
@@ -1489,14 +1520,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:
@@ -1507,6 +1574,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
@@ -1522,13 +1592,46 @@
#
# 3. The uri format is the same as for -incoming
#
+# 5. For now, number of migration streams is restricted to one, i.e
+# number of items in 'channels' list is just 1.
+#
+# 4. The 'uri' and 'channels' arguments are mutually exclusive;
+# exactly one of the two should be present.
+#
# Example:
#
# -> { "execute": "migrate-incoming",
# "arguments": { "uri": "tcp::4446" } }
# <- { "return": {} }
+#
+# -> { "execute": "migrate",
+# "arguments": {
+# "channels": [ { "channel-type": "main",
+# "addr": { "transport": "socket",
+# "type": "inet",
+# "host": "10.12.34.9",
+# "port": "1050" } } ] } }
+# <- { "return": {} }
+#
+# -> { "execute": "migrate",
+# "arguments": {
+# "channels": [ { "channel-type": "main",
+# "addr": { "transport": "exec",
+# "args": [ "/bin/nc", "-p", "6000",
+# "/some/sock" ] } } ] } }
+# <- { "return": {} }
+#
+# -> { "execute": "migrate",
+# "arguments": {
+# "channels": [ { "channel-type": "main",
+# "addr": { "transport": "rdma",
+# "host": "10.12.34.9",
+# "port": "1050" } } ] } }
+# <- { "return": {} }
##
-{ 'command': 'migrate-incoming', 'data': {'uri': 'str' } }
+{ 'command': 'migrate-incoming',
+ 'data': {'*uri': 'str',
+ '*channels': [ 'MigrationChannel' ] } }
##
# @xen-save-devices-state:
diff --git a/softmmu/vl.c b/softmmu/vl.c
index b0b96f67fa..d811f3f878 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2651,7 +2651,7 @@ void qmp_x_exit_preconfig(Error **errp)
if (incoming) {
Error *local_err = NULL;
if (strcmp(incoming, "defer") != 0) {
- qmp_migrate_incoming(incoming, &local_err);
+ qmp_migrate_incoming(incoming, false, NULL, &local_err);
if (local_err) {
error_reportf_err(local_err, "-incoming %s: ", incoming);
exit(1);
--
2.22.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 07/10] migration: modify migration_channels_and_uri_compatible() for new QAPI syntax
2023-07-21 14:49 [PATCH v9 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
` (5 preceding siblings ...)
2023-07-21 14:49 ` [PATCH v9 06/10] migration: New migrate and migrate-incoming argument 'channels' Het Gala
@ 2023-07-21 14:49 ` Het Gala
2023-07-21 14:49 ` [PATCH v9 08/10] migration: Implement MigrateChannelList to qmp migration flow Het Gala
` (2 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Het Gala @ 2023-07-21 14:49 UTC (permalink / raw)
To: qemu-devel
Cc: Prerna Saxena, quintela, dgilbert, pbonzini, berrange, armbru,
eblake, Manish Mishra, Aravind Retnakaran, Het Gala
migration_channels_and_uri_compatible() check for transport mechanism
suitable for multifd migration gets executed when the caller calls old
uri syntax. It needs it to be run when using the 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 bd3a93fc8c..6e0a8beaf2 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -104,17 +104,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;
}
@@ -482,12 +485,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;
}
@@ -1718,12 +1721,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] 22+ messages in thread
* [PATCH v9 08/10] migration: Implement MigrateChannelList to qmp migration flow.
2023-07-21 14:49 [PATCH v9 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
` (6 preceding siblings ...)
2023-07-21 14:49 ` [PATCH v9 07/10] migration: modify migration_channels_and_uri_compatible() for new QAPI syntax Het Gala
@ 2023-07-21 14:49 ` Het Gala
2023-07-25 18:38 ` Daniel P. Berrangé
2023-07-21 14:49 ` [PATCH v9 09/10] migration: Implement MigrateChannelList to hmp " Het Gala
2023-07-21 14:49 ` [PATCH v9 10/10] migration: modify test_multifd_tcp_none() to use new QAPI syntax Het Gala
9 siblings, 1 reply; 22+ messages in thread
From: Het Gala @ 2023-07-21 14:49 UTC (permalink / raw)
To: qemu-devel
Cc: Prerna Saxena, quintela, dgilbert, pbonzini, berrange, armbru,
eblake, 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 | 77 ++++++++++++++++++++++++++++---------------
1 file changed, 50 insertions(+), 27 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 6e0a8beaf2..acf80b3590 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -426,9 +426,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 = &addr->u.socket;
InetSocketAddress *isock = &addr->u.rdma;
@@ -465,7 +466,9 @@ static bool migrate_uri_parse(const char *uri,
return false;
}
- *channel = addr;
+ val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;
+ val->addr = addr;
+ *channel = val;
return true;
}
@@ -473,7 +476,8 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
MigrationChannelList *channels,
Error **errp)
{
- g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
+ g_autoptr(MigrationChannel) channel = g_new0(MigrationChannel, 1);
+ g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
/*
* Having preliminary checks for uri and channel
@@ -483,20 +487,29 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
"exclusive; exactly one of the two should be present in "
"'migrate-incoming' qmp command ");
return;
+ } 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 {
+ /* caller uses the old URI syntax */
+ if (uri && !migrate_uri_parse(uri, &channel, errp)) {
+ 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) {
@@ -505,11 +518,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 {
error_setg(errp, "unknown migration protocol: %s", uri);
}
@@ -1709,7 +1722,8 @@ void qmp_migrate(const char *uri, bool has_channels,
bool resume_requested;
Error *local_err = NULL;
MigrationState *s = migrate_get_current();
- g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
+ g_autoptr(MigrationChannel) channel = g_new0(MigrationChannel, 1);
+ g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
/*
* Having preliminary checks for uri and channel
@@ -1719,14 +1733,23 @@ void qmp_migrate(const char *uri, bool has_channels,
"exclusive; exactly one of the two should be present in "
"'migrate' qmp command ");
return;
+ } else if (channels) {
+ /* To verify that Migrate channel list has only item */
+ if (channels->next) {
+ error_setg(errp, "Channel list has more than one entries");
+ return;
+ }
+ channel = channels->value;
+ } else {
+ /* caller uses the old URI syntax */
+ if (uri && !migrate_uri_parse(uri, &channel, errp)) {
+ 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;
}
@@ -1743,8 +1766,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) {
@@ -1753,11 +1776,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 (!resume_requested) {
yank_unregister_instance(MIGRATION_YANK_INSTANCE);
--
2.22.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 09/10] migration: Implement MigrateChannelList to hmp migration flow.
2023-07-21 14:49 [PATCH v9 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
` (7 preceding siblings ...)
2023-07-21 14:49 ` [PATCH v9 08/10] migration: Implement MigrateChannelList to qmp migration flow Het Gala
@ 2023-07-21 14:49 ` Het Gala
2023-07-25 19:15 ` Daniel P. Berrangé
2023-07-21 14:49 ` [PATCH v9 10/10] migration: modify test_multifd_tcp_none() to use new QAPI syntax Het Gala
9 siblings, 1 reply; 22+ messages in thread
From: Het Gala @ 2023-07-21 14:49 UTC (permalink / raw)
To: qemu-devel
Cc: Prerna Saxena, quintela, dgilbert, pbonzini, berrange, armbru,
eblake, 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 | 16 +++++++++++++---
migration/migration.c | 5 ++---
migration/migration.h | 3 ++-
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 49b150f33f..25f51ec99c 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -423,10 +423,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 = g_new0(MigrationChannel, 1);
- qmp_migrate_incoming(uri, false, NULL, &err);
+ migrate_uri_parse(uri, &channel, &err);
+ QAPI_LIST_PREPEND(caps, channel);
- hmp_handle_error(mon, err);
+ qmp_migrate_incoming(NULL, true, caps, &err);
+ qapi_free_MigrationChannelList(caps);
}
void hmp_migrate_recover(Monitor *mon, const QDict *qdict)
@@ -704,9 +708,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 = g_new0(MigrationChannel, 1);
+
+ migrate_uri_parse(uri, &channel, &err);
+ QAPI_LIST_PREPEND(caps, channel);
- qmp_migrate(uri, false, NULL, !!blk, blk, !!inc, inc,
+ 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 acf80b3590..cf063a76df 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -425,9 +425,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 b7c8b67542..a8268394ca 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -501,7 +501,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] 22+ messages in thread
* [PATCH v9 10/10] migration: modify test_multifd_tcp_none() to use new QAPI syntax.
2023-07-21 14:49 [PATCH v9 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
` (8 preceding siblings ...)
2023-07-21 14:49 ` [PATCH v9 09/10] migration: Implement MigrateChannelList to hmp " Het Gala
@ 2023-07-21 14:49 ` Het Gala
2023-07-25 19:15 ` Daniel P. Berrangé
9 siblings, 1 reply; 22+ messages in thread
From: Het Gala @ 2023-07-21 14:49 UTC (permalink / raw)
To: qemu-devel
Cc: Prerna Saxena, quintela, dgilbert, pbonzini, berrange, armbru,
eblake, 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 e256da1216..376fad8311 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2185,7 +2185,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': [ { 'channeltype': 'main',"
+ " 'addr': { 'transport': 'socket',"
+ " 'type': 'inet',"
+ " 'host': '127.0.0.1',"
+ " 'port': '0' } } ] } }");
return NULL;
}
--
2.22.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v9 06/10] migration: New migrate and migrate-incoming argument 'channels'
2023-07-21 14:49 ` [PATCH v9 06/10] migration: New migrate and migrate-incoming argument 'channels' Het Gala
@ 2023-07-25 18:34 ` Daniel P. Berrangé
2023-07-25 18:37 ` Daniel P. Berrangé
0 siblings, 1 reply; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-07-25 18:34 UTC (permalink / raw)
To: Het Gala
Cc: qemu-devel, Prerna Saxena, quintela, dgilbert, pbonzini, armbru,
eblake, Manish Mishra, Aravind Retnakaran
On Fri, Jul 21, 2023 at 02:49:31PM +0000, Het Gala wrote:
> 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>
> ---
> migration/migration-hmp-cmds.c | 6 +-
> migration/migration.c | 34 ++++++++--
> qapi/migration.json | 109 ++++++++++++++++++++++++++++++++-
> softmmu/vl.c | 2 +-
> 4 files changed, 139 insertions(+), 12 deletions(-)
>
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 9885d7c9f7..49b150f33f 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -424,7 +424,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);
> }
> @@ -705,8 +705,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 f37b388876..bd3a93fc8c 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -466,10 +466,22 @@ static bool migrate_uri_parse(const char *uri,
> return true;
> }
>
> -static void qemu_start_incoming_migration(const char *uri, Error **errp)
> +static void qemu_start_incoming_migration(const char *uri, bool has_channels,
> + MigrationChannelList *channels,
> + Error **errp)
> {
> g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
>
> + /*
> + * Having preliminary checks for uri and channel
> + */
> + if (uri && has_channels) {
> + error_setg(errp, "'uri' and 'channels' arguments are mutually "
> + "exclusive; exactly one of the two should be present in "
> + "'migrate-incoming' qmp command ");
> + return;
> + }
This checks is both are present.
Also needs a check if neither are present as that's invalid.
> @@ -1694,6 +1708,16 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> MigrationState *s = migrate_get_current();
> g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
>
> + /*
> + * Having preliminary checks for uri and channel
> + */
> + if (uri && has_channels) {
> + error_setg(errp, "'uri' and 'channels' arguments are mutually "
> + "exclusive; exactly one of the two should be present in "
> + "'migrate' qmp command ");
> + return;
> + }
Same here
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 06/10] migration: New migrate and migrate-incoming argument 'channels'
2023-07-25 18:34 ` Daniel P. Berrangé
@ 2023-07-25 18:37 ` Daniel P. Berrangé
2023-07-25 19:32 ` Het Gala
2023-07-25 19:54 ` Het Gala
0 siblings, 2 replies; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-07-25 18:37 UTC (permalink / raw)
To: Het Gala, qemu-devel, Prerna Saxena, quintela, dgilbert,
pbonzini, armbru, eblake, Manish Mishra, Aravind Retnakaran
On Tue, Jul 25, 2023 at 07:34:09PM +0100, Daniel P. Berrangé wrote:
> On Fri, Jul 21, 2023 at 02:49:31PM +0000, Het Gala wrote:
> > 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>
> > ---
> > migration/migration-hmp-cmds.c | 6 +-
> > migration/migration.c | 34 ++++++++--
> > qapi/migration.json | 109 ++++++++++++++++++++++++++++++++-
> > softmmu/vl.c | 2 +-
> > 4 files changed, 139 insertions(+), 12 deletions(-)
> >
> > diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> > index 9885d7c9f7..49b150f33f 100644
> > --- a/migration/migration-hmp-cmds.c
> > +++ b/migration/migration-hmp-cmds.c
> > @@ -424,7 +424,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);
> > }
> > @@ -705,8 +705,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 f37b388876..bd3a93fc8c 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -466,10 +466,22 @@ static bool migrate_uri_parse(const char *uri,
> > return true;
> > }
> >
> > -static void qemu_start_incoming_migration(const char *uri, Error **errp)
> > +static void qemu_start_incoming_migration(const char *uri, bool has_channels,
> > + MigrationChannelList *channels,
> > + Error **errp)
> > {
> > g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
> >
> > + /*
> > + * Having preliminary checks for uri and channel
> > + */
> > + if (uri && has_channels) {
> > + error_setg(errp, "'uri' and 'channels' arguments are mutually "
> > + "exclusive; exactly one of the two should be present in "
> > + "'migrate-incoming' qmp command ");
> > + return;
> > + }
>
> This checks is both are present.
>
> Also needs a check if neither are present as that's invalid.
Also it should (temporarily) raise an error if "has_channels" is
set, as while we've added the parameter in QAPI, we've not
implemented it yet. IOW, raise an error now, and remove the
error in a later patch.
>
>
> > @@ -1694,6 +1708,16 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> > MigrationState *s = migrate_get_current();
> > g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
> >
> > + /*
> > + * Having preliminary checks for uri and channel
> > + */
> > + if (uri && has_channels) {
> > + error_setg(errp, "'uri' and 'channels' arguments are mutually "
> > + "exclusive; exactly one of the two should be present in "
> > + "'migrate' qmp command ");
> > + return;
> > + }
>
> Same here
>
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 08/10] migration: Implement MigrateChannelList to qmp migration flow.
2023-07-21 14:49 ` [PATCH v9 08/10] migration: Implement MigrateChannelList to qmp migration flow Het Gala
@ 2023-07-25 18:38 ` Daniel P. Berrangé
2023-07-25 19:49 ` Het Gala
0 siblings, 1 reply; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-07-25 18:38 UTC (permalink / raw)
To: Het Gala
Cc: qemu-devel, Prerna Saxena, quintela, dgilbert, pbonzini, armbru,
eblake, Manish Mishra, Aravind Retnakaran
On Fri, Jul 21, 2023 at 02:49:34PM +0000, Het Gala wrote:
> 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 | 77 ++++++++++++++++++++++++++++---------------
> 1 file changed, 50 insertions(+), 27 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 6e0a8beaf2..acf80b3590 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -426,9 +426,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 = &addr->u.socket;
> InetSocketAddress *isock = &addr->u.rdma;
> @@ -465,7 +466,9 @@ static bool migrate_uri_parse(const char *uri,
> return false;
> }
>
> - *channel = addr;
> + val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;
> + val->addr = addr;
> + *channel = val;
> return true;
> }
>
> @@ -473,7 +476,8 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
> MigrationChannelList *channels,
> Error **errp)
> {
> - g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
> + g_autoptr(MigrationChannel) channel = g_new0(MigrationChannel, 1);
> + g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
>
> /*
> * Having preliminary checks for uri and channel
> @@ -483,20 +487,29 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
> "exclusive; exactly one of the two should be present in "
> "'migrate-incoming' qmp command ");
> return;
> + } 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 {
> + /* caller uses the old URI syntax */
> + if (uri && !migrate_uri_parse(uri, &channel, errp)) {
> + 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) {
> @@ -505,11 +518,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 {
> error_setg(errp, "unknown migration protocol: %s", uri);
> }
> @@ -1709,7 +1722,8 @@ void qmp_migrate(const char *uri, bool has_channels,
> bool resume_requested;
> Error *local_err = NULL;
> MigrationState *s = migrate_get_current();
> - g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
> + g_autoptr(MigrationChannel) channel = g_new0(MigrationChannel, 1);
> + g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
>
> /*
> * Having preliminary checks for uri and channel
> @@ -1719,14 +1733,23 @@ void qmp_migrate(const char *uri, bool has_channels,
> "exclusive; exactly one of the two should be present in "
> "'migrate' qmp command ");
> return;
> + } else if (channels) {
> + /* To verify that Migrate channel list has only item */
> + if (channels->next) {
> + error_setg(errp, "Channel list has more than one entries");
> + return;
> + }
> + channel = channels->value;
> + } else {
} else if (uri) {
if (!migrate_uri_parse(....))
} else {
...error - neither uri or has_channels ...
}
> + /* caller uses the old URI syntax */
> + if (uri && !migrate_uri_parse(uri, &channel, errp)) {
> + 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;
> }
>
> @@ -1743,8 +1766,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) {
> @@ -1753,11 +1776,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 (!resume_requested) {
> yank_unregister_instance(MIGRATION_YANK_INSTANCE);
> --
> 2.22.3
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 09/10] migration: Implement MigrateChannelList to hmp migration flow.
2023-07-21 14:49 ` [PATCH v9 09/10] migration: Implement MigrateChannelList to hmp " Het Gala
@ 2023-07-25 19:15 ` Daniel P. Berrangé
2023-07-25 19:51 ` Het Gala
0 siblings, 1 reply; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-07-25 19:15 UTC (permalink / raw)
To: Het Gala
Cc: qemu-devel, Prerna Saxena, quintela, dgilbert, pbonzini, armbru,
eblake, Manish Mishra, Aravind Retnakaran
On Fri, Jul 21, 2023 at 02:49:35PM +0000, Het Gala wrote:
> 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 | 16 +++++++++++++---
> migration/migration.c | 5 ++---
> migration/migration.h | 3 ++-
> 3 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 49b150f33f..25f51ec99c 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -423,10 +423,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 = g_new0(MigrationChannel, 1);
>
> - qmp_migrate_incoming(uri, false, NULL, &err);
> + migrate_uri_parse(uri, &channel, &err);
> + QAPI_LIST_PREPEND(caps, channel);
>
> - hmp_handle_error(mon, err);
> + qmp_migrate_incoming(NULL, true, caps, &err);
> + qapi_free_MigrationChannelList(caps);
IIRC, you still need the hmp_handle_error call to print any
error message.
> }
>
> void hmp_migrate_recover(Monitor *mon, const QDict *qdict)
> @@ -704,9 +708,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 = g_new0(MigrationChannel, 1);
> +
> + migrate_uri_parse(uri, &channel, &err);
> + QAPI_LIST_PREPEND(caps, channel);
>
> - qmp_migrate(uri, false, NULL, !!blk, blk, !!inc, inc,
> + 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 acf80b3590..cf063a76df 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -425,9 +425,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 b7c8b67542..a8268394ca 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -501,7 +501,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
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 10/10] migration: modify test_multifd_tcp_none() to use new QAPI syntax.
2023-07-21 14:49 ` [PATCH v9 10/10] migration: modify test_multifd_tcp_none() to use new QAPI syntax Het Gala
@ 2023-07-25 19:15 ` Daniel P. Berrangé
0 siblings, 0 replies; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-07-25 19:15 UTC (permalink / raw)
To: Het Gala
Cc: qemu-devel, Prerna Saxena, quintela, dgilbert, pbonzini, armbru,
eblake, Manish Mishra, Aravind Retnakaran
On Fri, Jul 21, 2023 at 02:49:37PM +0000, Het Gala wrote:
> 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(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 06/10] migration: New migrate and migrate-incoming argument 'channels'
2023-07-25 18:37 ` Daniel P. Berrangé
@ 2023-07-25 19:32 ` Het Gala
2023-07-25 19:54 ` Het Gala
1 sibling, 0 replies; 22+ messages in thread
From: Het Gala @ 2023-07-25 19:32 UTC (permalink / raw)
To: Daniel P. Berrangé,
qemu-devel, Prerna Saxena, quintela, dgilbert, pbonzini, armbru,
eblake, Manish Mishra, Aravind Retnakaran
On 26/07/23 12:07 am, Daniel P. Berrangé wrote:
> On Tue, Jul 25, 2023 at 07:34:09PM +0100, Daniel P. Berrangé wrote:
>> On Fri, Jul 21, 2023 at 02:49:31PM +0000, Het Gala wrote:
>>> 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>
>>> ---
>>> migration/migration-hmp-cmds.c | 6 +-
>>> migration/migration.c | 34 ++++++++--
>>> qapi/migration.json | 109 ++++++++++++++++++++++++++++++++-
>>> softmmu/vl.c | 2 +-
>>> 4 files changed, 139 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
>>> index 9885d7c9f7..49b150f33f 100644
>>> --- a/migration/migration-hmp-cmds.c
>>> +++ b/migration/migration-hmp-cmds.c
>>> @@ -424,7 +424,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);
>>> }
>>> @@ -705,8 +705,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 f37b388876..bd3a93fc8c 100644
>>> --- a/migration/migration.c
>>> +++ b/migration/migration.c
>>> @@ -466,10 +466,22 @@ static bool migrate_uri_parse(const char *uri,
>>> return true;
>>> }
>>>
>>> -static void qemu_start_incoming_migration(const char *uri, Error **errp)
>>> +static void qemu_start_incoming_migration(const char *uri, bool has_channels,
>>> + MigrationChannelList *channels,
>>> + Error **errp)
>>> {
>>> g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
>>>
>>> + /*
>>> + * Having preliminary checks for uri and channel
>>> + */
>>> + if (uri && has_channels) {
>>> + error_setg(errp, "'uri' and 'channels' arguments are mutually "
>>> + "exclusive; exactly one of the two should be present in "
>>> + "'migrate-incoming' qmp command ");
>>> + return;
>>> + }
>> This checks is both are present.
>>
>> Also needs a check if neither are present as that's invalid.
> Also it should (temporarily) raise an error if "has_channels" is
> set, as while we've added the parameter in QAPI, we've not
> implemented it yet. IOW, raise an error now, and remove the
> error in a later patch.
Ack. So in total there should be 3 checks right. 1) if 'has_channels' is
set, 2) if 'uri' and 'channels' both are present, 3) if 'uri' and
'channels' both are absent. Basically right now only uri should allowed
and should atleast be present.
I think overall only 1) would be enough and should be checked before
'migration_channels_and_uri_compatible()' and if 'has_channels' is set,
just return for now. With this 2) would not be necessary or not come
into play in this patch. 3) will be taken care by
'migration_channels_and_uri_compatible()' itself IMO.
Let me know if I am missing something here.
>>
>>> @@ -1694,6 +1708,16 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>>> MigrationState *s = migrate_get_current();
>>> g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
>>>
>>> + /*
>>> + * Having preliminary checks for uri and channel
>>> + */
>>> + if (uri && has_channels) {
>>> + error_setg(errp, "'uri' and 'channels' arguments are mutually "
>>> + "exclusive; exactly one of the two should be present in "
>>> + "'migrate' qmp command ");
>>> + return;
>>> + }
>> Same here
>>
>>
>> With regards,
>> Daniel
>> --
>> |: https://urldefense.proofpoint.com/v2/url?u=https-3A__berrange.com&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=i0lmmIrs7N4r3uqLYCdRVXLFaEavt77Ltkec0hIlE4aUQITo9-8povsDHWELv-vE&s=AQ1C7WPg2jLYjNXU29Xw7trQcjmB96Yy3-God3-UaIQ&e= -o- https://urldefense.proofpoint.com/v2/url?u=https-3A__www.flickr.com_photos_dberrange&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=i0lmmIrs7N4r3uqLYCdRVXLFaEavt77Ltkec0hIlE4aUQITo9-8povsDHWELv-vE&s=aiGUx76ySVL-epTmaFIZUyZbkzeXGedVaXGvFw4xcgo&e= :|
>> |: https://urldefense.proofpoint.com/v2/url?u=https-3A__libvirt.org&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=i0lmmIrs7N4r3uqLYCdRVXLFaEavt77Ltkec0hIlE4aUQITo9-8povsDHWELv-vE&s=beFwppzRJ_eYlYPZKHlSZpaysLC5AExPh5_inAZBu_k&e= -o- https://urldefense.proofpoint.com/v2/url?u=https-3A__fstop138.berrange.com&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=i0lmmIrs7N4r3uqLYCdRVXLFaEavt77Ltkec0hIlE4aUQITo9-8povsDHWELv-vE&s=q6fb1PbChin197qfXnyXa1FpzaC7GecQJOzWO1EUQTo&e= :|
>> |: https://urldefense.proofpoint.com/v2/url?u=https-3A__entangle-2Dphoto.org&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=i0lmmIrs7N4r3uqLYCdRVXLFaEavt77Ltkec0hIlE4aUQITo9-8povsDHWELv-vE&s=lUJpjpbEMjPFyDvx1GXWR5yrxWfQqTyLI42J5oxavgs&e= -o- https://urldefense.proofpoint.com/v2/url?u=https-3A__www.instagram.com_dberrange&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=i0lmmIrs7N4r3uqLYCdRVXLFaEavt77Ltkec0hIlE4aUQITo9-8povsDHWELv-vE&s=u9HElHtI7mdNiWq8I7j1GpnUObfjfopMmpLdvGZRI8k&e= :|
>>
>>
> With regards,
> Daniel
Regards,
Het Gala
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 08/10] migration: Implement MigrateChannelList to qmp migration flow.
2023-07-25 18:38 ` Daniel P. Berrangé
@ 2023-07-25 19:49 ` Het Gala
0 siblings, 0 replies; 22+ messages in thread
From: Het Gala @ 2023-07-25 19:49 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Prerna Saxena, quintela, dgilbert, pbonzini, armbru,
eblake, Manish Mishra, Aravind Retnakaran
On 26/07/23 12:08 am, Daniel P. Berrangé wrote:
> On Fri, Jul 21, 2023 at 02:49:34PM +0000, Het Gala wrote:
>> 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 | 77 ++++++++++++++++++++++++++++---------------
>> 1 file changed, 50 insertions(+), 27 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 6e0a8beaf2..acf80b3590 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -426,9 +426,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 = &addr->u.socket;
>> InetSocketAddress *isock = &addr->u.rdma;
>> @@ -465,7 +466,9 @@ static bool migrate_uri_parse(const char *uri,
>> return false;
>> }
>>
>> - *channel = addr;
>> + val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;
>> + val->addr = addr;
>> + *channel = val;
>> return true;
>> }
>>
>> @@ -473,7 +476,8 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
>> MigrationChannelList *channels,
>> Error **errp)
>> {
>> - g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
>> + g_autoptr(MigrationChannel) channel = g_new0(MigrationChannel, 1);
>> + g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
>>
>> /*
>> * Having preliminary checks for uri and channel
>> @@ -483,20 +487,29 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
>> "exclusive; exactly one of the two should be present in "
>> "'migrate-incoming' qmp command ");
>> return;
>> + } 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 {
>> + /* caller uses the old URI syntax */
>> + if (uri && !migrate_uri_parse(uri, &channel, errp)) {
>> + 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) {
>> @@ -505,11 +518,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 {
>> error_setg(errp, "unknown migration protocol: %s", uri);
>> }
>> @@ -1709,7 +1722,8 @@ void qmp_migrate(const char *uri, bool has_channels,
>> bool resume_requested;
>> Error *local_err = NULL;
>> MigrationState *s = migrate_get_current();
>> - g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
>> + g_autoptr(MigrationChannel) channel = g_new0(MigrationChannel, 1);
>> + g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
>>
>> /*
>> * Having preliminary checks for uri and channel
>> @@ -1719,14 +1733,23 @@ void qmp_migrate(const char *uri, bool has_channels,
>> "exclusive; exactly one of the two should be present in "
>> "'migrate' qmp command ");
>> return;
>> + } else if (channels) {
>> + /* To verify that Migrate channel list has only item */
>> + if (channels->next) {
>> + error_setg(errp, "Channel list has more than one entries");
>> + return;
>> + }
>> + channel = channels->value;
>> + } else {
> } else if (uri) {
> if (!migrate_uri_parse(....))
> } else {
> ...error - neither uri or has_channels ...
>
> }
Ack. Will add this here.
>> + /* caller uses the old URI syntax */
>> + if (uri && !migrate_uri_parse(uri, &channel, errp)) {
>> + 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;
>> }
>>
>> @@ -1743,8 +1766,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) {
>> @@ -1753,11 +1776,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 (!resume_requested) {
>> yank_unregister_instance(MIGRATION_YANK_INSTANCE);
>> --
>> 2.22.3
>>
> With regards,
> Daniel
Regards,
Het Gala
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 09/10] migration: Implement MigrateChannelList to hmp migration flow.
2023-07-25 19:15 ` Daniel P. Berrangé
@ 2023-07-25 19:51 ` Het Gala
0 siblings, 0 replies; 22+ messages in thread
From: Het Gala @ 2023-07-25 19:51 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Prerna Saxena, quintela, dgilbert, pbonzini, armbru,
eblake, Manish Mishra, Aravind Retnakaran
On 26/07/23 12:45 am, Daniel P. Berrangé wrote:
> On Fri, Jul 21, 2023 at 02:49:35PM +0000, Het Gala wrote:
>> 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 | 16 +++++++++++++---
>> migration/migration.c | 5 ++---
>> migration/migration.h | 3 ++-
>> 3 files changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
>> index 49b150f33f..25f51ec99c 100644
>> --- a/migration/migration-hmp-cmds.c
>> +++ b/migration/migration-hmp-cmds.c
>> @@ -423,10 +423,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 = g_new0(MigrationChannel, 1);
>>
>> - qmp_migrate_incoming(uri, false, NULL, &err);
>> + migrate_uri_parse(uri, &channel, &err);
>> + QAPI_LIST_PREPEND(caps, channel);
>>
>> - hmp_handle_error(mon, err);
>> + qmp_migrate_incoming(NULL, true, caps, &err);
>> + qapi_free_MigrationChannelList(caps);
> IIRC, you still need the hmp_handle_error call to print any
> error message.
Yes, sorry, I missed that out while adding the statements. Will add
hmp_handle_error call here.
>> }
>>
>> void hmp_migrate_recover(Monitor *mon, const QDict *qdict)
>> @@ -704,9 +708,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 = g_new0(MigrationChannel, 1);
>> +
>> + migrate_uri_parse(uri, &channel, &err);
>> + QAPI_LIST_PREPEND(caps, channel);
>>
>> - qmp_migrate(uri, false, NULL, !!blk, blk, !!inc, inc,
>> + 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 acf80b3590..cf063a76df 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -425,9 +425,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 b7c8b67542..a8268394ca 100644
>> --- a/migration/migration.h
>> +++ b/migration/migration.h
>> @@ -501,7 +501,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
>>
> With regards,
> Daniel
Regards,
Het Gala
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 06/10] migration: New migrate and migrate-incoming argument 'channels'
2023-07-25 18:37 ` Daniel P. Berrangé
2023-07-25 19:32 ` Het Gala
@ 2023-07-25 19:54 ` Het Gala
2023-07-26 8:35 ` Daniel P. Berrangé
1 sibling, 1 reply; 22+ messages in thread
From: Het Gala @ 2023-07-25 19:54 UTC (permalink / raw)
To: Daniel P. Berrangé,
qemu-devel, Prerna Saxena, quintela, dgilbert, pbonzini, armbru,
eblake, Manish Mishra, Aravind Retnakaran
Sorry, last reply on this patch was accidently replied only to Daniel.
Pasting the reply again so it is received by all the active maintianers
here. Apologies for the error 😅
On 26/07/23 12:07 am, Daniel P. Berrangé wrote:
> On Tue, Jul 25, 2023 at 07:34:09PM +0100, Daniel P. Berrangé wrote:
>> On Fri, Jul 21, 2023 at 02:49:31PM +0000, Het Gala wrote:
>>> 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>
>>> ---
>>> migration/migration-hmp-cmds.c | 6 +-
>>> migration/migration.c | 34 ++++++++--
>>> qapi/migration.json | 109 ++++++++++++++++++++++++++++++++-
>>> softmmu/vl.c | 2 +-
>>> 4 files changed, 139 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
>>> index 9885d7c9f7..49b150f33f 100644
>>> --- a/migration/migration-hmp-cmds.c
>>> +++ b/migration/migration-hmp-cmds.c
>>> @@ -424,7 +424,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);
>>> }
>>> @@ -705,8 +705,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 f37b388876..bd3a93fc8c 100644
>>> --- a/migration/migration.c
>>> +++ b/migration/migration.c
>>> @@ -466,10 +466,22 @@ static bool migrate_uri_parse(const char *uri,
>>> return true;
>>> }
>>>
>>> -static void qemu_start_incoming_migration(const char *uri, Error **errp)
>>> +static void qemu_start_incoming_migration(const char *uri, bool has_channels,
>>> + MigrationChannelList *channels,
>>> + Error **errp)
>>> {
>>> g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
>>>
>>> + /*
>>> + * Having preliminary checks for uri and channel
>>> + */
>>> + if (uri && has_channels) {
>>> + error_setg(errp, "'uri' and 'channels' arguments are mutually "
>>> + "exclusive; exactly one of the two should be present in "
>>> + "'migrate-incoming' qmp command ");
>>> + return;
>>> + }
>> This checks is both are present.
>>
>> Also needs a check if neither are present as that's invalid.
> Also it should (temporarily) raise an error if "has_channels" is
> set, as while we've added the parameter in QAPI, we've not
> implemented it yet. IOW, raise an error now, and remove the
> error in a later patch.
Ack. So in total there should be 3 checks right. 1) if 'has_channels' is
set, 2) if 'uri' and 'channels' both are present, 3) if 'uri' and
'channels' both are absent. Basically right now only uri should allowed
and should atleast be present.
I think overall only 1) would be enough and should be checked before
'migration_channels_and_uri_compatible()' and if 'has_channels' is set,
just return for now. With this 2) would not be necessary or not come
into play in this patch. 3) will be taken care by
'migration_channels_and_uri_compatible()' itself IMO.
Let me know if I am missing something here.
>>
>>> @@ -1694,6 +1708,16 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>>> MigrationState *s = migrate_get_current();
>>> g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
>>>
>>> + /*
>>> + * Having preliminary checks for uri and channel
>>> + */
>>> + if (uri && has_channels) {
>>> + error_setg(errp, "'uri' and 'channels' arguments are mutually "
>>> + "exclusive; exactly one of the two should be present in "
>>> + "'migrate' qmp command ");
>>> + return;
>>> + }
>> Same here
>>
>>
>> With regards,
>> Daniel
>> --
>> |: https://urldefense.proofpoint.com/v2/url?u=https-3A__berrange.com&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=i0lmmIrs7N4r3uqLYCdRVXLFaEavt77Ltkec0hIlE4aUQITo9-8povsDHWELv-vE&s=AQ1C7WPg2jLYjNXU29Xw7trQcjmB96Yy3-God3-UaIQ&e= -o- https://urldefense.proofpoint.com/v2/url?u=https-3A__www.flickr.com_photos_dberrange&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=i0lmmIrs7N4r3uqLYCdRVXLFaEavt77Ltkec0hIlE4aUQITo9-8povsDHWELv-vE&s=aiGUx76ySVL-epTmaFIZUyZbkzeXGedVaXGvFw4xcgo&e= :|
>> |: https://urldefense.proofpoint.com/v2/url?u=https-3A__libvirt.org&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=i0lmmIrs7N4r3uqLYCdRVXLFaEavt77Ltkec0hIlE4aUQITo9-8povsDHWELv-vE&s=beFwppzRJ_eYlYPZKHlSZpaysLC5AExPh5_inAZBu_k&e= -o- https://urldefense.proofpoint.com/v2/url?u=https-3A__fstop138.berrange.com&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=i0lmmIrs7N4r3uqLYCdRVXLFaEavt77Ltkec0hIlE4aUQITo9-8povsDHWELv-vE&s=q6fb1PbChin197qfXnyXa1FpzaC7GecQJOzWO1EUQTo&e= :|
>> |: https://urldefense.proofpoint.com/v2/url?u=https-3A__entangle-2Dphoto.org&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=i0lmmIrs7N4r3uqLYCdRVXLFaEavt77Ltkec0hIlE4aUQITo9-8povsDHWELv-vE&s=lUJpjpbEMjPFyDvx1GXWR5yrxWfQqTyLI42J5oxavgs&e= -o- https://urldefense.proofpoint.com/v2/url?u=https-3A__www.instagram.com_dberrange&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=i0lmmIrs7N4r3uqLYCdRVXLFaEavt77Ltkec0hIlE4aUQITo9-8povsDHWELv-vE&s=u9HElHtI7mdNiWq8I7j1GpnUObfjfopMmpLdvGZRI8k&e= :|
>>
>>
> With regards,
> Daniel
Regards,
Het Gala
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 06/10] migration: New migrate and migrate-incoming argument 'channels'
2023-07-25 19:54 ` Het Gala
@ 2023-07-26 8:35 ` Daniel P. Berrangé
2023-07-26 10:02 ` Het Gala
0 siblings, 1 reply; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-07-26 8:35 UTC (permalink / raw)
To: Het Gala
Cc: qemu-devel, Prerna Saxena, quintela, dgilbert, pbonzini, armbru,
eblake, Manish Mishra, Aravind Retnakaran
On Wed, Jul 26, 2023 at 01:24:48AM +0530, Het Gala wrote:
> Sorry, last reply on this patch was accidently replied only to Daniel.
> Pasting the reply again so it is received by all the active maintianers
> here. Apologies for the error 😅
>
> On 26/07/23 12:07 am, Daniel P. Berrangé wrote:
> > On Tue, Jul 25, 2023 at 07:34:09PM +0100, Daniel P. Berrangé wrote:
> > > On Fri, Jul 21, 2023 at 02:49:31PM +0000, Het Gala wrote:
> > > > 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>
> > > > ---
> > > > migration/migration-hmp-cmds.c | 6 +-
> > > > migration/migration.c | 34 ++++++++--
> > > > qapi/migration.json | 109 ++++++++++++++++++++++++++++++++-
> > > > softmmu/vl.c | 2 +-
> > > > 4 files changed, 139 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> > > > index 9885d7c9f7..49b150f33f 100644
> > > > --- a/migration/migration-hmp-cmds.c
> > > > +++ b/migration/migration-hmp-cmds.c
> > > > @@ -424,7 +424,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);
> > > > }
> > > > @@ -705,8 +705,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 f37b388876..bd3a93fc8c 100644
> > > > --- a/migration/migration.c
> > > > +++ b/migration/migration.c
> > > > @@ -466,10 +466,22 @@ static bool migrate_uri_parse(const char *uri,
> > > > return true;
> > > > }
> > > > -static void qemu_start_incoming_migration(const char *uri, Error **errp)
> > > > +static void qemu_start_incoming_migration(const char *uri, bool has_channels,
> > > > + MigrationChannelList *channels,
> > > > + Error **errp)
> > > > {
> > > > g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
> > > > + /*
> > > > + * Having preliminary checks for uri and channel
> > > > + */
> > > > + if (uri && has_channels) {
> > > > + error_setg(errp, "'uri' and 'channels' arguments are mutually "
> > > > + "exclusive; exactly one of the two should be present in "
> > > > + "'migrate-incoming' qmp command ");
> > > > + return;
> > > > + }
> > > This checks is both are present.
> > >
> > > Also needs a check if neither are present as that's invalid.
> > Also it should (temporarily) raise an error if "has_channels" is
> > set, as while we've added the parameter in QAPI, we've not
> > implemented it yet. IOW, raise an error now, and remove the
> > error in a later patch.
> Ack. So in total there should be 3 checks right. 1) if 'has_channels' is
> set, 2) if 'uri' and 'channels' both are present, 3) if 'uri' and 'channels'
> both are absent. Basically right now only uri should allowed and should
> atleast be present.
Correct.
> I think overall only 1) would be enough and should be checked before
> 'migration_channels_and_uri_compatible()' and if 'has_channels' is set, just
> return for now. With this 2) would not be necessary or not come into play in
> this patch. 3) will be taken care by
> 'migration_channels_and_uri_compatible()' itself IMO.
I think all the checks should be in this method, as it is the entrypoint
for execution of the QMP command and thus where parameter validation
should live. Spreading it across methods will make it very easy to open
bugs by refactoring code and not realizing the new codepaths don't do
the right checks.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 06/10] migration: New migrate and migrate-incoming argument 'channels'
2023-07-26 8:35 ` Daniel P. Berrangé
@ 2023-07-26 10:02 ` Het Gala
0 siblings, 0 replies; 22+ messages in thread
From: Het Gala @ 2023-07-26 10:02 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Prerna Saxena, quintela, dgilbert, pbonzini, armbru,
eblake, Manish Mishra, Aravind Retnakaran
On 26/07/23 2:05 pm, Daniel P. Berrangé wrote:
> On Wed, Jul 26, 2023 at 01:24:48AM +0530, Het Gala wrote:
>> Sorry, last reply on this patch was accidently replied only to Daniel.
>> Pasting the reply again so it is received by all the active maintianers
>> here. Apologies for the error 😅
>>
>> On 26/07/23 12:07 am, Daniel P. Berrangé wrote:
>>> On Tue, Jul 25, 2023 at 07:34:09PM +0100, Daniel P. Berrangé wrote:
>>>> On Fri, Jul 21, 2023 at 02:49:31PM +0000, Het Gala wrote:
>>>>> 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>
>>>>> ---
>>>>> migration/migration-hmp-cmds.c | 6 +-
>>>>> migration/migration.c | 34 ++++++++--
>>>>> qapi/migration.json | 109 ++++++++++++++++++++++++++++++++-
>>>>> softmmu/vl.c | 2 +-
>>>>> 4 files changed, 139 insertions(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
>>>>> index 9885d7c9f7..49b150f33f 100644
>>>>> --- a/migration/migration-hmp-cmds.c
>>>>> +++ b/migration/migration-hmp-cmds.c
>>>>> @@ -424,7 +424,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);
>>>>> }
>>>>> @@ -705,8 +705,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 f37b388876..bd3a93fc8c 100644
>>>>> --- a/migration/migration.c
>>>>> +++ b/migration/migration.c
>>>>> @@ -466,10 +466,22 @@ static bool migrate_uri_parse(const char *uri,
>>>>> return true;
>>>>> }
>>>>> -static void qemu_start_incoming_migration(const char *uri, Error **errp)
>>>>> +static void qemu_start_incoming_migration(const char *uri, bool has_channels,
>>>>> + MigrationChannelList *channels,
>>>>> + Error **errp)
>>>>> {
>>>>> g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
>>>>> + /*
>>>>> + * Having preliminary checks for uri and channel
>>>>> + */
>>>>> + if (uri && has_channels) {
>>>>> + error_setg(errp, "'uri' and 'channels' arguments are mutually "
>>>>> + "exclusive; exactly one of the two should be present in "
>>>>> + "'migrate-incoming' qmp command ");
>>>>> + return;
>>>>> + }
>>>> This checks is both are present.
>>>>
>>>> Also needs a check if neither are present as that's invalid.
>>> Also it should (temporarily) raise an error if "has_channels" is
>>> set, as while we've added the parameter in QAPI, we've not
>>> implemented it yet. IOW, raise an error now, and remove the
>>> error in a later patch.
>> Ack. So in total there should be 3 checks right. 1) if 'has_channels' is
>> set, 2) if 'uri' and 'channels' both are present, 3) if 'uri' and 'channels'
>> both are absent. Basically right now only uri should allowed and should
>> atleast be present.
> Correct.
Ack.
>> I think overall only 1) would be enough and should be checked before
>> 'migration_channels_and_uri_compatible()' and if 'has_channels' is set, just
>> return for now. With this 2) would not be necessary or not come into play in
>> this patch. 3) will be taken care by
>> 'migration_channels_and_uri_compatible()' itself IMO.
> I think all the checks should be in this method, as it is the entrypoint
> for execution of the QMP command and thus where parameter validation
> should live. Spreading it across methods will make it very easy to open
> bugs by refactoring code and not realizing the new codepaths don't do
> the right checks.
Okay, I got the concern here. Will add these 3 checks in this patch and
later remove 'has_channels' check in the patch where we are implementing
MigrateChannel.
> With regards,
> Daniel
Regards,
Het Gala
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2023-07-26 10:02 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-21 14:49 [PATCH v9 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration Het Gala
2023-07-21 14:49 ` [PATCH v9 01/10] migration: New QAPI type 'MigrateAddress' Het Gala
2023-07-21 14:49 ` [PATCH v9 02/10] migration: convert migration 'uri' into 'MigrateAddress' Het Gala
2023-07-21 14:49 ` [PATCH v9 03/10] migration: convert socket backend to accept MigrateAddress Het Gala
2023-07-21 14:49 ` [PATCH v9 04/10] migration: convert rdma " Het Gala
2023-07-21 14:49 ` [PATCH v9 05/10] migration: convert exec " Het Gala
2023-07-21 14:49 ` [PATCH v9 06/10] migration: New migrate and migrate-incoming argument 'channels' Het Gala
2023-07-25 18:34 ` Daniel P. Berrangé
2023-07-25 18:37 ` Daniel P. Berrangé
2023-07-25 19:32 ` Het Gala
2023-07-25 19:54 ` Het Gala
2023-07-26 8:35 ` Daniel P. Berrangé
2023-07-26 10:02 ` Het Gala
2023-07-21 14:49 ` [PATCH v9 07/10] migration: modify migration_channels_and_uri_compatible() for new QAPI syntax Het Gala
2023-07-21 14:49 ` [PATCH v9 08/10] migration: Implement MigrateChannelList to qmp migration flow Het Gala
2023-07-25 18:38 ` Daniel P. Berrangé
2023-07-25 19:49 ` Het Gala
2023-07-21 14:49 ` [PATCH v9 09/10] migration: Implement MigrateChannelList to hmp " Het Gala
2023-07-25 19:15 ` Daniel P. Berrangé
2023-07-25 19:51 ` Het Gala
2023-07-21 14:49 ` [PATCH v9 10/10] migration: modify test_multifd_tcp_none() to use new QAPI syntax Het Gala
2023-07-25 19:15 ` Daniel P. Berrangé
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.