On Mon, Dec 26, 2022 at 05:33:25AM +0000, Het Gala wrote:
> From: Author Het Gala <het.g...@nutanix.com>
>
> Existing 'migrate' QAPI design enforces transport mechanism, ip address
> of destination interface and corresponding port number in the form
> of a unified string 'uri' parameter. This scheme does seem to have an issue
> in it, i.e. double-level encoding of URIs.
>
> The current patch maps existing QAPI design into a well-defined data
> structure - 'MigrateChannel' only from the design perspective. Please note
> that
> the existing 'uri' parameter is kept untouched for backward compatibility.
>
> Suggested-by: Daniel P. Berrange <berra...@redhat.com>
> Suggested-by: Manish Mishra <manish.mis...@nutanix.com>
> Suggested-by: Aravind Retnakaran <aravind.retnaka...@nutanix.com>
> Signed-off-by: Het Gala <het.g...@nutanix.com>
> ---
> qapi/migration.json | 121 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 119 insertions(+), 2 deletions(-)
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 88ecf86ac8..753e187ce2 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1449,12 +1449,108 @@
> ##
> { 'command': 'migrate-continue', 'data': {'state': 'MigrationStatus'} }
>
> +##
> +# @MigrateTransport:
> +#
> +# The supported communication transport mechanisms for migration
> +#
> +# @socket: Supported communication type between two devices for migration.
> +# Socket is able to cover all of 'tcp', 'unix', 'vsock' and
> +# 'fd' already
> +#
> +# @exec: Supported communication type to redirect migration stream into file.
> +#
> +# @rdma: Supported communication type to redirect rdma type migration stream.
> +#
> +# Since 8.0
> +##
> +{ 'enum': 'MigrateTransport',
> + 'data': ['socket', 'exec', 'rdma'] }
> +
> +##
> +# @MigrateSocketAddr:
> +#
> +# To support different type of socket.
> +#
> +# @socket-type: Different type of socket connections.
> +#
> +# Since 8.0
> +##
> +{ 'struct': 'MigrateSocketAddr',
> + 'data': {'socket-type': 'SocketAddress' } }
> +
> +##
> +# @MigrateExecAddr:
> + #
> + # Since 8.0
> + ##
> +{ 'struct': 'MigrateExecAddr',
> + 'data' : {'exec-str': 'str' } }
Currently for 'exec:cmdstr' the 'cmdstr' part is a shell command
that is passed
const char *argv[] = { "/bin/sh", "-c", command, NULL };
I have a strong preference for making it possible to avoid use
of shell when spawning commands, since much of thue time it is
not required and has the potential to open up vulnerabilities.
It would be nice to be able to just take the full argv directly
IOW
{ 'struct': 'MigrateExecAddr',
'data' : {'argv': ['str'] } }
If the caller wants to keep life safe and simple now they can
use
["/bin/nc", "-U", "/some/sock"]
but if they still want to send it via shell, they can also do
so
["/bin/sh", "-c", "...arbitrary shell script code...."]
> +
> +##
> +# @MigrateRdmaAddr:
> +#
> +# Since 8.0
> +##
> +{ 'struct': 'MigrateRdmaAddr',
> + 'data' : {'rdma-str': 'str' } }
Loooking at the RDMA code it takes the str, and treats it
as an IPv4 address:
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);
}
so we really ought to accept an InetSocketAddress struct
directly
{ 'struct': 'MigrateRdmaAddr',
'data' : {'rdma-str': 'InetSocketAddress' } }
> +
> +##
> +# @MigrateAddress:
> +#
> +# The options available for communication transport mechanisms for migration
> +#
> +# Since 8.0
> +##
> +{ 'union' : 'MigrateAddress',
> + 'base' : { 'transport' : 'MigrateTransport'},
> + 'discriminator' : 'transport',
> + 'data' : {
> + 'socket' : 'MigrateSocketAddr',
> + 'exec' : 'MigrateExecAddr',
> + 'rdma': 'MigrateRdmaAddr' } }
> +
> +##
> +# @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: SocketAddress 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
> +#
> +# @channel: Struct containing migration channel type, along with all
> +# the details of destination interface required for initiating
> +# a migration stream.
> #
> # @blk: do block migration (full disk copy)
> #
> @@ -1479,15 +1575,36 @@
> # 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. Both 'uri' and 'channel' arguments, are mututally exclusive but, atleast
Minor typos "mutually" "at
least"
> +# one of the two arguments should be present.
> +#
> # Example:
> #
> # -> { "execute": "migrate", "arguments": { "uri": "tcp:0:4446" } }
> # <- { "return": {} }
> #
> +# -> { "execute": "migrate",
> +# "arguments": {
> +# "channel": { "channeltype": "main",
> +# "addr": { "transport": "socket",
> +# "socket-type": { "type': "inet',
> +# "host": "10.12.34.9",
> +# "port": "1050" } } } } }
> +# <- { "return": {} }
> +#
> +# -> { "execute": "migrate",
> +# "arguments": { "channel": { "channeltype": "main",
> +# "addr": { "transport": "exec",
> +# "exec-str": "/tmp/exec" } } } }
Will need updating given my feedback above
> +# <- { "return": {} }
> +#
> ##
> { 'command': 'migrate',
> - 'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool',
> - '*detach': 'bool', '*resume': 'bool' } }
> + 'data': {'*uri': 'str', '*channel': 'MigrateChannel', '*blk': 'bool',
> + '*inc': 'bool', '*detach': 'bool', '*resume': 'bool' } }
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 :|
