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
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;
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list