diff mbox

[RFC,v2,12/15] cpu-model/s390: Extend QMP command query-cpu-definitions

Message ID 1424183053-4310-13-git-send-email-mimu@linux.vnet.ibm.com
State New
Headers show

Commit Message

Michael Mueller Feb. 17, 2015, 2:24 p.m. UTC
This patch implements the QMP command 'query-cpu-definitions' in the S390
context. The command returns a in terms of machine release date descending
sorted list of cpu model names in the current host context. A consumer may
successfully request each listed cpu model as long for a given accelerator
this model is runnable.

Thy QMP type AccelCpuModelInfo is introduced and the type CpuDefinitionInfo
is extended by the optional field 'accelerators'. It contains a list of named
accelerators and some indication whether the associated cpu model is runnable
or the default cpu model. The default cpu model is used either no specific cpu
was requested during QEMU startup or the cpu model with named 'host'.

request:
  {"execute": "query-cpu-definitions"}

answer:
  {"return":
    [{"name":"2964-ga1","accelerators":[{"name":"kvm","runnable":false,"default":false}]},
     {"name":"2828-ga1","accelerators":[{"name":"kvm","runnable":false,"default":false}]},
     {"name":"2827-ga2","accelerators":[{"name":"kvm","runnable":true,"default":true}]},
     {"name":"2827-ga1","accelerators":[{"name":"kvm","runnable":true,"default":false}]},
     {"name":"2818-ga1","accelerators":[{"name":"kvm","runnable":true,"default":false}]},
     ...
     {"name":"2064-ga1","accelerators":[{"runnable":true,"name":"kvm","default":false}]}
    ]
   }

Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
---
 qapi-schema.json          |  21 +++++++++-
 target-s390x/cpu-models.c |  15 +++++++
 target-s390x/cpu-models.h |   1 +
 target-s390x/cpu.c        | 100 +++++++++++++++++++++++++++++++++++++++++++---
 4 files changed, 130 insertions(+), 7 deletions(-)

Comments

Eric Blake Feb. 17, 2015, 6:09 p.m. UTC | #1
On 02/17/2015 07:24 AM, Michael Mueller wrote:
> This patch implements the QMP command 'query-cpu-definitions' in the S390
> context. The command returns a in terms of machine release date descending
> sorted list of cpu model names in the current host context.

returns a list of cpu model names sorted by descending release dates.

Does guaranteeing the sorting as part of the interface really matter, or
would it be better to just return the list with no documented sorting
(where callers treat it as unsorted)?

> A consumer may
> successfully request each listed cpu model as long for a given accelerator
> this model is runnable.
> 
> Thy QMP type AccelCpuModelInfo is introduced and the type CpuDefinitionInfo
> is extended by the optional field 'accelerators'. It contains a list of named
> accelerators and some indication whether the associated cpu model is runnable
> or the default cpu model. The default cpu model is used either no specific cpu
> was requested during QEMU startup or the cpu model with named 'host'.
> 
> request:
>   {"execute": "query-cpu-definitions"}
> 
> answer:
>   {"return":
>     [{"name":"2964-ga1","accelerators":[{"name":"kvm","runnable":false,"default":false}]},
>      {"name":"2828-ga1","accelerators":[{"name":"kvm","runnable":false,"default":false}]},
>      {"name":"2827-ga2","accelerators":[{"name":"kvm","runnable":true,"default":true}]},
>      {"name":"2827-ga1","accelerators":[{"name":"kvm","runnable":true,"default":false}]},
>      {"name":"2818-ga1","accelerators":[{"name":"kvm","runnable":true,"default":false}]},
>      ...
>      {"name":"2064-ga1","accelerators":[{"runnable":true,"name":"kvm","default":false}]}
>     ]
>    }
> 

Looks okay from an interface perspective.

> Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> ---
>  qapi-schema.json          |  21 +++++++++-
>  target-s390x/cpu-models.c |  15 +++++++
>  target-s390x/cpu-models.h |   1 +
>  target-s390x/cpu.c        | 100 +++++++++++++++++++++++++++++++++++++++++++---
>  4 files changed, 130 insertions(+), 7 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 9431fc2..a5d38ae 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2485,16 +2485,35 @@
>    'data': ['qtest', 'tcg', 'kvm', 'xen'  ] }
>  
>  ##
> +# @AccelCpuModelInfo:
> +#
> +# Accelerator specific CPU model data
> +#
> +# @id: the accelerator id
> +#

