| Message ID | 1705071910-174321-9-git-send-email-steven.sistare@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | allow cpr-reboot for vfio | expand |
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.sistare@oracle.com> Reviewed-by: Peter Xu <peterx@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 >
On 1/15/2024 2:37 AM, Peter Xu wrote: > 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.sistare@oracle.com> > > Reviewed-by: Peter Xu <peterx@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. Yes, if you are OK with it, I also prefer to simplify here. - Steve >> + error_report_err(local_err); >> + migrate_fd_cleanup(s); >> } >> >> static void migration_class_init(ObjectClass *klass, void *data) >> -- >> 1.8.3.1 >> >
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; /* * 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; + 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); + error_report_err(local_err); + migrate_fd_cleanup(s); } static void migration_class_init(ObjectClass *klass, void *data)
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.sistare@oracle.com> --- migration/migration.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)