diff mbox

[1/4] qemu: Automatically choose usable GIC version

Message ID 939cae58-9b78-09b3-cadd-38ba297c60c0@redhat.com
State New
Headers show

Commit Message

Cole Robinson May 13, 2016, 6:53 p.m. UTC 
On 05/12/2016 11:53 AM, Andrea Bolognani wrote:
> On Tue, 2016-05-10 at 18:42 -0400, Cole Robinson wrote:

>>>  

>>> +        if (virQEMUCapsFillDomainCaps(caps, qemuCaps, NULL, 0) < 0)

>>> +            goto cleanup;

>>> +

>>> +        gic = &(caps->gic);

>>> +

>>> +        /* Pick the best GIC version from those available */

>>> +        if (gic->supported) {

>>> +            virGICVersion version;

>>> +

>>> +            VIR_DEBUG("Looking for usable GIC version in domain capabilities");

>>> +            for (version = VIR_GIC_VERSION_LAST - 1;

>>> +                 version > VIR_GIC_VERSION_NONE;

>>> +                 version--) {

>>> +                if (VIR_DOMAIN_CAPS_ENUM_IS_SET(gic->version, version)) {

>>> +

>>> +                    VIR_DEBUG("Using GIC version %s",

>>> +                              virGICVersionTypeToString(version));

>>> +                    def->gic_version = version;

>>> +                    break;

>>> +                }

>>> +            }

>>>           }

>>  

>> Hmm that's a bit of a new pattern... it seems the only thing you really need

>> from domcaps is the bit of logic we encode via

>> virQEMUCapsFillDomainFeatureGICCaps. Maybe break that logic out into a public

>> function and call it here, rather than spinning up domcaps for a small bit of

>> info? Or is there more to it?

> 

> Nothing more to it :)

> 

> Do you mean I should make virQEMUCapsFillDomainFeatureGICCaps()

> public and use it here to fill only the part of the domain

> capabilities I'm actually going to use, or create a new function

> altogether?

> 

> Because right now I'm not seeing a way to do the latter without

> introducing some code duplication or making things quite a bit

> uglier... Maybe I'm just tired :)

> 


Can you break apart the logic like the attached patch, then call the new
function from the above code? I didn't try plugging it into your patches but
it looks to me like it should work

- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
diff mbox

Patch

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 1bddf43..f05efd2 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4256,6 +4256,32 @@  virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps,
 }
 
 
+static bool
+virQEMUCapsGICSupported(virArch arch,
+                        virDomainVirtType virttype,
+                        const char *machine,
+                        virGICCapabilityPtr cap)
+{
+    if (arch != VIR_ARCH_ARMV7L &&
+        arch != VIR_ARCH_AARCH64)
+        return false;
+
+    if (STRNEQ(machine, "virt") &&
+        !STRPREFIX(machine, "virt-"))
+        return false;
+
+    if (virttype == VIR_DOMAIN_VIRT_KVM &&
+        !(cap->implementation & VIR_GIC_IMPLEMENTATION_KERNEL))
+        return false;
+
+    if (virttype == VIR_DOMAIN_VIRT_QEMU &&
+        !(cap->implementation & VIR_GIC_IMPLEMENTATION_EMULATED))
+        return false;
+
+    return true;
+}
+
+
 /**
  * virQEMUCapsFillDomainFeatureGICCaps:
  * @qemuCaps: QEMU capabilities
@@ -4282,23 +4308,12 @@  virQEMUCapsFillDomainFeatureGICCaps(virQEMUCapsPtr qemuCaps,
     virDomainCapsFeatureGICPtr gic = &domCaps->gic;
     size_t i;
 
-    if (domCaps->arch != VIR_ARCH_ARMV7L &&
-        domCaps->arch != VIR_ARCH_AARCH64)
-        return 0;
-
-    if (STRNEQ(domCaps->machine, "virt") &&
-        !STRPREFIX(domCaps->machine, "virt-"))
-        return 0;
-
     for (i = 0; i < qemuCaps->ngicCapabilities; i++) {
         virGICCapabilityPtr cap = &qemuCaps->gicCapabilities[i];
-
-        if (domCaps->virttype == VIR_DOMAIN_VIRT_KVM &&
-            !(cap->implementation & VIR_GIC_IMPLEMENTATION_KERNEL))
-            continue;
-
-        if (domCaps->virttype == VIR_DOMAIN_VIRT_QEMU &&
-            !(cap->implementation & VIR_GIC_IMPLEMENTATION_EMULATED))
+        if (!virQEMUCapsGICSupported(domCaps->arch,
+                                     domCaps->virttype,
+                                     domCaps->machine,
+                                     cap))
             continue;
 
         gic->supported = true;