[v2,15/19] s390x: print CPU definitions in sorted order

Other architectures provide nicely sorted lists, let's do it similarly on
s390x.

While at it, clean up the code we have to touch either way.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu_models.c | 63 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 41 insertions(+), 22 deletions(-)

On Mon,  4 Sep 2017 17:43:12 +0200
David Hildenbrand <david@redhat.com> wrote:

> Other architectures provide nicely sorted lists, let's do it similarly on
> s390x.
> 
> While at it, clean up the code we have to touch either way.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/cpu_models.c | 63 ++++++++++++++++++++++++++++++-----------------
>  1 file changed, 41 insertions(+), 22 deletions(-)
> 
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 8e20e7637b..092db34258 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -270,41 +270,60 @@ const S390CPUDef *s390_find_cpu_def(uint16_t type, uint8_t gen, uint8_t ec_ga,
>      return last_compatible;
>  }
>  
> -struct S390PrintCpuListInfo {
> -    FILE *f;
> -    fprintf_function print;
> -};
> -
> -static void print_cpu_model_list(ObjectClass *klass, void *opaque)
> +static void s390_print_cpu_model_list_entry(gpointer data, gpointer user_data)
>  {
> -    struct S390PrintCpuListInfo *info = opaque;
> -    S390CPUClass *scc = S390_CPU_CLASS(klass);
> -    char *name = g_strdup(object_class_get_name(klass));
> -    const char *details = "";
> -
> -    if (scc->is_static) {
> -        details = "(static, migration-safe)";
> -    } else if (scc->is_migration_safe) {
> -        details = "(migration-safe)";
> -    }
> +    CPUListState *s = user_data;
> +    const S390CPUClass *scc = S390_CPU_CLASS((ObjectClass *)data);
> +    char *name = g_strdup(object_class_get_name((ObjectClass *)data));
>  
>      /* strip off the -s390-cpu */
>      g_strrstr(name, "-" TYPE_S390_CPU)[0] = 0;
> -    (*info->print)(info->f, "s390 %-15s %-35s %s\n", name, scc->desc,
> -                   details);
> +    (*s->cpu_fprintf)(s->file, "s390 %-15s %-35s (%smigration-safe)\n", name,
> +                      scc->desc, scc->is_static ? "static, " : "");

Hm, that now prints 's390 host' as 'migration safe'...

>      g_free(name);
>  }
>  
> +static gint s390_cpu_list_compare(gconstpointer a, gconstpointer b)
> +{
> +    const S390CPUClass *cc_a = S390_CPU_CLASS((ObjectClass *)a);
> +    const S390CPUClass *cc_b = S390_CPU_CLASS((ObjectClass *)b);
> +    const char *name_a = object_class_get_name((ObjectClass *)a);
> +    const char *name_b = object_class_get_name((ObjectClass *)b);
> +
> +    /* move qemu and host to the top of the list, qemu first, host second */
> +    if (name_a[0] == 'q') {
> +        return -1;
> +    } else if (name_b[0] == 'q') {
> +        return 1;
> +    } else if (name_a[0] == 'h') {
> +        return -1;
> +    } else if (name_b[0] == 'h') {
> +        return 1;
> +    }

Are we sure that there will never be models starting with q or h? But
we can worry about that later.

> +
> +    /* keep the same order we have in our table (sorted by release date) */
> +    if (cc_a->cpu_def != cc_b->cpu_def) {
> +        return cc_a->cpu_def - cc_b->cpu_def;
> +    }
> +
> +    /* exact same definition - list base model first */
> +    return cc_a->is_static ? -1 : 1;
> +}
> +
>  void s390_cpu_list(FILE *f, fprintf_function print)
>  {
> -    struct S390PrintCpuListInfo info = {
> -        .f = f,
> -        .print = print,
> +    CPUListState s = {
> +        .file = f,
> +        .cpu_fprintf = print,
>      };
>      S390FeatGroup group;
>      S390Feat feat;
> +    GSList *list;
>  
> -    object_class_foreach(print_cpu_model_list, TYPE_S390_CPU, false, &info);
> +    list = object_class_get_list(TYPE_S390_CPU, false);
> +    list = g_slist_sort(list, s390_cpu_list_compare);
> +    g_slist_foreach(list, s390_print_cpu_model_list_entry, &s);
> +    g_slist_free(list);
>  
>      (*print)(f, "\nRecognized feature flags:\n");
>      for (feat = 0; feat < S390_FEAT_MAX; feat++) {

Sorting looks good.

>> +    CPUListState *s = user_data;
>> +    const S390CPUClass *scc = S390_CPU_CLASS((ObjectClass *)data);
>> +    char *name = g_strdup(object_class_get_name((ObjectClass *)data));
>>  
>>      /* strip off the -s390-cpu */
>>      g_strrstr(name, "-" TYPE_S390_CPU)[0] = 0;
>> -    (*info->print)(info->f, "s390 %-15s %-35s %s\n", name, scc->desc,
>> -                   details);
>> +    (*s->cpu_fprintf)(s->file, "s390 %-15s %-35s (%smigration-safe)\n", name,
>> +                      scc->desc, scc->is_static ? "static, " : "");
> 
> Hm, that now prints 's390 host' as 'migration safe'...

Very good point, will fix, thanks!

> 
>>      g_free(name);
>>  }
>>  
>> +static gint s390_cpu_list_compare(gconstpointer a, gconstpointer b)
>> +{
>> +    const S390CPUClass *cc_a = S390_CPU_CLASS((ObjectClass *)a);
>> +    const S390CPUClass *cc_b = S390_CPU_CLASS((ObjectClass *)b);
>> +    const char *name_a = object_class_get_name((ObjectClass *)a);
>> +    const char *name_b = object_class_get_name((ObjectClass *)b);
>> +
>> +    /* move qemu and host to the top of the list, qemu first, host second */
>> +    if (name_a[0] == 'q') {
>> +        return -1;
>> +    } else if (name_b[0] == 'q') {
>> +        return 1;
>> +    } else if (name_a[0] == 'h') {
>> +        return -1;
>> +    } else if (name_b[0] == 'h') {
>> +        return 1;
>> +    }
> 
> Are we sure that there will never be models starting with q or h? But
> we can worry about that later.

Could be but unlikely. In the worst case sorting would be wrong, I think
we can live with that :)