| Message ID | 1701380247-340457-1-git-send-email-steven.sistare@oracle.com |
|---|---|
| Headers | show |
| Series | fix migration of suspended runstate | expand |
Hi Peter and Fabiano, Any comments on these patches, before I send V7? They are not affected by the other changes we have discussed, except for renaming runstate_is_started to runstate_is_live. [PATCH V6 04/14] cpus: vm_resume [PATCH V6 06/14] migration: preserve suspended runstate [PATCH V6 07/14] migration: preserve suspended for snapshot [PATCH V6 08/14] migration: preserve suspended for bg_migration - Steve On 11/30/2023 4:37 PM, 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" > > Steve Sistare (14): > cpus: pass runstate to vm_prepare_start > cpus: vm_was_suspended > cpus: stop vm in suspended runstate > 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 > tests/qtest: bootfile per vm > tests/qtest: background migration with suspend > > backends/tpm/tpm_emulator.c | 2 +- > gdbstub/system.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 | 19 ++- > migration/global_state.c | 10 ++ > migration/migration-hmp-cmds.c | 8 +- > migration/migration.c | 15 +-- > migration/savevm.c | 23 ++-- > qapi/misc.json | 10 +- > system/cpus.c | 49 +++++-- > 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 | 240 +++++++++++++++++++++++++---------- > 21 files changed, 382 insertions(+), 139 deletions(-) >
Steven Sistare <steven.sistare@oracle.com> writes: > Hi Peter and Fabiano, > > Any comments on these patches, before I send V7? They are not affected by > the other changes we have discussed, except for renaming runstate_is_started > to runstate_is_live. > > [PATCH V6 04/14] cpus: vm_resume > [PATCH V6 06/14] migration: preserve suspended runstate > [PATCH V6 07/14] migration: preserve suspended for snapshot Nothing catches my eye for now. > [PATCH V6 08/14] migration: preserve suspended for bg_migration This has my r-b already.
On Tue, Dec 05, 2023 at 01:52:23PM -0500, Steven Sistare wrote: > Hi Peter and Fabiano, > > Any comments on these patches, before I send V7? They are not affected by > the other changes we have discussed, except for renaming runstate_is_started > to runstate_is_live. > > [PATCH V6 04/14] cpus: vm_resume > [PATCH V6 06/14] migration: preserve suspended runstate > [PATCH V6 07/14] migration: preserve suspended for snapshot > [PATCH V6 08/14] migration: preserve suspended for bg_migration One nitpick I put in the reply there, all look good to me in general. Side note: I'll be away for the rest of the week. Thanks,
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" Steve Sistare (14): cpus: pass runstate to vm_prepare_start cpus: vm_was_suspended cpus: stop vm in suspended runstate 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 tests/qtest: bootfile per vm tests/qtest: background migration with suspend backends/tpm/tpm_emulator.c | 2 +- gdbstub/system.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 | 19 ++- migration/global_state.c | 10 ++ migration/migration-hmp-cmds.c | 8 +- migration/migration.c | 15 +-- migration/savevm.c | 23 ++-- qapi/misc.json | 10 +- system/cpus.c | 49 +++++-- 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 | 240 +++++++++++++++++++++++++---------- 21 files changed, 382 insertions(+), 139 deletions(-)