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

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 :)