[libvirt] [RFC 6/6] qemu: Cache GIC capabilities

[Cole Robinson]

On 04/06/2016 11:29 AM, Andrea Bolognani wrote:
> On Wed, 2016-03-30 at 16:29 -0400, John Ferlan wrote:
>> On 03/21/2016 01:28 PM, Andrea Bolognani wrote:
>>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>>> index 64007f0..23c3740 100644
>>> --- a/src/qemu/qemu_capabilities.c
>>> +++ b/src/qemu/qemu_capabilities.c
>>> @@ -2906,6 +2906,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;
>>  
>> Since these are processed as either/or in patch 5 based on virttype, I'm
>> beginning to think perhaps you'd have :
>>  
>> <gic version='#' type='{kernel|emulated}'/>
>>  
>> but this works, I'm still trying to process the whole domaincaps code
>> and what the use case would be. It looks like merely the output to
>> domcapabilities. In which case, I'm wonder if printing the gic_version
>> inside <arch> would be sufficient.
> 
> I believe all of these were addressed in my previous comments.
> 
> Let me know if that was actually not the case :)

I tried going through the patches and the comments but it was a bit hard to
follow, there was quite a few points/counterpoints :)

Andrea can you send a v2 addressing the obvious bits that John pointed out,
and anything you're still unsure of/disagreed with just highlight in v2 comments?

Thanks,
Cole