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