On Fri, Jun 20, 2014 at 04:19:10PM +0200, Michal Privoznik wrote:
> The commit produces this nice output:
>
> virsh # emulatorcaps /usr/bin/qemu-system-x86_64 kvm pc-1.3
> <emulatorCapabilities>
> <path>/usr/bin/qemu-system-x86_64</path>
> <domain>kvm</domain>
> <machine>pc-1.3</machine>
> <arch>x86_64</arch>
> <vcpu>255</vcpu>
> <devices>
> <hostdev>
> <driver>
> <enum name='driver'>
> <value>kvm</value>
> <value>vfio</value>
> </enum>
> </driver>
> </hostdev>
> </devices>
> </emulatorCapabilities>
Following on from my comments in the earlier patch, this XML looks
pretty much ok. Just s/emulatorCapabilities/domainCapabilities/
Also, within <devices> we would ultimately aim to have an entry for
every device type we support, with a supported=yes|no attribute eg
<devices>
<disk supported='no'/>
<hostdev supported='yes'>
<driver>
<enum name='driver'>
<value>kvm</value>
<value>vfio</value>
</enum>
</driver>
</hostdev>
</devices>
That way absence of information for a particular device type means
"no information available" rather than "unsupported". The "unsupported"
case would always be explicit, so apps can deal with case where we have
not fully converted all drivers to report this new info.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> docs/formatemulatorcaps.html.in | 63 +++++++++++++++
> docs/schemas/emulatorcapability.rng | 49 ++++++++++++
> src/libvirt_private.syms | 1 +
> src/qemu/qemu_capabilities.c | 78 ++++++++++--------
> src/qemu/qemu_capabilities.h | 3 +
> src/qemu/qemu_capabilitiespriv.h | 55 +++++++++++++
> src/qemu/qemu_driver.c | 92 ++++++++++++++++++++++
> tests/Makefile.am | 18 +++++
> .../viremulatorcaps-qemu-kvm-vfio.xml | 17 ++++
> tests/viremulatorcapabilitiestest.c | 72 ++++++++++++++++-
> tests/virhostdevmock.c | 40 ++++++++++
> 11 files changed, 456 insertions(+), 32 deletions(-)
> create mode 100644 src/qemu/qemu_capabilitiespriv.h
> create mode 100644 tests/viremulatorcapabilitiesdata/viremulatorcaps-qemu-kvm-vfio.xml
> create mode 100644 tests/virhostdevmock.c
>
> diff --git a/docs/formatemulatorcaps.html.in b/docs/formatemulatorcaps.html.in
> index beea1a9..ba87ff1 100644
> --- a/docs/formatemulatorcaps.html.in
> +++ b/docs/formatemulatorcaps.html.in
> @@ -48,5 +48,68 @@
> <dd>The domain's <a href="formatdomain.html#elementsOSBIOS">machine
> type</a>.</dd>
> </dl>
> +
> + <p>Besides these three elements, depending on the hypervisor and the driver
> + used other elements may occur. For example:</p>
> +<pre>
> +<arch>x86_64</arch>
> +<vcpu>255</vcpu>
> +<devices/>
> +</pre>
> + <p>Where:</p>
> + <dl>
> + <dt>arch</dt>
> + <dd>Denotes the <a
> + href="formatdomain.html#elementsOSBIOS">architecture</a> of the
> + domain.</dd>
> +
> + <dt>vcpu</dt>
> + <dd>Tells the maximum virtual CPU count supported by the underlying
> + hypervisor.</dd>
> +
> + <dt>devices</dt>
> + <dd>Contains info on supported <a href="#elementsDevices">devices</a> features</dd>
> + </dl>
> +
> + <h3><a name="elementsDevices">Devices</a></h3>
> + <p>The final set of XML elements is used to describe device capabilities
> + with respect to the hypervisor and host capabilities. For example:</p>
> +<pre>
> +<devices>
> + <hostdev>
> + <driver>
> + <enum name='driver'>
> + <value>kvm</value>
> + <value>vfio</value>
> + </enum>
> + </driver>
> + </hostdev>
> +</devices>
> +</pre>
> + <p>The aim is to keep the element names as consistent with <a
> + href="formatdomain.html">domain element names</a> as possible.</p>
> +
> + <h4><a name="elementsHostDev">Host device assignment</a></h4>
> + <p>When deciding on the most suitable device passthrough mode, look into
> + this part of the XML document. For instance, in the following snippet both
> + VFIO and legacy KVM passthrough are supported:</p>
> +<pre>
> +<devices>
> + <hostdev>
> + <driver>
> + <enum name='driver'>
> + <value>kvm</value>
> + <value>vfio</value>
> + </enum>
> + </driver>
> + </hostdev>
> +</devices>
> +</pre>
> +
> + <p>The <code>hostdev</code> element can contain the following elements:</p>
> + <dl>
> + <dt><code>driver</code></dt>
> + <dd>Here are the information on hostdev driver gathered.</dd>
> + </dl>
> </body>
> </html>
> diff --git a/docs/schemas/emulatorcapability.rng b/docs/schemas/emulatorcapability.rng
> index 2548cef..0c4a0cb 100644
> --- a/docs/schemas/emulatorcapability.rng
> +++ b/docs/schemas/emulatorcapability.rng
> @@ -20,7 +20,56 @@
> <element name='machine'>
> <text/>
> </element>
> + <optional>
> + <element name='arch'>
> + <text/>
> + </element>
> + <element name='vcpu'>
> + <ref name='unsignedInt'/>
> + </element>
> + <ref name='devices'/>
> + </optional>
> </interleave>
> </element>
> </define>
> +
> + <define name='devices'>
> + <element name='devices'>
> + <interleave>
> + <ref name='hostdev'/>
> + </interleave>
> + </element>
> + </define>
> +
> + <define name='hostdev'>
> + <element name='hostdev'>
> + <interleave>
> + <ref name='driver'/>
> + </interleave>
> + </element>
> + </define>
> +
> + <define name='driver'>
> + <element name='driver'>
> + <interleave>
> + <ref name='enum-driver'/>
> + </interleave>
> + </element>
> + </define>
> +
> + <define name='enum-driver'>
> + <element name='enum'>
> + <attribute name='name'>
> + <value>driver</value>
> + </attribute>
> + <zeroOrMore>
> + <element name='value'>
> + <choice>
> + <value>kvm</value>
> + <value>vfio</value>
> + </choice>
> + </element>
> + </zeroOrMore>
> + </element>
> + </define>
> </grammar>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 2784610..68ad1c7 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -426,6 +426,7 @@ virDomainVideoTypeFromString;
> virDomainVideoTypeToString;
> virDomainVirtioEventIdxTypeFromString;
> virDomainVirtioEventIdxTypeToString;
> +virDomainVirtTypeFromString;
> virDomainVirtTypeToString;
> virDomainWatchdogActionTypeFromString;
> virDomainWatchdogActionTypeToString;
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 245d6b5..68828e3 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -24,6 +24,7 @@
> #include <config.h>
>
> #include "qemu_capabilities.h"
> +#include "qemu_capabilitiespriv.h"
> #include "viralloc.h"
> #include "vircrypto.h"
> #include "virlog.h"
> @@ -39,6 +40,7 @@
> #include "virnodesuspend.h"
> #include "qemu_monitor.h"
> #include "virstring.h"
> +#include "virhostdev.h"
>
> #include <fcntl.h>
> #include <sys/stat.h>
> @@ -259,37 +261,6 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
> );
>
>
> -/*
> - * Update the XML parser/formatter when adding more
> - * information to this struct so that it gets cached
> - * correctly. It does not have to be ABI-stable, as
> - * the cache will be discarded & repopulated if the
> - * timestamp on the libvirtd binary changes.
> - */
> -struct _virQEMUCaps {
> - virObject object;
> -
> - bool usedQMP;
> -
> - char *binary;
> - time_t ctime;
> -
> - virBitmapPtr flags;
> -
> - unsigned int version;
> - unsigned int kvmVersion;
> -
> - virArch arch;
> -
> - size_t ncpuDefinitions;
> - char **cpuDefinitions;
> -
> - size_t nmachineTypes;
> - char **machineTypes;
> - char **machineAliases;
> - unsigned int *machineMaxCpus;
> -};
> -
> struct _virQEMUCapsCache {
> virMutex lock;
> virHashTablePtr binaries;
> @@ -3509,3 +3480,48 @@ virQEMUCapsSupportsChardev(virDomainDefPtr def,
> (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
> chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO));
> }
> +
> +int
> +virQEMUCapsFormatBuf(virBufferPtr buf,
> + void *opaque,
> + const char *machine)
> +{
> + virQEMUCapsPtr qemuCaps = (virQEMUCapsPtr) opaque;
> + bool hostkvm = virHostdevHostSupportsPassthroughLegacy();
> + bool hostvfio = virHostdevHostSupportsPassthroughVFIO();
> + bool qemukvm = virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCIDEVICE) ||
> + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE);
> + bool qemuvfio = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI);
> + int maxcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps, machine);
> +
> + virBufferAsprintf(buf, "<arch>%s</arch>\n", virArchToString(qemuCaps->arch));
> +
> + if (maxcpus > 0)
> + virBufferAsprintf(buf, "<vcpu>%d</vcpu>\n", maxcpus);
> +
> + /* So far we want to print the rest iff both host and qemu support kvm/vfio */
> + if (!((hostkvm && qemukvm) || (hostvfio && qemuvfio)))
> + return 0;
> +
> + virBufferAddLit(buf, "<devices>\n");
> + virBufferAdjustIndent(buf, 2);
> + virBufferAddLit(buf, "<hostdev>\n");
> + virBufferAdjustIndent(buf, 2);
> + virBufferAddLit(buf, "<driver>\n");
> + virBufferAdjustIndent(buf, 2);
> + virBufferAddLit(buf, "<enum name='driver'>\n");
> + virBufferAdjustIndent(buf, 2);
> + if (hostkvm && qemukvm)
> + virBufferAddLit(buf, "<value>kvm</value>\n");
> + if (hostvfio && qemuvfio)
> + virBufferAddLit(buf, "<value>vfio</value>\n");
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</enum>\n");
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</driver>\n");
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</hostdev>\n");
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</devices>\n");
> + return 0;
> +}
Yes, this inline XML formatting is something we want to avoid in the virt
drivers.
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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list