Re: [QEMU PATCH] kvmclock: advance clock by time window between vm_stop and pre_save

Marcelo Tosatti
2016-11-07T19:42:44Z

On Mon, Nov 07, 2016 at 05:31:49PM +0300, Roman Kagan wrote:
> On Fri, Nov 04, 2016 at 06:16:06PM +0100, Radim Krčmář wrote:
> > 2016-11-04 16:57+0100, Paolo Bonzini:
> > > On 04/11/2016 16:48, Radim Krčmář wrote:
> > >> 2016-11-04 16:33+0100, Paolo Bonzini:
> > >>> On 04/11/2016 16:25, Radim Krčmář wrote:
> > >>>>>>
> > >>>>>> + if (s->advance_clock && s->clock + s->advance_clock > s->clock) {
> > >>>>>> + s->clock += s->advance_clock;
> > >>>>>> + s->advance_clock = 0;
> > >>>>>> + }
> > >>>> Can't the advance_clock added to the migrated KVMClockState instead of
> > >>>> passing it as another parameter?
> > >>>>
> > >>>> (It is sad that we can't just query KVMClockState in kvmclock_pre_save
> > >>>> because of the Linux bug.)
> > >>>
> > >>> What Linux bug? The one that makes us use kvmclock_current_nsec?
> > >>
> > >> No, the one that forced Marcelo to add the 10 minute limit to the
> > >> advance_clock. We wouldn't need this advance_clock hack if we could
> > >> just call KVM_GET_CLOCK like we did before 00f4d64ee76e ("kvmclock:
> > >> clock should count only if vm is running").
> > >
> > > There are two cases:
> > >
> > > - migrating a paused guest
> > >
> > > - pausing at the end of migration
> > >
> > > In the first case, kvmclock_vm_state_change's !running branch will see
> > > state == RUN_STATE_FINISH_MIGRATE && s->clock_valid. In the second
> > > case, it will see state == RUN_STATE_FINISH_MIGRATE && !s->clock_valid.
> >
> > I lift my case, marcelo's said that stopping the time is a feature ...
> > (*kittens die*)
>
> Sorry to chime in in the middle of the thread, but I wonder how happy
> the guests are with this behavior. Intuitively pausing or snapshotting
> feels like closing the lid of a laptop, so every time I see the guest
> waking up in the past after a pause I get confused. It may also be
> unexpected by Windows guests who never had this overflow problem but
> now, being tied up with kvmclock, have to stop the time while in pause,
> too.
>
> Roman.

Waking up should be using guest-set-time QGA API:

http://bugzilla.redhat.com/show_bug.cgi?id=1102411

Check "virsh domtime".