There is no 'id' field below, did you mean 'name'?

> +# @default: cpu model for 'host'
> +#
> +# @runnable: cpu model can be activated on hosting machine
> +#
> +# Since: 2.3.0
> +#
> +##
> +{ 'type': 'AccelCpuModelInfo',
> +  'data': { 'name': 'AccelId', 'default': 'bool', 'runnable': 'bool' } }
> +
> +##
>  # @CpuDefinitionInfo:
>  #
>  # Virtual CPU definition.
>  #
>  # @name: the name of the CPU definition
>  #
> +# @accelerators: #optional cpu model offered per accelerator (since 2.3.0)
> +#

Must the field be optional, or will we always provide it?  Since this is
an output-only field, it is okay for back-compat to make the new field
unconditional.
Michael Mueller Feb. 18, 2015, 9:29 a.m. UTC | #2
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Tue, 17 Feb 2015 11:09:34 -0700
Eric Blake <eblake@redhat.com> wrote:

> On 02/17/2015 07:24 AM, Michael Mueller wrote:

> > This patch implements the QMP command 'query-cpu-definitions' in the S390

> > context. The command returns a in terms of machine release date descending

> > sorted list of cpu model names in the current host context.

> 

> returns a list of cpu model names sorted by descending release dates.

> 

> Does guaranteeing the sorting as part of the interface really matter, or

> would it be better to just return the list with no documented sorting

> (where callers treat it as unsorted)?


Yep, that is an implementation detail and I don't depend on that. If a sequence would be required
one cold model a field named "order". But as said, I don't require that and will update the
comment by dropping the "sorted list" part.

> 

> > A consumer may

> > successfully request each listed cpu model as long for a given accelerator

> > this model is runnable.

> > 

> > Thy QMP type AccelCpuModelInfo is introduced and the type CpuDefinitionInfo

> > is extended by the optional field 'accelerators'. It contains a list of named

> > accelerators and some indication whether the associated cpu model is runnable

> > or the default cpu model. The default cpu model is used either no specific cpu

> > was requested during QEMU startup or the cpu model with named 'host'.

> > 

> > request:

> >   {"execute": "query-cpu-definitions"}

> > 

> > answer:

> >   {"return":

> >     [{"name":"2964-ga1","accelerators":[{"name":"kvm","runnable":false,"default":false}]},

> >      {"name":"2828-ga1","accelerators":[{"name":"kvm","runnable":false,"default":false}]},

> >      {"name":"2827-ga2","accelerators":[{"name":"kvm","runnable":true,"default":true}]},

> >      {"name":"2827-ga1","accelerators":[{"name":"kvm","runnable":true,"default":false}]},

> >      {"name":"2818-ga1","accelerators":[{"name":"kvm","runnable":true,"default":false}]},

> >      ...

> >      {"name":"2064-ga1","accelerators":[{"runnable":true,"name":"kvm","default":false}]}

> >     ]

> >    }

> > 

> 

> Looks okay from an interface perspective.

> 

> > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>

> > ---

> >  qapi-schema.json          |  21 +++++++++-

> >  target-s390x/cpu-models.c |  15 +++++++

> >  target-s390x/cpu-models.h |   1 +

> >  target-s390x/cpu.c        | 100 +++++++++++++++++++++++++++++++++++++++++++---

> >  4 files changed, 130 insertions(+), 7 deletions(-)

> > 

> > diff --git a/qapi-schema.json b/qapi-schema.json

> > index 9431fc2..a5d38ae 100644

> > --- a/qapi-schema.json

> > +++ b/qapi-schema.json

> > @@ -2485,16 +2485,35 @@

> >    'data': ['qtest', 'tcg', 'kvm', 'xen'  ] }

> >  

> >  ##

> > +# @AccelCpuModelInfo:

> > +#

> > +# Accelerator specific CPU model data

> > +#

> > +# @id: the accelerator id

> > +#

> 

> There is no 'id' field below, did you mean 'name'?


I did rename that one time to often :-), will s/id/name/g ...

> 

> > +# @default: cpu model for 'host'

> > +#

> > +# @runnable: cpu model can be activated on hosting machine

> > +#

> > +# Since: 2.3.0

> > +#

> > +##

> > +{ 'type': 'AccelCpuModelInfo',

> > +  'data': { 'name': 'AccelId', 'default': 'bool', 'runnable': 'bool' } }

> > +

> > +##

> >  # @CpuDefinitionInfo:

