[libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL

Wed May 11 15:25:50 UTC 2016

On 05/11/2016 04:49 AM, Daniel P. Berrange wrote:
> On Tue, May 10, 2016 at 07:15:01PM -0400, Cole Robinson wrote:
>> On 05/10/2016 11:50 AM, Daniel P. Berrange wrote:
>>> On Tue, May 10, 2016 at 11:26:29AM -0400, Cole Robinson wrote:
>>>> On 05/10/2016 05:10 AM, Daniel P. Berrange wrote:
>>>>> On Mon, May 09, 2016 at 06:53:01PM -0400, Cole Robinson wrote:
>>>>>> On 05/09/2016 09:48 AM, Daniel P. Berrange wrote:
>>>>>
>>>>> IMHO it is a total failure if we require the application to extend its
>>>>> parser every time we add a new enum to the domain capabilities. We have
>>>>> the ability to design something that is data driven - we should not build
>>>>> something it is forced to be code driven with code changes for every
>>>>> libvirt addition.
>>>>
>>>> This is ignoring the point I made previously that the schema is not already
>>>> fully introspectable. Most of the current enums cannot be programmatically
>>>> consumed anyways, because there's no way to map an enum name to its domain XML
>>>> property. So if that's our goal to be data driven, we should address that
>>>> issue first.
>>>
>>> Knowing the mapping of the capabilities enums to the domain schema is
>>> a pre-requisite for consuming the capabilities data, no matter which
>>> approach discussed in this thread is chosen. Assuming the app knows
>>> that mapping, then the enum conditionals can be programmatically
>>> handled with the approach I describe.
>>>
>>> Providing a way for the app to automatically determine the mapping
>>> from capabilities enums to the domain schema would be a nice
>>> addition, but it isn't a pre-requisite blocker for what's discussed
>>> here. It is something that can be deal with in parallel.
>>>
>>>> I can't really think of a good way to represent that without nesting
>>>> deeply or using specially formatted strings. Do you have a suggestion
>>>> for that?
>>>
>>> Probably the best thing is to use a very simplified xpath style notation.
>>> If we add a 'mapping' attribute to the <enum> that expresses the attribute
>>> or element it is associated with, relative to the parent container element.
>>>
>>> eg, consider the tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
>>> data file with the mapping info:
>>>
>>> <domainCapabilities>
>>>   <path>/usr/bin/qemu-system-x86_64</path>
>>>   <domain>kvm</domain>
>>>   <machine>pc-1.2</machine>
>>>   <arch>x86_64</arch>
>>>   <os supported='yes'>
>>>     <loader supported='yes'>
>>>       <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
>>>       <value>/usr/share/OVMF/OVMF_CODE.fd</value>
>>>       <enum name='type' mapping="@type">
>>>         <value>rom</value>
>>>         <value>pflash</value>
>>>       </enum>
>>>       <enum name='readonly' mapping="@readonly">
>>>         <value>yes</value>
>>>         <value>no</value>
>>>       </enum>
>>>     </loader>
>>>   </os>
>>>   <devices>
>>>     <disk supported='yes'>
>>>       <enum name='diskDevice' mapping="@device">
>>>         <value>disk</value>
>>>         <value>cdrom</value>
>>>         <value>floppy</value>
>>>         <value>lun</value>
>>>       </enum>
>>>       <enum name='bus' mapping="target/@bus">
>>>         <value>ide</value>
>>>         <value>fdc</value>
>>>         <value>scsi</value>
>>>         <value>virtio</value>
>>>         <value>usb</value>
>>>       </enum>
>>>     </disk>
>>>     <hostdev supported='yes'>
>>>       <enum name='mode' mapping="@mode">
>>>         <value>subsystem</value>
>>>       </enum>
>>>       <enum name='startupPolicy' mapping="source/@startupPolicy">
>>>         <value>default</value>
>>>         <value>mandatory</value>
>>>         <value>requisite</value>
>>>         <value>optional</value>
>>>       </enum>
>>>       <enum name='subsysType' mapping="@type">
>>>         <value>usb</value>
>>>         <value>pci</value>
>>>         <value>scsi</value>
>>>       </enum>
>>>       <enum name='capsType' mapping="@type"/>
>>>       <enum name='pciBackend' mapping="driver/@name">
>>>         <value>default</value>
>>>         <value>kvm</value>
>>>         <value>vfio</value>
>>>       </enum>
>>>     </hostdev>
>>>   </devices>
>>>   <features>
>>>     <gic supported='no'/>
>>>   </features>
>>> </domainCapabilities>
>>>
>>>
>>> NB, with my proposed conditionals, the hostdev enums above would actually
>>> want to be different. eg it would want to look like this:
>>>
>>>       <enum name='mode' mapping="@mode">
>>>         <value>subsystem</value>
>>>       </enum>
>>>       <enum name='startupPolicy' mapping="source/@startupPolicy">
>>>         <value>default</value>
>>>         <value>mandatory</value>
>>>         <value>requisite</value>
>>>         <value>optional</value>
>>>       </enum>
>>>       <enum name='type' mapping="@type">
>>>         <condition>
>>> 	  <enum name='mode' value='subsystem'/>
>>> 	</condition>
>>>         <value>usb</value>
>>>         <value>pci</value>
>>>         <value>scsi</value>
>>>       </enum>
>>>       <enum name='driver' mapping="driver/@name">
>>>         <condition>
>>> 	  <enum name='mode' value='subsystem'/>
>>> 	  <enum name='type' value='pci'/>
>>> 	</condition>
>>>         <value>default</value>
>>>         <value>kvm</value>
>>>         <value>vfio</value>
>>>       </enum>
>>>
>>
>> Yeah that mapping= bit makes sense to me.
>>
>> I don't have time to see pick this up now though, so I've stuffed it in a bug:
> 
> Thinking about this some more last night, I realize that once we have
> the 'mapping' attribute, then the choice of values in the 'name' attribute
> becomes totally irrelevant.
> 
> IOW, we could use your suggestion for giving each enum a unique name,
> eg 'spiceGL', 'vncGL', etc, etc. So apps which want to consume this
> data have a choice between just matching on explicit enum names and
> ignoring the <condition> rules, or they can match on the 'mapping'
> attribute and use the <condition> rules.. So we get the best of both
> ideas.

Hmm, but does that mean that once we assign an enum name, its semantics are
locked in stone? Can we add new conditions to an existing enum to clarify
semantics, or does extending the condition list always mandate a new enum?

- Cole