On Fri, May 12, 2023 at 02:32:38PM +0000, Het Gala wrote:
> MigrateChannelList ideally allows to connect accross multiple interfaces.
>
> Added MigrateChannelList struct as argument to 'migrate' qapi. Introduced
> MigrateChannelList in hmp_migrate() and qmp_migrate() functions.
>
> Future patchset series plans to include multiple MigrateChannels
> for multiple interfaces to be connected. That is the reason,
> 'MigrateChannelList'
> is the preferred choice of argument over 'MigrateChannel' and making 'migrate'
> QAPI future proof.
>
> For current patch, have just limit size of MigrateChannelList to 1 element as
> a runtime check.
>
> Suggested-by: Aravind Retnakaran <aravind.retnaka...@nutanix.com>
> Signed-off-by: Het Gala <het.g...@nutanix.com>
> ---
> migration/migration-hmp-cmds.c | 113 ++++++++++++++++++++++++++++++++-
> migration/migration.c | 17 ++++-
> qapi/migration.json | 69 +++++++++++++++++++-
> 3 files changed, 192 insertions(+), 7 deletions(-)
>
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 4e9f00e7dc..f098d04542 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -51,6 +51,101 @@ static void migration_global_dump(Monitor *mon)
> ms->clear_bitmap_shift);
> }
>
> +static bool
> +migrate_channel_from_qdict(MigrateChannel **channel,
> + const QDict *qdict, Error **errp)
> +{
> + Error *err = NULL;
> + const char *channeltype = qdict_get_try_str(qdict, "channeltype");
> + const char *transport_str = qdict_get_try_str(qdict, "transport");
> + const char *socketaddr_type = qdict_get_try_str(qdict, "type");
> + const char *inet_host = qdict_get_try_str(qdict, "host");
> + const char *inet_port = qdict_get_try_str(qdict, "port");
> + const char *unix_path = qdict_get_try_str(qdict, "path");
> + const char *vsock_cid = qdict_get_try_str(qdict, "cid");
> + const char *vsock_port = qdict_get_try_str(qdict, "port");
> + const char *fd = qdict_get_try_str(qdict, "str");
> + QList *exec = qdict_get_qlist(qdict, "exec");
THis seems to be implicitly defining a huge set of extra parameters
for the migrate 'HMP' command, but none of it is actually defined
at the hmp-commands.hx
Do we even need todo this ? For HMP it is not unreasonable to just
keep using the URI syntax forever?
> + MigrateChannel *val = g_new0(MigrateChannel, 1);
> + MigrateChannelType channel_type;
> + MigrateTransport transport;
> + MigrateAddress *addr = g_new0(MigrateAddress, 1);
> + SocketAddress *saddr = g_new(SocketAddress, 1);
> + SocketAddressType type;
> +
> + channel_type = qapi_enum_parse(&MigrateChannelType_lookup,
> + channeltype, -1, &err);
> + if (channel_type < 0) {
> + goto end;
> + }
> +
> + transport = qapi_enum_parse(&MigrateTransport_lookup,
> + transport_str, -1, &err);
> + if (transport < 0) {
> + goto end;
> + }
> +
> + type = qapi_enum_parse(&SocketAddressType_lookup,
> + socketaddr_type, -1, &err);
> + if (type < 0) {
> + goto end;
> + }
> +
> + addr->transport = transport;
> +
> + switch (transport) {
> + case MIGRATE_TRANSPORT_SOCKET:
> + saddr->type = type;
> +
> + switch (type) {
> + case SOCKET_ADDRESS_TYPE_INET:
> + saddr->u.inet.host = (char *)inet_host;
> + saddr->u.inet.port = (char *)inet_port;
> + break;
> + case SOCKET_ADDRESS_TYPE_UNIX:
> + saddr->u.q_unix.path = (char *)unix_path;
> + break;
> + case SOCKET_ADDRESS_TYPE_VSOCK:
> + saddr->u.vsock.cid = (char *)vsock_cid;
> + saddr->u.vsock.port = (char *)vsock_port;
> + break;
> + case SOCKET_ADDRESS_TYPE_FD:
> + saddr->u.fd.str = (char *)fd;
> + break;
> + default:
> + error_setg(errp, "%s: Unknown socket type %d",
> + __func__, saddr->type);
> + goto end;
> + }
> +
> + addr->u.socket = *saddr;
> + break;
> + case MIGRATE_TRANSPORT_EXEC:
> + addr->u.exec.args = (strList *)exec;
> + break;
> + case MIGRATE_TRANSPORT_RDMA:
> + addr->u.rdma.host = (char *)inet_host;
> + addr->u.rdma.port = (char *)inet_port;
> + break;
> + default:
> + error_setg(errp, "%s: Unknown migrate transport type %d",
> + __func__, addr->transport);
> + goto end;
> + }
> +
> + val->channeltype = channel_type;
> + val->addr = addr;
> + *channel = val;
> + return true;
> +
> +end:
> + error_propagate(errp, err);
> + qapi_free_MigrateChannel(val);
> + qapi_free_MigrateAddress(addr);
> + qapi_free_SocketAddress(saddr);
> + return false;
> +}
> +
> void hmp_info_migrate(Monitor *mon, const QDict *qdict)
> {
> MigrationInfo *info;
> @@ -702,9 +797,18 @@ 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;
> + MigrateChannel *channel = g_new0(MigrateChannel, 1);
> + MigrateChannelList *caps = NULL;
> +
> + if (!migrate_channel_from_qdict(&channel, qdict, &err)) {
> + error_setg(&err, "error in retrieving channel from qdict");
> + goto end;
> + }
>
> - qmp_migrate(uri, !!blk, blk, !!inc, inc,
> - false, false, true, resume, &err);
> + QAPI_LIST_PREPEND(caps, channel);
> + qmp_migrate(uri, !!caps, caps, !!blk, blk, !!inc, inc,
> + false, false, true, resume, &err);
> + qapi_free_MigrateChannelList(caps);
> if (hmp_handle_error(mon, err)) {
> return;
> }
> @@ -725,6 +829,11 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
> status);
> timer_mod(status->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME));
> }
> +
> +end:
> + qapi_free_MigrateChannel(channel);
> + hmp_handle_error(mon, err);
> + return;
> }
>
> void migrate_set_capability_completion(ReadLineState *rs, int nb_args,
> diff --git a/migration/migration.c b/migration/migration.c
> index 6abd69df8d..78f16e5041 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1694,15 +1694,26 @@ static bool migrate_prepare(MigrationState *s, bool
> blk, bool blk_inc,
> return true;
> }
>
> -void qmp_migrate(const char *uri, bool has_blk, bool blk,
> - bool has_inc, bool inc, bool has_detach, bool detach,
> - bool has_resume, bool resume, Error **errp)
> +void qmp_migrate(const char *uri, bool has_channels,
> + MigrateChannelList *channels, bool has_blk, bool blk,
> + bool has_inc, bool inc, bool has_detach,
> + bool detach, bool has_resume, bool resume, Error **errp)
> {
> Error *local_err = NULL;
> MigrationState *s = migrate_get_current();
> MigrateAddress *channel = g_new0(MigrateAddress, 1);
> SocketAddress *saddr;
>
> + /*
> + * 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)) {
> goto out;
> diff --git a/qapi/migration.json b/qapi/migration.json
> index bf90bd8fe2..a71af87ffe 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1464,12 +1464,47 @@
> 'exec': 'MigrateExecCommand',
> 'rdma': 'InetSocketAddress' } }
>
> +##
> +# @MigrateChannelType:
> +#
> +# The supported options for migration channel type requests
> +#
> +# @main: Support request for main outbound migration control channel
> +#
> +# Since 8.0
> +##
> +{ 'enum': 'MigrateChannelType',
> + 'data': [ 'main' ] }
> +
> +##
> +# @MigrateChannel:
> +#
> +# Information regarding migration Channel-type for transferring packets,
> +# source and corresponding destination interface for socket connection
> +# and number of multifd channels over the interface.
> +#
> +# @channeltype: Name of Channel type for transfering packet information
> +#
> +# @addr: Information regarding migration parameters of destination interface
> +#
> +# Since 8.0
> +##
> +{ 'struct': 'MigrateChannel',
> + 'data': {
> + 'channeltype': 'MigrateChannelType',
> + 'addr': 'MigrateAddress' } }
> +
> ##
> # @migrate:
> #
> # Migrates the current running guest to another Virtual Machine.
> #
> # @uri: the Uniform Resource Identifier of the destination VM
> +# for migration thread
> +#
> +# @channels: Struct containing list of migration channel types, with all
> +# the information regarding destination interfaces required for
> +# initiating a migration stream.
> #
> # @blk: do block migration (full disk copy)
> #
> @@ -1494,15 +1529,45 @@
> # 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. The 'uri' and 'channel' 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": [ { "channeltype": "main",
> +# "addr": { "transport": "socket", "type": "inet",
> +# "host": "10.12.34.9",
> +# "port": "1050" } } ] } }
> +# <- { "return": {} }
> +#
> +# -> { "execute": "migrate",
> +# "arguments": {
> +# "channels": [ { "channeltype": "main",
> +# "addr": { "transport": "exec",
> +# "args": [ "/bin/nc", "-p", "6000",
> +# "/some/sock" ] } } ] } }
> +# <- { "return": {} }
> +#
> +# -> { "execute": "migrate",
> +# "arguments": {
> +# "channels": [ { "channeltype": "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': [ 'MigrateChannel' ], '*blk': 'bool',
> + '*inc': 'bool', '*detach': 'bool', '*resume': 'bool' } }
>
> ##
> # @migrate-incoming:
> --
> 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 :|
