qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: peterx@redhat.com
To: qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>
Cc: Fabiano Rosas <farosas@suse.de>,
	Steve Sistare <steven.sistare@oracle.com>,
	Juan Quintela <quintela@trasno.org>,
	peterx@redhat.com,
	Leonardo Bras Soares Passos <lsoaresp@redhat.com>,
	Avihai Horon <avihaih@nvidia.com>
Subject: [PULL 04/26] cpus: stop vm in suspended runstate
Date: Thu,  4 Jan 2024 12:31:49 +0800	[thread overview]
Message-ID: <20240104043213.431566-5-peterx@redhat.com> (raw)
In-Reply-To: <20240104043213.431566-1-peterx@redhat.com>

From: Steve Sistare <steven.sistare@oracle.com>

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:

  old behavior:
    RUN_STATE_SUSPENDED --> stop --> RUN_STATE_SUSPENDED

  new behavior:
    RUN_STATE_SUSPENDED --> stop --> RUN_STATE_PAUSED
    RUN_STATE_PAUSED    --> cont --> RUN_STATE_SUSPENDED

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>
Link: https://lore.kernel.org/r/1704312341-66640-3-git-send-email-steven.sistare@oracle.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 qapi/misc.json            | 11 +++++++++--
 qapi/run-state.json       |  6 +++---
 include/sysemu/runstate.h |  9 +++++++++
 system/cpus.c             | 23 +++++++++++++++--------
 system/runstate.c         |  3 +++
 5 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/qapi/misc.json b/qapi/misc.json
index cda2effa81..3622d98d01 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -134,7 +134,7 @@
 ##
 # @stop:
 #
-# Stop all guest VCPU execution.
+# Stop guest 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 VM execution.
 #
 # Since: 0.14
 #
@@ -165,6 +168,10 @@
 #     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/qapi/run-state.json b/qapi/run-state.json
index f216ba54ec..ca05502e0a 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -102,7 +102,7 @@
 ##
 # @StatusInfo:
 #
-# Information about VCPU run state
+# Information about VM run state
 #
 # @running: true if all VCPUs are runnable, false if not runnable
 #
@@ -130,9 +130,9 @@
 ##
 # @query-status:
 #
-# Query the run status of all VCPUs
+# Query the run status of the VM
 #
-# Returns: @StatusInfo reflecting all VCPUs
+# Returns: @StatusInfo reflecting the VM
 #
 # Since: 0.14
 #
diff --git a/include/sysemu/runstate.h b/include/sysemu/runstate.h
index 88a67e22b0..618eb491af 100644
--- a/include/sysemu/runstate.h
+++ b/include/sysemu/runstate.h
@@ -40,6 +40,15 @@ static inline bool shutdown_caused_by_guest(ShutdownCause cause)
     return cause >= SHUTDOWN_CAUSE_GUEST_SHUTDOWN;
 }
 
+/*
+ * In a "live" state, the vcpu clock is ticking, and the runstate notifiers
+ * think we are running.
+ */
+static inline bool runstate_is_live(RunState state)
+{
+    return state == RUN_STATE_RUNNING || state == RUN_STATE_SUSPENDED;
+}
+
 void vm_start(void);
 
 /**
diff --git a/system/cpus.c b/system/cpus.c
index 9f631ab734..f162435dd4 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 ea9d6c2a32..e2fa2040cb 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);
 }
 
 /*
-- 
2.41.0



  parent reply	other threads:[~2024-01-04  4:33 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-04  4:31 [PULL 00/26] Migration 20240104 patches peterx
2024-01-04  4:31 ` [PULL 01/26] MAINTAINERS: Leaving Migration peterx
2024-01-04  4:31 ` [PULL 02/26] MAINTAINERS: Remove myself as reviewer from Live Migration peterx
2024-01-04  4:31 ` [PULL 03/26] cpus: vm_was_suspended peterx
2024-01-04  4:31 ` peterx [this message]
2024-01-04  4:31 ` [PULL 05/26] cpus: check running not RUN_STATE_RUNNING peterx
2024-01-04  4:31 ` [PULL 06/26] cpus: vm_resume peterx
2024-01-04  4:31 ` [PULL 07/26] migration: propagate suspended runstate peterx
2024-01-04  4:31 ` [PULL 08/26] migration: preserve " peterx
2024-01-04  4:31 ` [PULL 09/26] migration: preserve suspended for snapshot peterx
2024-01-04  4:31 ` [PULL 10/26] migration: preserve suspended for bg_migration peterx
2024-01-04  4:31 ` [PULL 11/26] tests/qtest: migration events peterx
2024-01-04  4:31 ` [PULL 12/26] tests/qtest: option to suspend during migration peterx
2024-01-04  4:31 ` [PULL 13/26] tests/qtest: precopy migration with suspend peterx
2024-01-04  4:31 ` [PULL 14/26] tests/qtest: postcopy " peterx
2024-01-04  4:32 ` [PULL 15/26] migration: Remove migrate_max_downtime() declaration peterx
2024-01-04  4:32 ` [PULL 16/26] migration: Remove nulling of hostname in migrate_init() peterx
2024-01-04  4:32 ` [PULL 17/26] migration: Refactor migration_incoming_setup() peterx
2024-01-04  4:32 ` [PULL 18/26] migration: Remove errp parameter in migration_fd_process_incoming() peterx
2024-01-04  4:32 ` [PULL 19/26] migration/multifd: Fix error message in multifd_recv_initial_packet() peterx
2024-01-04  4:32 ` [PULL 20/26] migration/multifd: Simplify multifd_channel_connect() if else statement peterx
2024-01-04  4:32 ` [PULL 21/26] migration/multifd: Fix leaking of Error in TLS error flow peterx
2024-01-04  4:32 ` [PULL 22/26] migration/multifd: Remove error_setg() in migration_ioc_process_incoming() peterx
2024-01-04  4:32 ` [PULL 23/26] migration: Fix migration_channel_read_peek() error path peterx
2024-01-04  4:32 ` [PULL 24/26] migration: Remove unnecessary usage of local Error peterx
2024-01-04  4:32 ` [PULL 25/26] migration/multifd: " peterx
2024-01-04  4:32 ` [PULL 26/26] migration: fix coverity migrate_mode finding peterx
2024-01-05 16:08 ` [PULL 00/26] Migration 20240104 patches Peter Maydell
2024-01-07 12:33   ` Peter Xu
2024-01-07 12:41     ` Stefan Hajnoczi
2024-01-07 15:23       ` Peter Maydell
2024-01-07 16:28         ` Stefan Hajnoczi
2024-01-08  2:10           ` Peter Xu
2024-01-08  2:34             ` Peter Xu
2024-01-08 15:22               ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240104043213.431566-5-peterx@redhat.com \
    --to=peterx@redhat.com \
    --cc=avihaih@nvidia.com \
    --cc=farosas@suse.de \
    --cc=lsoaresp@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@trasno.org \
    --cc=stefanha@redhat.com \
    --cc=steven.sistare@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).