From 4a228bcc994ea8ab05d4927e23e7916f32cc1168 Mon Sep 17 00:00:00 2001 From: Fabiano Rosas Date: Thu, 13 Feb 2025 14:59:24 -0300 Subject: [PATCH] migration: Don't set FAILED state when cancelling The expected outcome from qmp_migrate_cancel() is that the source migration goes to the terminal state MIGRATION_STATUS_CANCELLED. Anything different from this is a bug when cancelling. Make sure there is never a state transition from an unspecified state into FAILED. Code that sets FAILED, should always either make sure that the old state is not CANCELLING or specify the old state. Note that the destination is allowed to go into FAILED, so there's no issue there. (I don't think this is relevant as a backport because cancelling does work, it just doesn't show the right state at the end) Fixes: 3dde8fdbad ("migration: Merge precopy/postcopy on switchover start") Fixes: d0edb8a173 ("migration: Create the postcopy preempt channel asynchronously") Fixes: 8518278a6a ("migration: implementation of background snapshot thread") Fixes: bf78a046b9 ("migration: refactor migrate_fd_connect failures") Reviewed-by: Peter Xu Message-ID: <20250213175927.19642-7-farosas@suse.de> Signed-off-by: Fabiano Rosas --- migration/migration.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index 48c9ad3c96..c597aa707e 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2648,7 +2648,10 @@ static int postcopy_start(MigrationState *ms, Error **errp) if (migrate_postcopy_preempt()) { migration_wait_main_channel(ms); if (postcopy_preempt_establish_channel(ms)) { - migrate_set_state(&ms->state, ms->state, MIGRATION_STATUS_FAILED); + if (ms->state != MIGRATION_STATUS_CANCELLING) { + migrate_set_state(&ms->state, ms->state, + MIGRATION_STATUS_FAILED); + } error_setg(errp, "%s: Failed to establish preempt channel", __func__); return -1; @@ -2986,7 +2989,9 @@ fail: error_free(local_err); } - migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED); + if (s->state != MIGRATION_STATUS_CANCELLING) { + migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED); + } } /** @@ -3009,7 +3014,7 @@ static void bg_migration_completion(MigrationState *s) qemu_put_buffer(s->to_dst_file, s->bioc->data, s->bioc->usage); qemu_fflush(s->to_dst_file); } else if (s->state == MIGRATION_STATUS_CANCELLING) { - goto fail; + return; } if (qemu_file_get_error(s->to_dst_file)) { @@ -3953,7 +3958,9 @@ void migration_connect(MigrationState *s, Error *error_in) fail: migrate_set_error(s, local_err); - migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED); + if (s->state != MIGRATION_STATUS_CANCELLING) { + migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED); + } error_report_err(local_err); migration_cleanup(s); } -- 2.30.2