Re: [PATCH v2 5/6] migration: Modified 'migrate-incoming' QAPI and HMP side changes on the destination interface.

[Het Gala]

On 09/02/23 4:01 pm, Daniel P. Berrangé wrote:
> On Wed, Feb 08, 2023 at 09:35:59AM +0000, Het Gala wrote:
> > 'migrate-incoming' QAPI design have been modified into well-defined
> > MigrateChannel struct to prevent multiple encoding of uri strings on
> > the destination side.'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>
> > ---
> >   migration/migration-hmp-cmds.c |  8 +++++++-
> >   migration/migration.c          |  3 ++-
> >   qapi/migration.json            | 22 ++++++++++++++++++++--
> >   softmmu/vl.c                   |  2 +-
> >   4 files changed, 30 insertions(+), 5 deletions(-)
> >
> > diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> > index a9f65ded7a..ae3c5ea5b2 100644
> > --- a/migration/migration-hmp-cmds.c
> > +++ b/migration/migration-hmp-cmds.c
> > @@ -500,8 +500,14 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict)
> >       Error *err = NULL;
> >       const char *uri = qdict_get_str(qdict, "uri");
> >   
> > -    qmp_migrate_incoming(uri, &err);
> > +    MigrateChannel *channel = g_new0(MigrateChannel, 1);
> > +    if (!migrate_channel_from_qdict(&channel, qdict, &err)) {
> > +        error_setg(&err, "error in retrieving channel from qdict");
> > +        return;
> > +    }
> >   
> > +    qmp_migrate_incoming(uri, channel, &err);
> > +    qapi_free_MigrateChannel(channel);
> >       hmp_handle_error(mon, err);
> >   }
> >   
> > diff --git a/migration/migration.c b/migration/migration.c
> > index accbf72a18..e22ce2dd15 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -2314,7 +2314,8 @@ void migrate_del_blocker(Error *reason)
> >       migration_blockers = g_slist_remove(migration_blockers, reason);
> >   }
> >   
> > -void qmp_migrate_incoming(const char *uri, Error **errp)
> > +void qmp_migrate_incoming(const char *uri, MigrateChannel *channel,
> > +                          Error **errp)
> >   {
> >       Error *local_err = NULL;
> >       static bool once = true;
> > diff --git a/qapi/migration.json b/qapi/migration.json
> > index 79acfcfe4e..3a88912f4d 100644
> > --- a/qapi/migration.json
> > +++ b/qapi/migration.json
> > @@ -1623,7 +1623,11 @@
> >   # with -incoming defer
> >   #
> >   # @uri: The Uniform Resource Identifier identifying the source or
> > -#       address to listen on
> > +#       the address to listen on
> > +#
> > +# @channel: Struct containing migration channel type, along with
> > +#           all the details of the destination interface required
> > +#           for the address to listen on for migration stream.
> >   #
> >   # Returns: nothing on success
> >   #
> > @@ -1640,14 +1644,28 @@
> >   #
> >   # 3. The uri format is the same as for -incoming
> >   #
> > +# 4. The 'uri' and 'channel' arguments are mutually exclusive but, atleast
> > +#    one of the two arguments should be present.
> > +#
> >   # Example:
> >   #
> >   # -> { "execute": "migrate-incoming",
> >   #      "arguments": { "uri": "tcp::4446" } }
> >   # <- { "return": {} }
> >   #
> > +# -> { "execute": "migrate-incoming",
> > +#      "arguments": {
> > +#          "channel": { "channeltype": "main",
> > +#                        "addr": { "transport": "socket",
> > +#                                  "socket-type": { "type": "inet",
> > +#                                                   "host": "10.12.34.9",
> > +#                                                   "port": "1050" } } } } }
> > +# <- { "return": {} }
> > +#
> >   ##
> > -{ 'command': 'migrate-incoming', 'data': {'uri': 'str' } }
> > +{ 'command': 'migrate-incoming',
> > +             'data': {'*uri': 'str',
> > +                      '*channel': 'MigrateChannel'} }
> Same question of whether we need to future proof now by making this
>
>    '*channels': ['MigrateChannel']
>
> ?
>
> Otherwise we'll have to add this 'channels' field later, and
> end up with 'channel' and 'channels' and 'uri'

Why do we need 3 fields ? We can directly convert 'channel' into 'channels'.

'*channels' will be of the form ['MigrateChannel']. Ex: In the list the 
first channel will be of type 'main' and other channel could be of type 
'data' for multifd.

> With regards,
> Daniel
Regards,
Het Gala