[libvirt] [PATCH 4/7] qemu: Default to GIC v2
jferlan at redhat.com
Mon Feb 8 13:46:24 UTC 2016
On 02/08/2016 08:31 AM, Andrea Bolognani wrote:
> On Sun, 2016-02-07 at 09:38 -0500, John Ferlan wrote:
>>> static int
>>> +qemuDomainDefAddDefaultFeatures(virDomainDefPtr def)
>> 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
> 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?
Enable seems OK. I assume you were perhaps following the lead of
>>> + /* 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.
Well yes, always good to update the docs especially when you add or
change the valid values of a documented XML attribute.
More information about the libvir-list