[libvirt] [PATCH 0/5] Expose IOMMU and VFIO host capabilities
This is basically a v2 of one of my previous attempts, but that was a different set. So practically it's still a v1. Michal Privoznik (5): virhostdev: Move IOMMU and VFIO funcs from qemu conf: Introduce viremulator_capabilities Introduce virConnectGetEmulatorCapabilities virsh: Expose virConnectGetEmulatorCapabilities qemu: Implement virConnectGetEmulatorCapabilities docs/formatemulatorcaps.html.in | 115 +++++++++++++ docs/schemas/Makefile.am | 1 + docs/schemas/emulatorcapability.rng | 75 +++++++++ docs/sitemap.html.in | 4 + include/libvirt/libvirt.h.in | 6 + libvirt.spec.in | 1 + mingw-libvirt.spec.in | 2 + src/Makefile.am | 3 +- src/conf/viremulator_capabilities.c | 139 +++++++++++++++ src/conf/viremulator_capabilities.h | 47 ++++++ src/driver.h | 7 + src/libvirt.c | 52 ++++++ src/libvirt_private.syms | 9 + src/libvirt_public.syms | 2 + src/qemu/qemu_capabilities.c | 78 +++++---- src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_capabilitiespriv.h | 55 ++++++ src/qemu/qemu_driver.c | 96 ++++++++++- src/qemu/qemu_hostdev.c | 76 +-------- src/qemu/qemu_hostdev.h | 2 - src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 19 ++- src/remote_protocol-structs | 10 ++ src/util/virhostdev.c | 73 ++++++++ src/util/virhostdev.h | 4 + tests/Makefile.am | 28 ++- .../viremulatorcaps-basic.xml | 5 + .../viremulatorcaps-qemu-kvm-vfio.xml | 17 ++ tests/viremulatorcapabilitiesschematest | 11 ++ tests/viremulatorcapabilitiestest.c | 187 +++++++++++++++++++++ tests/virhostdevmock.c | 40 +++++ tools/virsh-host.c | 74 ++++++++ tools/virsh.pod | 13 ++ 33 files changed, 1143 insertions(+), 112 deletions(-) create mode 100644 docs/formatemulatorcaps.html.in create mode 100644 docs/schemas/emulatorcapability.rng create mode 100644 src/conf/viremulator_capabilities.c create mode 100644 src/conf/viremulator_capabilities.h create mode 100644 src/qemu/qemu_capabilitiespriv.h create mode 100644 tests/viremulatorcapabilitiesdata/viremulatorcaps-basic.xml create mode 100644 tests/viremulatorcapabilitiesdata/viremulatorcaps-qemu-kvm-vfio.xml create mode 100755 tests/viremulatorcapabilitiesschematest create mode 100644 tests/viremulatorcapabilitiestest.c create mode 100644 tests/virhostdevmock.c -- 1.8.5.5
The functions in question are qemuHostdevHostSupportsPassthroughLegacy and qemuHostdevHostSupportsPassthroughVFIO. At the same time the functions are renamed to match 'vir' prefix instead of 'qemu' one. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 2 ++ src/qemu/qemu_driver.c | 4 +-- src/qemu/qemu_hostdev.c | 76 ++---------------------------------------------- src/qemu/qemu_hostdev.h | 2 -- src/util/virhostdev.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virhostdev.h | 4 +++ 6 files changed, 83 insertions(+), 78 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ac56782..1d29f15 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1345,6 +1345,8 @@ virHookPresent; # util/virhostdev.h +virHostdevHostSupportsPassthroughLegacy; +virHostdevHostSupportsPassthroughVFIO; virHostdevManagerGetDefault; virHostdevPCINodeDeviceDetach; virHostdevPCINodeDeviceReAttach; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 22a8ca5..ac6aee5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11355,8 +11355,8 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev, int ret = -1; virNodeDeviceDefPtr def = NULL; char *xml = NULL; - bool legacy = qemuHostdevHostSupportsPassthroughLegacy(); - bool vfio = qemuHostdevHostSupportsPassthroughVFIO(); + bool legacy = virHostdevHostSupportsPassthroughLegacy(); + bool vfio = virHostdevHostSupportsPassthroughVFIO(); virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; virCheckFlags(0, -1); diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 706db0c..39dacb2 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -23,7 +23,6 @@ #include <config.h> -#include <dirent.h> #include <fcntl.h> #include <sys/ioctl.h> #include <errno.h> @@ -84,84 +83,13 @@ qemuUpdateActiveSCSIHostdevs(virQEMUDriverPtr driver, } -bool -qemuHostdevHostSupportsPassthroughVFIO(void) -{ - DIR *iommuDir = NULL; - struct dirent *iommuGroup = NULL; - bool ret = false; - int direrr; - - /* condition 1 - /sys/kernel/iommu_groups/ contains entries */ - if (!(iommuDir = opendir("/sys/kernel/iommu_groups/"))) - goto cleanup; - - while ((direrr = virDirRead(iommuDir, &iommuGroup, NULL)) > 0) { - /* skip ./ ../ */ - if (STRPREFIX(iommuGroup->d_name, ".")) - continue; - - /* assume we found a group */ - break; - } - - if (direrr < 0 || !iommuGroup) - goto cleanup; - /* okay, iommu is on and recognizes groups */ - - /* condition 2 - /dev/vfio/vfio exists */ - if (!virFileExists("/dev/vfio/vfio")) - goto cleanup; - - ret = true; - - cleanup: - if (iommuDir) - closedir(iommuDir); - - return ret; -} - - -#if HAVE_LINUX_KVM_H -# include <linux/kvm.h> -bool -qemuHostdevHostSupportsPassthroughLegacy(void) -{ - int kvmfd = -1; - bool ret = false; - - if ((kvmfd = open("/dev/kvm", O_RDONLY)) < 0) - goto cleanup; - -# ifdef KVM_CAP_IOMMU - if ((ioctl(kvmfd, KVM_CHECK_EXTENSION, KVM_CAP_IOMMU)) <= 0) - goto cleanup; - - ret = true; -# endif - - cleanup: - VIR_FORCE_CLOSE(kvmfd); - - return ret; -} -#else -bool -qemuHostdevHostSupportsPassthroughLegacy(void) -{ - return false; -} -#endif - - static bool qemuPrepareHostdevPCICheckSupport(virDomainHostdevDefPtr *hostdevs, size_t nhostdevs, virQEMUCapsPtr qemuCaps) { - bool supportsPassthroughKVM = qemuHostdevHostSupportsPassthroughLegacy(); - bool supportsPassthroughVFIO = qemuHostdevHostSupportsPassthroughVFIO(); + bool supportsPassthroughKVM = virHostdevHostSupportsPassthroughLegacy(); + bool supportsPassthroughVFIO = virHostdevHostSupportsPassthroughVFIO(); size_t i; /* assign defaults for hostdev passthrough */ diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h index 05bd965..75cec9c 100644 --- a/src/qemu/qemu_hostdev.h +++ b/src/qemu/qemu_hostdev.h @@ -33,8 +33,6 @@ int qemuUpdateActiveUSBHostdevs(virQEMUDriverPtr driver, virDomainDefPtr def); int qemuUpdateActiveSCSIHostdevs(virQEMUDriverPtr driver, virDomainDefPtr def); -bool qemuHostdevHostSupportsPassthroughLegacy(void); -bool qemuHostdevHostSupportsPassthroughVFIO(void); int qemuPrepareHostdevPCIDevices(virQEMUDriverPtr driver, const char *name, const unsigned char *uuid, diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 9dd1df2..f791a10 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -24,6 +24,7 @@ #include <config.h> +#include <dirent.h> #include <fcntl.h> #include <sys/ioctl.h> #include <sys/types.h> @@ -32,6 +33,10 @@ #include <stdlib.h> #include <stdio.h> +#if HAVE_LINUX_KVM_H +# include <linux/kvm.h> +#endif + #include "virhostdev.h" #include "viralloc.h" #include "virstring.h" @@ -1635,3 +1640,71 @@ virHostdevUpdateDomainActiveDevices(virHostdevManagerPtr mgr, return 0; } + +#if HAVE_LINUX_KVM_H +bool +virHostdevHostSupportsPassthroughLegacy(void) +{ + int kvmfd = -1; + bool ret = false; + + if ((kvmfd = open("/dev/kvm", O_RDONLY)) < 0) + goto cleanup; + +# ifdef KVM_CAP_IOMMU + if ((ioctl(kvmfd, KVM_CHECK_EXTENSION, KVM_CAP_IOMMU)) <= 0) + goto cleanup; + + ret = true; +# endif + + cleanup: + VIR_FORCE_CLOSE(kvmfd); + + return ret; +} +#else +bool +virHostdevHostSupportsPassthroughLegacy(void) +{ + return false; +} +#endif + +bool +virHostdevHostSupportsPassthroughVFIO(void) +{ + DIR *iommuDir = NULL; + struct dirent *iommuGroup = NULL; + bool ret = false; + int direrr; + + /* condition 1 - /sys/kernel/iommu_groups/ contains entries */ + if (!(iommuDir = opendir("/sys/kernel/iommu_groups/"))) + goto cleanup; + + while ((direrr = virDirRead(iommuDir, &iommuGroup, NULL)) > 0) { + /* skip ./ ../ */ + if (STRPREFIX(iommuGroup->d_name, ".")) + continue; + + /* assume we found a group */ + break; + } + + if (direrr < 0 || !iommuGroup) + goto cleanup; + /* okay, iommu is on and recognizes groups */ + + /* condition 2 - /dev/vfio/vfio exists */ + if (!virFileExists("/dev/vfio/vfio")) + goto cleanup; + + ret = true; + + cleanup: + if (iommuDir) + closedir(iommuDir); + + return ret; +} diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h index 2036430..99640f5 100644 --- a/src/util/virhostdev.h +++ b/src/util/virhostdev.h @@ -155,4 +155,8 @@ int virHostdevPCINodeDeviceReset(virHostdevManagerPtr mgr, virPCIDevicePtr pci) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +/* KVM related functions */ +bool virHostdevHostSupportsPassthroughLegacy(void); +bool virHostdevHostSupportsPassthroughVFIO(void); + #endif /* __VIR_HOSTDEV_H__ */ -- 1.8.5.5
On Fri, Jun 20, 2014 at 04:19:06PM +0200, Michal Privoznik wrote:
The functions in question are qemuHostdevHostSupportsPassthroughLegacy and qemuHostdevHostSupportsPassthroughVFIO. At the same time the functions are renamed to match 'vir' prefix instead of 'qemu' one.
I'm not sure I see the reason for moving these functions ? They're QEMU specific surely ? eg
@@ -1635,3 +1640,71 @@ virHostdevUpdateDomainActiveDevices(virHostdevManagerPtr mgr,
return 0; } + +#if HAVE_LINUX_KVM_H +bool +virHostdevHostSupportsPassthroughLegacy(void) +{ + int kvmfd = -1; + bool ret = false; + + if ((kvmfd = open("/dev/kvm", O_RDONLY)) < 0) + goto cleanup;
is going to be useless for anything except QEMU driver. 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 :|
On 23.06.2014 16:05, Daniel P. Berrange wrote:
On Fri, Jun 20, 2014 at 04:19:06PM +0200, Michal Privoznik wrote:
The functions in question are qemuHostdevHostSupportsPassthroughLegacy and qemuHostdevHostSupportsPassthroughVFIO. At the same time the functions are renamed to match 'vir' prefix instead of 'qemu' one.
I'm not sure I see the reason for moving these functions ? They're QEMU specific surely ?
eg
@@ -1635,3 +1640,71 @@ virHostdevUpdateDomainActiveDevices(virHostdevManagerPtr mgr,
return 0; } + +#if HAVE_LINUX_KVM_H +bool +virHostdevHostSupportsPassthroughLegacy(void) +{ + int kvmfd = -1; + bool ret = false; + + if ((kvmfd = open("/dev/kvm", O_RDONLY)) < 0) + goto cleanup;
is going to be useless for anything except QEMU driver.
Regards, Daniel
Well, I thought other drivers might use it in the future, but the patch set can live without this patch. I'm fine with dropping it. Michal
The virEmulatorCapabilities is going to hold emulator capabilities, surprisingly. It's intended to be able to cover qemuCaps, lxcCaps (once we invent them, if ever) and so on. Among with adding the code itself, both some documentation and basic testing is introduced too. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatemulatorcaps.html.in | 52 ++++++++ docs/schemas/Makefile.am | 1 + docs/schemas/emulatorcapability.rng | 26 ++++ docs/sitemap.html.in | 4 + libvirt.spec.in | 1 + mingw-libvirt.spec.in | 2 + src/Makefile.am | 3 +- src/conf/viremulator_capabilities.c | 139 +++++++++++++++++++++ src/conf/viremulator_capabilities.h | 47 +++++++ src/libvirt_private.syms | 6 + tests/Makefile.am | 10 +- .../viremulatorcaps-basic.xml | 5 + tests/viremulatorcapabilitiesschematest | 11 ++ tests/viremulatorcapabilitiestest.c | 117 +++++++++++++++++ 14 files changed, 422 insertions(+), 2 deletions(-) create mode 100644 docs/formatemulatorcaps.html.in create mode 100644 docs/schemas/emulatorcapability.rng create mode 100644 src/conf/viremulator_capabilities.c create mode 100644 src/conf/viremulator_capabilities.h create mode 100644 tests/viremulatorcapabilitiesdata/viremulatorcaps-basic.xml create mode 100755 tests/viremulatorcapabilitiesschematest create mode 100644 tests/viremulatorcapabilitiestest.c diff --git a/docs/formatemulatorcaps.html.in b/docs/formatemulatorcaps.html.in new file mode 100644 index 0000000..beea1a9 --- /dev/null +++ b/docs/formatemulatorcaps.html.in @@ -0,0 +1,52 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> +<html xmlns="http://www.w3.org/1999/xhtml"> + <body> + <h1>Emulator capabilities XML format</h1> + + <ul id="toc"></ul> + + <h2><a name="Motivation">Motivation</a></h2> + + <p>Sometimes, when a new domain is to be created it may come handy to know + the capabilities of the hypervisor so the correct combination of devices and + drivers is used. For example, when management application is considering the + mode for a host device's passthrough there are several options depending not + only on host, but on hypervisor in question too. If the hypervisor is qemu + then it needs to be more recent to support VFIO, while legacy KVM is + achievable just fine with older one.</p> + + <p>The main difference between <a + href="formatcaps.html">virConnectGetCapabilities</a> and the emulator + capabilities API is, the former one aims more on the host capabilities (e.g. + NUMA topology, security models in effect, etc.) while the latter one + specializes on the hypervisor capabilities.</p> + + <h2><a name="elements">Element and attribute overview</a></h2> + + <p>The root element that emulator capability XML document starts with has + name <code>emulatorCapabilities</code>. It contains at least three direct + child elements:</p> + +<pre> +<emulatorCapabilities> + <path>/usr/bin/qemu-system-x86_64</path> + <domain>kvm</domain> + <machine>pc-i440fx-2.1</machine> + ... +</emulatorCapabilities> +</pre> + <dl> + <dt>path</dt> + <dd>The full path to the emulator binary.</dd> + + <dt>domain</dt> + <dd>Describes the <a href="formatdomain.html#elements">virtualization + type</a> (or so called domain type).</dd> + + <dt>machine</dt> + <dd>The domain's <a href="formatdomain.html#elementsOSBIOS">machine + type</a>.</dd> + </dl> + </body> +</html> diff --git a/docs/schemas/Makefile.am b/docs/schemas/Makefile.am index d71c327..3072f4f 100644 --- a/docs/schemas/Makefile.am +++ b/docs/schemas/Makefile.am @@ -21,6 +21,7 @@ schema_DATA = \ domain.rng \ domaincommon.rng \ domainsnapshot.rng \ + emulatorcapability.rng \ interface.rng \ network.rng \ networkcommon.rng \ diff --git a/docs/schemas/emulatorcapability.rng b/docs/schemas/emulatorcapability.rng new file mode 100644 index 0000000..2548cef --- /dev/null +++ b/docs/schemas/emulatorcapability.rng @@ -0,0 +1,26 @@ +<?xml version="1.0"?> +<!-- A Relax NG schema for the libvirt capabilities XML format --> +<grammar xmlns="http://relaxng.org/ns/structure/1.0" + datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + <include href='basictypes.rng'/> + <start> + <ref name='emulatorCapabilities'/> + </start> + + + <define name='emulatorCapabilities'> + <element name='emulatorCapabilities'> + <interleave> + <element name='path'> + <ref name="absFilePath"/> + </element> + <element name='domain'> + <text/> + </element> + <element name='machine'> + <text/> + </element> + </interleave> + </element> + </define> +</grammar> diff --git a/docs/sitemap.html.in b/docs/sitemap.html.in index 78e84e3..6ed4e4d 100644 --- a/docs/sitemap.html.in +++ b/docs/sitemap.html.in @@ -175,6 +175,10 @@ <span>The driver capabilities XML format</span> </li> <li> + <a href="formatemulatorcaps.html">Emulator capabilities</a> + <span>The emulator capabilities XML format</span> + </li> + <li> <a href="formatnode.html">Node Devices</a> <span>The host device XML format</span> </li> diff --git a/libvirt.spec.in b/libvirt.spec.in index 344748c..2545503 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2164,6 +2164,7 @@ exit 0 %{_datadir}/libvirt/schemas/domain.rng %{_datadir}/libvirt/schemas/domaincommon.rng %{_datadir}/libvirt/schemas/domainsnapshot.rng +%{_datadir}/libvirt/schemas/emulatorcapability.rng %{_datadir}/libvirt/schemas/interface.rng %{_datadir}/libvirt/schemas/network.rng %{_datadir}/libvirt/schemas/networkcommon.rng diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in index 1b505e6..ca1db40 100644 --- a/mingw-libvirt.spec.in +++ b/mingw-libvirt.spec.in @@ -205,6 +205,7 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw32_datadir}/libvirt/schemas/domain.rng %{mingw32_datadir}/libvirt/schemas/domaincommon.rng %{mingw32_datadir}/libvirt/schemas/domainsnapshot.rng +%{mingw32_datadir}/libvirt/schemas/emulatorcapability.rng %{mingw32_datadir}/libvirt/schemas/interface.rng %{mingw32_datadir}/libvirt/schemas/network.rng %{mingw32_datadir}/libvirt/schemas/networkcommon.rng @@ -265,6 +266,7 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw64_datadir}/libvirt/schemas/domain.rng %{mingw64_datadir}/libvirt/schemas/domaincommon.rng %{mingw64_datadir}/libvirt/schemas/domainsnapshot.rng +%{mingw64_datadir}/libvirt/schemas/emulatorcapability.rng %{mingw64_datadir}/libvirt/schemas/interface.rng %{mingw64_datadir}/libvirt/schemas/network.rng %{mingw64_datadir}/libvirt/schemas/networkcommon.rng diff --git a/src/Makefile.am b/src/Makefile.am index 2b9ac61..9db896d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -251,7 +251,8 @@ DOMAIN_CONF_SOURCES = \ conf/domain_conf.c conf/domain_conf.h \ conf/domain_audit.c conf/domain_audit.h \ conf/domain_nwfilter.c conf/domain_nwfilter.h \ - conf/snapshot_conf.c conf/snapshot_conf.h + conf/snapshot_conf.c conf/snapshot_conf.h \ + conf/viremulator_capabilities.c conf/viremulator_capabilities.h OBJECT_EVENT_SOURCES = \ conf/object_event.c conf/object_event.h \ diff --git a/src/conf/viremulator_capabilities.c b/src/conf/viremulator_capabilities.c new file mode 100644 index 0000000..8e7d4af --- /dev/null +++ b/src/conf/viremulator_capabilities.c @@ -0,0 +1,139 @@ +/* + * viremulator_capabilities.c: hypervisor capabilities + * + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Michal Privoznik <mprivozn@redhat.com> + */ + +#include <config.h> + +#include "viremulator_capabilities.h" +#include "virobject.h" +#include "viralloc.h" +#include "virbuffer.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_CAPABILITIES + +struct _virEmulatorCapabilities { + virObjectLockable parent; + + char *path; /* path to emulator binary */ + char *machine; /* machine type */ + virDomainVirtType virttype; /* virtualization type */ + + void *privateData; + virEmulatorCapabilitiesPrivateDataFormatFunc formatFunc; +}; + +static virClassPtr virEmulatorCapabilitiesClass; + +static void virEmulatorCapabilitiesDispose(void *obj); + +static int virEmulatorCapabilitiesOnceInit(void) +{ + if (!(virEmulatorCapabilitiesClass = virClassNew(virClassForObjectLockable(), + "virEmulatorCapabilities", + sizeof(virEmulatorCapabilities), + virEmulatorCapabilitiesDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virEmulatorCapabilities) + + +static void +virEmulatorCapabilitiesDispose(void *obj) +{ + virEmulatorCapabilitiesPtr caps = obj; + + VIR_FREE(caps->path); + VIR_FREE(caps->machine); +} + + +virEmulatorCapabilitiesPtr +virEmulatorCapabilitiesNew(const char *path, + const char *machine, + virDomainVirtType virttype) +{ + virEmulatorCapabilitiesPtr caps = NULL; + + if (virEmulatorCapabilitiesInitialize() < 0) + return NULL; + + if (!(caps = virObjectLockableNew(virEmulatorCapabilitiesClass))) + return NULL; + + if (VIR_STRDUP(caps->path, path) < 0 || + VIR_STRDUP(caps->machine, machine) < 0) + goto error; + caps->virttype = virttype; + + return caps; + error: + virObjectUnref(caps); + return NULL; +} + + +void +virEmulatorCapabilitiesSetPrivate(virEmulatorCapabilitiesPtr caps, + void *privateData, + virEmulatorCapabilitiesPrivateDataFormatFunc formatFunc) +{ + if (!caps) + return; + + caps->privateData = privateData; + caps->formatFunc = formatFunc; +} + + +static int +virEmulatorCapabilitiesFormatInternal(virEmulatorCapabilitiesPtr caps, + virBufferPtr buf) +{ + const char *virttype_str = virDomainVirtTypeToString(caps->virttype); + + virBufferAddLit(buf, "<emulatorCapabilities>\n"); + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<path>%s</path>\n", caps->path); + virBufferAsprintf(buf, "<domain>%s</domain>\n", virttype_str); + virBufferAsprintf(buf, "<machine>%s</machine>\n", caps->machine); + if (caps->formatFunc) + caps->formatFunc(buf, caps->privateData, caps->machine); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</emulatorCapabilities>\n"); + return 0; +} + + +char * +virEmulatorCapabilitiesFormat(virEmulatorCapabilitiesPtr caps) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (virEmulatorCapabilitiesFormatInternal(caps, &buf) < 0) + return NULL; + + return virBufferContentAndReset(&buf); + +} diff --git a/src/conf/viremulator_capabilities.h b/src/conf/viremulator_capabilities.h new file mode 100644 index 0000000..361f765 --- /dev/null +++ b/src/conf/viremulator_capabilities.h @@ -0,0 +1,47 @@ +/* + * viremulator_capabilities.h: hypervisor capabilities + * + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Michal Privoznik <mprivozn@redhat.com> + */ + +#ifndef __VIR_EMULATOR_CAPABILITIES_H +# define __VIR_EMULATOR_CAPABILITIES_H + +# include "domain_conf.h" + +typedef int +(*virEmulatorCapabilitiesPrivateDataFormatFunc)(virBufferPtr buf, + void *privateData, + const char *machine); + +typedef struct _virEmulatorCapabilities virEmulatorCapabilities; +typedef virEmulatorCapabilities *virEmulatorCapabilitiesPtr; + +virEmulatorCapabilitiesPtr +virEmulatorCapabilitiesNew(const char *path, + const char *machine, + virDomainVirtType virttype); + +void +virEmulatorCapabilitiesSetPrivate(virEmulatorCapabilitiesPtr caps, + void *privateData, + virEmulatorCapabilitiesPrivateDataFormatFunc formatFunc); + +char *virEmulatorCapabilitiesFormat(virEmulatorCapabilitiesPtr caps); +#endif diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1d29f15..2784610 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -759,6 +759,12 @@ virChrdevFree; virChrdevOpen; +# conf/viremulator_capabilities.h +virEmulatorCapabilitiesFormat; +virEmulatorCapabilitiesNew; +virEmulatorCapabilitiesSetPrivate; + + # cpu/cpu.h cpuBaseline; cpuBaselineXML; diff --git a/tests/Makefile.am b/tests/Makefile.am index c999061..e7d9f94 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -125,6 +125,8 @@ EXTRA_DIST = \ sysinfodata \ test-lib.sh \ vircaps2xmldata \ + viremulatorcapabilitiesdata\ + viremulatorcapabilitiesschematest \ vboxsnapshotxmldata \ virsh-uriprecedence \ virfiledata \ @@ -167,6 +169,7 @@ test_programs = virshtest sockettest \ virnetdevbandwidthtest \ virkmodtest \ vircapstest \ + viremulatorcapabilitiestest \ domainconftest \ virhostdevtest \ vircaps2xmltest \ @@ -319,7 +322,8 @@ test_scripts = \ nodedevschematest \ nwfilterschematest \ domainsnapshotschematest \ - secretschematest + secretschematest \ + viremulatorcapabilitiesschematest if WITH_LIBVIRTD test_scripts += \ @@ -824,6 +828,10 @@ vircaps2xmltest_SOURCES = \ vircaps2xmltest.c testutils.h testutils.c vircaps2xmltest_LDADD = $(LDADDS) +viremulatorcapabilitiestest_SOURCES = \ + viremulatorcapabilitiestest.c testutils.h testutils.c +viremulatorcapabilitiestest_LDADD = $(LDADDS) + if WITH_LIBVIRTD libvirtdconftest_SOURCES = \ libvirtdconftest.c testutils.h testutils.c \ diff --git a/tests/viremulatorcapabilitiesdata/viremulatorcaps-basic.xml b/tests/viremulatorcapabilitiesdata/viremulatorcaps-basic.xml new file mode 100644 index 0000000..a3fd457 --- /dev/null +++ b/tests/viremulatorcapabilitiesdata/viremulatorcaps-basic.xml @@ -0,0 +1,5 @@ +<emulatorCapabilities> + <path>/bin/emulatorbin</path> + <domain>uml</domain> + <machine>my-machine-type</machine> +</emulatorCapabilities> diff --git a/tests/viremulatorcapabilitiesschematest b/tests/viremulatorcapabilitiesschematest new file mode 100755 index 0000000..e4dd621 --- /dev/null +++ b/tests/viremulatorcapabilitiesschematest @@ -0,0 +1,11 @@ +#!/bin/sh + +: ${srcdir=.} +. $srcdir/test-lib.sh +. $abs_srcdir/schematestutils.sh + +DIRS="" +DIRS="$DIRS viremulatorcapabilitiesdata" +SCHEMA="emulatorcapability.rng" + +check_schema "$DIRS" "$SCHEMA" diff --git a/tests/viremulatorcapabilitiestest.c b/tests/viremulatorcapabilitiestest.c new file mode 100644 index 0000000..448f0cf --- /dev/null +++ b/tests/viremulatorcapabilitiestest.c @@ -0,0 +1,117 @@ +/* + * Copyright (C) Red Hat, Inc. 2014 + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Michal Privoznik <mprivozn@redhat.com> + */ + +#include <config.h> +#include <stdlib.h> + +#include "testutils.h" +#include "viremulator_capabilities.h" + + +#define VIR_FROM_THIS VIR_FROM_NONE + +static virEmulatorCapabilitiesPtr +buildVirEmulatorCapabilities(const char *emulatorbin, + const char *machine, + virDomainVirtType type, + void *privateData, + virEmulatorCapabilitiesPrivateDataFormatFunc formatFunc) +{ + virEmulatorCapabilitiesPtr emulCaps; + + if (!(emulCaps = virEmulatorCapabilitiesNew(emulatorbin, machine, type))) + goto cleanup; + + if (privateData) + virEmulatorCapabilitiesSetPrivate(emulCaps, privateData, formatFunc); + + cleanup: + return emulCaps; +} + +struct test_virEmulatorCapabilitiesFormatData { + const char *filename; + const char *emulatorbin; + const char *machine; + virDomainVirtType type; + void *privateData; + virEmulatorCapabilitiesPrivateDataFormatFunc formatFunc; +}; + +static int +test_virEmulatorCapabilitiesFormat(const void *opaque) +{ + struct test_virEmulatorCapabilitiesFormatData *data = + (struct test_virEmulatorCapabilitiesFormatData *) opaque; + virEmulatorCapabilitiesPtr emulCaps = NULL; + char *path = NULL; + char *emulCapsXML = NULL; + char *emulCapsFromFile = NULL; + int ret = -1; + + if (virAsprintf(&path, "%s/viremulatorcapabilitiesdata/viremulatorcaps-%s.xml", + abs_srcdir, data->filename) < 0) + goto cleanup; + + if (virFileReadAll(path, 8192, &emulCapsFromFile) < 0) + goto cleanup; + + if (!(emulCaps = buildVirEmulatorCapabilities(data->emulatorbin, data->machine, + data->type, data->privateData, + data->formatFunc))) + goto cleanup; + + if (!(emulCapsXML = virEmulatorCapabilitiesFormat(emulCaps))) + goto cleanup; + + if (STRNEQ(emulCapsFromFile, emulCapsXML)) { + virtTestDifference(stderr, emulCapsFromFile, emulCapsXML); + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(emulCapsFromFile); + VIR_FREE(emulCapsXML); + VIR_FREE(path); + virObjectUnref(emulCaps); + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + +#define DO_TEST(Filename, Emulatorbin, Machine, Type, ...) \ + do { \ + struct test_virEmulatorCapabilitiesFormatData data = {.filename = Filename, \ + .emulatorbin = Emulatorbin, .machine = Machine, .type = Type, __VA_ARGS__}; \ + if (virtTestRun(Filename, test_virEmulatorCapabilitiesFormat, &data) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST("basic", "/bin/emulatorbin", "my-machine-type", VIR_DOMAIN_VIRT_UML); + + return ret; +} + +VIRT_TEST_MAIN(mymain) -- 1.8.5.5
On Fri, Jun 20, 2014 at 04:19:07PM +0200, Michal Privoznik wrote:
--- docs/formatemulatorcaps.html.in | 52 ++++++++ docs/schemas/Makefile.am | 1 + docs/schemas/emulatorcapability.rng | 26 ++++ docs/sitemap.html.in | 4 + libvirt.spec.in | 1 + mingw-libvirt.spec.in | 2 + src/Makefile.am | 3 +- src/conf/viremulator_capabilities.c | 139 +++++++++++++++++++++ src/conf/viremulator_capabilities.h | 47 +++++++ src/libvirt_private.syms | 6 + tests/Makefile.am | 10 +- .../viremulatorcaps-basic.xml | 5 + tests/viremulatorcapabilitiesschematest | 11 ++ tests/viremulatorcapabilitiestest.c | 117 +++++++++++++++++ 14 files changed, 422 insertions(+), 2 deletions(-) create mode 100644 docs/formatemulatorcaps.html.in create mode 100644 docs/schemas/emulatorcapability.rng create mode 100644 src/conf/viremulator_capabilities.c create mode 100644 src/conf/viremulator_capabilities.h create mode 100644 tests/viremulatorcapabilitiesdata/viremulatorcaps-basic.xml create mode 100755 tests/viremulatorcapabilitiesschematest create mode 100644 tests/viremulatorcapabilitiestest.c
As a nitpick on naming I think I'd call this 'domain capabilities' since it is about representing metadata about stuff you can put in the domain conf XML schema. so eg src/conf/domain_capabilties.{c.h} and virDomainCapsPtr for struct.
diff --git a/src/conf/viremulator_capabilities.c b/src/conf/viremulator_capabilities.c new file mode 100644 index 0000000..8e7d4af --- /dev/null +++ b/src/conf/viremulator_capabilities.c @@ -0,0 +1,139 @@ +/* + * viremulator_capabilities.c: hypervisor capabilities + * + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Michal Privoznik <mprivozn@redhat.com> + */ + +#include <config.h> + +#include "viremulator_capabilities.h" +#include "virobject.h" +#include "viralloc.h" +#include "virbuffer.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_CAPABILITIES + +struct _virEmulatorCapabilities { + virObjectLockable parent; + + char *path; /* path to emulator binary */ + char *machine; /* machine type */ + virDomainVirtType virttype; /* virtualization type */ + + void *privateData; + virEmulatorCapabilitiesPrivateDataFormatFunc formatFunc; +};
+static int +virEmulatorCapabilitiesFormatInternal(virEmulatorCapabilitiesPtr caps, + virBufferPtr buf) +{ + const char *virttype_str = virDomainVirtTypeToString(caps->virttype); + + virBufferAddLit(buf, "<emulatorCapabilities>\n"); + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<path>%s</path>\n", caps->path); + virBufferAsprintf(buf, "<domain>%s</domain>\n", virttype_str); + virBufferAsprintf(buf, "<machine>%s</machine>\n", caps->machine); + if (caps->formatFunc) + caps->formatFunc(buf, caps->privateData, caps->machine);
I'm really not a fan of this - it lets every virt driver in libvirt invent its own thing to put in these capabilities, which has bitten us in the past, since we end up with divergence between drivers I think we need to spec out some approach to representing the data in this struct which drivers populate. To start with we're looking for info on what enum values are supported in each device type. This should probably be preface with a way to express what device types are actually supported by the driver. eg so you can discover that <channel> is not supported by Xen or LXC. How about something like this: typedef const char * (*virDomainCapsEnumFormat)(int va;ue); struct _virDomainCapsEnum { int values; /* Bitmask of values supported in the corresponding enum */ }; struct _virDomainCapsDevice { bool supported; /* true if <devtype> is supported by hypervisor */ }; struct _virDomainCapsDeviceDisk { struct virDomainCapsDevice; virDomainCapsEnum device; /* Info about virDomainDiskDevice enum values */ virDomainCapsEnum bus /* Info about virDomainDiskBus enum values */ ...rest of enums used in virDomainDiskDef or anything it references... }; struct _virDomainCapsDeviceHostdev { struct virDomainCapsDevice; virDomainCapsEnum mode; /* Info about virDomainHostdevMode */ virDomainCapsEnum startupPolicy; /* Info about virDomainStartupPolicy */ virDomainCapsEnum subsysType; /* Info about virDomainHostdevSubsysType */ virDomainCapsEnum capsType; /* Info about virDomainHostdevCapsType */ virDomainCapsEnum pciBackend; /* Info about virDomainHostdevSubsysPCIBackend */ }; And then in virEmulatorCapabilities (well virDomainCaps) have virDomainCaps { ... virDomainCapsDeviceDisk disk; virDomainCapsDeviceHostdev hostdev; ...other device types... } The virt drivers would then have todo something like this: caps.disk.supported = true; caps.disk.bus.values = (1 << VIR_DOMAIN_DISK_BUS_IDE) | (1 << VIR_DOMAIN_DISK_BUS_FDC) | (1 << VIR_DOMAIN_DISK_BUS_VIRTIO); caps.hostdev.supported = true; caps.hostdev.mode.values = (1 << VIR_HOSTDEV_MODE_SUBSYS) | (1 << VIR_HOSTDEV_MODE_CAPS); ...etc... The domain caps code would use the ToString functions from the relevant VIR_ENUMs to turn the bits in the 'values' into strings when generating the XML output. So now any time a virt driver decides to support a new device type, as well as doing its XML -> ARGV code, it should set the flag 'caps.<devtype>.supported = true' and populate the relevant enum information. 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 :|
On 06/20/14 16:19, Michal Privoznik wrote:
The virEmulatorCapabilities is going to hold emulator capabilities, surprisingly. It's intended to be able to cover qemuCaps, lxcCaps (once we invent them, if ever) and so on. Among with adding the code itself, both some documentation and basic testing is introduced too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
...
diff --git a/docs/formatemulatorcaps.html.in b/docs/formatemulatorcaps.html.in new file mode 100644 index 0000000..beea1a9 --- /dev/null +++ b/docs/formatemulatorcaps.html.in @@ -0,0 +1,52 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> +<html xmlns="http://www.w3.org/1999/xhtml"> + <body> + <h1>Emulator capabilities XML format</h1> + + <ul id="toc"></ul> + + <h2><a name="Motivation">Motivation</a></h2> + + <p>Sometimes, when a new domain is to be created it may come handy to know + the capabilities of the hypervisor so the correct combination of devices and + drivers is used. For example, when management application is considering the + mode for a host device's passthrough there are several options depending not + only on host, but on hypervisor in question too. If the hypervisor is qemu + then it needs to be more recent to support VFIO, while legacy KVM is + achievable just fine with older one.</p> + + <p>The main difference between <a + href="formatcaps.html">virConnectGetCapabilities</a> and the emulator + capabilities API is, the former one aims more on the host capabilities (e.g. + NUMA topology, security models in effect, etc.) while the latter one + specializes on the hypervisor capabilities.</p> + + <h2><a name="elements">Element and attribute overview</a></h2> + + <p>The root element that emulator capability XML document starts with has + name <code>emulatorCapabilities</code>. It contains at least three direct + child elements:</p>
We also have a <features> subelement of <guest> in the <capabilities> XML which is used for a similar thing although it doesn't support a per-machine-type output, only per-binary capabilities. Should we add this more granular approach and abandon the old one? Peter
+ +<pre> +<emulatorCapabilities> + <path>/usr/bin/qemu-system-x86_64</path> + <domain>kvm</domain> + <machine>pc-i440fx-2.1</machine> + ... +</emulatorCapabilities> +</pre> + <dl> + <dt>path</dt> + <dd>The full path to the emulator binary.</dd> + + <dt>domain</dt> + <dd>Describes the <a href="formatdomain.html#elements">virtualization + type</a> (or so called domain type).</dd> + + <dt>machine</dt> + <dd>The domain's <a href="formatdomain.html#elementsOSBIOS">machine + type</a>.</dd> + </dl> + </body> +</html>
On Tue, Jun 24, 2014 at 09:03:51AM +0200, Peter Krempa wrote:
On 06/20/14 16:19, Michal Privoznik wrote:
The virEmulatorCapabilities is going to hold emulator capabilities, surprisingly. It's intended to be able to cover qemuCaps, lxcCaps (once we invent them, if ever) and so on. Among with adding the code itself, both some documentation and basic testing is introduced too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
...
diff --git a/docs/formatemulatorcaps.html.in b/docs/formatemulatorcaps.html.in new file mode 100644 index 0000000..beea1a9 --- /dev/null +++ b/docs/formatemulatorcaps.html.in @@ -0,0 +1,52 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> +<html xmlns="http://www.w3.org/1999/xhtml"> + <body> + <h1>Emulator capabilities XML format</h1> + + <ul id="toc"></ul> + + <h2><a name="Motivation">Motivation</a></h2> + + <p>Sometimes, when a new domain is to be created it may come handy to know + the capabilities of the hypervisor so the correct combination of devices and + drivers is used. For example, when management application is considering the + mode for a host device's passthrough there are several options depending not + only on host, but on hypervisor in question too. If the hypervisor is qemu + then it needs to be more recent to support VFIO, while legacy KVM is + achievable just fine with older one.</p> + + <p>The main difference between <a + href="formatcaps.html">virConnectGetCapabilities</a> and the emulator + capabilities API is, the former one aims more on the host capabilities (e.g. + NUMA topology, security models in effect, etc.) while the latter one + specializes on the hypervisor capabilities.</p> + + <h2><a name="elements">Element and attribute overview</a></h2> + + <p>The root element that emulator capability XML document starts with has + name <code>emulatorCapabilities</code>. It contains at least three direct + child elements:</p>
We also have a <features> subelement of <guest> in the <capabilities> XML which is used for a similar thing although it doesn't support a per-machine-type output, only per-binary capabilities. Should we add this more granular approach and abandon the old one?
Yes, we should stop adding stuff related to the guest to the main <capabilities> XML since it doesn't scale. 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 :|
On 06/24/2014 03:39 AM, Daniel P. Berrange wrote:
We also have a <features> subelement of <guest> in the <capabilities> XML which is used for a similar thing although it doesn't support a per-machine-type output, only per-binary capabilities. Should we add this more granular approach and abandon the old one?
Yes, we should stop adding stuff related to the guest to the main <capabilities> XML since it doesn't scale.
Oh phooey - I just proposed yet another feature there: https://www.redhat.com/archives/libvir-list/2014-June/msg01097.html I'd like to turn on a witness for active commit support in the same release as we turn on the qemu implementation (and I'm hoping it still makes libvirt 1.2.6 - we haven't frozen yet, but it's near the end of the month, and we're still waiting on some patches to make it into qemu.git). If <features> is not the right place, then where should I advertise it? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 06/24/14 21:34, Eric Blake wrote:
On 06/24/2014 03:39 AM, Daniel P. Berrange wrote:
We also have a <features> subelement of <guest> in the <capabilities> XML which is used for a similar thing although it doesn't support a per-machine-type output, only per-binary capabilities. Should we add this more granular approach and abandon the old one?
Yes, we should stop adding stuff related to the guest to the main <capabilities> XML since it doesn't scale.
Oh phooey - I just proposed yet another feature there: https://www.redhat.com/archives/libvir-list/2014-June/msg01097.html
I'd like to turn on a witness for active commit support in the same release as we turn on the qemu implementation (and I'm hoping it still makes libvirt 1.2.6 - we haven't frozen yet, but it's near the end of the month, and we're still waiting on some patches to make it into qemu.git). If <features> is not the right place, then where should I advertise it?
Well, currently in the upstream state <features> is the only place where we can do it until Michal's new API gets in. We might need to add that particular feature there if this series doesn't make it, while active commit does. Peter
On Wed, Jun 25, 2014 at 08:14:12AM +0200, Peter Krempa wrote:
On 06/24/14 21:34, Eric Blake wrote:
On 06/24/2014 03:39 AM, Daniel P. Berrange wrote:
We also have a <features> subelement of <guest> in the <capabilities> XML which is used for a similar thing although it doesn't support a per-machine-type output, only per-binary capabilities. Should we add this more granular approach and abandon the old one?
Yes, we should stop adding stuff related to the guest to the main <capabilities> XML since it doesn't scale.
Oh phooey - I just proposed yet another feature there: https://www.redhat.com/archives/libvir-list/2014-June/msg01097.html
I'd like to turn on a witness for active commit support in the same release as we turn on the qemu implementation (and I'm hoping it still makes libvirt 1.2.6 - we haven't frozen yet, but it's near the end of the month, and we're still waiting on some patches to make it into qemu.git). If <features> is not the right place, then where should I advertise it?
Well, currently in the upstream state <features> is the only place where we can do it until Michal's new API gets in. We might need to add that particular feature there if this series doesn't make it, while active commit does.
Yeah, one more feature in existing capabilities XML isn't going to kill us. Might as well stick with what you have until we actually have the new API merged. 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 :|
On 24.06.2014 21:34, Eric Blake wrote:
On 06/24/2014 03:39 AM, Daniel P. Berrange wrote:
We also have a <features> subelement of <guest> in the <capabilities> XML which is used for a similar thing although it doesn't support a per-machine-type output, only per-binary capabilities. Should we add this more granular approach and abandon the old one?
Yes, we should stop adding stuff related to the guest to the main <capabilities> XML since it doesn't scale.
Oh phooey - I just proposed yet another feature there: https://www.redhat.com/archives/libvir-list/2014-June/msg01097.html
I'd like to turn on a witness for active commit support in the same release as we turn on the qemu implementation (and I'm hoping it still makes libvirt 1.2.6 - we haven't frozen yet, but it's near the end of the month, and we're still waiting on some patches to make it into qemu.git). If <features> is not the right place, then where should I advertise it?
I'm working on another version, but I'm not sure if I'll prepare patches prior to freeze. How critical is the active commit? I mean, can it wait a while (with possibility of slipping upcoming release)? Michal
On 06/25/2014 12:42 AM, Michal Privoznik wrote:
On 24.06.2014 21:34, Eric Blake wrote:
On 06/24/2014 03:39 AM, Daniel P. Berrange wrote:
We also have a <features> subelement of <guest> in the <capabilities> XML which is used for a similar thing although it doesn't support a per-machine-type output, only per-binary capabilities. Should we add this more granular approach and abandon the old one?
Yes, we should stop adding stuff related to the guest to the main <capabilities> XML since it doesn't scale.
Oh phooey - I just proposed yet another feature there: https://www.redhat.com/archives/libvir-list/2014-June/msg01097.html
I'd like to turn on a witness for active commit support in the same release as we turn on the qemu implementation (and I'm hoping it still makes libvirt 1.2.6 - we haven't frozen yet, but it's near the end of the month, and we're still waiting on some patches to make it into qemu.git). If <features> is not the right place, then where should I advertise it?
I'm working on another version, but I'm not sure if I'll prepare patches prior to freeze. How critical is the active commit? I mean, can it wait a while (with possibility of slipping upcoming release)?
Well, active commit implementation is also stalled waiting on qemu; although the API has been pushed already. I'm still playing it by ear for a couple more days, even if it means some of my patches go in (possibly with tweaks) after we freeze. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The API is supposed to fetch virEmulatorCapabilities once implemented in the hypervisor drivers. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt.h.in | 6 +++++ src/driver.h | 7 ++++++ src/libvirt.c | 52 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 ++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 19 +++++++++++++++- src/remote_protocol-structs | 10 +++++++++ 7 files changed, 96 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c83b20d..f71f732 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1585,6 +1585,12 @@ int virNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info); char * virConnectGetCapabilities (virConnectPtr conn); +char * virConnectGetEmulatorCapabilities(virConnectPtr conn, + const char *emulatorbin, + const char *machine, + const char *virttype, + unsigned int flags); + int virNodeGetCPUStats (virConnectPtr conn, int cpuNum, virNodeCPUStatsPtr params, diff --git a/src/driver.h b/src/driver.h index a0b258a..919cc44 100644 --- a/src/driver.h +++ b/src/driver.h @@ -126,6 +126,12 @@ typedef int typedef char * (*virDrvConnectGetCapabilities)(virConnectPtr conn); +typedef char * +(*virDrvConnectGetEmulatorCapabilities)(virConnectPtr conn, + const char *emulatorbin, + const char *machine, + const char *virttype, + unsigned int flags); typedef int (*virDrvConnectListDomains)(virConnectPtr conn, int *ids, @@ -1211,6 +1217,7 @@ struct _virDriver { virDrvConnectGetMaxVcpus connectGetMaxVcpus; virDrvNodeGetInfo nodeGetInfo; virDrvConnectGetCapabilities connectGetCapabilities; + virDrvConnectGetEmulatorCapabilities connectGetEmulatorCapabilities; virDrvConnectListDomains connectListDomains; virDrvConnectNumOfDomains connectNumOfDomains; virDrvConnectListAllDomains connectListAllDomains; diff --git a/src/libvirt.c b/src/libvirt.c index 83b7437..512630f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -7178,6 +7178,58 @@ virConnectGetCapabilities(virConnectPtr conn) /** + * virConnectGetEmulatorCapabilities: + * @conn: pointer to the hypervisor connection + * @emulatorbin: path to emulator + * @machine: machine type + * @virttype: virtualization type + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Prior creating a domain (for instance via virDomainCreateXML + * or virDomainDefineXML) it may be suitable to know what the + * underlying emulator and/or libvirt is capable of. For + * instance, if host, libvirt and qemu is capable of VFIO + * passthrough and so on. + * + * Returns NULL in case of error, or an XML string + * defining the capabilities. + * The client must free the returned string after use. + */ +char * +virConnectGetEmulatorCapabilities(virConnectPtr conn, + const char *emulatorbin, + const char *machine, + const char *virttype, + unsigned int flags) +{ + VIR_DEBUG("conn=%p emulatorbin=%s machine=%s virttype=%s flags=%x", + conn, emulatorbin, machine, virttype, flags); + + virResetLastError(); + + virCheckConnectReturn(conn, NULL); + + if (conn->driver->connectGetEmulatorCapabilities) { + char *ret; + ret = conn->driver->connectGetEmulatorCapabilities(conn, emulatorbin, + machine, virttype, + flags); + if (!ret) + goto error; + VIR_DEBUG("conn=%p emulatorbin=%s machine=%s virttype=%s flags=%x ret=%s", + conn, emulatorbin, machine, virttype, flags, ret); + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(conn); + return NULL; +} + + +/** * virNodeGetCPUStats: * @conn: pointer to the hypervisor connection. * @cpuNum: number of node cpu. (VIR_NODE_CPU_STATS_ALL_CPUS means total cpu diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 40d2c1a..e9987ad 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -661,6 +661,8 @@ LIBVIRT_1.2.5 { LIBVIRT_1.2.6 { global: virNodeGetFreePages; + virConnectGetEmulatorCapabilities; } LIBVIRT_1.2.5; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index bef9fd7..5838352 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7886,6 +7886,7 @@ static virDriver remote_driver = { .domainGetTime = remoteDomainGetTime, /* 1.2.5 */ .domainSetTime = remoteDomainSetTime, /* 1.2.5 */ .nodeGetFreePages = remoteNodeGetFreePages, /* 1.2.6 */ + .connectGetEmulatorCapabilities = remoteConnectGetEmulatorCapabilities, /* 1.2.6 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 50b1888..c76e5c4 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -475,6 +475,17 @@ struct remote_connect_get_capabilities_ret { remote_nonnull_string capabilities; }; +struct remote_connect_get_emulator_capabilities_args { + remote_string emulatorbin; + remote_string machine; + remote_string virttype; + unsigned int flags; +}; + +struct remote_connect_get_emulator_capabilities_ret { + remote_nonnull_string capabilities; +}; + struct remote_node_get_cpu_stats_args { int cpuNum; int nparams; @@ -5370,5 +5381,11 @@ enum remote_procedure { * @priority: high * @acl: connect:read */ - REMOTE_PROC_NODE_GET_FREE_PAGES = 340 + REMOTE_PROC_NODE_GET_FREE_PAGES = 340, + + /** + * @generate: both + * @acl: connect:read + */ + REMOTE_PROC_CONNECT_GET_EMULATOR_CAPABILITIES = 341 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 03268f5..f0e30b3 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -148,6 +148,15 @@ struct remote_node_get_info_ret { struct remote_connect_get_capabilities_ret { remote_nonnull_string capabilities; }; +struct remote_connect_get_emulator_capabilities_args { + remote_string emulatorbin; + remote_string machine; + remote_string virttype; + u_int flags; +}; +struct remote_connect_get_emulator_capabilities_ret { + remote_nonnull_string capabilities; +}; struct remote_node_get_cpu_stats_args { int cpuNum; int nparams; @@ -2826,4 +2835,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_TIME = 338, REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB_2 = 339, REMOTE_PROC_NODE_GET_FREE_PAGES = 340, + REMOTE_PROC_CONNECT_GET_EMULATOR_CAPABILITIES = 341, }; -- 1.8.5.5
On Fri, Jun 20, 2014 at 04:19:08PM +0200, Michal Privoznik wrote:
The API is supposed to fetch virEmulatorCapabilities once implemented in the hypervisor drivers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt.h.in | 6 +++++ src/driver.h | 7 ++++++ src/libvirt.c | 52 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 ++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 19 +++++++++++++++- src/remote_protocol-structs | 10 +++++++++ 7 files changed, 96 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c83b20d..f71f732 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1585,6 +1585,12 @@ int virNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info); char * virConnectGetCapabilities (virConnectPtr conn);
+char * virConnectGetEmulatorCapabilities(virConnectPtr conn, + const char *emulatorbin, + const char *machine, + const char *virttype, + unsigned int flags);
Following on from my prev comments lets call this virConnectGetDomainCapabilities since it is really about the <domain> XML schema and it is conceivable for a virt driver to not have any <emulator> at all. Hmm, this reminds me that we should have 'const char *arch' in there too. While for QEMU the architecture is implied by the emulator binary path provided, this doesn't hold true for say VMWare which doesn't use the emulator binary field. 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 :|
The API is exposed under emulatorcaps command. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-host.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 13 ++++++++++ 2 files changed, 87 insertions(+) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 13d4c5c..1f9dd0d 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -69,6 +69,74 @@ cmdCapabilities(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } /* + * "emulatorcaps" command + */ +static const vshCmdInfo info_emulatorcaps[] = { + {.name = "help", + .data = N_("emulator capabilities") + }, + {.name = "desc", + .data = N_("Returns capabilities of emulator with respect to host and libvirt.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_emulatorcaps[] = { + {.name = "emulatorbin", + .type = VSH_OT_STRING, + .help = N_("path to emulator binary"), + }, + {.name = "virttype", + .type = VSH_OT_STRING, + .help = N_("virtualization type (/domain/@type)"), + }, + {.name = "machine", + .type = VSH_OT_STRING, + .help = N_("machine type (/domain/os/type/@machine)"), + }, + {.name = NULL} +}; + +static bool +cmdEmulatorCaps(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; + char *caps; + const char *emulatorbin = NULL; + const char *machine = NULL; + const char *virttype = NULL; + const unsigned int flags = 0; /* No flags so far */ + + if (vshCommandOptString(cmd, "emulatorbin", &emulatorbin) < 0) { + vshError(ctl, "%s", _("ble")); + goto cleanup; + } + + if (vshCommandOptString(cmd, "virttype", &virttype) < 0) { + vshError(ctl, "%s", _("ble")); + goto cleanup; + } + + if (vshCommandOptString(cmd, "machine", &machine) < 0) { + vshError(ctl, "%s", _("ble")); + goto cleanup; + } + + caps = virConnectGetEmulatorCapabilities(ctl->conn, emulatorbin, + machine, virttype, flags); + if (!caps) { + vshError(ctl, "%s", _("failed to get emulator capabilities")); + goto cleanup; + } + + vshPrint(ctl, "%s\n", caps); + ret = true; + cleanup: + VIR_FREE(caps); + return ret; +} + +/* * "freecell" command */ static const vshCmdInfo info_freecell[] = { @@ -1131,6 +1199,12 @@ const vshCmdDef hostAndHypervisorCmds[] = { .info = info_cpu_models, .flags = 0 }, + {.name = "emulatorcaps", + .handler = cmdEmulatorCaps, + .opts = opts_emulatorcaps, + .info = info_emulatorcaps, + .flags = 0 + }, {.name = "freecell", .handler = cmdFreecell, .opts = opts_freecell, diff --git a/tools/virsh.pod b/tools/virsh.pod index 2c0f18a..155568f 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -505,6 +505,19 @@ B<virsh> list --title 0 Domain-0 running Mailserver 1 2 fedora paused +=item B<emulatorcaps> I<emulatorbin> I<virttype> [I<machine>] + +Print an XML document describing the capabilities of the +hypervisor we are currently connected to. This may be useful if +you intend to create a new domain and are curious if for instance +should use VFIO or legacy KVM device passthrough. The +I<emulatorbin> specifies the path to the emulator (this is same +as <emulator> element in the domain XML). Then, I<virttype> +specifies the virtualization used (the domain XML counterpart is +the 'type' attribute of the <domain/> top level element). The +last, optional argument overrides the default machine for the +emulator (can be found in domain XML under /domain/os/type). + =item B<freecell> [{ [I<--cellno>] B<cellno> | I<--all> }] Prints the available amount of memory on the machine or within a NUMA -- 1.8.5.5
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> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- 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; +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d755caa..121d1ba 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -307,4 +307,7 @@ int virQEMUCapsInitGuestFromBinary(virCapsPtr caps, virQEMUCapsPtr kvmbinCaps, virArch guestarch); +int virQEMUCapsFormatBuf(virBufferPtr buf, + void *opaque, + const char *machine); #endif /* __QEMU_CAPABILITIES_H__*/ diff --git a/src/qemu/qemu_capabilitiespriv.h b/src/qemu/qemu_capabilitiespriv.h new file mode 100644 index 0000000..9ed6853 --- /dev/null +++ b/src/qemu/qemu_capabilitiespriv.h @@ -0,0 +1,55 @@ +/* + * qemu_capabilitiespriv.h: private declarations for QEMU capabilities + * + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#ifndef __QEMU_CAPABILITIESPRIV_H__ +# define __QEMU_CAPABILITIESPRIV_H__ + +/* + * 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; +}; + +#endif /* __QEMU_CAPABILITIESPRIV_H__*/ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ac6aee5..b449770 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -95,6 +95,7 @@ #include "viraccessapicheckqemu.h" #include "storage/storage_driver.h" #include "virhostdev.h" +#include "viremulator_capabilities.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -16897,6 +16898,96 @@ qemuNodeGetFreePages(virConnectPtr conn, } +static char * +qemuConnectGetEmulatorCapabilities(virConnectPtr conn, + const char *emulatorbin, + const char *machine, + const char *virttype_str, + unsigned int flags) +{ + char *ret = NULL; + virQEMUDriverPtr driver = conn->privateData; + virQEMUCapsPtr qemuCaps = NULL; + virEmulatorCapabilitiesPtr emulCaps = NULL; + int virttype; /* virDomainVirtType */ + size_t ncanonicalMachine, i; + const char **canonicalMachine; + + virCheckFlags(0, ret); + virCheckNonNullArgGoto(emulatorbin, cleanup); + virCheckNonNullArgGoto(virttype_str, cleanup); + + if (virConnectGetEmulatorCapabilitiesEnsureACL(conn) < 0) + goto cleanup; + + if ((virttype = virDomainVirtTypeFromString(virttype_str)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown virttype: %s"), virttype_str); + goto cleanup; + } + + if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, + emulatorbin))) + goto cleanup; + + if (machine) { + const char *machine_tmp; + + if (!(machine_tmp = virQEMUCapsGetCanonicalMachine(qemuCaps, + machine))) { + /* This should never ever happen (TM) */ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("an error that should never " + "ever happen just occurred")); + goto cleanup; + } + machine = machine_tmp; + } + + /* The virQEMUCapsGetMachineTypes expects char *** but we want to stress + * the fact that we are not going to change machine types array, so we are + * using const char *** */ + if (!(ncanonicalMachine = virQEMUCapsGetMachineTypes(qemuCaps, + (char ***) &canonicalMachine))) { + virReportError(VIR_ERR_INVALID_ARG, + _(" emulator doesn't support any machines: %s"), + emulatorbin); + goto cleanup; + } + + if (machine) { + for (i = 0; i < ncanonicalMachine; i++) { + if (STREQ(machine, canonicalMachine[i])) + break; + } + + if (i == ncanonicalMachine) { + virReportError(VIR_ERR_INVALID_ARG, + _("the machine '%s' is not supported by emulator '%s'"), + machine, emulatorbin); + goto cleanup; + } + } else { + /* The default machine type is at the first position */ + machine = canonicalMachine[0]; + } + + if (!(emulCaps = virEmulatorCapabilitiesNew(emulatorbin, + machine, + virttype))) + goto cleanup; + + virEmulatorCapabilitiesSetPrivate(emulCaps, qemuCaps, virQEMUCapsFormatBuf); + + ret = virEmulatorCapabilitiesFormat(emulCaps); + + cleanup: + virObjectUnref(emulCaps); + virObjectUnref(qemuCaps); + return ret; +} + + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = QEMU_DRIVER_NAME, @@ -17092,6 +17183,7 @@ static virDriver qemuDriver = { .domainGetTime = qemuDomainGetTime, /* 1.2.5 */ .domainSetTime = qemuDomainSetTime, /* 1.2.5 */ .nodeGetFreePages = qemuNodeGetFreePages, /* 1.2.6 */ + .connectGetEmulatorCapabilities = qemuConnectGetEmulatorCapabilities, /* 1.2.6 */ }; diff --git a/tests/Makefile.am b/tests/Makefile.am index e7d9f94..fcd65b5 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -388,6 +388,9 @@ if WITH_QEMU test_libraries += libqemumonitortestutils.la \ qemuxml2argvmock.la \ $(NULL) +if WITH_LINUX +test_libraries += virhostdevmock.la +endif WITH_LINUX endif WITH_QEMU if WITH_BHYVE @@ -831,6 +834,21 @@ vircaps2xmltest_LDADD = $(LDADDS) viremulatorcapabilitiestest_SOURCES = \ viremulatorcapabilitiestest.c testutils.h testutils.c viremulatorcapabilitiestest_LDADD = $(LDADDS) +if WITH_QEMU +viremulatorcapabilitiestest_LDADD += $(qemu_LDADDS) +endif WITH_QEMU + +if WITH_LINUX +virhostdevmock_la_SOURCES = \ + virhostdevmock.c +virhostdevmock_la_CFLAGS = $(AM_CFLAGS) +virhostdevmock_la_LIBADD = $(GNULIB_LIBS) \ + ../src/libvirt.la +virhostdevmock_la_LDFLAGS = -module -avoid-version \ + -rpath /evil/libtool/hack/to/force/shared/lib/creation +else ! WITH_LINUX + EXTRA_DIST += virhostdevmock.c +endif ! WITH_LINUX if WITH_LIBVIRTD libvirtdconftest_SOURCES = \ diff --git a/tests/viremulatorcapabilitiesdata/viremulatorcaps-qemu-kvm-vfio.xml b/tests/viremulatorcapabilitiesdata/viremulatorcaps-qemu-kvm-vfio.xml new file mode 100644 index 0000000..e1d4bab --- /dev/null +++ b/tests/viremulatorcapabilitiesdata/viremulatorcaps-qemu-kvm-vfio.xml @@ -0,0 +1,17 @@ +<emulatorCapabilities> + <path>/usr/bin/qemu-system-x86_64</path> + <domain>kvm</domain> + <machine>pc-i440fx-2.1</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> diff --git a/tests/viremulatorcapabilitiestest.c b/tests/viremulatorcapabilitiestest.c index 448f0cf..25076a7 100644 --- a/tests/viremulatorcapabilitiestest.c +++ b/tests/viremulatorcapabilitiestest.c @@ -24,7 +24,8 @@ #include "testutils.h" #include "viremulator_capabilities.h" - +#include "qemu/qemu_capabilities.h" +#include "qemu/qemu_capabilitiespriv.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -96,11 +97,50 @@ test_virEmulatorCapabilitiesFormat(const void *opaque) return ret; } +static virQEMUCapsPtr +buildQEMUCaps(void) +{ + virQEMUCapsPtr qemuCaps = NULL; + + if (!(qemuCaps = virQEMUCapsNew())) + return NULL; + + if (VIR_ALLOC_N(qemuCaps->machineTypes, 1) < 0 || + VIR_ALLOC_N(qemuCaps->machineAliases, 1) < 0 || + VIR_ALLOC_N(qemuCaps->machineMaxCpus, 1) < 0) + goto error; + + if (VIR_STRDUP(qemuCaps->machineTypes[0], "pc-i440fx-2.1") < 0 || + VIR_STRDUP(qemuCaps->machineAliases[0], "pc") < 0) + goto error; + qemuCaps->machineMaxCpus[0] = 255; + qemuCaps->nmachineTypes++; + + qemuCaps->arch = VIR_ARCH_X86_64; + + virQEMUCapsSet(qemuCaps, QEMU_CAPS_PCIDEVICE); + virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI); + + return qemuCaps; + error: + virObjectUnref(qemuCaps); + return NULL; +} + static int mymain(void) { int ret = 0; +#ifdef WITH_QEMU + virQEMUCapsPtr qemuCaps; + + if (!(qemuCaps = buildQEMUCaps())) { + fprintf(stderr, "unable to build QEMU capabilities\n"); + return EXIT_FAILURE; + } +#endif + #define DO_TEST(Filename, Emulatorbin, Machine, Type, ...) \ do { \ struct test_virEmulatorCapabilitiesFormatData data = {.filename = Filename, \ @@ -109,9 +149,39 @@ mymain(void) ret = -1; \ } while (0) +#ifdef WITH_QEMU +# define DO_TEST_QEMU(Filename, Emulatorbin, Machine, Type, ...) \ + do { \ + struct test_virEmulatorCapabilitiesFormatData data = {.filename = Filename, \ + .emulatorbin = Emulatorbin, .machine = Machine, .type = Type, \ + .privateData = qemuCaps, .formatFunc = virQEMUCapsFormatBuf, __VA_ARGS__}; \ + if (virtTestRun(Filename, test_virEmulatorCapabilitiesFormat, &data) < 0) \ + ret = -1; \ + } while (0) +#endif + DO_TEST("basic", "/bin/emulatorbin", "my-machine-type", VIR_DOMAIN_VIRT_UML); +#ifdef WITH_QEMU + /* In the following test we are expecting host to support both KVM and + * VFIO. However, this is something we must mock and therefore the test can + * be run on the Linux platform only. */ +# ifdef __linux__ + DO_TEST_QEMU("qemu-kvm-vfio", "/usr/bin/qemu-system-x86_64", + "pc-i440fx-2.1", VIR_DOMAIN_VIRT_KVM); +# else +# error Port me +# endif +#endif + +#ifdef WITH_QEMU + virObjectUnref(qemuCaps); +#endif return ret; } +#ifdef __linux__ +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virhostdevmock.so") +#else VIRT_TEST_MAIN(mymain) +#endif diff --git a/tests/virhostdevmock.c b/tests/virhostdevmock.c new file mode 100644 index 0000000..cb08034 --- /dev/null +++ b/tests/virhostdevmock.c @@ -0,0 +1,40 @@ +/* + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Michal Privoznik <mprivozn@redhat.com> + */ + +#include <config.h> + +#ifdef __linux__ +# include "virhostdev.h" + +bool +virHostdevHostSupportsPassthroughLegacy(void) +{ + return true; +} + +bool +virHostdevHostSupportsPassthroughVFIO(void) +{ + return true; +} + +#else +/* Nothing to override on non-__linux__ platforms */ +#endif -- 1.8.5.5
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@redhat.com> --- 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 :|
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Michal Privoznik -
Peter Krempa