On Wed, Feb 08, 2023 at 09:35:56AM +0000, Het Gala wrote:
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 for initiating a migration stream.
This scheme has a significant flaw in it - double encoding of existing
URIs to extract migration info.
The current patch maps QAPI uri design onto well defined MigrateChannel
struct. This modified QAPI helps in preventing multi-level uri
encodings ('uri' parameter is kept for backward compatibility).
Suggested-by: Daniel P. Berrange <berrange@redhat.com>
Suggested-by: Manish Mishra <manish.mishra@nutanix.com>
Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
qapi/migration.json | 131 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 129 insertions(+), 2 deletions(-)
+
+##
+# @MigrateRdmaAddr:
+#
+# Since 8.0
+##
+{ 'struct': 'MigrateRdmaAddr',
+ 'data' : {'data': 'InetSocketAddress' } }
...while these branches supply everything else under 'data'. Also,
while you documented @socket-type above, you did not document @data in
either of these two types. [1]
+
+##
+# @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' } }
Another example of inconsistent spacing around :.
I'm guessing the reason you didn't go with 'socket': 'SocketAddress'
is that SocketAddress is itself a discriminated union, and Markus does
not yet have the QAPI generator wired up to support one union as a
branch of another larger union? It leads to extra nesting on the wire
[2]