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

Marcelo Tosatti
2016-11-04T16:25:26Z

On Fri, Nov 04, 2016 at 04:25:23PM +0100, Radim Krčmář wrote:
> 2016-11-04 07:43-0200, Marcelo Tosatti:
> > This patch, relative to pre-copy migration codepath,
> > measures the time between vm_stop() and pre_save(),
> > which includes copying the remaining RAM to destination,
> > and advances the clock by that amount.
> >
> > In a VM with 5 seconds downtime, this reduces the guest
> > clock difference on destination from 5s to 0.2s.
> >
> > Please do not apply this yet as some codepaths still need
> > checking, submitting early for comments.
>
> The time computation looks ok.
>
> We could make it slightly more precise by returning the CLOCK_MONOTONIC
> at which KVM_GET_CLOCK is read with the IOCTL, but we don't account the
> migration time anyway, so that precision would be wasted.

Yes, can be enhanced later... That should be about 1us? (return to
userspace).

> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> >
> > diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
> > @@ -100,6 +106,11 @@ static void kvmclock_vm_state_change(void *opaque, int running,
> > s->clock = time_at_migration;
> > }
> >
> > + 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?

Don't want to be fragile to ->ns being overwritten by pre_save executing
on the destination and overwriting the value passed in from
migration...

> (It is sad that we can't just query KVMClockState in kvmclock_pre_save
> because of the Linux bug.)
>
> > @@ -135,6 +146,18 @@ static void kvmclock_vm_state_change(void *opaque, int running,
> > abort();
> > }
> > s->clock = data.clock;
> > + /*
> > + * Transition from VM-running to VM-stopped via migration?
> > + * Record when the VM was stopped.
> > + */
> > +
> > + if (state == RUN_STATE_FINISH_MIGRATE &&
> > + !migration_in_postcopy(migrate_get_current())) {
> > + clock_gettime(CLOCK_MONOTONIC, &s->t_aftervmstop);
>
> How big (more like small) was the clock delta between here and
> kvmclock_pre_save with postcopy?

Haven't checked, but the clock delta is now 0.2s.
So i assume postcopy (stopping the VM, saving the device states)
should be about 0.2s.

I'll run and let you know.