On 8/17/2023 2:23 PM, Peter Xu wrote:
> On Mon, Aug 14, 2023 at 11:54:26AM -0700, 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,
>> for a restored snapshot, the automatic wakeup fails. The runstate is
>> RUNNING, but the guest is not. See the commit messages for the details.
>
> Hi Steve,
>
> I drafted two small patches to show what I meant, on top of this series.
> Before applying these two, one needs to revert patch 1 in this series.
>
> After applied, it should also pass all three new suspend tests. We can
> continue the discussion here based on the patches.
Your 2 patches look good. I suggest we keep patch 1, and I squash patch 2
into the other patches.
There is one more fix needed: on the sending side, if the state is suspended,
then ticks must be disabled so the tick globals are updated before they are
written to vmstate. Otherwise, tick starts at 0 in the receiver when
cpu_enable_ticks is called.
-------------------------------------------
diff --git a/migration/migration.c b/migration/migration.c
index b004475..89d56a8 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -63,6 +63,7 @@
#include "sysemu/cpus.h"
#include "yank_functions.h"
#include "sysemu/qtest.h"
+#include "sysemu/cpu-timers.h"
#include "options.h"
#include "sysemu/dirtylimit.h"
@@ -2125,6 +2126,9 @@ static int postcopy_start(MigrationState *ms, Error **errp
trace_postcopy_start_set_run();
global_state_store();
+ if (runstate_check(RUN_STATE_SUSPENDED)) {
+ cpu_disable_ticks();
+ }
ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
if (ret < 0) {
goto fail;
@@ -2333,6 +2337,9 @@ static void migration_completion(MigrationState *s)
s->vm_old_state = runstate_get();
global_state_store();
+ if (runstate_check(RUN_STATE_SUSPENDED)) {
+ cpu_disable_ticks();
+ }
ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
trace_migration_completion_vm_stop(ret);
if (ret >= 0) {
diff --git a/migration/savevm.c b/migration/savevm.c
index 7b9c477..eff6976 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -58,6 +58,7 @@
#include "qemu/cutils.h"
#include "io/channel-buffer.h"
#include "io/channel-file.h"
+#include "sysemu/cpu-timers.h"
#include "sysemu/replay.h"
#include "sysemu/runstate.h"
#include "sysemu/sysemu.h"
@@ -2969,6 +2970,9 @@ bool save_snapshot(const char *name, bool overwrite, const
saved_vm_running = runstate_is_running();
global_state_store();
+ if (runstate_check(RUN_STATE_SUSPENDED)) {
+ cpu_disable_ticks();
+ }
vm_stop(RUN_STATE_SAVE_VM);
bdrv_drain_all_begin();
@@ -3037,6 +3041,8 @@ bool save_snapshot(const char *name, bool overwrite, const
if (saved_vm_running) {
vm_start();
+ } else if (runstate_check(RUN_STATE_SUSPENDED)) {
+ cpu_enable_ticks();
}
return ret == 0;
}
-------------------------------------------
- Steve
