Re: [PATCH v1 0/5] target/arm/kvm: Provide an option to adjust virtual time

[Andrew Jones]

On Tue, Dec 10, 2019 at 04:47:49PM +0100, Andrea Bolognani wrote:
> On Tue, 2019-12-10 at 15:33 +0100, Andrew Jones wrote:
> > On Tue, Dec 10, 2019 at 03:21:02PM +0100, Andrea Bolognani wrote:
> > > I agree with everything except the naming: why would
> > > 
> > >   kvm-no-adjvtime=off  vtime is adjusted      (default)
> > >   kvm-no-adjvtime=on   vtime is not adjusted
> > > 
> > > be better than
> > > 
> > >   kvm-adjvtime=on   vtime is adjusted      (default)
> > >   kvm-adjvtime=off  vtime is not adjusted
> > > 
> > > ? Both offer the exact same amount of flexibility, but the latter has
> > > the advantage of not introducing any unwieldy double negatives.
> > 
> > A default of 'off' == 0 means not setting anything at all. There's
> > already precedent for 'kvm-no*' prefixed cpu features,
> > 
> > kvm-no-smi-migration
> > kvm-nopiodelay
> 
> Sorry, I'm not sure I understand. Do you mean that the array where
> you store CPU features is 0-inizialized, so it's more convenient to
> have off (0) as the default because it means you don't have to touch
> it beforehand? Or something different?
>

Right. The CPU feature flag (a boolean member of the CPU state) will
be zero by default because C. It's not a big deal, though, because the
property default can easily be set to true while it's added to a cpu
type.

I don't have a strong enough opinion about kvm-adjvtime vs.
kvm-no-adjvtime to insist one way or another. I agree double inversions
are easier to mess up, but I also like the way the '-no-' better
communicates that the default is [probably] 'yes'.

All interested parties, please vote. I'll be sending v2 soon and I can
call this thing anything the majority (or the dominate minority) prefer.

Thanks,
drew