[libvirt] [PATCH v2 11/33] qemu: Store more types in qemuMonitorCPUModelInfo

[Jiri Denemark]

On Tue, Feb 21, 2017 at 09:27:45 -0500, John Ferlan wrote:
> 
> 
> On 02/15/2017 11:44 AM, Jiri Denemark wrote:
> > While query-cpu-model-expansion returns only boolean features on s390,
> > but x86_64 reports some integer and string properties which we are
> > interested in.
> > 
> > Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> > ---
> > 
> > Notes:
> >     Version 2:
> >     - no change
> > 
> >  src/qemu/qemu_capabilities.c                     | 84 ++++++++++++++++--------
> >  src/qemu/qemu_monitor.c                          | 22 ++++++-
> >  src/qemu/qemu_monitor.h                          | 23 +++++--
> >  src/qemu/qemu_monitor_json.c                     | 37 ++++++++---
> >  tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml |  7 ++
> >  5 files changed, 133 insertions(+), 40 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index aab336954..466852d13 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -3074,14 +3074,16 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
> >      cpu->nfeatures = 0;
> >  
> >      for (i = 0; i < modelInfo->nprops; i++) {
> > -        if (VIR_STRDUP(cpu->features[i].name, modelInfo->props[i].name) < 0)
> > +        virCPUFeatureDefPtr feature = cpu->features + cpu->nfeatures;
> > +        qemuMonitorCPUPropertyPtr prop = modelInfo->props + i;
> > +
> > +        if (prop->type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN)
> > +            continue;
> 
> So s390 only supports "boolean" or "default" types?

S390 only supports boolean properties; there's no "default" type.

...
> > @@ -3187,30 +3188,47 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
> >          hostCPU->nprops = n;
> >  
> >          for (i = 0; i < n; i++) {
> > -            hostCPU->props[i].name = virXMLPropString(featureNodes[i], "name");
> > -            if (!hostCPU->props[i].name) {
> > +            qemuMonitorCPUPropertyPtr prop = hostCPU->props + i;
> > +            ctxt->node = featureNodes[i];
> > +
> > +            if (!(prop->name = virXMLPropString(ctxt->node, "name"))) {
> >                  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >                                 _("missing 'name' attribute for a host CPU"
> >                                   " model property in QEMU capabilities cache"));
> >                  goto cleanup;
> >              }
> 
> If you follow the suggestion I have in the previous patch, then if you
> don't find the property named "type", then you know the 'value' is
> either "yes" or "no"
> 
> If there is a type then the "value" property is either "string" or "number"

As explained in my previous reply, there's no need to support missing
type attribute.

...
> > diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> > index 8811d8501..112f041f1 100644
> > --- a/src/qemu/qemu_monitor.h
> > +++ b/src/qemu/qemu_monitor.h
> > @@ -921,16 +921,31 @@ int qemuMonitorGetCPUDefinitions(qemuMonitorPtr mon,
> >                                   qemuMonitorCPUDefInfoPtr **cpus);
> >  void qemuMonitorCPUDefInfoFree(qemuMonitorCPUDefInfoPtr cpu);
> >  
> > +typedef enum {
> > +    QEMU_MONITOR_CPU_PROPERTY_BOOLEAN,
> 
> As stated in previous patch, I think should should be "DEFAULT"

No.

> 
> > +    QEMU_MONITOR_CPU_PROPERTY_STRING,
> > +    QEMU_MONITOR_CPU_PROPERTY_ULL,
> 
> likewise, "NUMBER" not "ULL"

Yes.

Jirka