On Mon, Nov 14, 2016 at 03:09:46PM +0100, Juan Quintela wrote:
> Marcelo Tosatti <mtosatti@xxxxxxxxxx> wrote:
> > Check for KVM_CAP_ADJUST_CLOCK capability KVM_CLOCK_TSC_STABLE, which
> > indicates that KVM_GET_CLOCK returns a value as seen by the guest at
> > that moment.
> >
> > For new machine types, use this value rather than reading
> > from guest memory.
> >
> > This reduces kvmclock difference on migration from 5s to 0.1s
> > (when max_downtime == 5s).
> >
> > Signed-off-by: Marcelo Tosatti <mtosatti@xxxxxxxxxx>
>
> Acked-by: Juan Quintela <quintela@xxxxxxxxxx>
>
> But, if you have to respin it ....
>
>
> > + /* whether machine supports reliable KVM_GET_CLOCK */
> > + bool mach_use_reliable_get_clock;
> > +
> > + /* whether source host supported reliable KVM_GET_CLOCK */
> > + bool src_use_reliable_get_clock;
>
> This two names are really long, but I don't have better suggesitons :-()
> > if (running) {
> > struct kvm_clock_data data = {};
> > - uint64_t time_at_migration = kvmclock_current_nsec(s);
> > + uint64_t time_at_migration = 0;
>
> This was not "time_at_migration", it was not already before, but just
> now it looks really weird. (as it was already faulty, this is why it is
> only a suggestion.)
>
> >
> > - s->clock_valid = false;
> > + /* 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);
> > + }
> > + /* 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
> > + */
>
> This comment is just weird. Simplifying
> /* If A and B do C */
> if (!A and || !B) {
> then D();
> }
>
> Doing the opposite comment?
>
> Migration code looks rigth.
Fixed those.
> Once said that, I continue hating clocks.
Me too, especially the biological one.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
