Re: [PATCH V6 07/14] migration: preserve suspended for snapshot

[Peter Xu <peterx@redhat.com>]

On Thu, Nov 30, 2023 at 01:37:20PM -0800, Steve Sistare wrote:
> Restoring a snapshot can break a suspended guest.  Snapshots suffer from
> the same suspended-state issues that affect live migration, plus they must
> handle an additional problematic scenario, which is that a running vm must
> remain running if it loads a suspended snapshot.  Currently, after loading
> such a snapshot, the vm_start fails.  The runstate is RUNNING, but the guest
> is not.
> 
> To save, the vm_stop call now completely stops the suspended state, courtesy
> of a recent patch.  Finish with vm_resume to leave the vm in the state it had
> prior to the save, correctly restoring the suspended state.
> 
> To load, if the snapshot is not suspended, then vm_stop + vm_resume
> correctly handles all states, and leaves the vm in the state it had prior
> to the load.  However, if the snapshot is suspended, restoration is
> trickier.  First, call vm_resume to restore the state to suspended so the
> current state matches the saved state.  Then, if the pre-load state is
> running, call wakeup to resume running.
> 
> Prior to these changes, the vm_stop to RUN_STATE_SAVE_VM and
> RUN_STATE_RESTORE_VM did not change runstate if the current state was
> paused, suspended, or prelaunch, but now it does, so allow these
> transitions.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

One nitpick below:

[...]

> +void load_snapshot_resume(RunState state)
> +{
> +    vm_resume(state);
> +    if (state == RUN_STATE_RUNNING && runstate_get() == RUN_STATE_SUSPENDED) {
> +        qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, NULL);

Please avoid using NULL for Error**.  &error_abort seems apropriate.

Thanks,

-- 
Peter Xu