On Fri, 9 Jan 2026 at 18:13, Fabiano Rosas <[email protected]> wrote:
> In the next patches migration_cleanup() will be used in qmp_migrate(),
> which currently does not show an error message. Move the error
> reporting out of migration_cleanup() to avoid duplicated messages.

* duplicated -> duplicate OR double

> Reviewed-by: Peter Xu <[email protected]>
> Signed-off-by: Fabiano Rosas <[email protected]>
> ---
>  migration/migration.c | 29 +++++++++++++----------------
>  1 file changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 9204029c88..7bef787f00 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1528,10 +1528,6 @@ static void migration_cleanup(MigrationState *s)
>                            MIGRATION_STATUS_CANCELLED);
>      }
>
> -    if (s->error) {
> -        /* It is used on info migrate.  We can't free it */
> -        error_report_err(error_copy(s->error));
> -    }
>      type = migration_has_failed(s) ? MIG_EVENT_PRECOPY_FAILED :
>                                       MIG_EVENT_PRECOPY_DONE;
>      migration_call_notifiers(type, NULL);
> @@ -1540,7 +1536,12 @@ static void migration_cleanup(MigrationState *s)
>
>  static void migration_cleanup_bh(void *opaque)
>  {
> -    migration_cleanup(opaque);
> +    MigrationState *s = opaque;
> +
> +    migration_cleanup(s);
> +    if (s->error) {
> +        error_report_err(error_copy(s->error));
> +    }
>  }
>
>  /*
> @@ -4025,18 +4026,12 @@ void migration_connect(MigrationState *s, Error 
> *error_in)
>      s->expected_downtime = migrate_downtime_limit();
>      if (error_in) {
>          migration_connect_error_propagate(s, error_in);
> -        if (resume) {
> -            /*
> -             * Don't do cleanup for resume if channel is invalid, but only 
> dump
> -             * the error.  We wait for another channel connect from the user.
> -             * The error_report still gives HMP user a hint on what failed.
> -             * It's normally done in migration_cleanup(), but call it here
> -             * explicitly.
> -             */
> -            error_report_err(error_copy(s->error));
> -        } else {
> +        if (!resume) {
>              migration_cleanup(s);
>          }
> +        if (s->error) {
> +            error_report_err(error_copy(s->error));
> +        }
>          return;
>      }
>
> @@ -4115,8 +4110,10 @@ fail:
>      if (s->state != MIGRATION_STATUS_CANCELLING) {
>          migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
>      }
> -    error_report_err(local_err);
>      migration_cleanup(s);
> +    if (s->error) {
> +        error_report_err(error_copy(s->error));
> +    }
>  }
>
>  static void migration_class_init(ObjectClass *klass, const void *data)
> --

* Change looks okay.
Reviewed-by: Prasad Pandit <[email protected]>

Thank you.
---
  - Prasad


Reply via email to