Re: [Qemu-arm] [PATCH 06/10] target-i386: print obsolete warnings if +-features are used

[Eduardo Habkost]

On Tue, Jun 07, 2016 at 03:26:58PM +0200, Igor Mammedov wrote:
> On Tue, 7 Jun 2016 15:00:04 +0200
> Paolo Bonzini <address@hidden> wrote:
> 
> > On 07/06/2016 14:54, Igor Mammedov wrote:
> > > On Tue, 7 Jun 2016 14:36:51 +0200
> > > Paolo Bonzini <address@hidden> wrote:
> > >   
> > >> On 07/06/2016 14:32, Igor Mammedov wrote:  
> > >>>>> Could you detect using +foo together with foo=off, and -foo together
> > >>>>> with foo=on?  Those are the really problematic cases, without them 
> > >>>>> +foo
> > >>>>> and -foo can become synonyms for =on and =off.    
> > >>> That's (legacy)current semantic of -cpu +-foo where it overrides any 
> > >>> foo=x,
> > >>> potentially it's possible to track foo=x locally in parser
> > >>> and then compare with +-foo both ways.
> > >>> But all we can do currently is to print warning about such use case.
> > >>>
> > >>> I think Eduardo's suggestion to just warn that +-foo is obsolete for now
> > >>> and drop support for it in several releases is sufficient(good) enough. 
> > >>>    
> > >>
> > >> kvm-unit-tests and libvirt both use it, especially because =on and =off
> > >> are relatively new I think?  It seems like it's really widespread.  
> > > 
> > > Yep, that's why it's not removed now.
> > > Looks like libvirt would be able to switch to foo=x syntax,
> > > I can take a look at kvm-unit-tests and make it use foo=x too.  
> > 
> > And all tutorials, and all scripts.  It's really too hard.
> > 
> > I'd really prefer to make an incompatible change straight in 2.7 for the
> > case of mixed foo=x and [+-]foo.
> I've tried to make a bit more extreme incompatible change starting from 2.7
> machine type in RFC (i.e. allow only foo=x syntax).

kvm-unit-tests would be another reason to not disable it
unconditionally on pc-2.7, as it uses the default machine-type.

> But Eduardo prefers to keep current +-foo, just marking it obsolete
> so that users would have time to adapt before support for legacy +-foo
> is dropped.
> 
> Anyways, we can introduce "mixed foo=x and [+-]foo" check on top of
> this series with warning and probably switch to fail later so not
> break existing users without giving them time to adapt.

We could make it fail in the case of mixing +foo and foo=x, but
we would need to be careful to not break existing valid cases
(like "vendor=AuthenticAMD,+foo", or "+foo,pmu=on". The code that
handles "foo=x" doesn't know if the property is a feature flag or
not. I'm not sure the extra code complexity is worth it.

If we plan to to be stuck with the "[+-]foo" code forever (I am
not completely against keeping it), why not just let people mix
it with the new format? I don't see the point of making the code
more convoluted just to prevent people from mixing both.

If we don't plan to keep the [+-]foo code forever, then let's
start printing a warning as soon as possible.

(Note that I am not talking about the weird "-foo,+foo" ordering
semantics. That's something else we need to warn about and
eventually obsolete.)

-- 
Eduardo