On Fri, 9 Jan 2026 at 18:12, Fabiano Rosas <farosas@suse.de> 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 <peterx@redhat.com>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> 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 <pjp@fedoraproject.org>
Thank you.
---
- Prasad