On Tue, 2016-05-17 at 16:10 -0400, John Ferlan wrote:
* enabled and configure default values related to those features. */ static void -qemuDomainDefEnableDefaultFeatures(virDomainDefPtr def) +qemuDomainDefEnableDefaultFeatures(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps) { - switch (def->os.arch) { - case VIR_ARCH_ARMV7L: - case VIR_ARCH_AARCH64: - if (qemuDomainMachineIsVirt(def)) { - /* GIC is always available to ARM virt machines */ - def->features[VIR_DOMAIN_FEATURE_GIC] = VIR_TRISTATE_SWITCH_ON; + do {
Not a fan... Isn't what you're doing the same as:
if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ABSENT && (def->os.arch == VIR_ARCH_ARMV7L || def->os.arch == VIR_ARCH_AARCH64) && qemuDomainMachineIsVirt(def)) {
The idea was to closely mirror virQEMUCapsFillDomainFeatureGICCaps(), but yeah, it's functionally identical to your version. I'm not exceedingly happy with cramming tons of conditions in the same if() - I think it makes for less readable code overall - but it's a well-established pattern used all over the place in libvirt, so let's stick with it. -- Andrea Bolognani Software Engineer - Virtualization Team