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)
>>
>> ?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?
>
Enable seems OK. I assume you were perhaps following the lead of
qemuDomainDefAddDefaultDevices.
>>> +{
>>> + /* 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.
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list