On Tuesday, July 18, 2023 1:44 PM, Isaku Yamahata wrote:
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -2058,6 +2058,21 @@ static int
> await_return_path_close_on_source(MigrationState *ms)
> > return ms->rp_state.error;
> > }
> >
> > +static int close_return_path_on_source(MigrationState *ms) {
> > + int ret;
> > +
> > + if (!ms->rp_state.rp_thread_created) {
> > + return 0;
> > + }
> > +
> > + trace_migration_return_path_end_before();
> > + ret = await_return_path_close_on_source(ms);
> > + trace_migration_return_path_end_after(ret);
> > +
> > + return ret;
> > +}
> > +
>
> There is only one caller, migration_completion(). We can consolidate two
> functions, await_return_path_close_on_source() and
> close_return_path_on_source(), into single function.
Sounds good, thanks.
> > +static int migration_completion_postcopy(MigrationState *s) {
> > + trace_migration_completion_postcopy_end();
> > +
> > + qemu_mutex_lock_iothread();
> > + qemu_savevm_state_complete_postcopy(s->to_dst_file);
> > + qemu_mutex_unlock_iothread();
> > +
> > + /*
> > + * Shutdown the postcopy fast path thread. This is only needed when
> dest
> > + * QEMU binary is old (7.1/7.2). QEMU 8.0+ doesn't need this.
> > + */
> > + if (migrate_postcopy_preempt() && s->preempt_pre_7_2) {
> > + postcopy_preempt_shutdown_file(s);
> > + }
> > +
> > + trace_migration_completion_postcopy_end_after_complete();
> > +
> > + return 0;
>
> Always return 0? Make it void.
OK.
> > +static void migration_completion(MigrationState *s) {
> > + int ret = -1;
> > + int current_active_state = s->state;
> > +
> > + if (s->state == MIGRATION_STATUS_ACTIVE) {
> > + ret = migration_completion_precopy(s, ¤t_active_state);
> > + } else if (s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE) {
> > + ret = migration_completion_postcopy(s);
>
> Here we can set ret = 0.
Yes, after migration_completion_postcopy is made "void".
