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

Radim Krčmář
2016-11-04T15:25:27Z

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.

> 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?

(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?

Thanks.