Re: [PATCH v9 06/10] migration: New migrate and migrate-incoming argument 'channels'

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Jul 25, 2023 at 07:34:09PM +0100, Daniel P. Berrangé wrote:
> On Fri, Jul 21, 2023 at 02:49:31PM +0000, Het Gala wrote:
> > MigrateChannelList allows to connect accross multiple interfaces.
> > Add MigrateChannelList struct as argument to migration QAPIs.
> > 
> > We plan to include multiple channels in future, to connnect
> > multiple interfaces. Hence, we choose 'MigrateChannelList'
> > as the new argument over 'MigrateChannel' to make migration
> > QAPIs future proof.
> > 
> > Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
> > Signed-off-by: Het Gala <het.gala@nutanix.com>
> > Acked-by: Markus Armbruster <armbru@redhat.com>
> > ---
> >  migration/migration-hmp-cmds.c |   6 +-
> >  migration/migration.c          |  34 ++++++++--
> >  qapi/migration.json            | 109 ++++++++++++++++++++++++++++++++-
> >  softmmu/vl.c                   |   2 +-
> >  4 files changed, 139 insertions(+), 12 deletions(-)
> > 
> > diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> > index 9885d7c9f7..49b150f33f 100644
> > --- a/migration/migration-hmp-cmds.c
> > +++ b/migration/migration-hmp-cmds.c
> > @@ -424,7 +424,7 @@ 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);
> > +    qmp_migrate_incoming(uri, false, NULL, &err);
> >  
> >      hmp_handle_error(mon, err);
> >  }
> > @@ -705,8 +705,8 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
> >      const char *uri = qdict_get_str(qdict, "uri");
> >      Error *err = NULL;
> >  
> > -    qmp_migrate(uri, !!blk, blk, !!inc, inc,
> > -                false, false, true, resume, &err);
> > +    qmp_migrate(uri, false, NULL, !!blk, blk, !!inc, inc,
> > +                 false, false, true, resume, &err);
> >      if (hmp_handle_error(mon, err)) {
> >          return;
> >      }
> > diff --git a/migration/migration.c b/migration/migration.c
> > index f37b388876..bd3a93fc8c 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -466,10 +466,22 @@ static bool migrate_uri_parse(const char *uri,
> >      return true;
> >  }
> >  
> > -static void qemu_start_incoming_migration(const char *uri, Error **errp)
> > +static void qemu_start_incoming_migration(const char *uri, bool has_channels,
> > +                                          MigrationChannelList *channels,
> > +                                          Error **errp)
> >  {
> >      g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
> >  
> > +    /*
> > +     * Having preliminary checks for uri and channel
> > +     */
> > +    if (uri && has_channels) {
> > +        error_setg(errp, "'uri' and 'channels' arguments are mutually "
> > +                   "exclusive; exactly one of the two should be present in "
> > +                   "'migrate-incoming' qmp command ");
> > +        return;
> > +    }
> 
> This checks is both are present.
> 
> Also needs a check if neither are present as that's invalid.

Also it should (temporarily) raise an error if "has_channels" is
set, as while we've added the parameter in QAPI, we've not
implemented it yet. IOW, raise an error now, and remove the
error in a later patch.

> 
> 
> > @@ -1694,6 +1708,16 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> >      MigrationState *s = migrate_get_current();
> >      g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
> >  
> > +    /*
> > +     * Having preliminary checks for uri and channel
> > +     */
> > +    if (uri && has_channels) {
> > +        error_setg(errp, "'uri' and 'channels' arguments are mutually "
> > +                   "exclusive; exactly one of the two should be present in "
> > +                   "'migrate' qmp command ");
> > +        return;
> > +    }
> 
> Same here 
> 
> 
> 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 :|
> 
> 

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 :|