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

[Roman Kagan <rkagan@virtuozzo.com>]

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.