| Message ID | 1693333086-392798-6-git-send-email-steven.sistare@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | fix migration of suspended runstate | expand |
On Tue, Aug 29, 2023 at 11:18:00AM -0700, Steve Sistare wrote: > Do not wake a suspended guest during bg_migration. > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > --- > migration/migration.c | 12 +++++------- > softmmu/runstate.c | 1 + > 2 files changed, 6 insertions(+), 7 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index a9ecb66..303d5a6 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -3064,7 +3064,9 @@ static void bg_migration_vm_start_bh(void *opaque) > qemu_bh_delete(s->vm_start_bh); > s->vm_start_bh = NULL; > > - vm_start(); > + if (!runstate_check(RUN_STATE_SUSPENDED)) { > + vm_start(); > + } > s->downtime = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - s->downtime_start; > } > > @@ -3134,16 +3136,12 @@ static void *bg_migration_thread(void *opaque) > > qemu_mutex_lock_iothread(); > > - /* > - * If VM is currently in suspended state, then, to make a valid runstate > - * transition in vm_stop_force_state() we need to wakeup it up. > - */ > - qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, NULL); > s->vm_old_state = runstate_get(); > > global_state_store(); > /* Forcibly stop VM before saving state of vCPUs and devices */ > - if (vm_stop_force_state(RUN_STATE_PAUSED)) { > + if (!runstate_check(RUN_STATE_SUSPENDED) && > + vm_stop_force_state(RUN_STATE_PAUSED)) { IIUC we need the vm_stop even for SUSPENDED? I think we need to make sure all backends are stopped before we start wr-protect the guest pages, so when wr-protect happens the guest pages should be in a consistent state. I'd think it proper to juse reuse the helper you introduced in the previous patches, but maybe you have a reason to not do so (I didn't see it mentioned anywhere, though)? > goto fail; > } > /* > diff --git a/softmmu/runstate.c b/softmmu/runstate.c > index 21d7407..4417527 100644 > --- a/softmmu/runstate.c > +++ b/softmmu/runstate.c > @@ -163,6 +163,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 }, > -- > 1.8.3.1 >
On 8/30/2023 12:35 PM, Peter Xu wrote: > On Tue, Aug 29, 2023 at 11:18:00AM -0700, Steve Sistare wrote: >> Do not wake a suspended guest during bg_migration. >> >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> >> --- >> migration/migration.c | 12 +++++------- >> softmmu/runstate.c | 1 + >> 2 files changed, 6 insertions(+), 7 deletions(-) >> >> diff --git a/migration/migration.c b/migration/migration.c >> index a9ecb66..303d5a6 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -3064,7 +3064,9 @@ static void bg_migration_vm_start_bh(void *opaque) >> qemu_bh_delete(s->vm_start_bh); >> s->vm_start_bh = NULL; >> >> - vm_start(); >> + if (!runstate_check(RUN_STATE_SUSPENDED)) { >> + vm_start(); >> + } >> s->downtime = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - s->downtime_start; >> } >> >> @@ -3134,16 +3136,12 @@ static void *bg_migration_thread(void *opaque) >> >> qemu_mutex_lock_iothread(); >> >> - /* >> - * If VM is currently in suspended state, then, to make a valid runstate >> - * transition in vm_stop_force_state() we need to wakeup it up. >> - */ >> - qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, NULL); >> s->vm_old_state = runstate_get(); >> >> global_state_store(); >> /* Forcibly stop VM before saving state of vCPUs and devices */ >> - if (vm_stop_force_state(RUN_STATE_PAUSED)) { >> + if (!runstate_check(RUN_STATE_SUSPENDED) && >> + vm_stop_force_state(RUN_STATE_PAUSED)) { > > IIUC we need the vm_stop even for SUSPENDED? I think we need to make sure > all backends are stopped before we start wr-protect the guest pages, so > when wr-protect happens the guest pages should be in a consistent state. Agreed, I stop everything for suspended in V5. > I'd think it proper to juse reuse the helper you introduced in the previous > patches, but maybe you have a reason to not do so (I didn't see it > mentioned anywhere, though)? This code is outgoing. The helper applies to the incoming side. - Steve >> goto fail; >> } >> /* >> diff --git a/softmmu/runstate.c b/softmmu/runstate.c >> index 21d7407..4417527 100644 >> --- a/softmmu/runstate.c >> +++ b/softmmu/runstate.c >> @@ -163,6 +163,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 }, >> -- >> 1.8.3.1 >> >
diff --git a/migration/migration.c b/migration/migration.c index a9ecb66..303d5a6 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -3064,7 +3064,9 @@ static void bg_migration_vm_start_bh(void *opaque) qemu_bh_delete(s->vm_start_bh); s->vm_start_bh = NULL; - vm_start(); + if (!runstate_check(RUN_STATE_SUSPENDED)) { + vm_start(); + } s->downtime = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - s->downtime_start; } @@ -3134,16 +3136,12 @@ static void *bg_migration_thread(void *opaque) qemu_mutex_lock_iothread(); - /* - * If VM is currently in suspended state, then, to make a valid runstate - * transition in vm_stop_force_state() we need to wakeup it up. - */ - qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, NULL); s->vm_old_state = runstate_get(); global_state_store(); /* Forcibly stop VM before saving state of vCPUs and devices */ - if (vm_stop_force_state(RUN_STATE_PAUSED)) { + if (!runstate_check(RUN_STATE_SUSPENDED) && + vm_stop_force_state(RUN_STATE_PAUSED)) { goto fail; } /* diff --git a/softmmu/runstate.c b/softmmu/runstate.c index 21d7407..4417527 100644 --- a/softmmu/runstate.c +++ b/softmmu/runstate.c @@ -163,6 +163,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 },
Do not wake a suspended guest during bg_migration. Signed-off-by: Steve Sistare <steven.sistare@oracle.com> --- migration/migration.c | 12 +++++------- softmmu/runstate.c | 1 + 2 files changed, 6 insertions(+), 7 deletions(-)