On Fri, Jan 12, 2024 at 07:05:07AM -0800, Steve Sistare wrote:
> Move common code for the error path in migrate_fd_connect to a shared
> fail label. No functional change.
>
> Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
Reviewed-by: Peter Xu <pet...@redhat.com>
One nitpick below:
> ---
> migration/migration.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index e9914aa..c828ba7 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3549,6 +3549,7 @@ void migrate_fd_connect(MigrationState *s, Error
> *error_in)
> Error *local_err = NULL;
> uint64_t rate_limit;
> bool resume = s->state == MIGRATION_STATUS_POSTCOPY_PAUSED;
> + int expect_state = s->state;
IMHO we can drop this.
>
> /*
> * If there's a previous error, free it and prepare for another one.
> @@ -3603,11 +3604,7 @@ void migrate_fd_connect(MigrationState *s, Error
> *error_in)
> if (migrate_postcopy_ram() || migrate_return_path()) {
> if (open_return_path_on_source(s)) {
> error_setg(&local_err, "Unable to open return-path for
> postcopy");
> - migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
> - migrate_set_error(s, local_err);
> - error_report_err(local_err);
> - migrate_fd_cleanup(s);
> - return;
> + goto fail;
> }
> }
>
> @@ -3629,12 +3626,8 @@ void migrate_fd_connect(MigrationState *s, Error
> *error_in)
> }
>
> if (multifd_save_setup(&local_err) != 0) {
> - migrate_set_error(s, local_err);
> - error_report_err(local_err);
> - migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> - MIGRATION_STATUS_FAILED);
> - migrate_fd_cleanup(s);
> - return;
> + expect_state = MIGRATION_STATUS_SETUP;
Then drop this.
> + goto fail;
> }
>
> if (migrate_background_snapshot()) {
> @@ -3645,6 +3638,13 @@ void migrate_fd_connect(MigrationState *s, Error
> *error_in)
> migration_thread, s, QEMU_THREAD_JOINABLE);
> }
> s->migration_thread_running = true;
> + return;
> +
> +fail:
> + migrate_set_error(s, local_err);
> + migrate_set_state(&s->state, expect_state, MIGRATION_STATUS_FAILED);
Then constantly use s->state.
> + error_report_err(local_err);
> + migrate_fd_cleanup(s);
> }
>
> static void migration_class_init(ObjectClass *klass, void *data)
> --
> 1.8.3.1
>
--
Peter Xu
