On Fri, 9 Jan 2026 at 18:12, Fabiano Rosas <[email protected]> wrote: > Whenever an error occurs between migrate_init() and the start of > migration_thread, do cleanup immediately after.
* .. immediately after -> immediately. > The cleanup at qmp_migrate_finish_cb can also be removed because it > will always be reached wither via the error path at * wither -> whether OR either > qmp_migrate_finish->migration_connect_error_propagate or via the > migrate_cleanup_bh. > > The yank_unregister_instance at qmp_migrate() is now replaced by the > one at migration_cleanup(). > > Reviewed-by: Peter Xu <[email protected]> > Signed-off-by: Fabiano Rosas <[email protected]> > --- > migration/migration.c | 26 +++++++++++++++++--------- > 1 file changed, 17 insertions(+), 9 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index 164cb26c48..d57cc2dc3b 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -1576,15 +1576,21 @@ static void > migration_connect_error_propagate(MigrationState *s, Error *error) > { > MigrationStatus current = s->state; > MigrationStatus next = MIGRATION_STATUS_NONE; > + bool resume = false; > > switch (current) { > case MIGRATION_STATUS_SETUP: > next = MIGRATION_STATUS_FAILED; > break; > > + case MIGRATION_STATUS_POSTCOPY_PAUSED: > + resume = true; > + break; > + > case MIGRATION_STATUS_POSTCOPY_RECOVER_SETUP: > /* Never fail a postcopy migration; switch back to PAUSED instead */ > next = MIGRATION_STATUS_POSTCOPY_PAUSED; > + resume = true; > break; > > case MIGRATION_STATUS_CANCELLING: > @@ -1609,6 +1615,10 @@ static void > migration_connect_error_propagate(MigrationState *s, Error *error) > } > > migrate_error_propagate(s, error); > + > + if (!resume) { > + migration_cleanup(s); > + } > } > > void migration_cancel(void) > @@ -2211,9 +2221,6 @@ static gboolean qmp_migrate_finish_cb(QIOChannel > *channel, > MigrationAddress *addr = opaque; > > qmp_migrate_finish(addr, NULL); > - > - cpr_state_close(); > - migrate_hup_delete(migrate_get_current()); > qapi_free_MigrationAddress(addr); > return G_SOURCE_REMOVE; > } > @@ -2222,7 +2229,6 @@ void qmp_migrate(const char *uri, bool has_channels, > MigrationChannelList *channels, bool has_detach, bool > detach, > bool has_resume, bool resume, Error **errp) > { > - Error *local_err = NULL; > MigrationState *s = migrate_get_current(); > g_autoptr(MigrationChannel) channel = NULL; > MigrationAddress *addr = NULL; > @@ -2279,6 +2285,13 @@ void qmp_migrate(const char *uri, bool has_channels, > return; > } > > + /* > + * The migrate_prepare() above calls migrate_init(). From this > + * point on, until the end of migration, make sure any failures > + * eventually result in a call to migration_cleanup(). > + */ * +1 > + Error *local_err = NULL; > + > if (!cpr_state_save(cpr_channel, &local_err)) { > goto out; > } > @@ -2303,7 +2316,6 @@ void qmp_migrate(const char *uri, bool has_channels, > > out: > if (local_err) { > - yank_unregister_instance(MIGRATION_YANK_INSTANCE); > migration_connect_error_propagate(s, error_copy(local_err)); > error_propagate(errp, local_err); > } > @@ -4026,9 +4038,6 @@ 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) { > - migration_cleanup(s); > - } > if (s->error) { > error_report_err(error_copy(s->error)); > } > @@ -4107,7 +4116,6 @@ void migration_connect(MigrationState *s, Error > *error_in) > > fail: > migration_connect_error_propagate(s, local_err); > - migration_cleanup(s); > if (s->error) { > error_report_err(error_copy(s->error)); > } > -- > 2.51.0 * Change looks okay. Reviewed-by: Prasad Pandit <[email protected]> Thank you. --- - Prasad
