On Wed, Jan 21, 2015 at 06:00:37PM +0100, Radim Krčmář wrote:
> 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?)
Yes.
> > > 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.
delta = pvclock_data.tsc_timestamp - RDTSC
Guest expects delta to be smaller than a given threshold. It does
not expect greater precision.
Size of delta does not affect precision.
> 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?)
One of the features of virtualization is to be able to run old
operating systems?
> 3) The patch made me ask more silly questions than it answered :)
> (Why can't other software depend on previous behavior?
Documentation/virtual/kvm/msr.txt:
whose data will be filled in by the hypervisor periodically.
Only one write, or registration, is needed for each VCPU. The interval
between updates of this structure is arbitrary and implementation-dependent.
The hypervisor may update this structure at any time it sees fit until
anything with bit0 == 0 is written to it.
> Why can't kvmclock without master clock still fail?
It can, given a loaded system.
> Why can't we improve the master clock?)
Out of context question.
> > > 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
All guests which depend on KVM_FEATURE_CLOCKSOURCE will timedrift.
> + 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.)
This phrase is awkward. Overlapping goals are negative,
then? (think of a large number of totally overlapping goals).
--
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
