[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 8/9] target-i386: Use "-" instead of "_" on all
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH 8/9] target-i386: Use "-" instead of "_" on all feature names |
Date: |
Fri, 27 May 2016 17:32:34 -0300 |
User-agent: |
Mutt/1.6.1 (2016-04-27) |
On Tue, May 24, 2016 at 03:22:27PM +0200, Igor Mammedov wrote:
> On Tue, 24 May 2016 09:34:05 -0300
> Eduardo Habkost <address@hidden> wrote:
>
> > On Tue, May 24, 2016 at 02:17:03PM +0200, Igor Mammedov wrote:
> > > On Fri, 6 May 2016 15:11:31 -0300
> > > Eduardo Habkost <address@hidden> wrote:
[...]
> > > > -/* Convert all '_' in a feature string option name to '-', to make
> > > > feature
> > > > - * name conform to QOM property naming rule, which uses '-' instead of
> > > > '_'.
> > > > +/* Convert all '_' in a feature string option name to '-', to keep
> > > > compatibility
> > > > + * with old feature names that used "_" instead of "-".
> > > > */
> > > > static inline void feat2prop(char *s)
> > > > {
> > > > @@ -1925,8 +1925,10 @@ static void x86_cpu_parse_featurestr(CPUState
> > > > *cs, char *features,
> > > > while (featurestr) {
> > > > char *val;
> > > I'd place a single feat2prop() here
> > > and delete it from other call sites in this function.
> >
> > A previous version of this patch had it. But it would change the
> > property value too, not just the property name (breaking stuff
> > like "model-id=some_string").
> >
> it's bug in feat2prop(), which probably should be fixed there,
> so it would do what comment above it says. Or as alternative:
The comment above it doesn't say anything about stopping at a '='
delimiter. I always expected it to just replace "_" with "-" in a
null-terminated string.
(I am not completely against making it stop at '=', but I believe
your suggestion below sounds better).
>
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index ca2a893..e46e4c3 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1941,14 +1941,16 @@ static void x86_cpu_parse_featurestr(CPUState *cs,
> char *features
> featurestr = features ? strtok(features, ",") : NULL;
>
> while (featurestr) {
> - char *val;
> + char *val = strchr(featurestr, '=');
> + if (val) {
> + *val = 0; val++;
> + }
> + feat2prop(featurestr);
This would make "+feature=FOO" treated as a valid option, and it
isn't. It would keep the existing behavior if we did this:
- if (featurestr[0] == '+') {
+ if (featurestr[0] == '+' && !val) {
add_flagname_to_bitmaps(featurestr + 1, plus_features,
&local_err);
- } else if (featurestr[0] == '-') {
+ if (featurestr[0] == '+' && !val) {
add_flagname_to_bitmaps(featurestr + 1, minus_features,
&local_err);
In either case, I prefer to get this optimization reviewed as a
separate patch. Can you send it as a follow-up?
> - } else if ((val = strchr(featurestr, '='))) {
> - *val = 0; val++;
> - feat2prop(featurestr);
> + } else if (val) {
> if (!strcmp(featurestr, "xlevel")) {
> char *err;
> char num[32];
> @@ -2000,7 +2002,6 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char
> *features,
> object_property_parse(OBJECT(cpu), val, featurestr,
> &local_err);
> }
> } else {
> - feat2prop(featurestr);
> object_property_parse(OBJECT(cpu), "on", featurestr, &local_err);
> }
> if (local_err) {
>
>
--
Eduardo
- Re: [Qemu-devel] [PATCH 3/9] target-i386: Call cpu_exec_init() on realize, (continued)
[Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions, Eduardo Habkost, 2016/05/06
- Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions, Markus Armbruster, 2016/05/09
- Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions, David Hildenbrand, 2016/05/09
- Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions, Eduardo Habkost, 2016/05/09
- Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions, David Hildenbrand, 2016/05/09
- Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions, Eduardo Habkost, 2016/05/09
- Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions, David Hildenbrand, 2016/05/09
- Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions, Eduardo Habkost, 2016/05/09
- Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions, David Hildenbrand, 2016/05/10