| Message ID | 20230713105713.236883-9-het.gala@nutanix.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration | expand |
On Thu, Jul 13, 2023 at 10:57:12AM +0000, Het Gala wrote: > Integrate MigrateChannelList with all transport backends > (socket, exec and rdma) for both src and dest migration > endpoints. > > Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com> > Signed-off-by: Het Gala <het.gala@nutanix.com> > --- > migration/migration.c | 77 ++++++++++++++++++++++++++++--------------- > migration/socket.c | 5 ++- > 2 files changed, 54 insertions(+), 28 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index 62894952ca..43c7e4b525 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; > @@ -464,7 +465,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; > } > > @@ -472,7 +475,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 > @@ -482,20 +486,29 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels, > "exclusive; exactly one of the two should be present in " > "'migrate-incoming' qmp command "); > return; > - } > - > - if (uri && !migrate_uri_parse(uri, &channel, errp)) { > - return; > + } else if (channels) { > + /* To verify that Migrate channel list has only item */ > + if (channels->next) { > + error_setg(errp, "Channel list has more than one entries"); > + return; > + } > + channel = channels->value; > + addr = channel->addr; > + } else { > + /* caller uses the old URI syntax */ > + if (uri && !migrate_uri_parse(uri, &channel, errp)) { > + return; > + } > } > > /* transport mechanism not suitable for migration? */ > - if (!migration_channels_and_transport_compatible(channel, errp)) { > + if (!migration_channels_and_transport_compatible(addr, errp)) { This is strange - the API contract of migration_channels_and_transport_compatible hasn't changed, so replacing MigrationChannel object with MigrationAddress is surely not going to compile. Also the '} else {' branch above doesn't set 'addr' so it will be NULL in that codepath > 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) { > @@ -504,11 +517,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); > } > @@ -1708,7 +1721,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 > @@ -1718,14 +1732,23 @@ void qmp_migrate(const char *uri, bool has_channels, > "exclusive; exactly one of the two should be present in " > "'migrate' qmp command "); > return; > - } > - > - if (!migrate_uri_parse(uri, &channel, errp)) { > - return; > + } else if (channels) { > + /* To verify that Migrate channel list has only item */ > + if (channels->next) { > + error_setg(errp, "Channel list has more than one entries"); > + return; > + } > + channel = channels->value; > + addr = channel->addr; > + } else { > + /* caller uses the old URI syntax */ > + if (uri && !migrate_uri_parse(uri, &channel, errp)) { > + return; > + } > } > > /* transport mechanism not suitable for migration? */ > - if (!migration_channels_and_transport_compatible(channel, errp)) { > + if (!migration_channels_and_transport_compatible(addr, errp)) { > return; > } > > @@ -1742,8 +1765,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) { > @@ -1752,11 +1775,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); > 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); This patch isn't changing the API contract of socket_start_outgoing_migration, so if this QAPI_CLONE is indeed required, that suggests it should have been in an earlier patch which introduced the pre-requisite to clone. With regards, Daniel
diff --git a/migration/migration.c b/migration/migration.c index 62894952ca..43c7e4b525 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; @@ -464,7 +465,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; } @@ -472,7 +475,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 @@ -482,20 +486,29 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels, "exclusive; exactly one of the two should be present in " "'migrate-incoming' qmp command "); return; - } - - if (uri && !migrate_uri_parse(uri, &channel, errp)) { - return; + } else if (channels) { + /* To verify that Migrate channel list has only item */ + if (channels->next) { + error_setg(errp, "Channel list has more than one entries"); + return; + } + channel = channels->value; + addr = channel->addr; + } else { + /* caller uses the old URI syntax */ + if (uri && !migrate_uri_parse(uri, &channel, errp)) { + return; + } } /* transport mechanism not suitable for migration? */ - if (!migration_channels_and_transport_compatible(channel, errp)) { + if (!migration_channels_and_transport_compatible(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) { @@ -504,11 +517,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); } @@ -1708,7 +1721,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 @@ -1718,14 +1732,23 @@ void qmp_migrate(const char *uri, bool has_channels, "exclusive; exactly one of the two should be present in " "'migrate' qmp command "); return; - } - - if (!migrate_uri_parse(uri, &channel, errp)) { - return; + } else if (channels) { + /* To verify that Migrate channel list has only item */ + if (channels->next) { + error_setg(errp, "Channel list has more than one entries"); + return; + } + channel = channels->value; + addr = channel->addr; + } else { + /* caller uses the old URI syntax */ + if (uri && !migrate_uri_parse(uri, &channel, errp)) { + return; + } } /* transport mechanism not suitable for migration? */ - if (!migration_channels_and_transport_compatible(channel, errp)) { + if (!migration_channels_and_transport_compatible(addr, errp)) { return; } @@ -1742,8 +1765,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) { @@ -1752,11 +1775,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); 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);
Integrate MigrateChannelList with all transport backends (socket, exec and rdma) for both src and dest migration endpoints. Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com> Signed-off-by: Het Gala <het.gala@nutanix.com> --- migration/migration.c | 77 ++++++++++++++++++++++++++++--------------- migration/socket.c | 5 ++- 2 files changed, 54 insertions(+), 28 deletions(-)