Re: [Qemu-devel] [PATCH v7 07/22] migration: Make migrate_fd_error() the owner of the Error

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Eric Blake (eblake@redhat.com) wrote:
> On 09/06/2017 06:51 AM, Juan Quintela wrote:
> > So far, we had to free the error after each caller, so just do it
> > here.  Once there, tls.c was leaking the error.
> 
> You mention tls.c,
> 
> > 
> > Signed-off-by: Juan Quintela <quintela@redhat.com>
> > ---
> >  migration/channel.c   |  1 -
> >  migration/migration.c | 10 ++++------
> >  migration/migration.h |  4 ++--
> >  migration/socket.c    |  1 -
> >  4 files changed, 6 insertions(+), 10 deletions(-)
> 
> but don't touch it.  Am I missing something?


hmm well I see in migration/tls.c:

    if (qio_task_propagate_error(task, &err)) {
        trace_migration_tls_outgoing_handshake_error(error_get_pretty(err));
        migrate_fd_error(s, err);
        error_free(err);
    } else {
        trace_migration_tls_outgoing_handshake_complete();
        migration_channel_connect(s, ioc, NULL);
    }

so I think that error_free has to go?

Dave

> >  
> > -void migrate_fd_error(MigrationState *s, const Error *error)
> > +void migrate_fd_error(MigrationState *s, Error *error)
> >  {
> 
> No comments at definition,
> 
> > +++ b/migration/migration.h
> > @@ -163,8 +163,8 @@ bool  migration_has_all_channels(void);
> >  
> >  uint64_t migrate_max_downtime(void);
> >  
> > -void migrate_set_error(MigrationState *s, const Error *error);
> > -void migrate_fd_error(MigrationState *s, const Error *error);
> > +void migrate_set_error(MigrationState *s, Error *error);
> > +void migrate_fd_error(MigrationState *s, Error *error);
> 
> or at declaration. That would be worth adding at some point, but this
> patch isn't making it worse.
> 
> The code looks okay in isolation, so if it is only the commit message
> that needs fixing,
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 



--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK