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