[libvirt] [PATCH 6/7] qemu: Cache GIC capabilities

[Cole Robinson]

On 04/18/2016 01:44 PM, Andrea Bolognani wrote:
> Implement support for saving GIC capabilities in the cache and
> read them back.
> ---
>  src/qemu/qemu_capabilities.c | 87 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
> 

I was surprised by lack of test cases, but it seems a common problem in this
area of the code, so nothing to handle for this patch series...

One comment below

> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index ee80be2..be3e420 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -2926,6 +2926,77 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename,
>      }
>      VIR_FREE(nodes);
>  
> +    if ((n = virXPathNodeSet("./gic", ctxt, &nodes)) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("failed to parse qemu capabilities gic"));
> +        goto cleanup;
> +    }
> +    if (n > 0) {
> +        unsigned int uintValue;
> +        bool boolValue;
> +
> +        qemuCaps->ngicCapabilities = n;
> +        if (VIR_ALLOC_N(qemuCaps->gicCapabilities, n) < 0)
> +            goto cleanup;
> +
> +        for (i = 0; i < n; i++) {
> +            virGICCapabilityPtr cap = &qemuCaps->gicCapabilities[i];
> +
> +            if (!(str = virXMLPropString(nodes[i], "version"))) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("missing GIC version "
> +                                 "in QEMU capabilities cache"));
> +                goto cleanup;
> +            }
> +            if (str &&
> +                virStrToLong_ui(str, NULL, 10, &uintValue) < 0) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("malformed GIC version "
> +                                 "in QEMU capabilities cache"));
> +                goto cleanup;
> +            }
> +            cap->version = uintValue;
> +            VIR_FREE(str);
> +
> +            if (!(str = virXMLPropString(nodes[i], "kernel"))) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("missing in-kernel GIC information "
> +                                 "in QEMU capabilities cache"));
> +                goto cleanup;
> +            }
> +            if (str &&
> +                !(boolValue = STREQ(str, "true")) &&
> +                STRNEQ(str, "false")) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("malformed in-kernel GIC information "
> +                                 "in QEMU capabilities cache"));
> +                goto cleanup;
> +            }
> +            if (boolValue)
> +                cap->implementation |= VIR_GIC_IMPLEMENTATION_KERNEL;
> +            VIR_FREE(str);
> +
> +            if (!(str = virXMLPropString(nodes[i], "emulated"))) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("missing emulated GIC information "
> +                                 "in QEMU capabilities cache"));
> +                goto cleanup;
> +            }
> +            if (str &&
> +                !(boolValue = STREQ(str, "true")) &&
> +                STRNEQ(str, "false")) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("malformed emulated GIC information "
> +                                 "in QEMU capabilities cache"));
> +                goto cleanup;
> +            }
> +            if (boolValue)
> +                cap->implementation |= VIR_GIC_IMPLEMENTATION_EMULATED;
> +            VIR_FREE(str);
> +        }
> +    }
> +    VIR_FREE(nodes);
> +
>      ret = 0;
>   cleanup:
>      VIR_FREE(str);
> @@ -2992,6 +3063,22 @@ virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const char *filename)
>                            qemuCaps->machineMaxCpus[i]);
>      }
>  
> +    for (i = 0; i < qemuCaps->ngicCapabilities; i++) {
> +        virGICCapabilityPtr cap;
> +        bool kernel;
> +        bool emulated;
> +
> +        cap = &qemuCaps->gicCapabilities[i];
> +        kernel = (cap->implementation & VIR_GIC_IMPLEMENTATION_KERNEL);
> +        emulated = (cap->implementation & VIR_GIC_IMPLEMENTATION_EMULATED);
> +
> +        virBufferAsprintf(&buf,
> +                          "<gic version='%d' kernel='%s' emulated='%s'/>\n",
> +                          cap->version,
> +                          kernel ? "true" : "false",
> +                          emulated ? "true" : "false");
> +    }
> +
>      virBufferAdjustIndent(&buf, -2);
>      virBufferAddLit(&buf, "</qemuCaps>\n");
>  
> 

Use of literal 'true'/'false' isn't common in our XML formats. This isn't user
facing, but still probably better to use 'yes'/'no', if only so that some
future cleanup can streamline things here :)

ACK otherwise

- Cole