| Message ID | 1596122076-341293-12-git-send-email-steven.sistare@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Live Update | expand |
* Steve Sistare (steven.sistare@oracle.com) wrote: > After cprload, the guest console misbehaves. You must type 8 characters > before any are echoed to the terminal. Qemu was not sending interrupts > to the guest because the QEMU_CLOCK_VIRTUAL timers_state.cpu_clock_offset > was bad. The offset is usually updated at cprsave time by the path > > save_cpr_snapshot() > vm_stop() > do_vm_stop() > if (runstate_is_running()) > cpu_disable_ticks(); > timers_state.cpu_clock_offset = cpu_get_clock_locked(); > > However, if the guest is in RUN_STATE_SUSPENDED, then cpu_disable_ticks is > not called. Further, the earlier transition to suspended in > qemu_system_suspend did not disable ticks. To fix, call cpu_disable_ticks > from save_cpr_snapshot. > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> Are you saying this is really a more generic bug with migrating when suspended and we should fix this anyway? Dave > --- > migration/savevm.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/migration/savevm.c b/migration/savevm.c > index f101039..00f493b 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -2729,6 +2729,11 @@ void save_cpr_snapshot(const char *file, const char *mode, Error **errp) > return; > } > > + /* Update timers_state before saving. Suspend did not so do. */ > + if (runstate_check(RUN_STATE_SUSPENDED)) { > + cpu_disable_ticks(); > + } > + > vm_stop(RUN_STATE_SAVE_VM); > > ret = qemu_savevm_state(f, op, errp); > -- > 1.8.3.1 >
On 9/11/2020 1:53 PM, Dr. David Alan Gilbert wrote: > * Steve Sistare (steven.sistare@oracle.com) wrote: >> After cprload, the guest console misbehaves. You must type 8 characters >> before any are echoed to the terminal. Qemu was not sending interrupts >> to the guest because the QEMU_CLOCK_VIRTUAL timers_state.cpu_clock_offset >> was bad. The offset is usually updated at cprsave time by the path >> >> save_cpr_snapshot() >> vm_stop() >> do_vm_stop() >> if (runstate_is_running()) >> cpu_disable_ticks(); >> timers_state.cpu_clock_offset = cpu_get_clock_locked(); >> >> However, if the guest is in RUN_STATE_SUSPENDED, then cpu_disable_ticks is >> not called. Further, the earlier transition to suspended in >> qemu_system_suspend did not disable ticks. To fix, call cpu_disable_ticks >> from save_cpr_snapshot. >> >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > > Are you saying this is really a more generic bug with migrating when > suspended and we should fix this anyway? Yes. Or when suspended and calling save_vmstate(), or qmp_xen_save_devices_state(). Each of those functions needs the same fix unless someone identifies a more centralized way in the state transition logic to disable ticks. - Steve >> --- >> migration/savevm.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/migration/savevm.c b/migration/savevm.c >> index f101039..00f493b 100644 >> --- a/migration/savevm.c >> +++ b/migration/savevm.c >> @@ -2729,6 +2729,11 @@ void save_cpr_snapshot(const char *file, const char *mode, Error **errp) >> return; >> } >> >> + /* Update timers_state before saving. Suspend did not so do. */ >> + if (runstate_check(RUN_STATE_SUSPENDED)) { >> + cpu_disable_ticks(); >> + } >> + >> vm_stop(RUN_STATE_SAVE_VM); >> >> ret = qemu_savevm_state(f, op, errp); >> -- >> 1.8.3.1 >>
* Steven Sistare (steven.sistare@oracle.com) wrote: > On 9/11/2020 1:53 PM, Dr. David Alan Gilbert wrote: > > * Steve Sistare (steven.sistare@oracle.com) wrote: > >> After cprload, the guest console misbehaves. You must type 8 characters > >> before any are echoed to the terminal. Qemu was not sending interrupts > >> to the guest because the QEMU_CLOCK_VIRTUAL timers_state.cpu_clock_offset > >> was bad. The offset is usually updated at cprsave time by the path > >> > >> save_cpr_snapshot() > >> vm_stop() > >> do_vm_stop() > >> if (runstate_is_running()) > >> cpu_disable_ticks(); > >> timers_state.cpu_clock_offset = cpu_get_clock_locked(); > >> > >> However, if the guest is in RUN_STATE_SUSPENDED, then cpu_disable_ticks is > >> not called. Further, the earlier transition to suspended in > >> qemu_system_suspend did not disable ticks. To fix, call cpu_disable_ticks > >> from save_cpr_snapshot. > >> > >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > > > > Are you saying this is really a more generic bug with migrating when > > suspended and we should fix this anyway? > > Yes. Or when suspended and calling save_vmstate(), or qmp_xen_save_devices_state(). > Each of those functions needs the same fix unless someone identifies a more > centralized way in the state transition logic to disable ticks. OK, in that case please split this out of the series and we can take a fix as normal; please cc in mtosatti@redhat.com . Dave > > - Steve > > >> --- > >> migration/savevm.c | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> > >> diff --git a/migration/savevm.c b/migration/savevm.c > >> index f101039..00f493b 100644 > >> --- a/migration/savevm.c > >> +++ b/migration/savevm.c > >> @@ -2729,6 +2729,11 @@ void save_cpr_snapshot(const char *file, const char *mode, Error **errp) > >> return; > >> } > >> > >> + /* Update timers_state before saving. Suspend did not so do. */ > >> + if (runstate_check(RUN_STATE_SUSPENDED)) { > >> + cpu_disable_ticks(); > >> + } > >> + > >> vm_stop(RUN_STATE_SAVE_VM); > >> > >> ret = qemu_savevm_state(f, op, errp); > >> -- > >> 1.8.3.1 > >> >
diff --git a/migration/savevm.c b/migration/savevm.c index f101039..00f493b 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -2729,6 +2729,11 @@ void save_cpr_snapshot(const char *file, const char *mode, Error **errp) return; } + /* Update timers_state before saving. Suspend did not so do. */ + if (runstate_check(RUN_STATE_SUSPENDED)) { + cpu_disable_ticks(); + } + vm_stop(RUN_STATE_SAVE_VM); ret = qemu_savevm_state(f, op, errp);
After cprload, the guest console misbehaves. You must type 8 characters before any are echoed to the terminal. Qemu was not sending interrupts to the guest because the QEMU_CLOCK_VIRTUAL timers_state.cpu_clock_offset was bad. The offset is usually updated at cprsave time by the path save_cpr_snapshot() vm_stop() do_vm_stop() if (runstate_is_running()) cpu_disable_ticks(); timers_state.cpu_clock_offset = cpu_get_clock_locked(); However, if the guest is in RUN_STATE_SUSPENDED, then cpu_disable_ticks is not called. Further, the earlier transition to suspended in qemu_system_suspend did not disable ticks. To fix, call cpu_disable_ticks from save_cpr_snapshot. Signed-off-by: Steve Sistare <steven.sistare@oracle.com> --- migration/savevm.c | 5 +++++ 1 file changed, 5 insertions(+)