Hi Markus, Eric, any comment about this change in stop/cont behavior before
it gets pulled? There is no change since V6.
- Steve
On 12/6/2023 12:23 PM, Steve Sistare wrote:
> Currently, a vm in the suspended state is not completely stopped. The VCPUs
> have been paused, but the cpu clock still runs, and runstate notifiers for
> the transition to stopped have not been called. This causes problems for
> live migration. Stale cpu timers_state is saved to the migration stream,
> causing time errors in the guest when it wakes from suspend, and state that
> would have been modified by runstate notifiers is wrong.
>
> Modify vm_stop to completely stop the vm if the current state is suspended,
> transition to RUN_STATE_PAUSED, and remember that the machine was suspended.
> Modify vm_start to restore the suspended state.
>
> This affects all callers of vm_stop and vm_start, notably, the qapi stop and
> cont commands. For example:
>
> (qemu) info status
> VM status: paused (suspended)
>
> (qemu) stop
> (qemu) info status
> VM status: paused
>
> (qemu) system_wakeup
> Error: Unable to wake up: guest is not in suspended state
>
> (qemu) cont
> (qemu) info status
> VM status: paused (suspended)
>
> (qemu) system_wakeup
> (qemu) info status
> VM status: running
>
> Suggested-by: Peter Xu <pet...@redhat.com>
> Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
> Reviewed-by: Peter Xu <pet...@redhat.com>
> ---
> include/sysemu/runstate.h | 5 +++++
> qapi/misc.json | 10 ++++++++--
> system/cpus.c | 23 +++++++++++++++--------
> system/runstate.c | 3 +++
> 4 files changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/include/sysemu/runstate.h b/include/sysemu/runstate.h
> index 88a67e2..867e53c 100644
> --- a/include/sysemu/runstate.h
> +++ b/include/sysemu/runstate.h
> @@ -40,6 +40,11 @@ static inline bool shutdown_caused_by_guest(ShutdownCause
> cause)
> return cause >= SHUTDOWN_CAUSE_GUEST_SHUTDOWN;
> }
>
> +static inline bool runstate_is_live(RunState state)
> +{
> + return state == RUN_STATE_RUNNING || state == RUN_STATE_SUSPENDED;
> +}
> +
> void vm_start(void);
>
> /**
> diff --git a/qapi/misc.json b/qapi/misc.json
> index cda2eff..efb8d44 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -134,7 +134,7 @@
> ##
> # @stop:
> #
> -# Stop all guest VCPU execution.
> +# Stop all guest VCPU and VM execution.
> #
> # Since: 0.14
> #
> @@ -143,6 +143,9 @@
> # the guest remains paused once migration finishes, as if the -S
> # option was passed on the command line.
> #
> +# In the "suspended" state, it will completely stop the VM and
> +# cause a transition to the "paused" state. (Since 9.0)
> +#
> # Example:
> #
> # -> { "execute": "stop" }
> @@ -153,7 +156,7 @@
> ##
> # @cont:
> #
> -# Resume guest VCPU execution.
> +# Resume guest VCPU and VM execution.
> #
> # Since: 0.14
> #
> @@ -165,6 +168,9 @@
> # guest starts once migration finishes, removing the effect of the
> # -S command line option if it was passed.
> #
> +# If the VM was previously suspended, and not been reset or woken,
> +# this command will transition back to the "suspended" state. (Since 9.0)
> +#
> # Example:
> #
> # -> { "execute": "cont" }
> diff --git a/system/cpus.c b/system/cpus.c
> index 9f631ab..f162435 100644
> --- a/system/cpus.c
> +++ b/system/cpus.c
> @@ -277,11 +277,15 @@ bool vm_get_suspended(void)
> static int do_vm_stop(RunState state, bool send_stop)
> {
> int ret = 0;
> + RunState oldstate = runstate_get();
>
> - if (runstate_is_running()) {
> + if (runstate_is_live(oldstate)) {
> + vm_was_suspended = (oldstate == RUN_STATE_SUSPENDED);
> runstate_set(state);
> cpu_disable_ticks();
> - pause_all_vcpus();
> + if (oldstate == RUN_STATE_RUNNING) {
> + pause_all_vcpus();
> + }
> vm_state_notify(0, state);
> if (send_stop) {
> qapi_event_send_stop();
> @@ -694,11 +698,13 @@ int vm_stop(RunState state)
>
> /**
> * Prepare for (re)starting the VM.
> - * Returns -1 if the vCPUs are not to be restarted (e.g. if they are already
> - * running or in case of an error condition), 0 otherwise.
> + * Returns 0 if the vCPUs should be restarted, -1 on an error condition,
> + * and 1 otherwise.
> */
> int vm_prepare_start(bool step_pending)
> {
> + int ret = vm_was_suspended ? 1 : 0;
> + RunState state = vm_was_suspended ? RUN_STATE_SUSPENDED :
> RUN_STATE_RUNNING;
> RunState requested;
>
> qemu_vmstop_requested(&requested);
> @@ -729,9 +735,10 @@ int vm_prepare_start(bool step_pending)
> qapi_event_send_resume();
>
> cpu_enable_ticks();
> - runstate_set(RUN_STATE_RUNNING);
> - vm_state_notify(1, RUN_STATE_RUNNING);
> - return 0;
> + runstate_set(state);
> + vm_state_notify(1, state);
> + vm_was_suspended = false;
> + return ret;
> }
>
> void vm_start(void)
> @@ -745,7 +752,7 @@ void vm_start(void)
> current state is forgotten forever */
> int vm_stop_force_state(RunState state)
> {
> - if (runstate_is_running()) {
> + if (runstate_is_live(runstate_get())) {
> return vm_stop(state);
> } else {
> int ret;
> diff --git a/system/runstate.c b/system/runstate.c
> index ea9d6c2..e2fa204 100644
> --- a/system/runstate.c
> +++ b/system/runstate.c
> @@ -108,6 +108,7 @@ static const RunStateTransition
> runstate_transitions_def[] = {
> { RUN_STATE_PAUSED, RUN_STATE_POSTMIGRATE },
> { RUN_STATE_PAUSED, RUN_STATE_PRELAUNCH },
> { RUN_STATE_PAUSED, RUN_STATE_COLO},
> + { RUN_STATE_PAUSED, RUN_STATE_SUSPENDED},
>
> { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING },
> { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE },
> @@ -161,6 +162,7 @@ static const RunStateTransition
> runstate_transitions_def[] = {
> { RUN_STATE_SUSPENDED, RUN_STATE_FINISH_MIGRATE },
> { RUN_STATE_SUSPENDED, RUN_STATE_PRELAUNCH },
> { RUN_STATE_SUSPENDED, RUN_STATE_COLO},
> + { RUN_STATE_SUSPENDED, RUN_STATE_PAUSED},
>
> { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
> { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
> @@ -502,6 +504,7 @@ void qemu_system_reset(ShutdownCause reason)
> qapi_event_send_reset(shutdown_caused_by_guest(reason), reason);
> }
> cpu_synchronize_all_post_reset();
> + vm_set_suspended(false);
> }
>
> /*
