Re: [PATCH V6 06/14] migration: preserve suspended runstate

2023-12-05 Thread Peter Xu
On Thu, Nov 30, 2023 at 01:37:19PM -0800, Steve Sistare wrote:
> A guest that is migrated in the suspended state automaticaly wakes and
> continues execution.  This is wrong; the guest should end migration in
> the same state it started.  The root cause is that the outgoing migration
> code automatically wakes the guest, then saves the RUNNING runstate in
> global_state_store(), hence the incoming migration code thinks the guest is
> running and continues the guest if autostart is true.
> 
> On the outgoing side, delete the call to qemu_system_wakeup_request().
> Now that vm_stop completely stops a vm in the suspended state (from the
> preceding patches), the existing call to vm_stop_force_state is sufficient
> to correctly migrate all vmstate.
> 
> On the incoming side, call vm_start if the pre-migration state was running
> or suspended.  For the latter, vm_start correctly restores the suspended
> state, and a future system_wakeup monitor request will cause the vm to
> resume running.
> 
> Signed-off-by: Steve Sistare 

Reviewed-by: Peter Xu 

-- 
Peter Xu




[PATCH V6 06/14] migration: preserve suspended runstate

2023-11-30 Thread Steve Sistare
A guest that is migrated in the suspended state automaticaly wakes and
continues execution.  This is wrong; the guest should end migration in
the same state it started.  The root cause is that the outgoing migration
code automatically wakes the guest, then saves the RUNNING runstate in
global_state_store(), hence the incoming migration code thinks the guest is
running and continues the guest if autostart is true.

On the outgoing side, delete the call to qemu_system_wakeup_request().
Now that vm_stop completely stops a vm in the suspended state (from the
preceding patches), the existing call to vm_stop_force_state is sufficient
to correctly migrate all vmstate.

On the incoming side, call vm_start if the pre-migration state was running
or suspended.  For the latter, vm_start correctly restores the suspended
state, and a future system_wakeup monitor request will cause the vm to
resume running.

Signed-off-by: Steve Sistare 
---
 migration/migration.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 28a34c9..d1d94c4 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -603,7 +603,7 @@ static void process_incoming_migration_bh(void *opaque)
  */
 if (!migrate_late_block_activate() ||
  (autostart && (!global_state_received() ||
-global_state_get_runstate() == RUN_STATE_RUNNING))) {
+runstate_is_started(global_state_get_runstate() {
 /* Make sure all file formats throw away their mutable metadata.
  * If we get an error here, just don't restart the VM yet. */
 bdrv_activate_all(_err);
@@ -627,7 +627,7 @@ static void process_incoming_migration_bh(void *opaque)
 dirty_bitmap_mig_before_vm_start();
 
 if (!global_state_received() ||
-global_state_get_runstate() == RUN_STATE_RUNNING) {
+runstate_is_started(global_state_get_runstate())) {
 if (autostart) {
 vm_start();
 } else {
@@ -2415,7 +2415,6 @@ static int postcopy_start(MigrationState *ms, Error 
**errp)
 
 migration_downtime_start(ms);
 
-qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, NULL);
 global_state_store();
 ret = migration_stop_vm(RUN_STATE_FINISH_MIGRATE);
 if (ret < 0) {
@@ -2614,7 +2613,6 @@ static int migration_completion_precopy(MigrationState *s,
 
 qemu_mutex_lock_iothread();
 migration_downtime_start(s);
-qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, NULL);
 
 s->vm_old_state = runstate_get();
 global_state_store();
@@ -3135,7 +3133,7 @@ static void migration_iteration_finish(MigrationState *s)
 case MIGRATION_STATUS_FAILED:
 case MIGRATION_STATUS_CANCELLED:
 case MIGRATION_STATUS_CANCELLING:
-if (s->vm_old_state == RUN_STATE_RUNNING) {
+if (runstate_is_started(s->vm_old_state)) {
 if (!runstate_check(RUN_STATE_SHUTDOWN)) {
 vm_start();
 }
-- 
1.8.3.1