Re: [Qemu-devel] [qemu patch 2/2] kvmclock: reduce kvmclock difference on migration

[Paolo Bonzini]

On 14/11/2016 13:36, Marcelo Tosatti wrote:
> +        /* local (running VM) restore */
> +        if (s->clock_valid) {
> +            /*
> +             * if host does not support reliable KVM_GET_CLOCK,
> +             * read kvmclock value from memory
> +             */
> +            if (!kvm_has_adjust_clock_stable()) {
> +                time_at_migration = kvmclock_current_nsec(s);

Just assign to s->clock here...


> +            }
> +        /* migration/savevm/init restore */
> +        } else {
> +            /*
> +             * use s->clock in case machine uses reliable
> +             * get clock and host where vm was executing
> +             * supported reliable get clock
> +             */
> +            if (!s->mach_use_reliable_get_clock ||
> +                !s->src_use_reliable_get_clock) {
> +                time_at_migration = kvmclock_current_nsec(s);

... and here, so that time_at_migration is not needed anymore.

Also here it's enough to look at s->src_user_reliable_get_clock, because
if s->mach_use_reliable_get_clock is false,
s->src_use_reliable_get_clock will be false as well.

> +            }
> +        }
>  
> -        /* We can't rely on the migrated clock value, just discard it */
> +        /* We can't rely on the saved clock value, just discard it */
>          if (time_at_migration) {
>              s->clock = time_at_migration;

[...]

> 
> +static bool kvmclock_src_use_reliable_get_clock(void *opaque)
> +{
> +    KVMClockState *s = opaque;
> +
> +    /*
> +     * On machine types that support reliable KVM_GET_CLOCK,
> +     * if host kernel does provide reliable KVM_GET_CLOCK,
> +     * set src_use_reliable_get_clock=true so that destination
> +     * avoids reading kvmclock from memory.
> +     */
> +    if (s->mach_use_reliable_get_clock && kvm_has_adjust_clock_stable()) {
> +        s->src_use_reliable_get_clock = true;
> +    }
> +
> +    return s->src_use_reliable_get_clock;
> +}

Here you can just return s->mach_use_reliable_get_clock.  To set
s->src_use_reliable_get_clock, after issuing KVM_GET_CLOCK you can look
at the KVM_CLOCK_TSC_STABLE bit in the kvm_clock struct's flags.

You don't actually need kvm_has_adjust_clock_stable(), but please place
the KVM_GET_CLOCK code for kvmclock_pre_save and
kvmclock_vm_state_change in a common function.

Also, just another small nit: please make your scripts use the "-p"
option on diff. :)

Thanks,

Paolo