[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH V1 11/32] cpu: disable ticks when suspended
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [PATCH V1 11/32] cpu: disable ticks when suspended |
Date: |
Fri, 25 Sep 2020 10:03:40 +0100 |
User-agent: |
Mutt/1.14.6 (2020-07-11) |
* 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
> >>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK