[libvirt] [PATCH v2 10/33] qemu: Rename hostCPU/feature element in capabilities cache

[John Ferlan]

On 02/15/2017 11:44 AM, Jiri Denemark wrote:
> The element will be generalized in the following commit.
> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
> 
> Notes:
>     Version 2:
>     - no change
> 
>  src/qemu/qemu_capabilities.c                     |  14 +-
>  tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml  |  30 +--
>  tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 322 +++++++++++------------
>  3 files changed, 183 insertions(+), 183 deletions(-)
> 

This is essentially assuming the capabilities cache is being re-created
and not re-read... Hopefully that works correctly... It was the change
of an element name without some sort of pseudonym added or failure
returned if "feature" was found.  I didn't check callers - but if
something here failed would it *ensure* that the capabilities cache was
re-created?


> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 688d19504..aab336954 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -3180,7 +3180,7 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
>  
>      ctxt->node = hostCPUNode;
>  
> -    if ((n = virXPathNodeSet("./feature", ctxt, &featureNodes)) > 0) {
> +    if ((n = virXPathNodeSet("./property", ctxt, &featureNodes)) > 0) {

featureNodes should be renamed to propertyNodes too...

>          if (VIR_ALLOC_N(hostCPU->props, n) < 0)
>              goto cleanup;
>  
> @@ -3191,14 +3191,14 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
>              if (!hostCPU->props[i].name) {
>                  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                                 _("missing 'name' attribute for a host CPU"
> -                                 " model feature in QEMU capabilities cache"));
> +                                 " model property in QEMU capabilities cache"));
>                  goto cleanup;
>              }
>  
> -            if (!(str = virXMLPropString(featureNodes[i], "supported"))) {
> +            if (!(str = virXMLPropString(featureNodes[i], "boolean"))) {
>                  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                               _("missing 'supported' attribute for a host CPU"
> -                                 " model feature in QEMU capabilities cache"));
> +                               _("missing 'boolean' attribute for a host CPU"
> +                                 " model property in QEMU capabilities cache"));
>                  goto cleanup;
>              }
>              if (STREQ(str, "yes")) {
> @@ -3207,7 +3207,7 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
>                  hostCPU->props[i].supported = false;
>              } else {
>                  virReportError(VIR_ERR_INTERNAL_ERROR,
> -                               _("invalid supported value: '%s'"), str);
> +                               _("invalid boolean value: '%s'"), str);

boolean's are typically true/false - so if "boolean='no'", then does
that mean it's something else ;-)

I'm not sure boolean is the best name, but I see from the next patch
it's used because of the "type" being read from monitor/json.

Perhaps rather than "boolean" go with "value"... That way when you
implement more types in the next patch you get:

property name='%s' value='yes|no'
property name='%s' type='string' value='%s'
property name='%s' type='ull' value=%ull

If "type" isn't found, then value is assumed to be a boolean... In the
following patch qemuMonitorCPUPropertyType alters BOOLEAN to DEFAULT.

Not a fan of 'ull' either - I think it should just be 'number' and the
implementation details is that the number is a ULL.

John

>                  goto cleanup;
>              }
>              VIR_FREE(str);
> @@ -3552,7 +3552,7 @@ virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
>      virBufferAdjustIndent(buf, 2);
>  
>      for (i = 0; i < model->nprops; i++) {
> -        virBufferAsprintf(buf, "<feature name='%s' supported='%s'/>\n",
> +        virBufferAsprintf(buf, "<property name='%s' boolean='%s'/>\n",
>                            model->props[i].name,
>                            model->props[i].supported ? "yes" : "no");
>      }

naturally adjustments here...

[...]