[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v3 08/28] qemu: Store more types in qemuMonitorCPUModelInfo



On Thu, Feb 23, 2017 at 03:15:06PM +0100, 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 redhat com>
> ---
> 
> Notes:
>     Version 3:
>     - change the XML element to
>         <property name='...' type='boolean|string|number' value='...'/>
>     
>     Version 2:
>     - no change
> 
>  src/qemu/qemu_capabilities.c                     | 100 +++++++++++++++++------
>  src/qemu/qemu_monitor.c                          |  25 +++++-
>  src/qemu/qemu_monitor.h                          |  27 +++++-
>  src/qemu/qemu_monitor_json.c                     |  42 ++++++++--
>  tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml |   7 ++
>  5 files changed, 163 insertions(+), 38 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index e037d5f95..0b611c323 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -3076,14 +3076,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;
> +
> +        if (VIR_STRDUP(feature->name, prop->name) < 0)
>              return -1;
> -
> -        if (modelInfo->props[i].supported)
> -            cpu->features[i].policy = VIR_CPU_FEATURE_REQUIRE;
> -        else
> -            cpu->features[i].policy = VIR_CPU_FEATURE_DISABLE;
> -
> +        feature->policy = prop->value.boolean ? VIR_CPU_FEATURE_REQUIRE
> +                                              : VIR_CPU_FEATURE_DISABLE;
>          cpu->nfeatures++;
>      }
>  
> @@ -3189,30 +3191,59 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
>          hostCPU->nprops = n;
>  
>          for (i = 0; i < n; i++) {
> -            hostCPU->props[i].name = virXMLPropString(nodes[i], "name");
> -            if (!hostCPU->props[i].name) {
> +            qemuMonitorCPUPropertyPtr prop = hostCPU->props + i;
> +            int type;
> +
> +            ctxt->node = nodes[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 (!(str = virXMLPropString(nodes[i], "value"))) {
> +            if (!(str = virXMLPropString(ctxt->node, "type")) ||
> +                (type = qemuMonitorCPUPropertyTypeFromString(str)) < 0) {
>                  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                               _("missing 'value' attribute for a host CPU"
> -                                 " model property in QEMU capabilities cache"));
> -                goto cleanup;
> -            }
> -            if (STREQ(str, "true")) {
> -                hostCPU->props[i].supported = true;
> -            } else if (STREQ(str, "false")) {
> -                hostCPU->props[i].supported = false;
> -            } else {
> -                virReportError(VIR_ERR_INTERNAL_ERROR,
> -                               _("invalid boolean  value: '%s'"), str);
> +                               _("missing CPU model property type in QEMU "
> +                                 "capabilities cache"));

There should be a different error message for the case when type is missing
and when the type is unknown.  Or just modify the error message to cover both
cases.

ACK with that fixed.

Pavel

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]