> >  #

> >  # Virtual CPU definition.

> >  #

> >  # @name: the name of the CPU definition

> >  #

> > +# @accelerators: #optional cpu model offered per accelerator (since 2.3.0)

> > +#

> 

> Must the field be optional, or will we always provide it?  Since this is

> an output-only field, it is okay for back-compat to make the new field

> unconditional.


It will be always provided when an accelerator supports cpu models and implements the
probing mode. As I'm currently adopting it for s390/kvm only, I can't enforce it for all
other arch/accelerator combinations as it is an extension of an existing command...

Thanks
Michael

> 


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBAgAGBQJU5Ft5AAoJELPcPLQSJsKQ520P/A3EQqyY7buhBZWwVDQcA49J
FSzjyEt2JAJmZAlMFMaxbVDwJcm5PXEbCHR1+NuDXuyEYsPxqG7TxvP+3yLR2lLa
QbucHGd9M789Tg0hy2YPifIIB93LY5Kb3SNxhL52olyIrnsovHoBCbboMlmmKTk7
KyH6q2KXddjWtZbHy9WGQY91r56yMdsbfIxbYMJuJbJ/9Hr0lh0xB6W77mL8GIrV
dbURjaZXwgoGecVAlyEQcpInS2fl6XSX7Y2rCAq9Jp/9ZNfSQdOai19Md2cQ1JLi
92qYwgT8XV6bZOHJ1E8xc7+KlJRRH4MvYbWTNCVHEA3ewE5rYkspe92fEj82GZnc
eJV+hjZcq9cNaOF8bvhpjy+9zW8WdrwsQKbXNEeJDzDmnYhaaKcdKRa6qUDwYLQW
eg+TAfO+G9YNYfEpJH5okCo3t7elpYlkdcOvNtj1X2gFAmpjkbeVOUWSD1JmtJ5u
+s3XUagvQdzoIdpsziob0NEpLU62QFcqAa3ZNSY/FE7itTMnZs2+rvbYUxGyRjqz
BbEwPDoAMcFCO6CdK/hoZxV8RbCRH+MoDy+oLKXbxsF1rJcFfe5VGUQBTbYJUNEO
l87sUJBw5AqcU5/VpnuQn/unVCupQxour63T6WxzobvFT+rpMIR8mUQXaAEe9RfJ
8G8A5VXn8C4zrC2Am5G5
=cpfo
-----END PGP SIGNATURE-----
diff mbox

Patch

diff --git a/qapi-schema.json b/qapi-schema.json
index 9431fc2..a5d38ae 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2485,16 +2485,35 @@ 
   'data': ['qtest', 'tcg', 'kvm', 'xen'  ] }
 
 ##
+# @AccelCpuModelInfo:
+#
+# Accelerator specific CPU model data
+#
+# @id: the accelerator id
+#
+# @default: cpu model for 'host'
+#
+# @runnable: cpu model can be activated on hosting machine
+#
+# Since: 2.3.0
+#
+##
+{ 'type': 'AccelCpuModelInfo',
+  'data': { 'name': 'AccelId', 'default': 'bool', 'runnable': 'bool' } }
+
+##
 # @CpuDefinitionInfo:
 #
 # Virtual CPU definition.
 #
 # @name: the name of the CPU definition
 #
+# @accelerators: #optional cpu model offered per accelerator (since 2.3.0)
+#
 # Since: 1.2.0
 ##
 { 'type': 'CpuDefinitionInfo',
-  'data': { 'name': 'str' } }
+  'data': { 'name': 'str', '*accelerators': ['AccelCpuModelInfo'] } }
 
 ##
 # @query-cpu-definitions:
diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
index 4d03adc..c63b4c2 100644
--- a/target-s390x/cpu-models.c
+++ b/target-s390x/cpu-models.c
@@ -202,6 +202,21 @@  int set_s390_cpu_alias(const char *name, const char *model)
     return 0;
 }
 
