Re: [libvirt] [PATCH 11/12] Expose IOMMU and VFIO capabilities from virCaps

On 30.05.2014 11:11, Daniel P. Berrange wrote: >>diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c >>index 9561ba3..a91f37b 100644 >>--- a/src/conf/capabilities.c >>+++ b/src/conf/capabilities.c >>@@ -901,6 +901,10 @@ virCapabilitiesFormatXML(virCapsPtr caps) >> virBufferAddLit(&buf, "</secmodel>\n"); >> } >> >>+ /* KVM and VFIO features */ >>+ virBufferAsprintf(&buf, "<kvm>%s</kvm>\n", caps->host.legacyKVMPassthrough ? "true" : "false"); >>+ virBufferAsprintf(&buf, "<vfio>%s</vfio>\n", caps->host.VFIOPassthrough ? "true" : "false"); > >Ah, so this is missing from the previous patch. > >In fact the splt between this & previous patch is a bit confusing. > >I'd expect the previous patch to only change the src/conf/capabilities* >files and generic test cases. While this patch only change the driver >code and driver specific test cases. > > >So this XML is really trying to provide a list of the valid enumeration >options for the <driver> element of <hostdev> devices. This is quite a >common request for many domain XML elements, so I feel we ought to do >something that is not so ad-hoc here. When we get into this world of >providing info about supported options for devices, we really need to >be able to express this on a fine granularity that the capabilities >XML allows for. > >For example, different emulator binaries will support different options, >as will many different architectures, or even different machine types of >virtualization types. > >So it feels like we need a new API for this, that accepts info about >the machine we're trying to launch. eg > > char * virConnectGetEmulatorCapabilties(virConnectPtr conn, > const char *emulatorbin, > const char *machine, > const char *virttype); What's this virttype argument?

> >NB I didn't include 'architecture' since that's implicit in the >emulatorbin chosen. The 'char *' return value would be an XML >schema > >As for the XML schema, I haven't given it huge thought, but perhaps >something that loosely mirrors the XML schema is desirable. the XML schema? You mean the capabilities XML schema?

> >So for the <hostdev> driver type enum I could imagine starting off with: > > <emulatorCapabilities> > <path>/usr/bin/qemyu-system-x86_64</path> > <domain>kvm</domain> > <arch>x86_64</arch> > <machine>pc-1.0</machine > <devices> > <hostdev> > <driver> I find this misleading. I mean, why kvm and vfio is under devices/hostdev/driver?

> <enum name="driver"> > <value>kvm</value> > <value>vfio</value> > </enum> > </driver> > </hostdev> > </devices> > </emulatorCapabilities> > >I would expect that the 'maxCpus' hack we stuffed into the existing >capabilities XML would be added to this too eg <vcpus max="120"/> >at the top level So let me see if I understand correctly. To represent emulatorCapabilities in memory we need another structure, say: typedef struct _virEmulatorCapabilities virEmulatorCapabilities; typedef virEmulatorCapabilities *virEmulatorCapabilitiesPtr; struct _virEmulatorCapabilities { <some values here> }; which will live in src/conf/capabilities.*. Can you shed a light on the structure internals and where it should be created? IIUC, it'll be created in the virConnectGetEmulatorCapabilties(), then formated and immediately disposed.