Re: [PATCH for-11.0 v2 4/7] migration: Make migration_connect_set_error() own the error

[Markus Armbruster <armbru@redhat.com>]

Peter Xu <peterx@redhat.com> writes:

> Make migration_connect_set_error() take ownership of the error always.
> Paving way for making migrate_set_error() to take ownership.
>
> When at it, renaming it to migration_connect_error_propagate(), following
> Error API, to imply the Error object ownership transition.
>
> NOTE: this patch also makes migration_connect() to take ownership of the
> Error passed in.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/channel.c   | 1 -
>  migration/migration.c | 9 +++++----
>  2 files changed, 5 insertions(+), 5 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..0ff8b31a88 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_error_propagate(MigrationState *s, Error *error)
>  {
>      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_error_propagate(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_error_propagate(s, error_copy(local_err));
>          error_propagate(errp, local_err);
>          return;
>      }
> @@ -4039,7 +4040,7 @@ void migration_connect(MigrationState *s, Error *error_in)
>  
>      s->expected_downtime = migrate_downtime_limit();
>      if (error_in) {
> -        migration_connect_set_error(s, error_in);
> +        migration_connect_error_propagate(s, error_in);
>          if (resume) {
>              /*
>               * Don't do cleanup for resume if channel is invalid, but only dump

Could be split for slightly easier review: first change
migration_connect(), then migrate_set_error().  I doubt it's worth the
bother now.

Reviewed-by: Markus Armbruster <armbru@redhat.com>