On 6/12/23 21:48, Philippe Mathieu-Daudé wrote:
On 6/12/23 20:19, Steven Sistare wrote:
On 12/6/2023 1:45 PM, Philippe Mathieu-Daudé wrote:
Hi Steve,
On 6/12/23 18:23, 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;
+}
Not being familiar with (live) migration, from a generic vCPU PoV
I don't get what "runstate_is_live" means. Can we add a comment
explaining what this helper is for?
Sure. "live" means the cpu clock is ticking, and the runstate
notifiers think
we are running. It is everything that is enabled in vm_start except
for starting
the vcpus.
What about runstate_is_vcpu_clock_ticking() then?
Since this is a migration particular case, maybe we can be verbose
in vm_resume() and keep runstate_is_live() -- eventually undocumented
-- in migration/migration.c.
runstate_is_live is about cpu and vm state, not migration state (the
"live" is not
live migration), and is used in multiple places in cpus code and
elsewhere, so I would
like to keep it in runstate.h. It has a specific meaning, and it is
useful to search
for it to see who handles "liveness", and distinguish it from code
that checks the
running and suspended states for other reasons.
OK then, no objection, but please add a comment describing.
Thanks,
Phil.
