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. > > The current patch is continuation of patchset series > https://www.mail-archive.com/qemu-devel@nongnu.org/msg901274.html > and reply to the ongoing discussion for better QAPI design here > https://www.mail-archive.com/qemu-devel@nongnu.org/msg903753.html. > > Suggested-by: Daniel P. Berrange <berra...@redhat.com> > Suggested-by: Aravind Retnakaran <aravind.retnaka...@nutanix.com> > Suggested-by: Manish Mishra <manish.mis...@nutanix.com> > Signed-off-by: Het Gala <het.g...@nutanix.com>
Hi 1st of all, I can't see how this is 7.1 material, I guess we need to move it to 8.0. > --- > qapi/migration.json | 127 +++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 125 insertions(+), 2 deletions(-) > > diff --git a/qapi/migration.json b/qapi/migration.json > index 88ecf86ac8..fd9286ea0f 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -1449,12 +1449,101 @@ > ## > { '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. > +# > +# Since 7.1 > +## > +{ 'enum': 'MigrateTransport', > + 'data': ['socket', 'exec'] } I haven't looked too much into this, but as Danield told in the past, I can see where the rdma falls into this scheme. I guess it is going to be its own, but who knows. > +# The supported options for migration channel type requests > +# > +# @control: Support request for main outbound migration control channel > +# > +# @data: Supported Channel type for multifd data channels > +# > +# @async: Supported Channel type for post-copy async requests > +# > +# Since 7.1 > +## > +{ 'enum': 'MigrateChannelType', > + 'data': ['control', 'data', 'async'] } > + 'data': ['main', 'data', 'ram-async'] } ??? I don't like the 'control' name because without multifd we still pass everything through it. And with multifd, we still pass all devices through it. About the asynchronous channel, I don't know if calling it postcopy is better. > +{ 'struct': 'MigrateChannel', > + 'data' : { > + 'channeltype' : 'MigrateChannelType', > + '*src-addr' : 'MigrateAddress', > + 'dest-addr' : 'MigrateAddress', Why do we want *both* addresses? > + '*multifd-count' : 'int' } } And if we are passing a list, why do we want to pass the real number? > # -> { "execute": "migrate", "arguments": { "uri": "tcp:0:4446" } } > # <- { "return": {} } > # > +# -> { "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 think that "uri" bit should be dropped, right? > + '*inc': 'bool', '*detach': 'bool', '*resume': 'bool' } } > > ## > # @migrate-incoming: I can't see how to make the old one to work on top of this one (i.e. we would have to create strings from lists on QAPI, I think that is just too much). So I think that the best way (I know I am contradicting myself) is to create a new migrate command and just let the old one alone. That way: - you can drop blk and blk - you can do anything that you want with the uris, as assuming that they are always sockets. - 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. What do you think? Later, Juan.