Re: [Qemu-devel] [PULL 10/10] target-i386: Print obsolete warnings if +-features are used

[Paolo Bonzini]

----- Original Message -----
> From: "Eduardo Habkost" <address@hidden>
> To: "Paolo Bonzini" <address@hidden>
> Cc: "Peter Maydell" <address@hidden>, "Andreas Färber" <address@hidden>, 
> address@hidden, "Richard
> Henderson" <address@hidden>, "Igor Mammedov" <address@hidden>
> Sent: Tuesday, June 14, 2016 11:31:03 PM
> Subject: Re: [PULL 10/10] target-i386: Print obsolete warnings if +-features 
> are used
> 
> On Tue, Jun 14, 2016 at 05:16:40PM -0400, Paolo Bonzini wrote:
> > ----- Original Message -----
> > > From: "Eduardo Habkost" <address@hidden>
> > > To: "Peter Maydell" <address@hidden>
> > > Cc: "Andreas Färber" <address@hidden>, address@hidden, "Richard
> > > Henderson" <address@hidden>, "Paolo
> > > Bonzini" <address@hidden>, "Igor Mammedov" <address@hidden>
> > > Sent: Tuesday, June 14, 2016 10:59:08 PM
> > > Subject: [PULL 10/10] target-i386: Print obsolete warnings if +-features
> > > are used
> > > 
> > > From: Igor Mammedov <address@hidden>
> > > 
> > > Signed-off-by: Igor Mammedov <address@hidden>
> > > [ehabkost: Changed to use error_report()]
> > > Signed-off-by: Eduardo Habkost <address@hidden>
> > > ---
> > >  target-i386/cpu.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > > index 3665fec..baa3783 100644
> > > --- a/target-i386/cpu.c
> > > +++ b/target-i386/cpu.c
> > > @@ -1980,9 +1980,15 @@ static void x86_cpu_parse_featurestr(CPUState *cs,
> > > char *features,
> > >          /* Compatibility syntax: */
> > >          if (featurestr[0] == '+') {
> > >              add_flagname_to_bitmaps(featurestr + 1, plus_features,
> > >              &local_err);
> > > +            error_report(
> > > +                "'+%s' is obsolete and will be removed in future, use
> > > '%s=on'",
> > > +                featurestr + 1, featurestr + 1);
> > >              continue;
> > >          } else if (featurestr[0] == '-') {
> > >              add_flagname_to_bitmaps(featurestr + 1, minus_features,
> > >              &local_err);
> > > +            error_report(
> > > +                "'-%s' is obsolete and will be removed in future, use
> > > '%s=off'",
> > > +                featurestr + 1, featurestr + 1);
> > >              continue;
> > >          }
> > 
> > I still disagree with this change.
> 
> I've just removed the patch from the x86-pull-request tag, while
> we sort this out.
> 
> Do you suggest supporting the "[+-]feature" syntax forever?

I suggest supporting it, but removing the awful interaction with 
"feature=on/off"
as soon as possible.  This shouldn't block this pull request, of course.

I just believe it's not practical to remove the feature.  For example
kvm-unit-tests can be used with new kernel and old QEMU, so I don't think
it will move away from [+-]feature very soon.

Regarding libvirt, is "feature=on/off" introspectable?  That would also be
a problem for libvirt to support both old and new QEMU.

Paolo