Re: [PATCH 2/4] migration: Accept MigrationStatus in migration_has_failed()

[Peter Xu <peterx@redhat.com>]

On Mon, Sep 15, 2025 at 01:59:13PM +0200, Juraj Marcin wrote:
> From: Juraj Marcin <jmarcin@redhat.com>
> 
> This allows to reuse the helper also with MigrationIncomingState.

I get the point, but just to mention that this helper doesn't really change
much on incoming side on simplifying the code or function-wise, because we
don't have CANCELLING/CANCELLED state on deste QEMU.. which is definitely
not obvious.. :(

So:

  migration_has_failed(incoming->state)

Is exactly the same as:

  incoming->state == MIGRATION_STATUS_FAILED

Except it will make src start to pass in s->state.. which is slightly more
awkward.

Maybe we keep the MIGRATION_STATUS_FAILED check in your next patch, and
drop this one for now until it grows more than FAILED on dest?

> 
> Signed-off-by: Juraj Marcin <jmarcin@redhat.com>
> ---
>  migration/migration.c | 8 ++++----
>  migration/migration.h | 2 +-
>  migration/multifd.c   | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 54dac3db88..2c0b3a7229 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1542,7 +1542,7 @@ static void migration_cleanup(MigrationState *s)
>          /* 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 :
> +    type = migration_has_failed(s->state) ? MIG_EVENT_PRECOPY_FAILED :
>                                       MIG_EVENT_PRECOPY_DONE;
>      migration_call_notifiers(s, type, NULL);
>      yank_unregister_instance(MIGRATION_YANK_INSTANCE);
> @@ -1700,10 +1700,10 @@ int migration_call_notifiers(MigrationState *s, MigrationEventType type,
>      return ret;
>  }
>  
> -bool migration_has_failed(MigrationState *s)
> +bool migration_has_failed(MigrationStatus state)
>  {
> -    return (s->state == MIGRATION_STATUS_CANCELLED ||
> -            s->state == MIGRATION_STATUS_FAILED);
> +    return (state == MIGRATION_STATUS_CANCELLED ||
> +            state == MIGRATION_STATUS_FAILED);
>  }
>  
>  bool migration_in_postcopy(void)
> diff --git a/migration/migration.h b/migration/migration.h
> index 01329bf824..2c2331f40d 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -535,7 +535,7 @@ bool migration_is_blocked(Error **errp);
>  bool migration_in_postcopy(void);
>  bool migration_postcopy_is_alive(MigrationStatus state);
>  MigrationState *migrate_get_current(void);
> -bool migration_has_failed(MigrationState *);
> +bool migration_has_failed(MigrationStatus state);
>  bool migrate_mode_is_cpr(MigrationState *);
>  
>  uint64_t ram_get_total_transferred_pages(void);
> diff --git a/migration/multifd.c b/migration/multifd.c
> index b255778855..c569f91f2c 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -568,7 +568,7 @@ void multifd_send_shutdown(void)
>               * already failed. If the migration succeeded, errors are
>               * not expected but there's no need to kill the source.
>               */
> -            if (local_err && !migration_has_failed(migrate_get_current())) {
> +            if (local_err && !migration_has_failed(migrate_get_current()->state)) {
>                  warn_report(
>                      "multifd_send_%d: Failed to terminate TLS connection: %s",
>                      p->id, error_get_pretty(local_err));
> -- 
> 2.51.0
> 

-- 
Peter Xu