On Tue, May 16, 2023 at 11:18:16AM +0530, Het Gala wrote:
>
> On 15/05/23 4:06 pm, Daniel P. Berrangé wrote:
> > On Fri, May 12, 2023 at 02:32:38PM +0000, Het Gala wrote:
> > > MigrateChannelList ideally allows to connect accross multiple interfaces.
> > >
> > > Added MigrateChannelList struct as argument to 'migrate' qapi. Introduced
> > > MigrateChannelList in hmp_migrate() and qmp_migrate() functions.
> > >
> > > Future patchset series plans to include multiple MigrateChannels
> > > for multiple interfaces to be connected. That is the reason,
> > > 'MigrateChannelList'
> > > is the preferred choice of argument over 'MigrateChannel' and making
> > > 'migrate'
> > > QAPI future proof.
> > >
> > > For current patch, have just limit size of MigrateChannelList to 1
> > > element as
> > > a runtime check.
> > >
> > > Suggested-by: Aravind Retnakaran <aravind.retnaka...@nutanix.com>
> > > Signed-off-by: Het Gala <het.g...@nutanix.com>
> > > ---
> > > migration/migration-hmp-cmds.c | 113 ++++++++++++++++++++++++++++++++-
> > > migration/migration.c | 17 ++++-
> > > qapi/migration.json | 69 +++++++++++++++++++-
> > > 3 files changed, 192 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/migration/migration-hmp-cmds.c
> > > b/migration/migration-hmp-cmds.c
> > > index 4e9f00e7dc..f098d04542 100644
> > > --- a/migration/migration-hmp-cmds.c
> > > +++ b/migration/migration-hmp-cmds.c
> > > @@ -51,6 +51,101 @@ static void migration_global_dump(Monitor *mon)
> > > ms->clear_bitmap_shift);
> > > }
> > > +static bool
> > > +migrate_channel_from_qdict(MigrateChannel **channel,
> > > + const QDict *qdict, Error **errp)
> > > +{
> > > + Error *err = NULL;
> > > + const char *channeltype = qdict_get_try_str(qdict, "channeltype");
> > > + const char *transport_str = qdict_get_try_str(qdict, "transport");
> > > + const char *socketaddr_type = qdict_get_try_str(qdict, "type");
> > > + const char *inet_host = qdict_get_try_str(qdict, "host");
> > > + const char *inet_port = qdict_get_try_str(qdict, "port");
> > > + const char *unix_path = qdict_get_try_str(qdict, "path");
> > > + const char *vsock_cid = qdict_get_try_str(qdict, "cid");
> > > + const char *vsock_port = qdict_get_try_str(qdict, "port");
> > > + const char *fd = qdict_get_try_str(qdict, "str");
> > > + QList *exec = qdict_get_qlist(qdict, "exec");
> > THis seems to be implicitly defining a huge set of extra parameters
> > for the migrate 'HMP' command, but none of it is actually defined
> > at the hmp-commands.hx
> I don't have a lot of knowledge on HMP commands. I had code changes here in
> HMP merely to to keep it compatible with QMP command as it used to call
> qmp_migrate() function.
> > Do we even need todo this ? For HMP it is not unreasonable to just
> > keep using the URI syntax forever?
>
> Daniel, I didn't completely understand your ask here. Are you implying that
> for the HMP wrapper on top of QMP, we should pass it as a string only to
> qmp_migrate() function ?
Yeah, that's my thought. HMP is targetted at humans and aims for
user friendliness, and does not need to have 100% parity with QMP.
For the multi-channel setup, I think that's going to be something
only mgmt apps do, unlikely humans using HMP need this.
>
> In that case, we won't be needing migrate_channel_from_qdict() function at
> all right ? Please guide.
Yeah, I think it is redundant.
An earlier patch already added an API to convert a "char *uri" into a
MigrateChannel object. So we can keep HMP using a URI, but pass it to
QMP using the MigrateChannel struct.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
