| Message ID | 1348691578-17231-19-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Sep 26, 2012 at 8:32 PM, Igor Mammedov <imammedo@redhat.com> wrote: > cpu_model string does represent features in following format: > ([+-]feat)|(feat=foo)|(feat) > which makes it impossible directly use property infrastructure > to set features on CPU. > This patch introduces parser that splits CPU name from cpu_model and > converts legacy features string into canonized set of strings that > is compatible with property manipulation infrastructure. > > PS: > * later it could be used as a hook to convert legacy command line > features to global properties. Then marked as deprecated and > removed with -cpu option in the future. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > v2: > * compiler complains that it's unused function but I guess it is > easier for review this way, for pull req I'll squash it into next > patch > * fix spelling error > * initialize sptr, due to a CentOS6 compiler warning, that > breakes build when -Werror is set. suggested-by: Don Slutz > --- > target-i386/cpu.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 52 insertions(+), 0 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index b7a9cd6..afa12ca 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -1342,6 +1342,58 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp) > } > } > > +/* convert legacy cpumodel string to string cpu_name and > + * a uniform set of custom features that will be applied to CPU > + * using object_property_parse() > + */ > +static void compat_normalize_cpu_model(const char *cpu_model, char **cpu_name, > + QDict **features, Error **errp) > +{ > + > + char *s = g_strdup(cpu_model); > + char *featurestr, *sptr = NULL; > + > + *cpu_name = strtok_r(s, ",", &sptr); This would break build since strtok_r() isn't available on Mingw, it's not Posix either. How about g_strsplit()? > + *features = qdict_new(); > + > + featurestr = strtok_r(NULL, ",", &sptr); > + while (featurestr) { > + char *val; > + if (featurestr[0] == '+') { > + /* > + * preseve legacy behaviour, if feature was disabled once > + * do not allow to enable it again > + */ > + if (!qdict_haskey(*features, featurestr + 1)) { > + qdict_put(*features, featurestr + 1, qstring_from_str("on")); > + } > + } else if (featurestr[0] == '-') { > + qdict_put(*features, featurestr + 1, qstring_from_str("off")); > + } else { > + val = strchr(featurestr, '='); > + if (val) { > + *val = 0; val++; > + if (!strcmp(featurestr, "vendor")) { > + qdict_put(*features, "vendor-override", > + qstring_from_str("on")); > + qdict_put(*features, featurestr, qstring_from_str(val)); > + } else if (!strcmp(featurestr, "tsc_freq")) { > + qdict_put(*features, "tsc-frequency", > + qstring_from_str(val)); > + } else { > + qdict_put(*features, featurestr, qstring_from_str(val)); > + } > + } else { > + qdict_put(*features, featurestr, qstring_from_str("on")); > + } > + } > + > + featurestr = strtok_r(NULL, ",", &sptr); > + } > + > + return; > +} > + > static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > const char *cpu_model, Error **errp) > { > -- > 1.7.1 >
On Sun, 30 Sep 2012 12:16:27 +0000 Blue Swirl <blauwirbel@gmail.com> wrote: > This would break build since strtok_r() isn't available on Mingw, it's > not Posix either. How about g_strsplit()? Thanks! Updated series at: https://github.com/imammedo/qemu/tree/x86-cpu-properties.WIP I'll post fixed [7/22] and [18/22] patches as followup to this thread.
On 09/30/2012 06:16 AM, Blue Swirl wrote: > On Wed, Sep 26, 2012 at 8:32 PM, Igor Mammedov <imammedo@redhat.com> wrote: >> cpu_model string does represent features in following format: >> ([+-]feat)|(feat=foo)|(feat) >> which makes it impossible directly use property infrastructure >> to set features on CPU. >> This patch introduces parser that splits CPU name from cpu_model and >> converts legacy features string into canonized set of strings that >> is compatible with property manipulation infrastructure. >> >> + >> + *cpu_name = strtok_r(s, ",", &sptr); > > This would break build since strtok_r() isn't available on Mingw, it's Correct. Microsoft is stuck in the past when it comes to standard conformance. > not Posix either. Huh? http://pubs.opengroup.org/onlinepubs/9699919799/functions/strtok_r.html strtok_r has been part of POSIX since pthreads were added.
Il 02/10/2012 13:28, Eric Blake ha scritto: >>> >> + *cpu_name = strtok_r(s, ",", &sptr); >> > >> > This would break build since strtok_r() isn't available on Mingw, it's > Correct. Microsoft is stuck in the past when it comes to standard > conformance. Luckily, in the common case of a non-varying delimiter g_strsplit is just as simple: gchar ** g_strsplit (const gchar *string, const gchar *delimiter, gint max_tokens); Splits a string into a maximum of max_tokens pieces, using the given delimiter. If max_tokens is reached, the remainder of string is appended to the last token. As a special case, the result of splitting the empty string "" is an empty vector, not a vector containing a single string. The reason for this special case is that being able to represent a empty vector is typically more useful than consistent handling of empty elements. If you do need to represent empty elements, you'll need to check for the empty string before calling g_strsplit(). string : a string to split. delimiter : a string which specifies the places at which to split the string. The delimiter is not included in any of the resulting strings, unless max_tokens is reached. max_tokens : the maximum number of pieces to split string into. If this is less than 1, the string is split completely. Returns : a newly-allocated NULL-terminated array of strings. Use g_strfreev() to free it.
On Tue, Oct 2, 2012 at 11:28 AM, Eric Blake <eblake@redhat.com> wrote: > On 09/30/2012 06:16 AM, Blue Swirl wrote: >> On Wed, Sep 26, 2012 at 8:32 PM, Igor Mammedov <imammedo@redhat.com> wrote: >>> cpu_model string does represent features in following format: >>> ([+-]feat)|(feat=foo)|(feat) >>> which makes it impossible directly use property infrastructure >>> to set features on CPU. >>> This patch introduces parser that splits CPU name from cpu_model and >>> converts legacy features string into canonized set of strings that >>> is compatible with property manipulation infrastructure. >>> > >>> + >>> + *cpu_name = strtok_r(s, ",", &sptr); >> >> This would break build since strtok_r() isn't available on Mingw, it's > > Correct. Microsoft is stuck in the past when it comes to standard > conformance. > >> not Posix either. > > Huh? http://pubs.opengroup.org/onlinepubs/9699919799/functions/strtok_r.html > strtok_r has been part of POSIX since pthreads were added. Sorry, I missed that strtok_r() was sharing the entry with strtok(). > > -- > Eric Blake eblake@redhat.com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
On Wed, 3 Oct 2012 20:09:52 +0000 Blue Swirl <blauwirbel@gmail.com> wrote: > On Tue, Oct 2, 2012 at 11:28 AM, Eric Blake <eblake@redhat.com> wrote: > > On 09/30/2012 06:16 AM, Blue Swirl wrote: > >> On Wed, Sep 26, 2012 at 8:32 PM, Igor Mammedov <imammedo@redhat.com> wrote: > >>> cpu_model string does represent features in following format: > >>> ([+-]feat)|(feat=foo)|(feat) > >>> which makes it impossible directly use property infrastructure > >>> to set features on CPU. > >>> This patch introduces parser that splits CPU name from cpu_model and > >>> converts legacy features string into canonized set of strings that > >>> is compatible with property manipulation infrastructure. > >>> > > > >>> + > >>> + *cpu_name = strtok_r(s, ",", &sptr); > >> > >> This would break build since strtok_r() isn't available on Mingw, it's > > > > Correct. Microsoft is stuck in the past when it comes to standard > > conformance. > > > >> not Posix either. > > > > Huh? http://pubs.opengroup.org/onlinepubs/9699919799/functions/strtok_r.html > > strtok_r has been part of POSIX since pthreads were added. > > Sorry, I missed that strtok_r() was sharing the entry with strtok(). Mingw maintainers rejected to implement it though: http://sourceforge.net/tracker/?func=detail&aid=2673480&group_id=2435&atid=352435 Anyway, I've already replace it with g_strsplit(), resulting patch became even simpler. > > > > > -- > > Eric Blake eblake@redhat.com +1-919-301-3266 > > Libvirt virtualization library http://libvirt.org > >
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index b7a9cd6..afa12ca 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1342,6 +1342,58 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp) } } +/* convert legacy cpumodel string to string cpu_name and + * a uniform set of custom features that will be applied to CPU + * using object_property_parse() + */ +static void compat_normalize_cpu_model(const char *cpu_model, char **cpu_name, + QDict **features, Error **errp) +{ + + char *s = g_strdup(cpu_model); + char *featurestr, *sptr = NULL; + + *cpu_name = strtok_r(s, ",", &sptr); + *features = qdict_new(); + + featurestr = strtok_r(NULL, ",", &sptr); + while (featurestr) { + char *val; + if (featurestr[0] == '+') { + /* + * preseve legacy behaviour, if feature was disabled once + * do not allow to enable it again + */ + if (!qdict_haskey(*features, featurestr + 1)) { + qdict_put(*features, featurestr + 1, qstring_from_str("on")); + } + } else if (featurestr[0] == '-') { + qdict_put(*features, featurestr + 1, qstring_from_str("off")); + } else { + val = strchr(featurestr, '='); + if (val) { + *val = 0; val++; + if (!strcmp(featurestr, "vendor")) { + qdict_put(*features, "vendor-override", + qstring_from_str("on")); + qdict_put(*features, featurestr, qstring_from_str(val)); + } else if (!strcmp(featurestr, "tsc_freq")) { + qdict_put(*features, "tsc-frequency", + qstring_from_str(val)); + } else { + qdict_put(*features, featurestr, qstring_from_str(val)); + } + } else { + qdict_put(*features, featurestr, qstring_from_str("on")); + } + } + + featurestr = strtok_r(NULL, ",", &sptr); + } + + return; +} + static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, const char *cpu_model, Error **errp) {
cpu_model string does represent features in following format: ([+-]feat)|(feat=foo)|(feat) which makes it impossible directly use property infrastructure to set features on CPU. This patch introduces parser that splits CPU name from cpu_model and converts legacy features string into canonized set of strings that is compatible with property manipulation infrastructure. PS: * later it could be used as a hook to convert legacy command line features to global properties. Then marked as deprecated and removed with -cpu option in the future. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- v2: * compiler complains that it's unused function but I guess it is easier for review this way, for pull req I'll squash it into next patch * fix spelling error * initialize sptr, due to a CentOS6 compiler warning, that breakes build when -Werror is set. suggested-by: Don Slutz --- target-i386/cpu.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 52 insertions(+), 0 deletions(-)