qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH V7 02/12] cpus: stop vm in suspended runstate


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH V7 02/12] cpus: stop vm in suspended runstate
Date: Wed, 6 Dec 2023 22:09:32 +0100
User-agent: Mozilla Thunderbird

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 <peterx@redhat.com>
Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Reviewed-by: Peter Xu <peterx@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.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]