[libvirt] [PATCH 1/7] gic: Introduce virGICVersion enumeration

[Cole Robinson]

On 02/08/2016 10:15 AM, Andrea Bolognani wrote:
> On Sat, 2016-02-06 at 18:27 -0500, Cole Robinson wrote:
>> On 02/03/2016 03:25 PM, Andrea Bolognani wrote:
>>> We currently blindly accept any numeric value as a GIC version, even
>>> thought only GIC v2 and GIC v3 actually exist; on the other hand, we
>>> reject "host", which is a perfectly legitimate value for QEMU guests.
>>>  
>>> This new enumeration contains all GIC versions libvirt is aware of.
>>> ---
>>>   src/Makefile.am   |  1 +
>>>   src/util/virgic.c | 33 +++++++++++++++++++++++++++++++++
>>>   src/util/virgic.h | 38 ++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 72 insertions(+)
>>>   create mode 100644 src/util/virgic.c
>>>   create mode 100644 src/util/virgic.h
>>  
>> Seems a bit overkill to give this its own util/ file, but I assume this may
>> grow extra host probing bits in the future?
> 
> That might or might not be the case, as I mentioned in the cover
> letter: it mostly depends on whether we end up probing for
> supported GIC versions ourselves (in which case the code would
> neatly fit here) or rely on QEMU.
> 
> I can move this to device_conf.h and take it out later if we add
> more GIC-related stuff, or leave it here and move it later. Having
> two files just for an enumeration does indeed look a bit silly;
> then again device_conf.h is a huge beast at 3000 lines long.
> 

I assume you mean domain_conf.h here? Maybe we should have a separate header
and .c file for just the enums, if only to slim down domain_conf.* lengths.
Not needed for this series though.

> Also, at some point we will need to expose information about
> GIC in the capabilites or domcapabilities XML, and conf/ will
> not be a great fit anymore either.
> 

hmm, do we not share enums between domain conf and capabilities? maybe that's
another argument for splitting out the domain enums to their own file so they
are easier to share.

I'm fine with the current util/ layout, or domain_conf. Whatever you think is best

- Cole