[libvirt] [PATCH 4/7] qemu: Default to GIC v2

[John Ferlan]

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