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