On Sun, 2016-02-07 at 09:38 -0500, John Ferlan wrote:
> > static int
> > +qemuDomainDefAddDefaultFeatures(virDomainDefPtr def)
>
> ?qemuDomainDefSetDefaultFeatures?
>
> You're not adding gic, it's already added, you're just setting the
> default version... although this could be unnecessary if host were the
> default...
Patch 5/7 actually enables the VIR_DOMAIN_FEATURE_GIC feature in the
same function, so the name makes more sense after that one has been
applied as well, but maybe I can keep the two steps separated and
have both AddDefaultFeatures (for turning on features that should be
turned on by default) and SetDefaultFeatures (for setting the specific
value in cases like this, where it's not a simple boolean)?
Or change the name to qemuDomainDefEnableDefaultFeatures() and still
do both things in one place?
> > +{
> > + /* Default to GIC v2 if no version was specified */
> > + if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON &&
> > + def->gic_version == VIR_GIC_VERSION_NONE)
> > + def->gic_version = VIR_GIC_VERSION_2;
> > +
> > + return 0;
>
> Since there is no other return value, this should be a void
Okay, we can change the return type later if we start doing something
that might fail.
> BTW: Somewhere along the way docs/formatdomain.html.in needs an
> adjustment to describe the options (host, 2, 3) and how they work.
There is already some documentation for GIC in
docs/formatdomain.html.in, but yeah, some improvements would definitely
be a nice addition.
Cheers.
--
Andrea Bolognani
Software Engineer - Virtualization Team
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list