[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 07/22] migration: Make migrate_fd_error() the
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v7 07/22] migration: Make migrate_fd_error() the owner of the Error |
Date: |
Mon, 11 Sep 2017 19:58:18 +0100 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
* Eric Blake (address@hidden) 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 <address@hidden>
> > ---
> > 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 <address@hidden>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- Re: [Qemu-devel] [PATCH v7 01/22] Revert "io: add new qio_channel_{readv, writev, read, write}_all functions", (continued)
- [Qemu-devel] [PATCH v7 03/22] migration: Teach it about G_SOURCE_REMOVE, Juan Quintela, 2017/09/06
- [Qemu-devel] [PATCH v7 04/22] migration: Add comments to channel functions, Juan Quintela, 2017/09/06
- [Qemu-devel] [PATCH v7 06/22] migration: Improve migration thread error handling, Juan Quintela, 2017/09/06
- [Qemu-devel] [PATCH v7 05/22] migration: Create migration_has_all_channels, Juan Quintela, 2017/09/06
- [Qemu-devel] [PATCH v7 07/22] migration: Make migrate_fd_error() the owner of the Error, Juan Quintela, 2017/09/06
- [Qemu-devel] [PATCH v7 09/22] migration: Add multifd capability, Juan Quintela, 2017/09/06
- [Qemu-devel] [PATCH v7 08/22] qio: Create new qio_channel_{readv, writev}_all, Juan Quintela, 2017/09/06
- [Qemu-devel] [PATCH v7 11/22] migration: Create x-multifd-group parameter, Juan Quintela, 2017/09/06
- [Qemu-devel] [PATCH v7 10/22] migration: Create x-multifd-threads parameter, Juan Quintela, 2017/09/06
- [Qemu-devel] [PATCH v7 12/22] migration: Create multifd migration threads, Juan Quintela, 2017/09/06
- [Qemu-devel] [PATCH v7 13/22] migration: Split migration_fd_process_incoming, Juan Quintela, 2017/09/06
- [Qemu-devel] [PATCH v7 14/22] migration: Start of multiple fd work, Juan Quintela, 2017/09/06