Hi Peter, On 2025-09-19 10:57, Peter Xu wrote: > On Mon, Sep 15, 2025 at 01:59:13PM +0200, Juraj Marcin wrote: > > From: Juraj Marcin <[email protected]> > > > > 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?
Sure, that sounds like a good solution. > > > > > Signed-off-by: Juraj Marcin <[email protected]> > > --- > > 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 >
