Re: [pve-devel] [PATCH qemu-server] migrate: keep VM paused after migration if it was before

[Fabian Ebner <f.ebner@proxmox.com>]

Am 21.04.22 um 11:15 schrieb Fabian Grünbichler:
> 
> this confused me, so I tried to look at different code paths - a 
> transition from phase2 (after migration has started, any error before 
> that is irrelevant since it couldn't have triggered a pause) to 
> phase2_cleanup with $err / $self->{errors} set can only happen if
> 
> A) migration reached 'completed' state, VM state transitioned to 
> FINISH_MIGRATE, but block-job completing code after the query-migrate 
> loop died
> B) query-migrate failed too often, VM state unknown
> C) query-migrate returned no or an invalid status, VM state unknown
> D) query-migrate returned failed or cancelled status, VM state unknown
> E) migration was interrupted
> 
> in case A (which was my original sequence of events), the VM is now 
> paused for sure (by the migration), and we only want to resume it (via 
> migrate_cancel?) if it wasn't previously paused. so does migrate_cancel 
> always trigger a resume? if so, do we want to (re-)pause the VM 
> depending on whether it was originally paused?
> 
> in case B or C we don't know the state or what it means. we can attempt 
> to migrate_cancel (which might resume the VM?), which might give us the 
> same situation as A
> 
> in case D or E the VM might have been paused (and resumed again) by the 
> migration, or maybe not. so we might still want to do the same as with 
> situation A.
> 
> looking at the qemu source it should keep track of whether the VM was
> running before it paused during the migration completion 
> (MigrationState->vm_was_running).
> 
> this gets set when the migration converges (in migration_completion), 
> before the VM transitions to FINISH_MIGRATE (i.e., paused while waiting 
> for completion). from there on we have two options - either the 
> migration status switches to failed, or to completed, and we BREAK the 
> migration iteration (migration_iteration_run), and we end up in 
> migration_iteration_finish.
> 
> migration_iteration_finish will transition the VM to POSTMIGRATE if the 
> migration was completed. otherwise, if the migration got cancelled, it 
> will start the VM again if it was marked as previously running but 
> paused by migration (see above), or else if it entered convergence but 
> didn't complete yet it will be switched to POSTMIGRATE (without 
> vm_start). cancelling before the migration converged simple does nothing 
> except cleaning up the migration - the VM is still running or paused 
> depending on its original state, since the migration never paused it it 
> also had no need to resume it.
> 
> so there is case A, where we should end up in POSTMIGRATE and want to 
> return to RUNNING or PAUSED, depending on original state. there is case 
> B, where cancelling before convergence is fine, cancelling after 
> convergence but before completion should resume iff originally running 
> (fine), but end up in POSTMIGRATE instead of PAUSED if not (wrong), with 
> cases C, D and E being similar but likely not resumed/completed.

If the VM was running before, I think we can just check if the state is
POSTMIGRATE (after migrate_cancel, both cases where we know for sure we
want to resume should end up there; if there is some weird corner case
we missed, I'd say adding it once it pops up is the way to go) and try
to go back to RUNNING by issuing a QMP 'cont'.

We can't actually go from POSTMIGRATE back to PAUSED directly though:

typedef struct {
    RunState from;
    RunState to;
} RunStateTransition;

static const RunStateTransition runstate_transitions_def[] = {
(...)

    { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING },
    { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE },
    { RUN_STATE_POSTMIGRATE, RUN_STATE_PRELAUNCH },

(...)

Not sure what we can do besides keeping it in POSTMIGRATE there? It
shouldn't be as relevant as in the "was running" case IMHO. Well,
enabling the start button in the UI would help ;)