Integrated MigrateChannelList with all transport backends (socket, exec
and rdma) for both source and destination migration code flow.
Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
migration/migration.c | 95 +++++++++++++++++++++++++++----------------
migration/socket.c | 5 ++-
2 files changed, 64 insertions(+), 36 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index de058643a6..a37eba29e3 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -410,10 +410,11 @@ void migrate_add_address(SocketAddress *address)
}
static bool migrate_uri_parse(const char *uri,
- MigrateAddress **channel,
+ MigrateChannel **channel,
Error **errp)
{
Error *local_err = NULL;
+ MigrateChannel *val = g_new0(MigrateChannel, 1);
MigrateAddress *addrs = g_new0(MigrateAddress, 1);
SocketAddress *saddr;
InetSocketAddress *isock = &addrs->u.rdma;
@@ -441,6 +442,7 @@ static bool migrate_uri_parse(const char *uri,
}
if (local_err) {
+ qapi_free_MigrateChannel(val);
qapi_free_MigrateAddress(addrs);
qapi_free_SocketAddress(saddr);
qapi_free_InetSocketAddress(isock);
@@ -448,7 +450,9 @@ static bool migrate_uri_parse(const char *uri,
return false;
}
- *channel = addrs;
+ val->channeltype = MIGRATE_CHANNEL_TYPE_MAIN;
+ val->addr = addrs;
+ *channel = val;
return true;
}
@@ -457,8 +461,9 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
Error **errp)
{
Error *local_err = NULL;
- MigrateAddress *channel = g_new0(MigrateAddress, 1);
+ MigrateAddress *addrs;
SocketAddress *saddr;
+ MigrateChannel *channel = NULL;
/*
* Having preliminary checks for uri and channel
@@ -467,22 +472,30 @@ static void qemu_start_incoming_migration(const char *uri, bool 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)) {
goto out;
- }
+ } 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");
+ goto out;
+ }
+ channel = channels->value;
+ } else {
+ /* URI is not suitable for migration? */
+ if (!migration_channels_and_uri_compatible(uri, errp)) {
+ goto out;
+ }
- if (uri && !migrate_uri_parse(uri, &channel, errp)) {
- error_setg(errp, "Error parsing uri");
- goto out;
+ if (uri && !migrate_uri_parse(uri, &channel, errp)) {
+ error_setg(errp, "Error parsing uri");
+ goto out;
+ }
}
- saddr = &channel->u.socket;
+ addrs = channel->addr;
+ saddr = &channel->addr->u.socket;
qapi_event_send_migration(MIGRATION_STATUS_SETUP);
- if (channel->transport == MIGRATE_TRANSPORT_SOCKET) {
+ if (addrs->transport == MIGRATE_TRANSPORT_SOCKET) {
if (saddr->type == SOCKET_ADDRESS_TYPE_INET ||
saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) {
@@ -491,23 +504,25 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
fd_start_incoming_migration(saddr->u.fd.str, &local_err);
}
#ifdef CONFIG_RDMA
- } else if (channel->transport == MIGRATE_TRANSPORT_RDMA) {
- rdma_start_incoming_migration(&channel->u.rdma, &local_err);
+ } else if (addrs->transport == MIGRATE_TRANSPORT_RDMA) {
+ rdma_start_incoming_migration(&addrs->u.rdma, &local_err);
#endif
- } else if (channel->transport == MIGRATE_TRANSPORT_EXEC) {
- exec_start_incoming_migration(channel->u.exec.args, &local_err);
+ } else if (addrs->transport == MIGRATE_TRANSPORT_EXEC) {
+ exec_start_incoming_migration(addrs->u.exec.args, &local_err);
} else {
error_setg(errp, "unknown migration protocol: %s", uri);
}
if (local_err) {
+ qapi_free_MigrateAddress(addrs);
qapi_free_SocketAddress(saddr);
error_propagate(errp, local_err);
return;
}
out:
- qapi_free_MigrateAddress(channel);
+ qapi_free_MigrateChannel(channel);
+ return;
}
static void process_incoming_migration_bh(void *opaque)
@@ -1714,8 +1729,9 @@ void qmp_migrate(const char *uri, bool has_channels,
{
Error *local_err = NULL;
MigrationState *s = migrate_get_current();
- MigrateAddress *channel = g_new0(MigrateAddress, 1);
+ MigrateAddress *addrs;
SocketAddress *saddr;
+ MigrateChannel *channel = NULL;
/*
* Having preliminary checks for uri and channel
@@ -1724,17 +1740,24 @@ void qmp_migrate(const char *uri, bool 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)) {
goto out;
- }
+ } 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");
+ goto out;
+ }
+ channel = channels->value;
+ } else {
+ /* URI is not suitable for migration? */
+ if (!migration_channels_and_uri_compatible(uri, errp)) {
+ goto out;
+ }
- if (!migrate_uri_parse(uri, &channel, &local_err)) {
- error_setg(errp, "Error parsing uri");
- goto out;
+ if (!migrate_uri_parse(uri, &channel, &local_err)) {
+ error_setg(errp, "Error parsing uri");
+ goto out;
+ }
}
if (!migrate_prepare(s, has_blk && blk, has_inc && inc,
@@ -1749,8 +1772,9 @@ void qmp_migrate(const char *uri, bool has_channels,
}
}
- saddr = &channel->u.socket;
- if (channel->transport == MIGRATE_TRANSPORT_SOCKET) {
+ addrs = channel->addr;
+ saddr = &channel->addr->u.socket;
+ if (addrs->transport == MIGRATE_TRANSPORT_SOCKET) {
if (saddr->type == SOCKET_ADDRESS_TYPE_INET ||
saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) {
@@ -1759,11 +1783,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 == MIGRATE_TRANSPORT_RDMA) {
- rdma_start_outgoing_migration(s, &channel->u.rdma, &local_err);
+ } else if (addrs->transport == MIGRATE_TRANSPORT_RDMA) {
+ rdma_start_outgoing_migration(s, &addrs->u.rdma, &local_err);
#endif
- } else if (channel->transport == MIGRATE_TRANSPORT_EXEC) {
- exec_start_outgoing_migration(s, channel->u.exec.args, &local_err);
+ } else if (addrs->transport == MIGRATE_TRANSPORT_EXEC) {
+ exec_start_outgoing_migration(s, addrs->u.exec.args, &local_err);
} else {
if (!(has_resume && resume)) {
yank_unregister_instance(MIGRATION_YANK_INSTANCE);
@@ -1780,6 +1804,7 @@ void qmp_migrate(const char *uri, bool has_channels,
if (!(has_resume && resume)) {
yank_unregister_instance(MIGRATION_YANK_INSTANCE);
}
+ qapi_free_MigrateAddress(addrs);
qapi_free_SocketAddress(saddr);
migrate_fd_error(s, local_err);
error_propagate(errp, local_err);
diff --git a/migration/socket.c b/migration/socket.c
index 8e7430b266..98e3ea1514 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -28,6 +28,8 @@
#include "trace.h"
#include "postcopy-ram.h"
#include "options.h"
+#include "qapi/clone-visitor.h"
+#include "qapi/qapi-visit-sockets.h"
struct SocketOutgoingArgs {
SocketAddress *saddr;
@@ -114,12 +116,13 @@ void socket_start_outgoing_migration(MigrationState *s,
{
QIOChannelSocket *sioc = qio_channel_socket_new();
struct SocketConnectData *data = g_new0(struct SocketConnectData, 1);
+ SocketAddress *addr = QAPI_CLONE(SocketAddress, saddr);
data->s = s;
/* in case previous migration leaked it */
qapi_free_SocketAddress(outgoing_args.saddr);
- outgoing_args.saddr = saddr;
+ outgoing_args.saddr = addr;
if (saddr->type == SOCKET_ADDRESS_TYPE_INET) {
data->hostname = g_strdup(saddr->u.inet.host);
--
2.22.3On Fri, May 12, 2023 at 02:32:40PM +0000, Het Gala wrote:
> Integrated MigrateChannelList with all transport backends (socket, exec
> and rdma) for both source and destination migration code flow.
>
> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> ---
> migration/migration.c | 95 +++++++++++++++++++++++++++----------------
> migration/socket.c | 5 ++-
> 2 files changed, 64 insertions(+), 36 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index de058643a6..a37eba29e3 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -410,10 +410,11 @@ void migrate_add_address(SocketAddress *address)
> }
>
> static bool migrate_uri_parse(const char *uri,
> - MigrateAddress **channel,
> + MigrateChannel **channel,
> Error **errp)
> {
> Error *local_err = NULL;
> + MigrateChannel *val = g_new0(MigrateChannel, 1);
> MigrateAddress *addrs = g_new0(MigrateAddress, 1);
> SocketAddress *saddr;
> InetSocketAddress *isock = &addrs->u.rdma;
> @@ -441,6 +442,7 @@ static bool migrate_uri_parse(const char *uri,
> }
>
> if (local_err) {
> + qapi_free_MigrateChannel(val);
> qapi_free_MigrateAddress(addrs);
> qapi_free_SocketAddress(saddr);
> qapi_free_InetSocketAddress(isock);
> @@ -448,7 +450,9 @@ static bool migrate_uri_parse(const char *uri,
> return false;
> }
>
> - *channel = addrs;
> + val->channeltype = MIGRATE_CHANNEL_TYPE_MAIN;
> + val->addr = addrs;
> + *channel = val;
> return true;
> }
>
> @@ -457,8 +461,9 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
> Error **errp)
> {
> Error *local_err = NULL;
> - MigrateAddress *channel = g_new0(MigrateAddress, 1);
> + MigrateAddress *addrs;
> SocketAddress *saddr;
> + MigrateChannel *channel = NULL;
>
> /*
> * Having preliminary checks for uri and channel
> @@ -467,22 +472,30 @@ static void qemu_start_incoming_migration(const char *uri, bool 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)) {
> goto out;
> - }
> + } 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");
> + goto out;
> + }
> + channel = channels->value;
> + } else {
> + /* URI is not suitable for migration? */
> + if (!migration_channels_and_uri_compatible(uri, errp)) {
> + goto out;
> + }
THis check only gets executed when the caller uses the old
URI syntax. We need to it be run when using the modern
MigrateChannel QAPI syntax too.
IOW, migration_channels_and_uri_compatible() needs converting
to take a 'MigrateChannel' object instead of URI, and then
the check can be run after the URI -> MigrateCHannel conversion
>
> - if (uri && !migrate_uri_parse(uri, &channel, errp)) {
> - error_setg(errp, "Error parsing uri");
> - goto out;
> + if (uri && !migrate_uri_parse(uri, &channel, errp)) {
> + error_setg(errp, "Error parsing uri");
> + goto out;
> + }
> }
>
> - saddr = &channel->u.socket;
> + addrs = channel->addr;
> + saddr = &channel->addr->u.socket;
> qapi_event_send_migration(MIGRATION_STATUS_SETUP);
> - if (channel->transport == MIGRATE_TRANSPORT_SOCKET) {
> + if (addrs->transport == MIGRATE_TRANSPORT_SOCKET) {
> if (saddr->type == SOCKET_ADDRESS_TYPE_INET ||
> saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
> saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) {
> @@ -491,23 +504,25 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
> fd_start_incoming_migration(saddr->u.fd.str, &local_err);
> }
> #ifdef CONFIG_RDMA
> - } else if (channel->transport == MIGRATE_TRANSPORT_RDMA) {
> - rdma_start_incoming_migration(&channel->u.rdma, &local_err);
> + } else if (addrs->transport == MIGRATE_TRANSPORT_RDMA) {
> + rdma_start_incoming_migration(&addrs->u.rdma, &local_err);
> #endif
> - } else if (channel->transport == MIGRATE_TRANSPORT_EXEC) {
> - exec_start_incoming_migration(channel->u.exec.args, &local_err);
> + } else if (addrs->transport == MIGRATE_TRANSPORT_EXEC) {
> + exec_start_incoming_migration(addrs->u.exec.args, &local_err);
> } else {
> error_setg(errp, "unknown migration protocol: %s", uri);
> }
>
> if (local_err) {
> + qapi_free_MigrateAddress(addrs);
> qapi_free_SocketAddress(saddr);
> error_propagate(errp, local_err);
> return;
> }
>
> out:
> - qapi_free_MigrateAddress(channel);
> + qapi_free_MigrateChannel(channel);
> + return;
> }
>
> static void process_incoming_migration_bh(void *opaque)
> @@ -1714,8 +1729,9 @@ void qmp_migrate(const char *uri, bool has_channels,
> {
> Error *local_err = NULL;
> MigrationState *s = migrate_get_current();
> - MigrateAddress *channel = g_new0(MigrateAddress, 1);
> + MigrateAddress *addrs;
> SocketAddress *saddr;
> + MigrateChannel *channel = NULL;
>
> /*
> * Having preliminary checks for uri and channel
> @@ -1724,17 +1740,24 @@ void qmp_migrate(const char *uri, bool 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)) {
> goto out;
> - }
> + } 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");
> + goto out;
> + }
> + channel = channels->value;
> + } else {
> + /* URI is not suitable for migration? */
> + if (!migration_channels_and_uri_compatible(uri, errp)) {
> + goto out;
> + }
>
> - if (!migrate_uri_parse(uri, &channel, &local_err)) {
> - error_setg(errp, "Error parsing uri");
> - goto out;
> + if (!migrate_uri_parse(uri, &channel, &local_err)) {
> + error_setg(errp, "Error parsing uri");
> + goto out;
> + }
> }
>
> if (!migrate_prepare(s, has_blk && blk, has_inc && inc,
> @@ -1749,8 +1772,9 @@ void qmp_migrate(const char *uri, bool has_channels,
> }
> }
>
> - saddr = &channel->u.socket;
> - if (channel->transport == MIGRATE_TRANSPORT_SOCKET) {
> + addrs = channel->addr;
> + saddr = &channel->addr->u.socket;
> + if (addrs->transport == MIGRATE_TRANSPORT_SOCKET) {
> if (saddr->type == SOCKET_ADDRESS_TYPE_INET ||
> saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
> saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) {
> @@ -1759,11 +1783,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 == MIGRATE_TRANSPORT_RDMA) {
> - rdma_start_outgoing_migration(s, &channel->u.rdma, &local_err);
> + } else if (addrs->transport == MIGRATE_TRANSPORT_RDMA) {
> + rdma_start_outgoing_migration(s, &addrs->u.rdma, &local_err);
> #endif
> - } else if (channel->transport == MIGRATE_TRANSPORT_EXEC) {
> - exec_start_outgoing_migration(s, channel->u.exec.args, &local_err);
> + } else if (addrs->transport == MIGRATE_TRANSPORT_EXEC) {
> + exec_start_outgoing_migration(s, addrs->u.exec.args, &local_err);
> } else {
> if (!(has_resume && resume)) {
> yank_unregister_instance(MIGRATION_YANK_INSTANCE);
> @@ -1780,6 +1804,7 @@ void qmp_migrate(const char *uri, bool has_channels,
> if (!(has_resume && resume)) {
> yank_unregister_instance(MIGRATION_YANK_INSTANCE);
> }
> + qapi_free_MigrateAddress(addrs);
> qapi_free_SocketAddress(saddr);
> migrate_fd_error(s, local_err);
> error_propagate(errp, local_err);
> diff --git a/migration/socket.c b/migration/socket.c
> index 8e7430b266..98e3ea1514 100644
> --- a/migration/socket.c
> +++ b/migration/socket.c
> @@ -28,6 +28,8 @@
> #include "trace.h"
> #include "postcopy-ram.h"
> #include "options.h"
> +#include "qapi/clone-visitor.h"
> +#include "qapi/qapi-visit-sockets.h"
>
> struct SocketOutgoingArgs {
> SocketAddress *saddr;
> @@ -114,12 +116,13 @@ void socket_start_outgoing_migration(MigrationState *s,
> {
> QIOChannelSocket *sioc = qio_channel_socket_new();
> struct SocketConnectData *data = g_new0(struct SocketConnectData, 1);
> + SocketAddress *addr = QAPI_CLONE(SocketAddress, saddr);
>
> data->s = s;
>
> /* in case previous migration leaked it */
> qapi_free_SocketAddress(outgoing_args.saddr);
> - outgoing_args.saddr = saddr;
> + outgoing_args.saddr = addr;
>
> if (saddr->type == SOCKET_ADDRESS_TYPE_INET) {
> data->hostname = g_strdup(saddr->u.inet.host);
> --
> 2.22.3
>
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 :|
On 15/05/23 4:12 pm, Daniel P. Berrangé wrote:
> On Fri, May 12, 2023 at 02:32:40PM +0000, Het Gala wrote:
>> Integrated MigrateChannelList with all transport backends (socket, exec
>> and rdma) for both source and destination migration code flow.
>>
>> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
>> Signed-off-by: Het Gala <het.gala@nutanix.com>
>> ---
>> migration/migration.c | 95 +++++++++++++++++++++++++++----------------
>> migration/socket.c | 5 ++-
>> 2 files changed, 64 insertions(+), 36 deletions(-)
>>
>> Error *local_err = NULL;
>> + MigrateChannel *val = g_new0(MigrateChannel, 1);
>> MigrateAddress *addrs = g_new0(MigrateAddress, 1);
>> SocketAddress *saddr;
>> InetSocketAddress *isock = &addrs->u.rdma;
>> @@ -441,6 +442,7 @@ static bool migrate_uri_parse(const char *uri,
>> }
>>
>> if (local_err) {
>> + qapi_free_MigrateChannel(val);
>> qapi_free_MigrateAddress(addrs);
>> qapi_free_SocketAddress(saddr);
>> qapi_free_InetSocketAddress(isock);
>> @@ -448,7 +450,9 @@ static bool migrate_uri_parse(const char *uri,
>> return false;
>> }
>>
>> - *channel = addrs;
>> + val->channeltype = MIGRATE_CHANNEL_TYPE_MAIN;
>> + val->addr = addrs;
>> + *channel = val;
>> return true;
>> }
>>
>> @@ -457,8 +461,9 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
>> Error **errp)
>> {
>> Error *local_err = NULL;
>> - MigrateAddress *channel = g_new0(MigrateAddress, 1);
>> + MigrateAddress *addrs;
>> SocketAddress *saddr;
>> + MigrateChannel *channel = NULL;
>>
>> /*
>> * Having preliminary checks for uri and channel
>> @@ -467,22 +472,30 @@ static void qemu_start_incoming_migration(const char *uri, bool 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)) {
>> goto out;
>> - }
>> + } 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");
>> + goto out;
>> + }
>> + channel = channels->value;
>> + } else {
>> + /* URI is not suitable for migration? */
>> + if (!migration_channels_and_uri_compatible(uri, errp)) {
>> + goto out;
>> + }
> THis check only gets executed when the caller uses the old
> URI syntax. We need to it be run when using the modern
> MigrateChannel QAPI syntax too.
>
> IOW, migration_channels_and_uri_compatible() needs converting
> to take a 'MigrateChannel' object instead of URI, and then
> the check can be run after the URI -> MigrateCHannel conversion
Yes, Daniel. Got your point. Will change it in the next version.
For Juan's comments, I have not explored the test side code still, so is
the idea to have some similar test functions like
test_precopy_tcp_plain, test_precopy_unix_plain but with the new syntax
? Please confirm this, and any advice on how appraoch this?
>>
>> - if (uri && !migrate_uri_parse(uri, &channel, errp)) {
>> - error_setg(errp, "Error parsing uri");
>> - goto out;
>> + if (uri && !migrate_uri_parse(uri, &channel, errp)) {
>> + error_setg(errp, "Error parsing uri");
>> + goto out;
>> + }
>> }
>>
>> - saddr = &channel->u.socket;
>> + addrs = channel->addr;
>> + saddr = &channel->addr->u.socket;
>> qapi_event_send_migration(MIGRATION_STATUS_SETUP);
>> - if (channel->transport == MIGRATE_TRANSPORT_SOCKET) {
>> + if (addrs->transport == MIGRATE_TRANSPORT_SOCKET) {
>> if (saddr->type == SOCKET_ADDRESS_TYPE_INET ||
>> saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
>> saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) {
>> @@ -491,23 +504,25 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
>> fd_start_incoming_migration(saddr->u.fd.str, &local_err);
>> }
>> #ifdef CONFIG_RDMA
>> - } else if (channel->transport == MIGRATE_TRANSPORT_RDMA) {
>> - rdma_start_incoming_migration(&channel->u.rdma, &local_err);
>> + } else if (addrs->transport == MIGRATE_TRANSPORT_RDMA) {
>> + rdma_start_incoming_migration(&addrs->u.rdma, &local_err);
>> #endif
>> - } else if (channel->transport == MIGRATE_TRANSPORT_EXEC) {
>> - exec_start_incoming_migration(channel->u.exec.args, &local_err);
>> + } else if (addrs->transport == MIGRATE_TRANSPORT_EXEC) {
>> + exec_start_incoming_migration(addrs->u.exec.args, &local_err);
>> } else {
>> error_setg(errp, "unknown migration protocol: %s", uri);
>> }
>>
>> if (local_err) {
>> + qapi_free_MigrateAddress(addrs);
>> qapi_free_SocketAddress(saddr);
>> error_propagate(errp, local_err);
>> return;
>> }
>>
>> out:
>> - qapi_free_MigrateAddress(channel);
>> + qapi_free_MigrateChannel(channel);
>> + return;
>> }
>>
>> static void process_incoming_migration_bh(void *opaque)
>> @@ -1714,8 +1729,9 @@ void qmp_migrate(const char *uri, bool has_channels,
>> {
>> Error *local_err = NULL;
>> MigrationState *s = migrate_get_current();
>> - MigrateAddress *channel = g_new0(MigrateAddress, 1);
>> + MigrateAddress *addrs;
>> SocketAddress *saddr;
>> + MigrateChannel *channel = NULL;
>>
>> struct SocketOutgoingArgs {
>> SocketAddress *saddr;
>> @@ -114,12 +116,13 @@ void socket_start_outgoing_migration(MigrationState *s,
>> {
>> QIOChannelSocket *sioc = qio_channel_socket_new();
>> struct SocketConnectData *data = g_new0(struct SocketConnectData, 1);
>> + SocketAddress *addr = QAPI_CLONE(SocketAddress, saddr);
>>
>> data->s = s;
>>
>> /* in case previous migration leaked it */
>> qapi_free_SocketAddress(outgoing_args.saddr);
>> - outgoing_args.saddr = saddr;
>> + outgoing_args.saddr = addr;
>>
>> if (saddr->type == SOCKET_ADDRESS_TYPE_INET) {
>> data->hostname = g_strdup(saddr->u.inet.host);
>> --
>> 2.22.3
>>
> With regards,
> Daniel
Regards,
Het Gala
Het Gala <het.gala@nutanix.com> wrote:
> On 15/05/23 4:12 pm, Daniel P. Berrangé wrote:
>> On Fri, May 12, 2023 at 02:32:40PM +0000, Het Gala wrote:
>>> Integrated MigrateChannelList with all transport backends (socket, exec
>>> and rdma) for both source and destination migration code flow.
>>>
>>> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
>>> Signed-off-by: Het Gala <het.gala@nutanix.com>
>>> ---
>>> migration/migration.c | 95 +++++++++++++++++++++++++++----------------
>>> migration/socket.c | 5 ++-
>>> 2 files changed, 64 insertions(+), 36 deletions(-)
>>>
>>> Error *local_err = NULL;
>>> + MigrateChannel *val = g_new0(MigrateChannel, 1);
>>> MigrateAddress *addrs = g_new0(MigrateAddress, 1);
>>> SocketAddress *saddr;
>>> InetSocketAddress *isock = &addrs->u.rdma;
>>> @@ -441,6 +442,7 @@ static bool migrate_uri_parse(const char *uri,
>>> }
>>> if (local_err) {
>>> + qapi_free_MigrateChannel(val);
>>> qapi_free_MigrateAddress(addrs);
>>> qapi_free_SocketAddress(saddr);
>>> qapi_free_InetSocketAddress(isock);
>>> @@ -448,7 +450,9 @@ static bool migrate_uri_parse(const char *uri,
>>> return false;
>>> }
>>> - *channel = addrs;
>>> + val->channeltype = MIGRATE_CHANNEL_TYPE_MAIN;
>>> + val->addr = addrs;
>>> + *channel = val;
>>> return true;
>>> }
>>> @@ -457,8 +461,9 @@ static void
>>> qemu_start_incoming_migration(const char *uri, bool has_channels,
>>> Error **errp)
>>> {
>>> Error *local_err = NULL;
>>> - MigrateAddress *channel = g_new0(MigrateAddress, 1);
>>> + MigrateAddress *addrs;
>>> SocketAddress *saddr;
>>> + MigrateChannel *channel = NULL;
>>> /*
>>> * Having preliminary checks for uri and channel
>>> @@ -467,22 +472,30 @@ static void qemu_start_incoming_migration(const char *uri, bool 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)) {
>>> goto out;
>>> - }
>>> + } 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");
>>> + goto out;
>>> + }
>>> + channel = channels->value;
>>> + } else {
>>> + /* URI is not suitable for migration? */
>>> + if (!migration_channels_and_uri_compatible(uri, errp)) {
>>> + goto out;
>>> + }
>> THis check only gets executed when the caller uses the old
>> URI syntax. We need to it be run when using the modern
>> MigrateChannel QAPI syntax too.
>>
>> IOW, migration_channels_and_uri_compatible() needs converting
>> to take a 'MigrateChannel' object instead of URI, and then
>> the check can be run after the URI -> MigrateCHannel conversion
>
> Yes, Daniel. Got your point. Will change it in the next version.
>
> For Juan's comments, I have not explored the test side code still, so
> is the idea to have some similar test functions like
> test_precopy_tcp_plain, test_precopy_unix_plain but with the new
> syntax ? Please confirm this, and any advice on how appraoch this?
There are several places that use this syntax:
rsp = wait_command(to, "{ 'execute': 'migrate-incoming',"
" 'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
Just change a couple of them to the new syntax.
I guess you will want to do the multifd tests with the new syntax, not
sure if it makes sense to also test the old one.
I guess you will also want to check that your are catching errors (like
different number of channels on source/destination).
Later, Juan.
Het Gala <het.gala@nutanix.com> wrote: > Integrated MigrateChannelList with all transport backends (socket, exec > and rdma) for both source and destination migration code flow. > > Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com> > Signed-off-by: Het Gala <het.gala@nutanix.com> Nothing especially wrong appears here, will wait for 2nd submission with the previous fixes done (specially the local_error bits). And in another order of events, you are not changing tests/qtest/migration-test.c My suggestion will be that for things that have more than one test (i.e. tcp/unix/...) just change some of them to use the new syntax for channels. What do you think? Later, Juan.
© 2016 - 2026 Red Hat, Inc.