| Message ID | 1702481421-375368-1-git-send-email-steven.sistare@oracle.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | fix migration of suspended runstate | expand |
Hi Peter, all have RB's, with all i's dotted and t's crossed - steve On 12/13/2023 10:30 AM, Steve Sistare wrote: > Migration of a guest in the suspended runstate is broken. The incoming > migration code automatically tries to wake the guest, which is wrong; > the guest should end migration in the same runstate it started. Further, > after saving a snapshot in the suspended state and loading it, the vm_start > fails. The runstate is RUNNING, but the guest is not. > > See the commit messages for the details. > > Changes in V2: > * simplify "start on wakeup request" > * fix postcopy, snapshot, and background migration > * refactor fixes for each type of migration > * explicitly handled suspended events and runstate in tests > * add test for postcopy and background migration > > Changes in V3: > * rebase to tip > * fix hang in new function migrate_wait_for_dirty_mem > > Changes in V4: > * rebase to tip > * add patch for vm_prepare_start (thanks Peter) > * add patch to preserve cpu ticks > > Changes in V5: > * rebase to tip > * added patches to completely stop vm in suspended state: > cpus: refactor vm_stop > cpus: stop vm in suspended state > * added patch to partially resume vm in suspended state: > cpus: start vm in suspended state > * modified "preserve suspended ..." patches to use the above. > * deleted patch "preserve cpu ticks if suspended". stop ticks in > vm_stop_force_state instead. > * deleted patch "add runstate function". defined new helper function > migrate_new_runstate in "preserve suspended runstate" > * Added some RB's, but removed other RB's because the patches changed. > > Changes in V6: > * all vm_stop calls completely stop the suspended state > * refactored and updated the "cpus" patches > * simplified the "preserve suspended" patches > * added patch "bootfile per vm" > > Changes in V7: > * rebase to tip, add RB-s > * fix backwards compatibility for global_state.vm_was_suspended > * delete vm_prepare_start state argument, and rename patch > "pass runstate to vm_prepare_start" to > "check running not RUN_STATE_RUNNING" > * drop patches: > tests/qtest: bootfile per vm > tests/qtest: background migration with suspend > * rename runstate_is_started to runstate_is_live > * move wait_for_suspend in tests > > Changes in V8: > * rebase to tip > * add RB's > * add comment for runstate_is_live > * simplify global_state - the needed function, and its use of vm_was_suspended > > Steve Sistare (12): > cpus: vm_was_suspended > cpus: stop vm in suspended runstate > cpus: check running not RUN_STATE_RUNNING > cpus: vm_resume > migration: propagate suspended runstate > migration: preserve suspended runstate > migration: preserve suspended for snapshot > migration: preserve suspended for bg_migration > tests/qtest: migration events > tests/qtest: option to suspend during migration > tests/qtest: precopy migration with suspend > tests/qtest: postcopy migration with suspend > > backends/tpm/tpm_emulator.c | 2 +- > hw/usb/hcd-ehci.c | 2 +- > hw/usb/redirect.c | 2 +- > hw/xen/xen-hvm-common.c | 2 +- > include/migration/snapshot.h | 7 ++ > include/sysemu/runstate.h | 20 ++++ > migration/global_state.c | 47 +++++---- > migration/migration-hmp-cmds.c | 8 +- > migration/migration.c | 15 +-- > migration/savevm.c | 23 +++-- > qapi/misc.json | 10 +- > system/cpus.c | 47 +++++++-- > system/runstate.c | 9 ++ > system/vl.c | 2 + > tests/migration/i386/Makefile | 5 +- > tests/migration/i386/a-b-bootblock.S | 50 +++++++++- > tests/migration/i386/a-b-bootblock.h | 26 +++-- > tests/qtest/migration-helpers.c | 27 ++---- > tests/qtest/migration-helpers.h | 11 ++- > tests/qtest/migration-test.c | 181 +++++++++++++++++++++++++---------- > 20 files changed, 352 insertions(+), 144 deletions(-) >
On Wed, Dec 13, 2023 at 10:35:33AM -0500, Steven Sistare wrote:
> Hi Peter, all have RB's, with all i's dotted and t's crossed - steve
Yes this seems to be more migration related so maybe good candidate for a
pull from migration submodule.
But since this is still solving a generic issue, I'm copying a few more
people from get_maintainers.pl that this series touches, just in case
they'll have something to say before dev cycle starts.
Thanks,
On 12/18/2023 12:14 AM, Peter Xu wrote: > On Wed, Dec 13, 2023 at 10:35:33AM -0500, Steven Sistare wrote: >> Hi Peter, all have RB's, with all i's dotted and t's crossed - steve > > Yes this seems to be more migration related so maybe good candidate for a > pull from migration submodule. > > But since this is still solving a generic issue, I'm copying a few more > people from get_maintainers.pl that this series touches, just in case > they'll have something to say before dev cycle starts. The key aspects are summarized by the cover letter and the commit messages pasted below for the first 6 patches: https://lore.kernel.org/qemu-devel/1702481421-375368-1-git-send-email-steven.sistare@oracle.com --------------------------------------------------------------------------- [PATCH V8 00/12] fix migration of suspended runstate Migration of a guest in the suspended runstate is broken. The incoming migration code automatically tries to wake the guest, which is wrong; the guest should end migration in the same runstate it started. Further, after saving a snapshot in the suspended state and loading it, the vm_start fails. The runstate is RUNNING, but the guest is not. --------------------------------------------------------------------------- [PATCH V8 01/12] cpus: vm_was_suspended Add a state variable to remember if a vm previously transitioned into a suspended state. --------------------------------------------------------------------------- [PATCH V8 02/12] cpus: stop vm in suspended runstate 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 --------------------------------------------------------------------------- [PATCH V8 03/12] cpus: check running not RUN_STATE_RUNNING When a vm transitions from running to suspended, runstate notifiers are not called, so the notifiers still think the vm is running. Hence, when we call vm_start to restore the suspended state, we call vm_state_notify with running=1. However, some notifiers check for RUN_STATE_RUNNING. They must check the running boolean instead. No functional change. --------------------------------------------------------------------------- [PATCH V8 04/12] cpus: vm_resume Define the vm_resume helper, for use in subsequent patches. --------------------------------------------------------------------------- [PATCH V8 05/12] migration: propagate suspended runstate If the outgoing machine was previously suspended, propagate that to the incoming side via global_state, so a subsequent vm_start restores the suspended state. To maintain backward and forward compatibility, reclaim some space from the runstate member. --------------------------------------------------------------------------- [PATCH V8 06/12] migration: preserve suspended runstate 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. ---------------------------------------------------------------------------
On Mon, Dec 18, 2023 at 01:14:51PM +0800, Peter Xu wrote: > On Wed, Dec 13, 2023 at 10:35:33AM -0500, Steven Sistare wrote: > > Hi Peter, all have RB's, with all i's dotted and t's crossed - steve > > Yes this seems to be more migration related so maybe good candidate for a > pull from migration submodule. > > But since this is still solving a generic issue, I'm copying a few more > people from get_maintainers.pl that this series touches, just in case > they'll have something to say before dev cycle starts. I did a quick smoke test of migrating a Xen guest. It works fine for me. Thanks,
On 12/20/2023 9:52 AM, Anthony PERARD wrote: > On Mon, Dec 18, 2023 at 01:14:51PM +0800, Peter Xu wrote: >> On Wed, Dec 13, 2023 at 10:35:33AM -0500, Steven Sistare wrote: >>> Hi Peter, all have RB's, with all i's dotted and t's crossed - steve >> >> Yes this seems to be more migration related so maybe good candidate for a >> pull from migration submodule. >> >> But since this is still solving a generic issue, I'm copying a few more >> people from get_maintainers.pl that this series touches, just in case >> they'll have something to say before dev cycle starts. > > I did a quick smoke test of migrating a Xen guest. It works fine for me. Thanks very much, I appreciate it - steve
Migration of a guest in the suspended runstate is broken. The incoming migration code automatically tries to wake the guest, which is wrong; the guest should end migration in the same runstate it started. Further, after saving a snapshot in the suspended state and loading it, the vm_start fails. The runstate is RUNNING, but the guest is not. See the commit messages for the details. Changes in V2: * simplify "start on wakeup request" * fix postcopy, snapshot, and background migration * refactor fixes for each type of migration * explicitly handled suspended events and runstate in tests * add test for postcopy and background migration Changes in V3: * rebase to tip * fix hang in new function migrate_wait_for_dirty_mem Changes in V4: * rebase to tip * add patch for vm_prepare_start (thanks Peter) * add patch to preserve cpu ticks Changes in V5: * rebase to tip * added patches to completely stop vm in suspended state: cpus: refactor vm_stop cpus: stop vm in suspended state * added patch to partially resume vm in suspended state: cpus: start vm in suspended state * modified "preserve suspended ..." patches to use the above. * deleted patch "preserve cpu ticks if suspended". stop ticks in vm_stop_force_state instead. * deleted patch "add runstate function". defined new helper function migrate_new_runstate in "preserve suspended runstate" * Added some RB's, but removed other RB's because the patches changed. Changes in V6: * all vm_stop calls completely stop the suspended state * refactored and updated the "cpus" patches * simplified the "preserve suspended" patches * added patch "bootfile per vm" Changes in V7: * rebase to tip, add RB-s * fix backwards compatibility for global_state.vm_was_suspended * delete vm_prepare_start state argument, and rename patch "pass runstate to vm_prepare_start" to "check running not RUN_STATE_RUNNING" * drop patches: tests/qtest: bootfile per vm tests/qtest: background migration with suspend * rename runstate_is_started to runstate_is_live * move wait_for_suspend in tests Changes in V8: * rebase to tip * add RB's * add comment for runstate_is_live * simplify global_state - the needed function, and its use of vm_was_suspended Steve Sistare (12): cpus: vm_was_suspended cpus: stop vm in suspended runstate cpus: check running not RUN_STATE_RUNNING cpus: vm_resume migration: propagate suspended runstate migration: preserve suspended runstate migration: preserve suspended for snapshot migration: preserve suspended for bg_migration tests/qtest: migration events tests/qtest: option to suspend during migration tests/qtest: precopy migration with suspend tests/qtest: postcopy migration with suspend backends/tpm/tpm_emulator.c | 2 +- hw/usb/hcd-ehci.c | 2 +- hw/usb/redirect.c | 2 +- hw/xen/xen-hvm-common.c | 2 +- include/migration/snapshot.h | 7 ++ include/sysemu/runstate.h | 20 ++++ migration/global_state.c | 47 +++++---- migration/migration-hmp-cmds.c | 8 +- migration/migration.c | 15 +-- migration/savevm.c | 23 +++-- qapi/misc.json | 10 +- system/cpus.c | 47 +++++++-- system/runstate.c | 9 ++ system/vl.c | 2 + tests/migration/i386/Makefile | 5 +- tests/migration/i386/a-b-bootblock.S | 50 +++++++++- tests/migration/i386/a-b-bootblock.h | 26 +++-- tests/qtest/migration-helpers.c | 27 ++---- tests/qtest/migration-helpers.h | 11 ++- tests/qtest/migration-test.c | 181 +++++++++++++++++++++++++---------- 20 files changed, 352 insertions(+), 144 deletions(-)