On 22/11/22 2:53 pm, Daniel P. Berrangé wrote:
On Mon, Nov 21, 2022 at 01:40:27PM +0100, Juan Quintela wrote:
Het Gala <het.g...@nutanix.com> wrote:
To prevent double data encoding of uris, instead of passing transport
mechanisms, host address and port all together in form of a single string
and writing different parsing functions, we intend the user to explicitly
mention most of the parameters through the medium of qmp command itself.
+{ 'struct': 'MigrateChannel',
+ 'data' : {
+ 'channeltype' : 'MigrateChannelType',
+ '*src-addr' : 'MigrateAddress',
+ 'dest-addr' : 'MigrateAddress',
Why do we want *both* addresses?
This is part of their goal to have source based routing, so that
traffic exits the src host via a particular NIC.
I think this patch would be better if we split it into two parts.
First introduce "MigrateChannel" struct with *only* the 'dest-addr'
field, and only allow one element in the list, making 'uri' optional.
Basically the first patch would *only* be about switching the
'migrate' command from using a plain string to a MigrateAddress
structure.
A second patch would then extend it to allow multiple MigrateChannel
elements, to support different destinations for each channel.
A third patch would then extend it to add src-addr to attain the
source based routing.
A fourth patch can then deprecate the existing 'uri' field.
> Thanks Daniel. This is a nice idea. I will break this patch into 4
different patches, so it would be clear to see how the QAPI design is
evolving.
+ '*multifd-count' : 'int' } }
And if we are passing a list, why do we want to pass the real number?
Yeah, technically I think this field is redundant, as you can just
add many entires to the 'channels' list, using the same addresses.
All this field does is allow a more compact JSON arg list. I'm
not sure this is neccessary, unless we're expecting huge numbers
of 'channels', and even then this isn't likely to be a performance
issue.
> I have tried to explain this to Juan. The main purpose is, if we want
to add 3 channels to one pair of interface and 4 pair of channels to
another pair of interface, instead of writing the same interface 3 or 4
times, this field saves that redundancy, and I personally feel, it gives
one clear representation of multifd capability.
+# -> { "execute": "migrate",
+# "arguments": {
+# "channels": [ { 'channeltype': 'control',
+# 'dest-addr': {'transport': 'socket',
+# 'type': 'inet',
+# 'host': '10.12.34.9', 'port':
'1050'}},
+# { 'channeltype': 'data',
+# 'src-addr': {'transport': 'socket',
+# 'type': 'inet',
+# 'host': '10.12.34.9',
+# 'port': '4000', 'ipv4': 'true'},
+# 'dest-addr': { 'transport': 'socket',
+# 'type': 'inet',
+# 'host': '10.12.34.92',
+# 'port': '1234', 'ipv4': 'true'},
+# 'multifd-count': 5 },
+# { 'channeltype': 'data',
+# 'src-addr': {'transport': 'socket',
+# 'type': 'inet',
+# 'host': '10.2.3.4', 'port': '1000'},
+# 'dest-addr': {'transport': 'socket',
+# 'type': 'inet',
+# 'host': '0.0.0.4', 'port': '3000'},
+# 'multifd-count': 3 } ] } }
+# <- { "return": {} }
+#
##
{ 'command': 'migrate',
- 'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool',
- '*detach': 'bool', '*resume': 'bool' } }
+ 'data': {'*uri': 'str', '*channels': ['MigrateChannel'], '*blk': 'bool',
- I would not care at all about the "exec" protocol, just leave that
alone in the deprecated command. Right now:
* we can't move it to multifd without a lot of PAIN
* there are patches on the list suggesting that what we really want is
to create a file that is the size of RAM and just write all the RAM
at the right place.
* that would make the way to create snapshots (I don't know if anyones
still wants them, much easier).
* I think that the only real use of exec migration was to create
snapshots, for real migration, using a socket is much, much saner.
* I.e. what I mean here is that for exec migration, we need to think
if we want to continue supporting it for normal migration, or only
as a way to create snapshots.
With regards,
Daniel
Thanks and regards,
Het Gala
