* Daniel P. Berrangé (berra...@redhat.com) wrote:
> 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' } }
I think that's probably the right thing to do; there is a native RDMA
addressing scheme that people occasionally (once a decade or so)
ask about but I don't think we've ever supported it.
Dave
>
>
> > +
> > +##
> > +# @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 :|
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
