2015-01-21 12:16-0200, Marcelo Tosatti:
> On Wed, Jan 21, 2015 at 03:09:27PM +0100, Radim Krčmář wrote:
> > 2015-01-20 15:54-0200, Marcelo Tosatti:
> > > SuSE's 2.6.16 kernel fails to boot if the delta between tsc_timestamp
> > > and rdtsc is larger than a given threshold:
> > [...]
> > > Disable masterclock support (which increases said delta) in case the
> > > boot vcpu does not use MSR_KVM_SYSTEM_TIME_NEW.
> > 
> > Why do we care about 2.6.16 bugs in upstream KVM?
> 
> Because people do use 2.6.16 guests.

(Those people probably won't use 3.19+ host ...
 Is this patch intended for stable?)

> > The code to benefit tradeoff of this patch seems bad to me ...
> 
> Can you state the tradeoff and then explain why it is bad ?

Additional code needs time to understand and is a source of bugs,
yet we still include it because we want to achieve something.

I meant the tradeoff between perceived value of "something" and
acceptability of the code.  (Ideally, computer programs would be a
shorter version of "Do what I want.\nEOF".)

There are three main points that made think it is bad,
1) The bug happens because a guest expects greater precision.
   I consider that as a guest problem.  kvmclock never guaranteed
   anything, so unmet expectations should be a recoverable error.

2) With time, the probability that 2.6.16 is used is getting lower,
   while people looking at KVM's code appear.
   - At what point are we going to drop 2.6.16 support?
     (We shouldn't let mistakes drag us down forever ...
      Or are we dooming KVM on purpose?)

3) The patch made me ask more silly questions than it answered :)
   (Why can't other software depend on previous behavior?
    Why can't kvmclock without master clock still fail?
    Why can't we improve the master clock?)

> > MSR_KVM_SYSTEM_TIME is deprecated -- we could remove it now, with
> > MSR_KVM_WALL_CLOCK (after hiding KVM_FEATURE_CLOCKSOURCE) if we want
> > to support old guests.
> 
> What is the benefit of removing support for MSR_KVM_SYSTEM_TIME ?

The maintainability of the code increases.  It would look as if we never
made the mistake with MSR_KVM_SYSTEM_TIME & MSR_KVM_WALL_CLOCK.
(I like when old code looks as if we wrote it from scratch.)

After comparing the (imperfectly evaluated) benefit of both variants,
 original patch:
   + 2.6.16 SUSE guests work
   - MSR_KVM_SYSTEM_TIME guests don't use master clock
   - KVM code is worse
 removal of KVM_FEATURE_CLOCKSOURCE:
   + 2.6.16 SUSE guests likely work
   + KVM code is better
   - MSR_KVM_SYSTEM_TIME guests use even worse clocksource

As KVM_FEATURE_CLOCKSOURCE2 was introduced in 2010, I found the removal
better even without waiting for the last MSR_KVM_SYSTEM_TIME guest to
perish.

> Supporting old guests is important.

It comes at a price.
(Mutually exclusive goals are important as well.)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to