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

[Fabian Grünbichler]

On April 21, 2022 9:44 am, Fabian Ebner wrote:
> Am 20.04.22 um 14:43 schrieb Fabian Grünbichler:
>> On March 18, 2022 8:51 am, Fabian Ebner wrote:
>>> Also cannot issue a guest agent command in that case.
>>>
>>> Reported in the community forum:
>>> https://forum.proxmox.com/threads/106618
>>>
>>> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
>>> ---
>>>
>>> Best viewed with -w.
>>>
>>>  PVE/QemuMigrate.pm | 54 ++++++++++++++++++++++++++--------------------
>>>  1 file changed, 31 insertions(+), 23 deletions(-)
>> 
>> patch looks good to me - it might make sense to restructure the 
>> conditionals a bit to log that resuming/fstrim was skipped though to 
>> reduce confusion (user that paused VM and user doing the migration might 
>> not be the same entity after all)?
>> 
>> one other thing I noticed (pre-existing, but the changes here made me 
>> look and my search came up short), inside phase2:
>> 
>> - start block job(s) without autocompletion and wait for them to 
>>   converge
>> - start RAM/state migration without autocompletion and wait for it to 
>>   converge
>> X both source and target VMs are paused now with "identical" state, 
>> irrespective of the source being paused or not initially
>> - cancel block job(s) (to close NBD writer(s) so that switchover can 
>>   proceed in phase3_cleanup)
>> 
>> if something happens after X in phase2, we enter phase2_cleanup, and 
>> attempt to cancel the migration
> 
> If migrate_cancel actually cancels the migration, the VM will be running
> on the source node again :)

see below - AFAIU migrate_cancel will resume if the migration reached 
the point of being completed and the VM was not paused, but running at 
that point.

> If migrate_cancel fails, resume might also fail?

possibly / probably, but at that point then error handling is already 
best-effort so it doesn't hurt to try I guess. the only way for 
migrate_cancel to fail (according to the docs ;)) is if QMP itself is 
not available..

> There is an edge case however:
> If migration actually finished, but we aborted because of e.g. too many
> query-migrate failures, then migrate_cancel will succeed (because there
> is no active migration) and the VM will be in post-migrate state on the
> source node. Here, resume would help.

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.

but all of that would require some confirmation via tracing and 
corresponding 'die' statements, possibly I didn't get the full picture 
of how cancelling interacts with completion for example ;)