On Wed, May 11, 2016 at 11:25:50AM -0400, Cole Robinson wrote: > 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?
Hmm, that's a good question - to too sure really, but I'd probably be inclined to say the latter. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