+/* compare order of two cpu classes for descending sort */
+gint s390_cpu_class_desc_order_compare(gconstpointer a, gconstpointer b)
+{
+    S390CPUClass *cc_a = S390_CPU_CLASS((ObjectClass *) a);
+    S390CPUClass *cc_b = S390_CPU_CLASS((ObjectClass *) b);
+
+    if (cc_a->mach->order < cc_b->mach->order) {
+        return 1;
+    }
+    if (cc_a->mach->order > cc_b->mach->order) {
+        return -1;
+    }
+    return 0;
+}
+
 /* return machine class for specific machine type */
 static void s390_machine_class_test_cpu_class(gpointer data, gpointer user_data)
 {
diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h
index 23ac2c4..d41fc37 100644
--- a/target-s390x/cpu-models.h
+++ b/target-s390x/cpu-models.h
@@ -106,6 +106,7 @@  static inline bool kvm_s390_probe_mode(void)
 
 int s390_setup_cpu_classes(AccelId accel, S390MachineProps *prop);
 gint s390_cpu_class_asc_order_compare(gconstpointer a, gconstpointer b);
+gint s390_cpu_class_desc_order_compare(gconstpointer a, gconstpointer b);
 void s390_cpu_list_entry(gpointer data, gpointer user_data);
 bool s390_cpu_classes_initialized(void);
 bool s390_test_facility(S390CPUClass *cc, unsigned long nr);
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index d98578b..42d38b0 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -66,18 +66,106 @@  void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 }
 
 #ifndef CONFIG_USER_ONLY
-CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+static AccelCpuModelInfoList *qmp_query_accel_entry(AccelId accel,
+                                                    S390CPUClass *cc,
+                                                    AccelCpuModelInfoList *prev)
+{
+    AccelCpuModelInfoList *list;
+    AccelCpuModelInfo *info;
+
+    info = g_try_new0(AccelCpuModelInfo, 1);
+    if (!info) {
+        goto out;
+    }
+    info->name = accel;
+    info->runnable = cc->is_active[accel];
+    info->q_default = cc->is_host[accel];
+    list = g_try_new0(AccelCpuModelInfoList, 1);
+    if (!list) {
+        goto out;
+    }
+    list->value = info;
+    list->next = prev;
+
+    return list;
+out:
+    g_free(info);
+    return prev;
+}
+
+static void qmp_query_cpu_definition_entry(gpointer data, gpointer user_data)
+{
+    ObjectClass *oc = (ObjectClass *) data;
+    S390CPUClass *cc = S390_CPU_CLASS(oc);
+    CpuDefinitionInfoList *list, **prev = user_data;
+    CpuDefinitionInfo *info;
+
+    if (!strcmp(object_class_get_name(oc), TYPE_S390_CPU)) {
+        return;
+    }
+    info = g_try_new0(CpuDefinitionInfo, 1);
+    if (!info) {
+        goto out;
+    }
+    info->name = strdup_s390_cpu_name(cc);
+    if (kvm_enabled()) {
+        info->accelerators =
+            qmp_query_accel_entry(ACCEL_ID_KVM, cc, info->accelerators);
+    }
+    info->has_accelerators = (info->accelerators) ? true : false;
+    list = g_try_new0(CpuDefinitionInfoList, 1);
+    if (!list) {
+        goto out;
+    }
+    list->value = info;
+    list->next = *prev;
+    *prev = list;
+    return;
+out:
+    if (info) {
+        g_free(info->name);
+        g_free(info);
+    }
+}
+
+static CpuDefinitionInfoList *qmp_query_cpu_definition_host(void)
 {
-    CpuDefinitionInfoList *entry;
+    CpuDefinitionInfoList *host = NULL;
     CpuDefinitionInfo *info;
 
-    info = g_malloc0(sizeof(*info));
+    info = g_try_new0(CpuDefinitionInfo, 1);
+    if (!info) {
+        goto out;
+    }
     info->name = g_strdup("host");
 
-    entry = g_malloc0(sizeof(*entry));
-    entry->value = info;
+    host = g_try_new0(CpuDefinitionInfoList, 1);
+    if (!host) {
+        g_free(info->name);
+        g_free(info);
+        goto out;
+    }
+    host->value = info;
+out:
+    return host;
+}
+
+CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+{
+    CpuDefinitionInfoList *list = NULL;
+    GSList *class_list;
+
+    if (!s390_probe_mode()) {
+        if (!s390_cpu_models_used()) {
+            return qmp_query_cpu_definition_host();
+        }
+    }
+    class_list = object_class_get_list(TYPE_S390_CPU, false);
+    class_list = g_slist_sort(class_list, s390_cpu_class_asc_order_compare);
+    g_slist_foreach(class_list, qmp_query_cpu_definition_entry, &list);
+    g_slist_free(class_list);
 
-    return entry;
+    return list;
 }
 
 CpuModelInfo *arch_query_cpu_model(Error **errp)