On 09/01/23 7:37 pm, Daniel P. Berrangé 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.
+##
+# @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...."]
Okay Daniel. I get your point and it makes sense too. This will also
have some code changes in exec.c file I assume.
+
+##
+# @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' } }
Yes, It resembles to InetSocketAddress. Will make the relevant changes
in rdma.c file.
With this, I had a small question in mind, do qemu need to develop /
leverage some functionality to check the correctness for host or port.
So that if the user enters an invalid host address, they get an error
message to enter correct address, if trying to migrate via qmp command
line interface.
Please correct me if I am wrong here.
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"
Thanks for pointing that out Daniel. Ack.
# 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
Yeah sure.Thanks for the feedback above.
With regards,
Daniel
Regards,
Het Gala
