Re: [PATCH v11 02/10] migration: convert migration 'uri' into 'MigrateAddress'

[Daniel P . Berrangé]

On Wed, Oct 04, 2023 at 11:43:12AM -0300, Fabiano Rosas wrote:
> Het Gala <het.gala@nutanix.com> writes:
> 
> > This patch parses 'migrate' and 'migrate-incoming' QAPI's 'uri'
> > string containing migration connection related information
> > and stores them inside well defined 'MigrateAddress' struct.
> >
> > Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
> > Signed-off-by: Het Gala <het.gala@nutanix.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  migration/exec.c      |  1 -
> >  migration/exec.h      |  4 ++++
> >  migration/migration.c | 55 +++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 59 insertions(+), 1 deletion(-)
> >
> > diff --git a/migration/exec.c b/migration/exec.c
> > index 2bf882bbe1..32f5143dfd 100644
> > --- a/migration/exec.c
> > +++ b/migration/exec.c
> > @@ -27,7 +27,6 @@
> >  #include "qemu/cutils.h"
> >  
> >  #ifdef WIN32
> > -const char *exec_get_cmd_path(void);
> >  const char *exec_get_cmd_path(void)
> >  {
> >      g_autofree char *detected_path = g_new(char, MAX_PATH);
> > diff --git a/migration/exec.h b/migration/exec.h
> > index b210ffde7a..736cd71028 100644
> > --- a/migration/exec.h
> > +++ b/migration/exec.h
> > @@ -19,6 +19,10 @@
> >  
> >  #ifndef QEMU_MIGRATION_EXEC_H
> >  #define QEMU_MIGRATION_EXEC_H
> > +
> > +#ifdef WIN32
> > +const char *exec_get_cmd_path(void);
> > +#endif
> >  void exec_start_incoming_migration(const char *host_port, Error **errp);
> >  
> >  void exec_start_outgoing_migration(MigrationState *s, const char 
> > *host_port,
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 6d3cf5d5cd..dcbd509d56 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -65,6 +65,7 @@
> >  #include "sysemu/qtest.h"
> >  #include "options.h"
> >  #include "sysemu/dirtylimit.h"
> > +#include "qemu/sockets.h"
> >  
> >  static NotifierList migration_state_notifiers =
> >      NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
> > @@ -427,15 +428,64 @@ void migrate_add_address(SocketAddress *address)
> >                        QAPI_CLONE(SocketAddress, address));
> >  }
> >  
> > +static bool migrate_uri_parse(const char *uri,
> > +                              MigrationAddress **channel,
> > +                              Error **errp)
> > +{
> > +    g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
> 
> This cannot be g_autoptr because you're passing it out of scope at the
> end of the function.

It is still good to use g_autoptr to deal with the error paths.

On the success path though you need   g_steal_pointer(&addr) to
prevent the autofree cleanup running.


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