Re: [Qemu-devel] [PULL 10/10] target-i386: Print obsolete warnings if +-features are used
On Tue, Jun 14, 2016 at 05:38:42PM -0400, Paolo Bonzini wrote: > > > - Original Message - > > From: "Eduardo Habkost" > > To: "Paolo Bonzini" > > Cc: "Peter Maydell" , "Andreas Färber" > > , qemu-devel@nongnu.org, "Richard > > Henderson" , "Igor Mammedov" > > 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" > > > > To: "Peter Maydell" > > > > Cc: "Andreas Färber" , qemu-devel@nongnu.org, "Richard > > > > Henderson" , "Paolo > > > > Bonzini" , "Igor Mammedov" > > > > 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 > > > > > > > > Signed-off-by: Igor Mammedov > > > > [ehabkost: Changed to use error_report()] > > > > Signed-off-by: Eduardo Habkost > > > > --- > > > > 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 plan to fix the awful ordering semantics. First with a warning for 1 or 2 releases (only when the weird semantics is really triggered), then +feature/-feature could be directly translated to feature=on/feature=off. > > 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. Good point. Removing the feature would require dozens of extra compatibility code to libvirt and kvm-unit-tests just to save 6 lines of code in QEMU. You convinced me. -- Eduardo
Re: [Qemu-devel] [PULL 10/10] target-i386: Print obsolete warnings if +-features are used
- Original Message - > From: "Eduardo Habkost" > To: "Paolo Bonzini" > Cc: "Peter Maydell" , "Andreas Färber" > , qemu-devel@nongnu.org, "Richard > Henderson" , "Igor Mammedov" > 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" > > > To: "Peter Maydell" > > > Cc: "Andreas Färber" , qemu-devel@nongnu.org, "Richard > > > Henderson" , "Paolo > > > Bonzini" , "Igor Mammedov" > > > 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 > > > > > > Signed-off-by: Igor Mammedov > > > [ehabkost: Changed to use error_report()] > > > Signed-off-by: Eduardo Habkost > > > --- > > > 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
Re: [Qemu-devel] [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" > > To: "Peter Maydell" > > Cc: "Andreas Färber" , qemu-devel@nongnu.org, "Richard > > Henderson" , "Paolo > > Bonzini" , "Igor Mammedov" > > 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 > > > > Signed-off-by: Igor Mammedov > > [ehabkost: Changed to use error_report()] > > Signed-off-by: Eduardo Habkost > > --- > > 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? If that's really what you prefer, I won't complain too loudly. It's only a few extra lines of code, after all. But if you have something else in mind, please clarify what you suggest. -- Eduardo
Re: [Qemu-devel] [PULL 10/10] target-i386: Print obsolete warnings if +-features are used
- Original Message - > From: "Eduardo Habkost" > To: "Peter Maydell" > Cc: "Andreas Färber" , qemu-devel@nongnu.org, "Richard > Henderson" , "Paolo > Bonzini" , "Igor Mammedov" > 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 > > Signed-off-by: Igor Mammedov > [ehabkost: Changed to use error_report()] > Signed-off-by: Eduardo Habkost > --- > 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. Paolo
[Qemu-devel] [PULL 10/10] target-i386: Print obsolete warnings if +-features are used
From: Igor Mammedov Signed-off-by: Igor Mammedov [ehabkost: Changed to use error_report()] Signed-off-by: Eduardo Habkost --- 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; } -- 2.5.5