Re: [Qemu-devel] [PATCH 18/22] target-i386: parse cpu_model string into set of stringified properties

[Blue Swirl]

On Wed, Sep 26, 2012 at 8:32 PM, Igor Mammedov <address@hidden> 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 <address@hidden>
> ---
>  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
>