Peter Xu <[email protected]> writes:
> Make migration_connect_set_error() take ownership of the error always.
> Paving way for making migrate_set_error() to take ownership.
>
> Signed-off-by: Peter Xu <[email protected]>
> ---
> migration/channel.c | 1 -
> migration/migration.c | 7 ++++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/migration/channel.c b/migration/channel.c
> index 462cc183e1..92435fa7f7 100644
> --- a/migration/channel.c
> +++ b/migration/channel.c
> @@ -95,7 +95,6 @@ void migration_channel_connect(MigrationState *s,
> }
> }
> migration_connect(s, error);
> - error_free(error);
> }
>
>
> diff --git a/migration/migration.c b/migration/migration.c
> index b316ee01ab..4fe69cc2ef 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1575,7 +1575,7 @@ static void migrate_error_free(MigrationState *s)
> }
> }
>
> -static void migration_connect_set_error(MigrationState *s, const Error
> *error)
> +static void migration_connect_set_error(MigrationState *s, Error *error)
Recommend to rename for the same reason you rename migrate_set_error()
in the last patch.
Bonus: all calls become visible in the patch, easing review.
> {
> MigrationStatus current = s->state;
> MigrationStatus next;
> @@ -1602,6 +1602,7 @@ static void migration_connect_set_error(MigrationState
> *s, const Error *error)
>
> migrate_set_state(&s->state, current, next);
> migrate_set_error(s, error);
> + error_free(error);
> }
>
> void migration_cancel(void)
> @@ -2292,7 +2293,7 @@ void qmp_migrate(const char *uri, bool has_channels,
>
> out:
> if (local_err) {
> - migration_connect_set_error(s, local_err);
> + migration_connect_set_error(s, error_copy(local_err));
> error_propagate(errp, local_err);
> }
> }
> @@ -2337,7 +2338,7 @@ static void qmp_migrate_finish(MigrationAddress *addr,
> bool resume_requested,
> if (!resume_requested) {
> yank_unregister_instance(MIGRATION_YANK_INSTANCE);
> }
> - migration_connect_set_error(s, local_err);
> + migration_connect_set_error(s, error_copy(local_err));
> error_propagate(errp, local_err);
> return;
> }
There's one more call in migration_connect(). Doesn't it need an
error_copy() now? Oh, I see: it doesn't, because its only caller
migration_channel_connect() loses its error_free() in the first hunk.
So, migration_connect() *also* takes ownership now. The commit message
should cover this.
Aside: I'd be tempted to lift the if (error_in) code from
migration_connect() to migration_channel_connect() and drop the
@error_in parameter.
