[libvirt] [PATCH v4 00/20] Add vhost-user-gpu support

From: Marc-André Lureau <marcandre.lureau@redhat.com> Hi, This series adds support for running virtio GPUs in seperate processes, thanks to vhost-user backend. The QEMU support landed for 4.1. There are several benefits of running the GPU/virgl in an external process, since Mesa is rather heavy on the qemu main loop, and may block for a while, or crash. The external GPU process is started with one end of a socket pair, the other end is given to a QEMU chardev attached to a device. The external process is also added to the cgroup to limit resources usage. vhost-user is a generic mechanism that allows implementing virtio device dataplane handling in a separate userspace process. vhost-user-gpu here notably moves the virgl 3d handling out of the main qemu process. The external process will be /usr/libexec/vhost-user-gpu, which comes from qemu.git contrib/vhost-user-gpu code, first released in qemu-4.1. Part of this series deals with discovering the location on disk of the vhost-user-gpu binary, and what capabilities it provides. This uses a similar mechanism to firmware.json, described in qemu docs/interop/vhost-user.json https://github.com/qemu/qemu/blob/master/docs/interop/vhost-user.json qemu 4.1 ships a 50-qemu-gpu.json to match. I believe virtio-fs will use a similar mechanism when it lands in upstream qemu, as virtiofsd is a separate process that communicates with qemu over vhost-user. For a bit more background on vhost-user-gpu process handling and the json interop motivation, here's some of the qemu discussion: https://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg02610.html https://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg00807.html For this series, the XML to enable this is: <video model='virtio'> <driver name='vhostuser'/> <acceleration accel3d='yes' rendernode='/path/to/rendernode'/> </video> rendernode is optional qemu_vhost_user.c handles vhost-user.json qemu_vhost_user_gpu.c handles the process management for vhost-user-gpu v4: - rebased - simplify vhost-user-gpu pidfile checking - fix check/syntax-check v3: - rename qemu_configs -> qemu_interop_config - replace <model .. vhostuser='yes'/> with <driver name='vhostuser'/> - move vhost_user_binary to virDomainVideoDriverDef - some VIR_AUTO usage improvements - introduce qemuDomainVideoPrivate to store vhost-user fd - improved selection of -device model - use DO_TEST_CAPS_LATEST - allocate a rendernode with virHostGetDRMRenderNode() - but no clear idea how to have a test - add a patch to ignore EPERM in virDirOpenIfExists() - better domain checks/validate - added Ján r-b - no s-o-b from Cole, per request and commits taken from his git branch - rebase, indentation/style fixes, update doc, version.. v2: - rebase to master - if test file build by dropping LDADDS usage - syntax-check issues: * use #pragma once * if () bracket issues * jump label indent issues * error message %s usage * size_t for loops Marc-André Lureau (20): qemu: generalize qemuFetchConfigs util: ignore EACCESS in virDirOpenIfExists conf: format/parse/rng/docs for video <driver name='qemu|vhostuser'/> domain: add rendernode attribute on <accel> qemu-cgroup: allow accel rendernode access qemu: add vhost-user-gpu capabilities checks qemu: check that qemu is vhost-user-vga capable qemu: validate virtio-gpu with vhost-user qemu: restrict 'virgl=' option to non-vhostuser video type qemu: add vhost-user helpers qemu: add qemuSecurityStartVhostUserGPU helper conf: add privateData to virDomainVideoDef qemu: add qemuDomainVideoPrivate qemu: add vhost-user-gpu helper unit tests: mock execv/execve tests: wrap vhost-user paths in qemuxml2argvtest qemu: prepare domain for vhost-user GPU qemu: start/stop the vhost-user-gpu external device qemu: build vhost-user GPU devices tests: add vhost-user-gpu xml2argv tests docs/formatdomain.html.in | 18 +- docs/schemas/domaincommon.rng | 13 + src/conf/domain_conf.c | 87 +++- src/conf/domain_conf.h | 21 +- src/qemu/Makefile.inc.am | 6 + src/qemu/qemu_capabilities.c | 6 + src/qemu/qemu_capabilities.h | 4 + src/qemu/qemu_cgroup.c | 24 + src/qemu/qemu_command.c | 65 ++- src/qemu/qemu_domain.c | 52 ++- src/qemu/qemu_domain.h | 12 + src/qemu/qemu_extdevice.c | 74 ++- src/qemu/qemu_extdevice.h | 5 + src/qemu/qemu_firmware.c | 144 +----- src/qemu/qemu_interop_config.c | 183 ++++++++ src/qemu/qemu_interop_config.h | 25 ++ src/qemu/qemu_process.c | 61 ++- src/qemu/qemu_security.c | 40 ++ src/qemu/qemu_security.h | 6 + src/qemu/qemu_vhost_user.c | 422 ++++++++++++++++++ src/qemu/qemu_vhost_user.h | 48 ++ src/qemu/qemu_vhost_user_gpu.c | 276 ++++++++++++ src/qemu/qemu_vhost_user_gpu.h | 49 ++ src/util/virfile.c | 13 +- tests/Makefile.am | 9 + .../caps_4.1.0.x86_64.xml | 2 + .../etc/qemu/vhost-user/40-gpu.json | 1 + .../etc/qemu/vhost-user/50-gpu.json | 0 .../qemu/vhost-user/test-vhost-user-gpu | 11 + .../usr/share/qemu/vhost-user/30-gpu.json | 1 + .../usr/share/qemu/vhost-user/50-gpu.json | 8 + .../usr/share/qemu/vhost-user/60-gpu.json | 1 + tests/qemuvhostusertest.c | 132 ++++++ ...host-user-gpu-secondary.x86_64-latest.args | 43 ++ .../vhost-user-gpu-secondary.xml | 46 ++ .../vhost-user-vga.x86_64-latest.args | 40 ++ tests/qemuxml2argvdata/vhost-user-vga.xml | 42 ++ tests/qemuxml2argvdata/virtio-options.args | 5 +- tests/qemuxml2argvdata/virtio-options.xml | 4 +- tests/qemuxml2argvtest.c | 16 +- tests/virfilewrapper.c | 22 + 41 files changed, 1827 insertions(+), 210 deletions(-) create mode 100644 src/qemu/qemu_interop_config.c create mode 100644 src/qemu/qemu_interop_config.h create mode 100644 src/qemu/qemu_vhost_user.c create mode 100644 src/qemu/qemu_vhost_user.h create mode 100644 src/qemu/qemu_vhost_user_gpu.c create mode 100644 src/qemu/qemu_vhost_user_gpu.h create mode 120000 tests/qemuvhostuserdata/etc/qemu/vhost-user/40-gpu.json create mode 100644 tests/qemuvhostuserdata/etc/qemu/vhost-user/50-gpu.json create mode 100755 tests/qemuvhostuserdata/usr/libexec/qemu/vhost-user/test-vhost-user-gpu create mode 120000 tests/qemuvhostuserdata/usr/share/qemu/vhost-user/30-gpu.json create mode 100644 tests/qemuvhostuserdata/usr/share/qemu/vhost-user/50-gpu.json create mode 120000 tests/qemuvhostuserdata/usr/share/qemu/vhost-user/60-gpu.json create mode 100644 tests/qemuvhostusertest.c create mode 100644 tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml create mode 100644 tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/vhost-user-vga.xml -- 2.23.0

From: Marc-André Lureau <marcandre.lureau@redhat.com> The same config files disovery & priority rules are used for vhost-user backends. No functional change, the only difference is that qemuInteropFetchConfigs() takes a "name" argument and construct paths with it (ex: "firmware"). Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_firmware.c | 144 +------------------------- src/qemu/qemu_interop_config.c | 183 +++++++++++++++++++++++++++++++++ src/qemu/qemu_interop_config.h | 25 +++++ 4 files changed, 212 insertions(+), 142 deletions(-) create mode 100644 src/qemu/qemu_interop_config.c create mode 100644 src/qemu/qemu_interop_config.h diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 48fd0332ec..7351dc0486 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -32,6 +32,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_hotplugpriv.h \ qemu/qemu_conf.c \ qemu/qemu_conf.h \ + qemu/qemu_interop_config.c \ + qemu/qemu_interop_config.h \ qemu/qemu_process.c \ qemu/qemu_process.h \ qemu/qemu_processpriv.h \ diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index f316a26a5b..aba2c359e7 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -23,13 +23,12 @@ #include <fnmatch.h> #include "qemu_firmware.h" +#include "qemu_interop_config.h" #include "configmake.h" #include "qemu_capabilities.h" #include "qemu_domain.h" #include "qemu_process.h" #include "virarch.h" -#include "virfile.h" -#include "virhash.h" #include "virjson.h" #include "virlog.h" #include "virstring.h" @@ -907,150 +906,11 @@ qemuFirmwareFormat(qemuFirmwarePtr fw) } -static int -qemuFirmwareBuildFileList(virHashTablePtr files, const char *dir) -{ - DIR *dirp; - struct dirent *ent = NULL; - int rc; - int ret = -1; - - if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) - return -1; - - if (rc == 0) - return 0; - - while ((rc = virDirRead(dirp, &ent, dir)) > 0) { - VIR_AUTOFREE(char *) filename = NULL; - VIR_AUTOFREE(char *) path = NULL; - struct stat sb; - - if (STRPREFIX(ent->d_name, ".")) - continue; - - if (VIR_STRDUP(filename, ent->d_name) < 0) - goto cleanup; - - if (virAsprintf(&path, "%s/%s", dir, filename) < 0) - goto cleanup; - - if (stat(path, &sb) < 0) { - virReportSystemError(errno, _("Unable to access %s"), path); - goto cleanup; - } - - if (!S_ISREG(sb.st_mode) && !S_ISLNK(sb.st_mode)) - continue; - - if (virHashUpdateEntry(files, filename, path) < 0) - goto cleanup; - - path = NULL; - } - - if (rc < 0) - goto cleanup; - - ret = 0; - cleanup: - virDirClose(&dirp); - return ret; -} - -static int -qemuFirmwareFilesSorter(const virHashKeyValuePair *a, - const virHashKeyValuePair *b) -{ - return strcmp(a->key, b->key); -} - -#define QEMU_FIRMWARE_SYSTEM_LOCATION PREFIX "/share/qemu/firmware" -#define QEMU_FIRMWARE_ETC_LOCATION SYSCONFDIR "/qemu/firmware" - int qemuFirmwareFetchConfigs(char ***firmwares, bool privileged) { - VIR_AUTOPTR(virHashTable) files = NULL; - VIR_AUTOFREE(char *) homeConfig = NULL; - VIR_AUTOFREE(char *) xdgConfig = NULL; - VIR_AUTOFREE(virHashKeyValuePairPtr) pairs = NULL; - virHashKeyValuePairPtr tmp = NULL; - - *firmwares = NULL; - - if (!privileged) { - /* This is a slight divergence from the specification. - * Since the system daemon runs as root, it doesn't make - * much sense to parse files in root's home directory. It - * makes sense only for session daemon which runs under - * regular user. */ - if (VIR_STRDUP(xdgConfig, getenv("XDG_CONFIG_HOME")) < 0) - return -1; - - if (!xdgConfig) { - VIR_AUTOFREE(char *) home = virGetUserDirectory(); - - if (!home) - return -1; - - if (virAsprintf(&xdgConfig, "%s/.config", home) < 0) - return -1; - } - - if (virAsprintf(&homeConfig, "%s/qemu/firmware", xdgConfig) < 0) - return -1; - } - - if (!(files = virHashCreate(10, virHashValueFree))) - return -1; - - if (qemuFirmwareBuildFileList(files, QEMU_FIRMWARE_SYSTEM_LOCATION) < 0) - return -1; - - if (qemuFirmwareBuildFileList(files, QEMU_FIRMWARE_ETC_LOCATION) < 0) - return -1; - - if (homeConfig && - qemuFirmwareBuildFileList(files, homeConfig) < 0) - return -1; - - /* At this point, the @files hash table contains unique set of filenames - * where each filename (as key) has the highest priority full pathname - * associated with it. */ - - if (virHashSize(files) == 0) - return 0; - - if (!(pairs = virHashGetItems(files, qemuFirmwareFilesSorter))) - return -1; - - for (tmp = pairs; tmp->key; tmp++) { - const char *path = tmp->value; - off_t len; - - if ((len = virFileLength(path, -1)) < 0) { - virReportSystemError(errno, - _("unable to get size of '%s'"), - path); - return -1; - } - - VIR_DEBUG("firmware description path '%s' len=%jd", - path, (intmax_t) len); - - if (len == 0) { - /* Empty files are used to mask less specific instances - * of the same file. */ - continue; - } - - if (virStringListAdd(firmwares, path) < 0) - return -1; - } - - return 0; + return qemuInteropFetchConfigs("firmware", firmwares, privileged); } diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c new file mode 100644 index 0000000000..1f39d4b576 --- /dev/null +++ b/src/qemu/qemu_interop_config.c @@ -0,0 +1,183 @@ +/* + * qemu_interop_config.c: QEMU firmware/vhost-user etc configs + * + * Copyright (C) 2019 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/>. + */ + +#include <config.h> + +#include "qemu_interop_config.h" +#include "configmake.h" +#include "viralloc.h" +#include "virenum.h" +#include "virfile.h" +#include "virhash.h" +#include "virlog.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +VIR_LOG_INIT("qemu.qemu_configs"); + +static int +qemuBuildFileList(virHashTablePtr files, const char *dir) +{ + DIR *dirp; + struct dirent *ent = NULL; + int rc; + int ret = -1; + + if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) + return -1; + + if (rc == 0) + return 0; + + while ((rc = virDirRead(dirp, &ent, dir)) > 0) { + VIR_AUTOFREE(char *) filename = NULL; + VIR_AUTOFREE(char *) path = NULL; + struct stat sb; + + if (STRPREFIX(ent->d_name, ".")) + continue; + + if (VIR_STRDUP(filename, ent->d_name) < 0) + goto cleanup; + + if (virAsprintf(&path, "%s/%s", dir, filename) < 0) + goto cleanup; + + if (stat(path, &sb) < 0) { + virReportSystemError(errno, _("Unable to access %s"), path); + goto cleanup; + } + + if (!S_ISREG(sb.st_mode) && !S_ISLNK(sb.st_mode)) + continue; + + if (virHashUpdateEntry(files, filename, path) < 0) + goto cleanup; + + path = NULL; + } + + if (rc < 0) + goto cleanup; + + ret = 0; + cleanup: + virDirClose(&dirp); + return ret; +} + +static int +qemuConfigFilesSorter(const virHashKeyValuePair *a, + const virHashKeyValuePair *b) +{ + return strcmp(a->key, b->key); +} + +#define QEMU_SYSTEM_LOCATION PREFIX "/share/qemu" +#define QEMU_ETC_LOCATION SYSCONFDIR "/qemu" + +int +qemuInteropFetchConfigs(const char *name, + char ***configs, + bool privileged) +{ + VIR_AUTOPTR(virHashTable) files = NULL; + VIR_AUTOFREE(char *) homeConfig = NULL; + VIR_AUTOFREE(char *) xdgConfig = NULL; + VIR_AUTOFREE(char *) sysLocation = virFileBuildPath(QEMU_SYSTEM_LOCATION, name, NULL); + VIR_AUTOFREE(char *) etcLocation = virFileBuildPath(QEMU_ETC_LOCATION, name, NULL); + VIR_AUTOFREE(virHashKeyValuePairPtr) pairs = NULL; + virHashKeyValuePairPtr tmp = NULL; + + *configs = NULL; + + if (!privileged) { + /* This is a slight divergence from the specification. + * Since the system daemon runs as root, it doesn't make + * much sense to parse files in root's home directory. It + * makes sense only for session daemon which runs under + * regular user. */ + if (VIR_STRDUP(xdgConfig, getenv("XDG_CONFIG_HOME")) < 0) + return -1; + + if (!xdgConfig) { + VIR_AUTOFREE(char *) home = virGetUserDirectory(); + + if (!home) + return -1; + + if (virAsprintf(&xdgConfig, "%s/.config", home) < 0) + return -1; + } + + if (virAsprintf(&homeConfig, "%s/qemu/%s", xdgConfig, name) < 0) + return -1; + } + + if (!(files = virHashCreate(10, virHashValueFree))) + return -1; + + if (qemuBuildFileList(files, sysLocation) < 0) + return -1; + + if (qemuBuildFileList(files, etcLocation) < 0) + return -1; + + if (homeConfig && + qemuBuildFileList(files, homeConfig) < 0) + return -1; + + /* At this point, the @files hash table contains unique set of filenames + * where each filename (as key) has the highest priority full pathname + * associated with it. */ + + if (virHashSize(files) == 0) + return 0; + + if (!(pairs = virHashGetItems(files, qemuConfigFilesSorter))) + return -1; + + for (tmp = pairs; tmp->key; tmp++) { + const char *path = tmp->value; + off_t len; + + if ((len = virFileLength(path, -1)) < 0) { + virReportSystemError(errno, + _("unable to get size of '%s'"), + path); + return -1; + } + + VIR_DEBUG("%s description path '%s' len=%jd", + name, path, (intmax_t) len); + + if (len == 0) { + /* Empty files are used to mask less specific instances + * of the same file. */ + continue; + } + + if (virStringListAdd(configs, path) < 0) + return -1; + } + + return 0; +} diff --git a/src/qemu/qemu_interop_config.h b/src/qemu/qemu_interop_config.h new file mode 100644 index 0000000000..9ac1fe9610 --- /dev/null +++ b/src/qemu/qemu_interop_config.h @@ -0,0 +1,25 @@ +/* + * qemu_interop_config.h: QEMU firmware/vhost-user etc configs + * + * Copyright (C) 2019 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/>. + */ + +#pragma once + +#include "internal.h" + +int qemuInteropFetchConfigs(const char *name, char ***configs, bool privileged); -- 2.23.0

On Fri, Sep 13, 2019 at 04:50:38PM +0400, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The same config files disovery & priority rules are used for vhost-user backends.
No functional change, the only difference is that qemuInteropFetchConfigs() takes a "name" argument and construct paths with it (ex: "firmware").
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_firmware.c | 144 +------------------------- src/qemu/qemu_interop_config.c | 183 +++++++++++++++++++++++++++++++++ src/qemu/qemu_interop_config.h | 25 +++++ 4 files changed, 212 insertions(+), 142 deletions(-) create mode 100644 src/qemu/qemu_interop_config.c create mode 100644 src/qemu/qemu_interop_config.h
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Marc-André Lureau <marcandre.lureau@redhat.com> Whether a directory exists or is not readable shouldn't make a big diffence. This removes errors when firmare or vhost-user config is looked up from a user session libvirt if /etc/libvirt is not readable. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/util/virfile.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index bb844c64e5..4d1fe50efc 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2855,7 +2855,8 @@ virFileRemove(const char *path, #endif /* WIN32 */ static int -virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT, bool quiet) +virDirOpenInternal(DIR **dirp, const char *name, + bool ignoreENOENT, bool ignoreEACCESS, bool quiet) { *dirp = opendir(name); /* exempt from syntax-check */ if (!*dirp) { @@ -2864,6 +2865,8 @@ virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT, bool quiet) if (ignoreENOENT && errno == ENOENT) return 0; + if (ignoreEACCESS && errno == EACCES) + return 0; virReportSystemError(errno, _("cannot open directory '%s'"), name); return -1; } @@ -2881,7 +2884,7 @@ virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT, bool quiet) int virDirOpen(DIR **dirp, const char *name) { - return virDirOpenInternal(dirp, name, false, false); + return virDirOpenInternal(dirp, name, false, false, false); } /** @@ -2890,13 +2893,13 @@ virDirOpen(DIR **dirp, const char *name) * @name: path of the directory * * Returns 1 on success. - * If opendir returns ENOENT, 0 is returned without reporting an error. + * If opendir returns ENOENT or EACCES, 0 is returned without reporting an error. * On other errors, -1 is returned and an error is reported. */ int virDirOpenIfExists(DIR **dirp, const char *name) { - return virDirOpenInternal(dirp, name, true, false); + return virDirOpenInternal(dirp, name, true, true, false); } /** @@ -2912,7 +2915,7 @@ virDirOpenIfExists(DIR **dirp, const char *name) int virDirOpenQuiet(DIR **dirp, const char *name) { - return virDirOpenInternal(dirp, name, false, true); + return virDirOpenInternal(dirp, name, false, false, true); } /** -- 2.23.0

On 9/13/19 8:50 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Whether a directory exists or is not readable shouldn't make a big diffence.
This removes errors when firmare or vhost-user config is looked up from a user session libvirt if /etc/libvirt is not readable.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/util/virfile.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
In some cases this makes sense, in others it isn't clear. For example: [:~] $ chmod 000 .config/libvirt/storage/ [:~] $ libvirtd ... 2019-09-20 19:45:13.691+0000: 327223: error : virDirOpenInternal:2846 : cannot open directory '/home/crobinso/.config/libvirt/storage': Permission denied 2019-09-20 19:45:13.691+0000: 327223: error : virStateInitialize:656 : Initialization of storage state driver failed: cannot open directory '/home/crobinso/.config/libvirt/storage': Permission denied 2019-09-20 19:45:13.691+0000: 327223: error : daemonRunStateInit:790 : Driver state initialization failed After the patches, this case doesn't explicitly fail, but the driver won't report any existing storage pools so it causes silent issues. I think erroring incase permissions are wrong is better, because libvirt doesn't have any code to attempt to fix that situation, unlike for missing directory Not sure if it's realistic for this case to happen but I noticed it through code inspection. Since this only applies to your particular case in the code in one area, you can do (untested) if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) { /* descriptive comment */ if (virLastErrorIsSystemErrno(EACCES)) { virResetLastError(); return 0; } return -1; }
diff --git a/src/util/virfile.c b/src/util/virfile.c index bb844c64e5..4d1fe50efc 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2855,7 +2855,8 @@ virFileRemove(const char *path, #endif /* WIN32 */
static int -virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT, bool quiet) +virDirOpenInternal(DIR **dirp, const char *name, + bool ignoreENOENT, bool ignoreEACCESS, bool quiet) { *dirp = opendir(name); /* exempt from syntax-check */ if (!*dirp) { @@ -2864,6 +2865,8 @@ virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT, bool quiet)
if (ignoreENOENT && errno == ENOENT) return 0; + if (ignoreEACCESS && errno == EACCES) + return 0; virReportSystemError(errno, _("cannot open directory '%s'"), name); return -1; } @@ -2881,7 +2884,7 @@ virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT, bool quiet) int virDirOpen(DIR **dirp, const char *name) { - return virDirOpenInternal(dirp, name, false, false); + return virDirOpenInternal(dirp, name, false, false, false); }
/** @@ -2890,13 +2893,13 @@ virDirOpen(DIR **dirp, const char *name) * @name: path of the directory * * Returns 1 on success. - * If opendir returns ENOENT, 0 is returned without reporting an error. + * If opendir returns ENOENT or EACCES, 0 is returned without reporting an error. * On other errors, -1 is returned and an error is reported. */ int virDirOpenIfExists(DIR **dirp, const char *name) { - return virDirOpenInternal(dirp, name, true, false); + return virDirOpenInternal(dirp, name, true, true, false); }
/** @@ -2912,7 +2915,7 @@ virDirOpenIfExists(DIR **dirp, const char *name) int virDirOpenQuiet(DIR **dirp, const char *name) { - return virDirOpenInternal(dirp, name, false, true); + return virDirOpenInternal(dirp, name, false, false, true); }
/**
- Cole

From: Marc-André Lureau <marcandre.lureau@redhat.com> Accept a new driver name attribute to specify usage of helper process, ex: <video> <driver name='vhostuser'/> <model type='virtio'/> </video> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- docs/formatdomain.html.in | 12 ++++++- docs/schemas/domaincommon.rng | 8 +++++ src/conf/domain_conf.c | 42 ++++++++++++++++++++++- src/conf/domain_conf.h | 12 +++++++ tests/qemuxml2argvdata/virtio-options.xml | 2 +- 5 files changed, 73 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 86a5261e47..60a47c812b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6997,6 +6997,7 @@ qemu-kvm -net nic,model=? /dev/null <model type='vga' vram='16384' heads='1'> <acceleration accel3d='yes' accel2d='yes'/> </model> + <driver name='qemu'/> </video> </devices> ...</pre> @@ -7097,7 +7098,16 @@ qemu-kvm -net nic,model=? /dev/null <dd> The subelement <code>driver</code> can be used to tune the device: <dl> - <dt>virtio options</dt> + <dt><code>name</code></dt> + <dd> + Specify the backend driver to use, either "qemu" or + "vhostuser" depending on the hypervisor features available + (<span class="since">since 5.8.0</span>). "qemu" is the + default QEMU backend. "vhostuser" will use a separate + vhost-user process backend (for <code>virtio</code> + device). + </dd> + <dt>virtio options</dt> <dd> <a href="#elementsVirtio">Virtio-specific options</a> can also be set (<span class="since">Since 3.5.0</span>) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index cae3be639e..7006d9f508 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3565,6 +3565,14 @@ <optional> <ref name="virtioOptions"/> </optional> + <optional> + <attribute name="name"> + <choice> + <value>qemu</value> + <value>vhostuser</value> + </choice> + </attribute> + </optional> <optional> <attribute name="vgaconf"> <choice> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 96e9223e21..6f81af2d4c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -728,6 +728,13 @@ VIR_ENUM_IMPL(virDomainPanicModel, "s390", ); +VIR_ENUM_IMPL(virDomainVideoBackend, + VIR_DOMAIN_VIDEO_BACKEND_TYPE_LAST, + "default", + "qemu", + "vhostuser", +); + VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, "default", @@ -6262,6 +6269,23 @@ virDomainVideoDefValidate(const virDomainVideoDef *video, } } + switch (video->backend) { + case VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER: + if (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'vhostuser' driver is only supported with 'virtio' device")); + return -1; + } + break; + case VIR_DOMAIN_VIDEO_BACKEND_TYPE_DEFAULT: + case VIR_DOMAIN_VIDEO_BACKEND_TYPE_QEMU: + break; + case VIR_DOMAIN_VIDEO_BACKEND_TYPE_LAST: + default: + virReportEnumRangeError(virDomainInputType, video->backend); + return -1; + } + return 0; } @@ -15405,6 +15429,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, xmlNodePtr cur; VIR_XPATH_NODE_AUTORESTORE(ctxt); VIR_AUTOFREE(char *) type = NULL; + VIR_AUTOFREE(char *) driver_name = NULL; VIR_AUTOFREE(char *) heads = NULL; VIR_AUTOFREE(char *) vram = NULL; VIR_AUTOFREE(char *) vram64 = NULL; @@ -15440,6 +15465,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, if (virXMLNodeNameEqual(cur, "driver")) { if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0) goto error; + driver_name = virXMLPropString(cur, "name"); } } cur = cur->next; @@ -15455,6 +15481,16 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, def->type = virDomainVideoDefaultType(dom); } + if (driver_name) { + if ((def->backend = virDomainVideoBackendTypeFromString(driver_name)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown video driver '%s'"), driver_name); + goto error; + } + } else { + def->backend = VIR_DOMAIN_VIDEO_BACKEND_TYPE_DEFAULT; + } + if (ram) { if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -26522,13 +26558,17 @@ virDomainVideoDefFormat(virBufferPtr buf, virDomainVirtioOptionsFormat(&driverBuf, def->virtio); if (virBufferCheckError(&driverBuf) < 0) goto cleanup; - if (virBufferUse(&driverBuf) || (def->driver && def->driver->vgaconf)) { + if (virBufferUse(&driverBuf) || (def->driver && def->driver->vgaconf) || + def->backend != VIR_DOMAIN_VIDEO_BACKEND_TYPE_DEFAULT) { virBufferAddLit(buf, "<driver"); if (virBufferUse(&driverBuf)) virBufferAddBuffer(buf, &driverBuf); if (def->driver && def->driver->vgaconf) virBufferAsprintf(buf, " vgaconf='%s'", virDomainVideoVGAConfTypeToString(def->driver->vgaconf)); + if (def->backend != VIR_DOMAIN_VIDEO_BACKEND_TYPE_DEFAULT) + virBufferAsprintf(buf, " name='%s'", + virDomainVideoBackendTypeToString(def->backend)); virBufferAddLit(buf, "/>\n"); } virBufferAsprintf(buf, "<model type='%s'", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 82631ecb07..7d60f3054c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1376,6 +1376,16 @@ struct _virDomainWatchdogDef { }; +/* the backend driver used for virtio interfaces */ +typedef enum { + VIR_DOMAIN_VIDEO_BACKEND_TYPE_DEFAULT, + VIR_DOMAIN_VIDEO_BACKEND_TYPE_QEMU, + VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER, + + VIR_DOMAIN_VIDEO_BACKEND_TYPE_LAST +} virDomainVideoBackendType; + + typedef enum { VIR_DOMAIN_VIDEO_TYPE_DEFAULT, VIR_DOMAIN_VIDEO_TYPE_VGA, @@ -1426,6 +1436,7 @@ struct _virDomainVideoDef { virDomainVideoDriverDefPtr driver; virDomainDeviceInfo info; virDomainVirtioOptionsPtr virtio; + virDomainVideoBackendType backend; }; /* graphics console modes */ @@ -3422,6 +3433,7 @@ VIR_ENUM_DECL(virDomainWatchdogModel); VIR_ENUM_DECL(virDomainWatchdogAction); VIR_ENUM_DECL(virDomainPanicModel); VIR_ENUM_DECL(virDomainVideo); +VIR_ENUM_DECL(virDomainVideoBackend); VIR_ENUM_DECL(virDomainHostdevMode); VIR_ENUM_DECL(virDomainHostdevSubsys); VIR_ENUM_DECL(virDomainHostdevCaps); diff --git a/tests/qemuxml2argvdata/virtio-options.xml b/tests/qemuxml2argvdata/virtio-options.xml index 773038a320..bdfadca22d 100644 --- a/tests/qemuxml2argvdata/virtio-options.xml +++ b/tests/qemuxml2argvdata/virtio-options.xml @@ -73,7 +73,7 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <video> - <driver iommu='on' ats='on'/> + <driver iommu='on' ats='on' name='vhostuser'/> <model type='virtio' heads='1' primary='yes'> <acceleration accel3d='yes'/> </model> -- 2.23.0

On Fri, Sep 13, 2019 at 04:50:40PM +0400, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Accept a new driver name attribute to specify usage of helper process, ex:
<video> <driver name='vhostuser'/> <model type='virtio'/> </video>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- docs/formatdomain.html.in | 12 ++++++- docs/schemas/domaincommon.rng | 8 +++++ src/conf/domain_conf.c | 42 ++++++++++++++++++++++- src/conf/domain_conf.h | 12 +++++++ tests/qemuxml2argvdata/virtio-options.xml | 2 +- 5 files changed, 73 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Marc-André Lureau <marcandre.lureau@redhat.com> vhost-user-gpu helper takes --render-node option to specify on which GPU should the renderning be done. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- docs/formatdomain.html.in | 6 ++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 17 ++++++++++++++++- src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/virtio-options.xml | 2 +- 5 files changed, 29 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 60a47c812b..93991aa3d1 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7082,6 +7082,12 @@ qemu-kvm -net nic,model=? /dev/null <dd>Enable 3D acceleration (for vbox driver <span class="since">since 0.7.1</span>, qemu driver <span class="since">since 1.3.0</span>)</dd> + + <dt><code>rendernode</code></dt> + <dd>Absolute path to a host's DRI device to be used for + rendering (for 'vhostuser' driver only, <span + class="since">since 5.8.0</span>). If none is specified, + libvirt will pick one available.</dd> </dl> </dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7006d9f508..40eb4a2d75 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3648,6 +3648,11 @@ <ref name="virYesNo"/> </attribute> </optional> + <optional> + <attribute name="rendernode"> + <ref name="absFilePath"/> + </attribute> + </optional> </element> </optional> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6f81af2d4c..15fb8dea17 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2840,6 +2840,8 @@ virDomainVideoDefClear(virDomainVideoDefPtr def) virDomainDeviceInfoClear(&def->info); + if (def->accel) + VIR_FREE(def->accel->rendernode); VIR_FREE(def->accel); VIR_FREE(def->virtio); VIR_FREE(def->driver); @@ -6279,6 +6281,11 @@ virDomainVideoDefValidate(const virDomainVideoDef *video, break; case VIR_DOMAIN_VIDEO_BACKEND_TYPE_DEFAULT: case VIR_DOMAIN_VIDEO_BACKEND_TYPE_QEMU: + if (video->accel && video->accel->rendernode) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unsupported rendernode accel attribute without 'vhostuser'")); + return -1; + } break; case VIR_DOMAIN_VIDEO_BACKEND_TYPE_LAST: default: @@ -15341,6 +15348,7 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node) int val; VIR_AUTOFREE(char *) accel2d = NULL; VIR_AUTOFREE(char *) accel3d = NULL; + VIR_AUTOFREE(char *) rendernode = NULL; cur = node->children; while (cur != NULL) { @@ -15349,12 +15357,13 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node) virXMLNodeNameEqual(cur, "acceleration")) { accel3d = virXMLPropString(cur, "accel3d"); accel2d = virXMLPropString(cur, "accel2d"); + rendernode = virXMLPropString(cur, "rendernode"); } } cur = cur->next; } - if (!accel3d && !accel2d) + if (!accel3d && !accel2d && !rendernode) return NULL; if (VIR_ALLOC(def) < 0) @@ -15378,6 +15387,8 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node) def->accel2d = val; } + VIR_STEAL_PTR(def->rendernode, rendernode); + cleanup: return def; } @@ -26535,6 +26546,10 @@ virDomainVideoAccelDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " accel2d='%s'", virTristateBoolTypeToString(def->accel2d)); } + if (def->rendernode) { + virBufferAsprintf(buf, " rendernode='%s'", + def->rendernode); + } virBufferAddLit(buf, "/>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7d60f3054c..16dacd4b43 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1417,6 +1417,7 @@ VIR_ENUM_DECL(virDomainVideoVGAConf); struct _virDomainVideoAccelDef { int accel2d; /* enum virTristateBool */ int accel3d; /* enum virTristateBool */ + char *rendernode; }; diff --git a/tests/qemuxml2argvdata/virtio-options.xml b/tests/qemuxml2argvdata/virtio-options.xml index bdfadca22d..dd9a4f4a01 100644 --- a/tests/qemuxml2argvdata/virtio-options.xml +++ b/tests/qemuxml2argvdata/virtio-options.xml @@ -75,7 +75,7 @@ <video> <driver iommu='on' ats='on' name='vhostuser'/> <model type='virtio' heads='1' primary='yes'> - <acceleration accel3d='yes'/> + <acceleration accel3d='yes' rendernode='/dev/dri/test'/> </model> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> -- 2.23.0

On Fri, Sep 13, 2019 at 04:50:41PM +0400, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
vhost-user-gpu helper takes --render-node option to specify on which GPU should the renderning be done.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- docs/formatdomain.html.in | 6 ++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 17 ++++++++++++++++- src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/virtio-options.xml | 2 +- 5 files changed, 29 insertions(+), 2 deletions(-)
@@ -15378,6 +15387,8 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node) def->accel2d = val; }
+ VIR_STEAL_PTR(def->rendernode, rendernode);
virFileSanitizePath should be called on user-provided paths
+ cleanup: return def; } @@ -26535,6 +26546,10 @@ virDomainVideoAccelDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " accel2d='%s'", virTristateBoolTypeToString(def->accel2d)); } + if (def->rendernode) { + virBufferAsprintf(buf, " rendernode='%s'", + def->rendernode); + }
And user-provided string should be escaped for XML by virBufferEscapeString (which is a no-op if the argument is NULL, so you can drop the if as well)
virBufferAddLit(buf, "/>\n"); }
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_cgroup.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index ecd96efb0a..eb6f993d8e 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -503,6 +503,25 @@ qemuSetupGraphicsCgroup(virDomainObjPtr vm, } +static int +qemuSetupVideoAccelCgroup(virDomainObjPtr vm, + virDomainVideoAccelDefPtr def) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret; + + if (!def->rendernode || + !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) + return 0; + + ret = virCgroupAllowDevicePath(priv->cgroup, def->rendernode, + VIR_CGROUP_DEVICE_RW, false); + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", def->rendernode, + "rw", ret); + return ret; +} + + static int qemuSetupBlkioCgroup(virDomainObjPtr vm) { @@ -803,6 +822,11 @@ qemuSetupDevicesCgroup(virDomainObjPtr vm) goto cleanup; } + for (i = 0; i < vm->def->nvideos; i++) { + if (qemuSetupVideoAccelCgroup(vm, vm->def->videos[i]->accel) < 0) + goto cleanup; + } + for (i = 0; i < vm->def->ninputs; i++) { if (qemuSetupInputCgroup(vm, vm->def->inputs[i]) < 0) goto cleanup; -- 2.23.0

On 9/13/19 8:50 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_cgroup.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index ecd96efb0a..eb6f993d8e 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -503,6 +503,25 @@ qemuSetupGraphicsCgroup(virDomainObjPtr vm, }
+static int +qemuSetupVideoAccelCgroup(virDomainObjPtr vm, + virDomainVideoAccelDefPtr def) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret; + + if (!def->rendernode || + !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) + return 0; + + ret = virCgroupAllowDevicePath(priv->cgroup, def->rendernode, + VIR_CGROUP_DEVICE_RW, false); + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", def->rendernode, + "rw", ret); + return ret; +} + + static int qemuSetupBlkioCgroup(virDomainObjPtr vm) { @@ -803,6 +822,11 @@ qemuSetupDevicesCgroup(virDomainObjPtr vm) goto cleanup; }
+ for (i = 0; i < vm->def->nvideos; i++) { + if (qemuSetupVideoAccelCgroup(vm, vm->def->videos[i]->accel) < 0) + goto cleanup; + } + for (i = 0; i < vm->def->ninputs; i++) { if (qemuSetupInputCgroup(vm, vm->def->inputs[i]) < 0) goto cleanup;
->accel can be NULL here otherwise we crash like I mentioned yesterday. All the other functions here act on a device as a whole so I think this should be qemuSetupVideoCgroup while you are here FWIW after fixing this the VM starts, but I see this in the logs with stock Fedora 31 qemu 4.1, not sure if it's indicative of a qemu error or something missing on libvirt side. The VM gets to the initrd load stage but then graphic output stops updating 2019-09-20T20:06:51.238950Z qemu-system-x86_64: Failed initializing vhost-user memory map, consider using -object memory-backend-file share=on 2019-09-20T20:06:51.238971Z qemu-system-x86_64: vhost_set_mem_table failed: Resource temporarily unavailable (11) 2019-09-20T20:06:51.238976Z qemu-system-x86_64: Error start vhost dev - Cole

From: Marc-André Lureau <marcandre.lureau@redhat.com> Those new devices are available since QEMU 4.1. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 6 ++++++ src/qemu/qemu_capabilities.h | 4 ++++ tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 2 ++ 3 files changed, 12 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9b19930964..49f0029f6f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -539,6 +539,10 @@ VIR_ENUM_IMPL(virQEMUCaps, "migration-file-drop-cache", "net-socket-dgram", "dbus-vmstate", + "vhost-user-gpu", + + /* 340 */ + "vhost-user-vga", ); @@ -1130,6 +1134,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "max-x86_64-cpu", QEMU_CAPS_X86_MAX_CPU }, { "bochs-display", QEMU_CAPS_DEVICE_BOCHS_DISPLAY }, { "dbus-vmstate", QEMU_CAPS_DBUS_VMSTATE }, + { "vhost-user-gpu", QEMU_CAPS_DEVICE_VHOST_USER_GPU }, + { "vhost-user-vga", QEMU_CAPS_DEVICE_VHOST_USER_VGA }, }; static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBalloon[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 54f91151c6..56d9649def 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -520,6 +520,10 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_MIGRATION_FILE_DROP_CACHE, /* migration with disk cache on is safe for type='file' disks */ QEMU_CAPS_NET_SOCKET_DGRAM, /* -net socket,fd= with dgram socket */ QEMU_CAPS_DBUS_VMSTATE, /* -object dbus-vmstate */ + QEMU_CAPS_DEVICE_VHOST_USER_GPU, /* -device vhost-user-gpu */ + + /* 340 */ + QEMU_CAPS_DEVICE_VHOST_USER_VGA, /* -device vhost-user-vga */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml index 05e6538e37..085f7e5aaa 100644 --- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml @@ -210,6 +210,8 @@ <flag name='bochs-display'/> <flag name='migration-file-drop-cache'/> <flag name='net-socket-dgram'/> + <flag name='vhost-user-gpu'/> + <flag name='vhost-user-vga'/> <version>4000050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100759</microcodeVersion> -- 2.23.0

From: Marc-André Lureau <marcandre.lureau@redhat.com> To support virtio VGA with vhost-user, vhost-user-vga device is necessary. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bd247628cb..24292b88bf 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -12697,9 +12697,14 @@ bool qemuDomainSupportsVideoVga(virDomainVideoDefPtr video, virQEMUCapsPtr qemuCaps) { - if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_VGA)) - return false; + if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { + if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_VGA)) + return false; + } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_VGA)) { + return false; + } + } return true; } -- 2.23.0

From: Marc-André Lureau <marcandre.lureau@redhat.com> Check qemu capability, and accept 3d acceleration. 3d acceleration support is checked when looking for a suitable vhost-user helper. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_process.c | 57 +++++++++++++++++++++++----------------- tests/qemuxml2argvtest.c | 3 +-- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 955ba4de4c..463b783966 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5266,34 +5266,43 @@ qemuProcessStartValidateVideo(virDomainObjPtr vm, for (i = 0; i < vm->def->nvideos; i++) { video = vm->def->videos[i]; - if ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("this QEMU does not support '%s' video device"), - virDomainVideoTypeToString(video->type)); - return -1; - } - - if (video->accel) { - if (video->accel->accel3d == VIR_TRISTATE_SWITCH_ON && - (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO || - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_VIRGL))) { + if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) { + if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_GPU)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU does not support 'vhost-user' video device")); + return -1; + } + } else { + if ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("%s 3d acceleration is not supported"), + _("this QEMU does not support '%s' video device"), virDomainVideoTypeToString(video->type)); return -1; } + + if (video->accel) { + if (video->accel->accel3d == VIR_TRISTATE_SWITCH_ON && + (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_VIRGL))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("%s 3d acceleration is not supported"), + virDomainVideoTypeToString(video->type)); + return -1; + } + } } } diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1f2ae5958a..f9eb74e7ed 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2894,8 +2894,7 @@ mymain(void) DO_TEST("virtio-options", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_KEYBOARD, QEMU_CAPS_VIRTIO_MOUSE, QEMU_CAPS_VIRTIO_TABLET, QEMU_CAPS_VIRTIO_INPUT_HOST, - QEMU_CAPS_DEVICE_VIRTIO_GPU, - QEMU_CAPS_VIRTIO_GPU_VIRGL, + QEMU_CAPS_DEVICE_VHOST_USER_GPU, QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, -- 2.23.0

On 9/13/19 8:50 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Check qemu capability, and accept 3d acceleration. 3d acceleration support is checked when looking for a suitable vhost-user helper.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_process.c | 57 +++++++++++++++++++++++----------------- tests/qemuxml2argvtest.c | 3 +-- 2 files changed, 34 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 955ba4de4c..463b783966 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5266,34 +5266,43 @@ qemuProcessStartValidateVideo(virDomainObjPtr vm, for (i = 0; i < vm->def->nvideos; i++) { video = vm->def->videos[i];
- if ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("this QEMU does not support '%s' video device"), - virDomainVideoTypeToString(video->type)); - return -1; - } - - if (video->accel) { - if (video->accel->accel3d == VIR_TRISTATE_SWITCH_ON && - (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO || - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_VIRGL))) { + if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) { + if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_GPU)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU does not support 'vhost-user' video device")); + return -1; + } + } else { + if ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("%s 3d acceleration is not supported"), + _("this QEMU does not support '%s' video device"), virDomainVideoTypeToString(video->type)); return -1; } + + if (video->accel) { + if (video->accel->accel3d == VIR_TRISTATE_SWITCH_ON && + (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_VIRGL))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("%s 3d acceleration is not supported"), + virDomainVideoTypeToString(video->type)); + return -1; + } + } } }
The quantity of indented code says to me this function should be split up, but I think all this should be moved to Validate time anyways, so it's something for the future Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole

From: Marc-André Lureau <marcandre.lureau@redhat.com> vhost-user device doesn't have a virgl option, it is passed to the vhost-user-gpu helper process instead. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_command.c | 9 ++++++--- tests/qemuxml2argvdata/virtio-options.args | 3 +-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f795f2e987..0a9110f3fa 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4555,9 +4555,12 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, virBufferAsprintf(&buf, ",id=%s", video->info.alias); - if (video->accel && video->accel->accel3d == VIR_TRISTATE_SWITCH_ON) { - virBufferAsprintf(&buf, ",virgl=%s", - virTristateSwitchTypeToString(video->accel->accel3d)); + if (video->backend != VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER && + video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { + if (video->accel && video->accel->accel3d == VIR_TRISTATE_SWITCH_ON) { + virBufferAsprintf(&buf, ",virgl=%s", + virTristateSwitchTypeToString(video->accel->accel3d)); + } } if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { diff --git a/tests/qemuxml2argvdata/virtio-options.args b/tests/qemuxml2argvdata/virtio-options.args index 40f85fdaa4..92bce8283c 100644 --- a/tests/qemuxml2argvdata/virtio-options.args +++ b/tests/qemuxml2argvdata/virtio-options.args @@ -49,8 +49,7 @@ ats=on \ ats=on \ -device virtio-input-host-pci,id=input3,evdev=/dev/input/event1234,bus=pci.0,\ addr=0x12,iommu_platform=on,ats=on \ --device virtio-gpu-pci,id=video0,virgl=on,bus=pci.0,addr=0x2,iommu_platform=on,\ -ats=on \ +-device virtio-gpu-pci,id=video0,bus=pci.0,addr=0x2,iommu_platform=on,ats=on \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xc,iommu_platform=on,\ ats=on \ -object rng-random,id=objrng0,filename=/dev/random \ -- 2.23.0

On 9/13/19 8:50 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
vhost-user device doesn't have a virgl option, it is passed to the vhost-user-gpu helper process instead.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_command.c | 9 ++++++--- tests/qemuxml2argvdata/virtio-options.args | 3 +-- 2 files changed, 7 insertions(+), 5 deletions(-)
Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole

From: Marc-André Lureau <marcandre.lureau@redhat.com> Add qemuVhostUserFetchConfigs() to discover vhost-user helpers. qemuVhostUserFillDomainGPU() will find the first matching GPU helper with the required capabilities and set the associated vhost_user_binary. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/conf/domain_conf.c | 2 + src/conf/domain_conf.h | 1 + src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_vhost_user.c | 422 ++++++++++++++++++ src/qemu/qemu_vhost_user.h | 48 ++ tests/Makefile.am | 9 + .../etc/qemu/vhost-user/40-gpu.json | 1 + .../etc/qemu/vhost-user/50-gpu.json | 0 .../qemu/vhost-user/test-vhost-user-gpu | 11 + .../usr/share/qemu/vhost-user/30-gpu.json | 1 + .../usr/share/qemu/vhost-user/50-gpu.json | 8 + .../usr/share/qemu/vhost-user/60-gpu.json | 1 + tests/qemuvhostusertest.c | 132 ++++++ 13 files changed, 638 insertions(+) create mode 100644 src/qemu/qemu_vhost_user.c create mode 100644 src/qemu/qemu_vhost_user.h create mode 120000 tests/qemuvhostuserdata/etc/qemu/vhost-user/40-gpu.json create mode 100644 tests/qemuvhostuserdata/etc/qemu/vhost-user/50-gpu.json create mode 100755 tests/qemuvhostuserdata/usr/libexec/qemu/vhost-user/test-vhost-user-gpu create mode 120000 tests/qemuvhostuserdata/usr/share/qemu/vhost-user/30-gpu.json create mode 100644 tests/qemuvhostuserdata/usr/share/qemu/vhost-user/50-gpu.json create mode 120000 tests/qemuvhostuserdata/usr/share/qemu/vhost-user/60-gpu.json create mode 100644 tests/qemuvhostusertest.c diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 15fb8dea17..9e1c8babf8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2844,6 +2844,8 @@ virDomainVideoDefClear(virDomainVideoDefPtr def) VIR_FREE(def->accel->rendernode); VIR_FREE(def->accel); VIR_FREE(def->virtio); + if (def->driver) + VIR_FREE(def->driver->vhost_user_binary); VIR_FREE(def->driver); memset(def, 0, sizeof(*def)); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 16dacd4b43..cec945de54 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1423,6 +1423,7 @@ struct _virDomainVideoAccelDef { struct _virDomainVideoDriverDef { virDomainVideoVGAConf vgaconf; + char *vhost_user_binary; }; struct _virDomainVideoDef { diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 7351dc0486..449550e64b 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -64,6 +64,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_slirp.h \ qemu/qemu_tpm.c \ qemu/qemu_tpm.h \ + qemu/qemu_vhost_user.c \ + qemu/qemu_vhost_user.h \ $(NULL) diff --git a/src/qemu/qemu_vhost_user.c b/src/qemu/qemu_vhost_user.c new file mode 100644 index 0000000000..bdc6cfb104 --- /dev/null +++ b/src/qemu/qemu_vhost_user.c @@ -0,0 +1,422 @@ +/* + * qemu_vhost_user.c: QEMU vhost-user + * + * Copyright (C) 2019 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/>. + */ + +#include <config.h> + +#include "qemu_vhost_user.h" +#include "qemu_interop_config.h" +#include "virjson.h" +#include "virlog.h" +#include "virstring.h" +#include "viralloc.h" +#include "virenum.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +VIR_LOG_INIT("qemu.qemu_vhost_user"); + +typedef enum { + QEMU_VHOST_USER_TYPE_NONE = 0, + QEMU_VHOST_USER_TYPE_9P, + QEMU_VHOST_USER_TYPE_BALLOON, + QEMU_VHOST_USER_TYPE_BLOCK, + QEMU_VHOST_USER_TYPE_CAIF, + QEMU_VHOST_USER_TYPE_CONSOLE, + QEMU_VHOST_USER_TYPE_CRYPTO, + QEMU_VHOST_USER_TYPE_GPU, + QEMU_VHOST_USER_TYPE_INPUT, + QEMU_VHOST_USER_TYPE_NET, + QEMU_VHOST_USER_TYPE_RNG, + QEMU_VHOST_USER_TYPE_RPMSG, + QEMU_VHOST_USER_TYPE_RPROC_SERIAL, + QEMU_VHOST_USER_TYPE_SCSI, + QEMU_VHOST_USER_TYPE_VSOCK, + QEMU_VHOST_USER_TYPE_FS, + + QEMU_VHOST_USER_TYPE_LAST +} qemuVhostUserType; + +VIR_ENUM_DECL(qemuVhostUserType); +VIR_ENUM_IMPL(qemuVhostUserType, + QEMU_VHOST_USER_TYPE_LAST, + "", + "9p", + "balloon", + "block", + "caif", + "console", + "crypto", + "gpu", + "input", + "net", + "rng", + "rpmsg", + "rproc-serial", + "scsi", + "vsock", + "fs", +); + +typedef enum { + QEMU_VHOST_USER_GPU_FEATURE_NONE = 0, + QEMU_VHOST_USER_GPU_FEATURE_VIRGL, + QEMU_VHOST_USER_GPU_FEATURE_RENDER_NODE, + + QEMU_VHOST_USER_GPU_FEATURE_LAST +} qemuVhostUserGPUFeature; + +VIR_ENUM_DECL(qemuVhostUserGPUFeature); +VIR_ENUM_IMPL(qemuVhostUserGPUFeature, + QEMU_VHOST_USER_GPU_FEATURE_LAST, + "", + "virgl", + "render-node", +); + +typedef struct _qemuVhostUserGPU qemuVhostUserGPU; +typedef qemuVhostUserGPU *qemuVhostUserGPUPtr; +struct _qemuVhostUserGPU { + size_t nfeatures; + qemuVhostUserGPUFeature *features; +}; + +struct _qemuVhostUser { + /* Description intentionally not parsed. */ + qemuVhostUserType type; + char *binary; + /* Tags intentionally not parsed. */ + + union { + qemuVhostUserGPU gpu; + } capabilities; +}; + + +static void +qemuVhostUserGPUFeatureFree(qemuVhostUserGPUFeature *features) +{ + VIR_FREE(features); +} + + +VIR_DEFINE_AUTOPTR_FUNC(qemuVhostUserGPUFeature, qemuVhostUserGPUFeatureFree); + + +void +qemuVhostUserFree(qemuVhostUserPtr vu) +{ + if (!vu) + return; + + if (vu->type == QEMU_VHOST_USER_TYPE_GPU) + VIR_FREE(vu->capabilities.gpu.features); + + VIR_FREE(vu->binary); + + VIR_FREE(vu); +} + + +/* 1MiB should be enough for everybody (TM) */ +#define DOCUMENT_SIZE (1024 * 1024) + + +static int +qemuVhostUserTypeParse(const char *path, + virJSONValuePtr doc, + qemuVhostUserPtr vu) +{ + const char *type = virJSONValueObjectGetString(doc, "type"); + int tmp; + + VIR_DEBUG("vhost-user description path '%s' type : %s", + path, type); + + if ((tmp = qemuVhostUserTypeTypeFromString(type)) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown vhost-user type: '%s'"), + type); + return -1; + } + + vu->type = tmp; + + return 0; +} + + +static int +qemuVhostUserBinaryParse(const char *path, + virJSONValuePtr doc, + qemuVhostUserPtr vu) +{ + const char *binary = virJSONValueObjectGetString(doc, "binary"); + + VIR_DEBUG("vhost-user description path '%s' binary : %s", + path, binary); + + if (VIR_STRDUP(vu->binary, binary) < 0) + return -1; + + return 0; +} + + +qemuVhostUserPtr +qemuVhostUserParse(const char *path) +{ + VIR_AUTOFREE(char *) cont = NULL; + VIR_AUTOPTR(virJSONValue) doc = NULL; + VIR_AUTOPTR(qemuVhostUser) vu = NULL; + qemuVhostUserPtr ret = NULL; + + if (virFileReadAll(path, DOCUMENT_SIZE, &cont) < 0) + return NULL; + + if (!(doc = virJSONValueFromString(cont))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to parse json file '%s'"), + path); + return NULL; + } + + if (VIR_ALLOC(vu) < 0) + return NULL; + + if (qemuVhostUserTypeParse(path, doc, vu) < 0) + return NULL; + + if (qemuVhostUserBinaryParse(path, doc, vu) < 0) + return NULL; + + VIR_STEAL_PTR(ret, vu); + return ret; +} + + +char * +qemuVhostUserFormat(qemuVhostUserPtr vu) +{ + VIR_AUTOPTR(virJSONValue) doc = NULL; + + if (!vu) + return NULL; + + if (!(doc = virJSONValueNewObject())) + return NULL; + + if (virJSONValueObjectAppendString(doc, "type", + qemuVhostUserTypeTypeToString(vu->type)) < 0) + return NULL; + + if (virJSONValueObjectAppendString(doc, "binary", vu->binary) < 0) + return NULL; + + return virJSONValueToString(doc, true); +} + + +int +qemuVhostUserFetchConfigs(char ***configs, + bool privileged) +{ + return qemuInteropFetchConfigs("vhost-user", configs, privileged); +} + + +static ssize_t +qemuVhostUserFetchParsedConfigs(bool privileged, + qemuVhostUserPtr **vhostuserRet, + char ***pathsRet) +{ + VIR_AUTOSTRINGLIST paths = NULL; + size_t npaths; + qemuVhostUserPtr *vus = NULL; + size_t i; + + if (qemuVhostUserFetchConfigs(&paths, privileged) < 0) + return -1; + + npaths = virStringListLength((const char **)paths); + + if (VIR_ALLOC_N(vus, npaths) < 0) + return -1; + + for (i = 0; i < npaths; i++) { + if (!(vus[i] = qemuVhostUserParse(paths[i]))) + goto error; + } + + VIR_STEAL_PTR(*vhostuserRet, vus); + if (pathsRet) + VIR_STEAL_PTR(*pathsRet, paths); + return npaths; + + error: + while (i > 0) + qemuVhostUserFree(vus[--i]); + VIR_FREE(vus); + return -1; +} + + +static int +qemuVhostUserGPUFillCapabilities(qemuVhostUserPtr vu, + virJSONValuePtr doc) +{ + qemuVhostUserGPUPtr gpu = &vu->capabilities.gpu; + virJSONValuePtr featuresJSON; + size_t nfeatures; + size_t i; + VIR_AUTOPTR(qemuVhostUserGPUFeature) features = NULL; + + if (!(featuresJSON = virJSONValueObjectGetArray(doc, "features"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to get features from '%s'"), + vu->binary); + return -1; + } + + nfeatures = virJSONValueArraySize(featuresJSON); + if (VIR_ALLOC_N(features, nfeatures) < 0) + return -1; + + for (i = 0; i < nfeatures; i++) { + virJSONValuePtr item = virJSONValueArrayGet(featuresJSON, i); + const char *tmpStr = virJSONValueGetString(item); + int tmp; + + if ((tmp = qemuVhostUserGPUFeatureTypeFromString(tmpStr)) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown feature %s"), + tmpStr); + continue; + } + + features[i] = tmp; + } + + VIR_STEAL_PTR(gpu->features, features); + gpu->nfeatures = nfeatures; + + return 0; +} + + +static bool +qemuVhostUserGPUHasFeature(qemuVhostUserGPUPtr gpu, + qemuVhostUserGPUFeature feature) +{ + size_t i; + + for (i = 0; i < gpu->nfeatures; i++) { + if (gpu->features[i] == feature) + return true; + } + + return false; +} + + +int +qemuVhostUserFillDomainGPU(virQEMUDriverPtr driver, + virDomainVideoDefPtr video) +{ + qemuVhostUserPtr *vus = NULL; + qemuVhostUserPtr vu = NULL; + ssize_t nvus = 0; + ssize_t i; + int ret = -1; + + if ((nvus = qemuVhostUserFetchParsedConfigs(driver->privileged, + &vus, NULL)) < 0) + goto end; + + for (i = 0; i < nvus; i++) { + VIR_AUTOPTR(virJSONValue) doc = NULL; + VIR_AUTOFREE(char *) output = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; + + vu = vus[i]; + if (vu->type != QEMU_VHOST_USER_TYPE_GPU) + continue; + + cmd = virCommandNewArgList(vu->binary, "--print-capabilities", NULL); + virCommandSetOutputBuffer(cmd, &output); + if (virCommandRun(cmd, NULL) < 0) + continue; + + if (!(doc = virJSONValueFromString(output))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to parse json capabilities '%s'"), + vu->binary); + continue; + } + + if (qemuVhostUserGPUFillCapabilities(vu, doc) < 0) + continue; + + if (video->accel) { + if (video->accel->accel3d && + !qemuVhostUserGPUHasFeature(&vu->capabilities.gpu, + QEMU_VHOST_USER_GPU_FEATURE_VIRGL)) + continue; + + if (video->accel->rendernode && + !qemuVhostUserGPUHasFeature(&vu->capabilities.gpu, + QEMU_VHOST_USER_GPU_FEATURE_RENDER_NODE)) + continue; + } + + if (!video->driver && VIR_ALLOC(video->driver) < 0) + goto end; + + VIR_FREE(video->driver->vhost_user_binary); + if (VIR_STRDUP(video->driver->vhost_user_binary, vu->binary) < 0) + goto end; + + break; + } + + if (i == nvus) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Unable to find a satisfying vhost-user-gpu")); + goto end; + } + + if (!video->accel && VIR_ALLOC(video->accel) < 0) + goto end; + + if (!video->accel->rendernode && + qemuVhostUserGPUHasFeature(&vu->capabilities.gpu, + QEMU_VHOST_USER_GPU_FEATURE_RENDER_NODE)) { + video->accel->rendernode = virHostGetDRMRenderNode(); + if (!video->accel->rendernode) + goto end; + } + + ret = 0; + + end: + for (i = 0; i < nvus; i++) + qemuVhostUserFree(vus[i]); + VIR_FREE(vus); + return ret; +} diff --git a/src/qemu/qemu_vhost_user.h b/src/qemu/qemu_vhost_user.h new file mode 100644 index 0000000000..76701dd1fa --- /dev/null +++ b/src/qemu/qemu_vhost_user.h @@ -0,0 +1,48 @@ +/* + * qemu_vhost_user.h: QEMU vhost-user + * + * Copyright (C) 2019 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/>. + */ + +#pragma once + +#include "domain_conf.h" +#include "qemu_conf.h" +#include "virautoclean.h" +#include "virarch.h" + +typedef struct _qemuVhostUser qemuVhostUser; +typedef qemuVhostUser *qemuVhostUserPtr; + +void +qemuVhostUserFree(qemuVhostUserPtr fw); + +VIR_DEFINE_AUTOPTR_FUNC(qemuVhostUser, qemuVhostUserFree); + +qemuVhostUserPtr +qemuVhostUserParse(const char *path); + +char * +qemuVhostUserFormat(qemuVhostUserPtr fw); + +int +qemuVhostUserFetchConfigs(char ***configs, + bool privileged); + +int +qemuVhostUserFillDomainGPU(virQEMUDriverPtr driver, + virDomainVideoDefPtr video); diff --git a/tests/Makefile.am b/tests/Makefile.am index 8c343857a5..9093b35bbc 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -291,6 +291,7 @@ test_programs += qemuxml2argvtest qemuxml2xmltest \ qemumigparamstest \ qemusecuritytest \ qemufirmwaretest \ + qemuvhostusertest \ $(NULL) test_helpers += qemucapsprobe test_libraries += libqemumonitortestutils.la \ @@ -693,6 +694,13 @@ qemufirmwaretest_SOURCES = \ $(NULL) qemufirmwaretest_LDADD = $(qemu_LDADDS) +qemuvhostusertest_SOURCES = \ + qemuvhostusertest.c \ + testutils.h testutils.c \ + virfilewrapper.c virfilewrapper.h \ + $(NULL) +qemuvhostusertest_LDADD = $(qemu_LDADDS) + else ! WITH_QEMU EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c \ qemudomaincheckpointxml2xmltest.c qemudomainsnapshotxml2xmltest.c \ @@ -707,6 +715,7 @@ EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c \ qemusecuritytest.c qemusecuritytest.h \ qemusecuritymock.c \ qemufirmwaretest.c \ + qemuvhostusertest.c \ $(QEMUMONITORTESTUTILS_SOURCES) endif ! WITH_QEMU diff --git a/tests/qemuvhostuserdata/etc/qemu/vhost-user/40-gpu.json b/tests/qemuvhostuserdata/etc/qemu/vhost-user/40-gpu.json new file mode 120000 index 0000000000..aa93864aa7 --- /dev/null +++ b/tests/qemuvhostuserdata/etc/qemu/vhost-user/40-gpu.json @@ -0,0 +1 @@ +../../../usr/share/qemu/vhost-user/50-gpu.json \ No newline at end of file diff --git a/tests/qemuvhostuserdata/etc/qemu/vhost-user/50-gpu.json b/tests/qemuvhostuserdata/etc/qemu/vhost-user/50-gpu.json new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qemuvhostuserdata/usr/libexec/qemu/vhost-user/test-vhost-user-gpu b/tests/qemuvhostuserdata/usr/libexec/qemu/vhost-user/test-vhost-user-gpu new file mode 100755 index 0000000000..a2c2ee0713 --- /dev/null +++ b/tests/qemuvhostuserdata/usr/libexec/qemu/vhost-user/test-vhost-user-gpu @@ -0,0 +1,11 @@ +#!/bin/sh + +cat <<EOF +{ + "type": "gpu", + "features": [ + "render-node", + "virgl" + ] +} +EOF diff --git a/tests/qemuvhostuserdata/usr/share/qemu/vhost-user/30-gpu.json b/tests/qemuvhostuserdata/usr/share/qemu/vhost-user/30-gpu.json new file mode 120000 index 0000000000..7051776593 --- /dev/null +++ b/tests/qemuvhostuserdata/usr/share/qemu/vhost-user/30-gpu.json @@ -0,0 +1 @@ +50-gpu.json \ No newline at end of file diff --git a/tests/qemuvhostuserdata/usr/share/qemu/vhost-user/50-gpu.json b/tests/qemuvhostuserdata/usr/share/qemu/vhost-user/50-gpu.json new file mode 100644 index 0000000000..4c751971db --- /dev/null +++ b/tests/qemuvhostuserdata/usr/share/qemu/vhost-user/50-gpu.json @@ -0,0 +1,8 @@ +{ + "description": "QEMU vhost-user-gpu", + "type": "gpu", + "binary": "/usr/libexec/qemu/vhost-user/test-vhost-user-gpu", + "tags": [ + "CONFIG_OPENGL_DMABUF=y" + ] +} diff --git a/tests/qemuvhostuserdata/usr/share/qemu/vhost-user/60-gpu.json b/tests/qemuvhostuserdata/usr/share/qemu/vhost-user/60-gpu.json new file mode 120000 index 0000000000..7051776593 --- /dev/null +++ b/tests/qemuvhostuserdata/usr/share/qemu/vhost-user/60-gpu.json @@ -0,0 +1 @@ +50-gpu.json \ No newline at end of file diff --git a/tests/qemuvhostusertest.c b/tests/qemuvhostusertest.c new file mode 100644 index 0000000000..52adf9834a --- /dev/null +++ b/tests/qemuvhostusertest.c @@ -0,0 +1,132 @@ +#include <config.h> + +#include <inttypes.h> + +#include "testutils.h" +#include "virfilewrapper.h" +#include "qemu/qemu_vhost_user.h" +#include "configmake.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +/* A very basic test. Parse given JSON vhostuser description into + * an internal structure, format it back and compare with the + * contents of the file (minus some keys that are not parsed). + */ +static int +testParseFormatVU(const void *opaque) +{ + const char *filename = opaque; + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOPTR(qemuVhostUser) vu = NULL; + VIR_AUTOFREE(char *) buf = NULL; + VIR_AUTOPTR(virJSONValue) json = NULL; + VIR_AUTOFREE(char *) expected = NULL; + VIR_AUTOFREE(char *) actual = NULL; + + if (virAsprintf(&path, "%s/qemuvhostuserdata/%s", + abs_srcdir, filename) < 0) + return -1; + + if (!(vu = qemuVhostUserParse(path))) + return -1; + + if (virFileReadAll(path, + 1024 * 1024, /* 1MiB */ + &buf) < 0) + return -1; + + if (!(json = virJSONValueFromString(buf))) + return -1; + + /* Description and tags are not parsed. */ + if (virJSONValueObjectRemoveKey(json, "description", NULL) < 0 || + virJSONValueObjectRemoveKey(json, "tags", NULL) < 0) + return -1; + + if (!(expected = virJSONValueToString(json, true))) + return -1; + + if (!(actual = qemuVhostUserFormat(vu))) + return -1; + + return virTestCompareToString(expected, actual); +} + + +static int +testVUPrecedence(const void *opaque ATTRIBUTE_UNUSED) +{ + VIR_AUTOFREE(char *) fakehome = NULL; + VIR_AUTOSTRINGLIST vuList = NULL; + size_t nvuList; + size_t i; + const char *expected[] = { + PREFIX "/share/qemu/vhost-user/30-gpu.json", + SYSCONFDIR "/qemu/vhost-user/40-gpu.json", + PREFIX "/share/qemu/vhost-user/60-gpu.json", + }; + const size_t nexpected = ARRAY_CARDINALITY(expected); + + if (VIR_STRDUP(fakehome, abs_srcdir "/qemuvhostuserdata/home/user/.config") < 0) + return -1; + + setenv("XDG_CONFIG_HOME", fakehome, 1); + + if (qemuVhostUserFetchConfigs(&vuList, false) < 0) + return -1; + + if (!vuList) { + fprintf(stderr, "Expected a non-NULL result, but got a NULL result\n"); + return -1; + } + + nvuList = virStringListLength((const char **)vuList); + + for (i = 0; i < MAX(nvuList, nexpected); i++) { + const char *e = i < nexpected ? expected[i] : NULL; + const char *f = i < nvuList ? vuList[i] : NULL; + + if (STRNEQ_NULLABLE(e, f)) { + fprintf(stderr, + "Unexpected path (i=%zu). Expected %s got %s \n", + i, NULLSTR(e), NULLSTR(f)); + return -1; + } + } + + return 0; +} + + +static int +mymain(void) +{ + int ret = 0; + + virFileWrapperAddPrefix(SYSCONFDIR "/qemu/vhost-user", + abs_srcdir "/qemuvhostuserdata/etc/qemu/vhost-user"); + virFileWrapperAddPrefix(PREFIX "/share/qemu/vhost-user", + abs_srcdir "/qemuvhostuserdata/usr/share/qemu/vhost-user"); + virFileWrapperAddPrefix("/home/user/.config/qemu/vhost-user", + abs_srcdir "/qemuvhostuserdata/home/user/.config/qemu/vhost-user"); + +#define DO_PARSE_TEST(filename) \ + do { \ + if (virTestRun("QEMU vhost-user " filename, \ + testParseFormatVU, filename) < 0) \ + ret = -1; \ + } while (0) + + DO_PARSE_TEST("usr/share/qemu/vhost-user/50-gpu.json"); + + if (virTestRun("QEMU vhost-user precedence test", testVUPrecedence, NULL) < 0) + ret = -1; + + virFileWrapperClearPrefixes(); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + + +VIR_TEST_MAIN(mymain) -- 2.23.0

On 9/13/19 8:50 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Add qemuVhostUserFetchConfigs() to discover vhost-user helpers.
qemuVhostUserFillDomainGPU() will find the first matching GPU helper with the required capabilities and set the associated vhost_user_binary.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole

From: Marc-André Lureau <marcandre.lureau@redhat.com> See function documentation. Used in a following patch. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_security.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_security.h | 6 ++++++ 2 files changed, 46 insertions(+) diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c index 91dd34f0e7..63808c2d17 100644 --- a/src/qemu/qemu_security.c +++ b/src/qemu/qemu_security.c @@ -433,6 +433,46 @@ qemuSecurityRestoreChardevLabel(virQEMUDriverPtr driver, } +/* + * qemuSecurityStartVhostUserGPU: + * + * @driver: the QEMU driver + * @vm: the domain object + * @cmd: the command to run + * @existstatus: pointer to int returning exit status of process + * @cmdret: pointer to int returning result of virCommandRun + * + * Start the vhost-user-gpu process with approriate labels. + * This function returns -1 on security setup error, 0 if all the + * setup was done properly. In case the virCommand failed to run + * 0 is returned but cmdret is set appropriately with the process + * exitstatus also set. + */ +int +qemuSecurityStartVhostUserGPU(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virCommandPtr cmd, + int *exitstatus, + int *cmdret) +{ + if (virSecurityManagerSetChildProcessLabel(driver->securityManager, + vm->def, cmd) < 0) + return -1; + + if (virSecurityManagerPreFork(driver->securityManager) < 0) + return -1; + + *cmdret = virCommandRun(cmd, exitstatus); + + virSecurityManagerPostFork(driver->securityManager); + + if (*cmdret < 0) + return -1; + + return 0; +} + + /* * qemuSecurityStartTPMEmulator: * diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h index 224a4d61c9..c8a4bd8220 100644 --- a/src/qemu/qemu_security.h +++ b/src/qemu/qemu_security.h @@ -77,6 +77,12 @@ int qemuSecurityRestoreChardevLabel(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainChrDefPtr chr); +int qemuSecurityStartVhostUserGPU(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virCommandPtr cmd, + int *exitstatus, + int *cmdret); + int qemuSecurityStartTPMEmulator(virQEMUDriverPtr driver, virDomainObjPtr vm, virCommandPtr cmd, -- 2.23.0

On 9/13/19 8:50 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
See function documentation. Used in a following patch.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_security.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_security.h | 6 ++++++ 2 files changed, 46 insertions(+)
Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole

From: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/conf/domain_conf.c | 26 +++++++++++++++++--------- src/conf/domain_conf.h | 7 +++++-- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9e1c8babf8..e99e52dc5e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2820,13 +2820,19 @@ void virDomainShmemDefFree(virDomainShmemDefPtr def) virDomainVideoDefPtr -virDomainVideoDefNew(void) +virDomainVideoDefNew(virDomainXMLOptionPtr xmlopt) { virDomainVideoDefPtr def; if (VIR_ALLOC(def) < 0) return NULL; + if (xmlopt && xmlopt->privateData.videoNew && + !(def->privateData = xmlopt->privateData.videoNew())) { + VIR_FREE(def); + return NULL; + } + def->heads = 1; return def; } @@ -2858,6 +2864,7 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def) return; virDomainVideoDefClear(def); + virObjectUnref(def->privateData); VIR_FREE(def); } @@ -5681,7 +5688,8 @@ virDomainDefPostParseVideo(virDomainDefPtr def, static int virDomainDefPostParseCommon(virDomainDefPtr def, - struct virDomainDefPostParseDeviceIteratorData *data) + struct virDomainDefPostParseDeviceIteratorData *data, + virDomainXMLOptionPtr xmlopt) { size_t i; @@ -5716,7 +5724,7 @@ virDomainDefPostParseCommon(virDomainDefPtr def, if (virDomainDefPostParseTimer(def) < 0) return -1; - if (virDomainDefAddImplicitDevices(def) < 0) + if (virDomainDefAddImplicitDevices(def, xmlopt) < 0) return -1; if (virDomainDefPostParseVideo(def, data) < 0) @@ -5842,7 +5850,7 @@ virDomainDefPostParse(virDomainDefPtr def, if (virDomainDefPostParseCheckFailure(def, parseFlags, ret) < 0) goto cleanup; - if ((ret = virDomainDefPostParseCommon(def, &data)) < 0) + if ((ret = virDomainDefPostParseCommon(def, &data, xmlopt)) < 0) goto cleanup; if (xmlopt->config.assignAddressesCallback) { @@ -15450,7 +15458,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_AUTOFREE(char *) vgamem = NULL; VIR_AUTOFREE(char *) primary = NULL; - if (!(def = virDomainVideoDefNew())) + if (!(def = virDomainVideoDefNew(xmlopt))) return NULL; ctxt->node = node; @@ -23771,7 +23779,7 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def) } static int -virDomainDefAddImplicitVideo(virDomainDefPtr def) +virDomainDefAddImplicitVideo(virDomainDefPtr def, virDomainXMLOptionPtr xmlopt) { int ret = -1; virDomainVideoDefPtr video = NULL; @@ -23781,7 +23789,7 @@ virDomainDefAddImplicitVideo(virDomainDefPtr def) if (def->ngraphics == 0 || def->nvideos > 0) return 0; - if (!(video = virDomainVideoDefNew())) + if (!(video = virDomainVideoDefNew(xmlopt))) goto cleanup; video->type = virDomainVideoDefaultType(def); if (VIR_APPEND_ELEMENT(def->videos, def->nvideos, video) < 0) @@ -23794,7 +23802,7 @@ virDomainDefAddImplicitVideo(virDomainDefPtr def) } int -virDomainDefAddImplicitDevices(virDomainDefPtr def) +virDomainDefAddImplicitDevices(virDomainDefPtr def, virDomainXMLOptionPtr xmlopt) { if (virDomainDefAddConsoleCompat(def) < 0) return -1; @@ -23802,7 +23810,7 @@ virDomainDefAddImplicitDevices(virDomainDefPtr def) if (virDomainDefAddImplicitControllers(def) < 0) return -1; - if (virDomainDefAddImplicitVideo(def) < 0) + if (virDomainDefAddImplicitVideo(def, xmlopt) < 0) return -1; return 0; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cec945de54..78aa1e83f2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1427,6 +1427,8 @@ struct _virDomainVideoDriverDef { }; struct _virDomainVideoDef { + virObjectPtr privateData; + int type; /* enum virDomainVideoType */ unsigned int ram; /* kibibytes (multiples of 1024) */ unsigned int vram; /* kibibytes (multiples of 1024) */ @@ -2729,6 +2731,7 @@ struct _virDomainXMLPrivateDataCallbacks { virDomainXMLPrivateDataNewFunc vsockNew; virDomainXMLPrivateDataNewFunc graphicsNew; virDomainXMLPrivateDataNewFunc networkNew; + virDomainXMLPrivateDataNewFunc videoNew; virDomainXMLPrivateDataFormatFunc format; virDomainXMLPrivateDataParseFunc parse; /* following function shall return a pointer which will be used as the @@ -2870,7 +2873,7 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def); void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def); void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def); void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def); -virDomainVideoDefPtr virDomainVideoDefNew(void); +virDomainVideoDefPtr virDomainVideoDefNew(virDomainXMLOptionPtr xmlopt); void virDomainVideoDefFree(virDomainVideoDefPtr def); void virDomainVideoDefClear(virDomainVideoDefPtr def); virDomainHostdevDefPtr virDomainHostdevDefNew(void); @@ -3062,7 +3065,7 @@ bool virDomainDefCheckABIStabilityFlags(virDomainDefPtr src, virDomainXMLOptionPtr xmlopt, unsigned int flags); -int virDomainDefAddImplicitDevices(virDomainDefPtr def); +int virDomainDefAddImplicitDevices(virDomainDefPtr def, virDomainXMLOptionPtr xmlopt); virDomainIOThreadIDDefPtr virDomainIOThreadIDFind(const virDomainDef *def, unsigned int iothread_id); -- 2.23.0

On 9/13/19 8:50 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/conf/domain_conf.c | 26 +++++++++++++++++--------- src/conf/domain_conf.h | 7 +++++-- 2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9e1c8babf8..e99e52dc5e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2820,13 +2820,19 @@ void virDomainShmemDefFree(virDomainShmemDefPtr def)
virDomainVideoDefPtr -virDomainVideoDefNew(void) +virDomainVideoDefNew(virDomainXMLOptionPtr xmlopt) { virDomainVideoDefPtr def;
if (VIR_ALLOC(def) < 0) return NULL;
+ if (xmlopt && xmlopt->privateData.videoNew && + !(def->privateData = xmlopt->privateData.videoNew())) { + VIR_FREE(def); + return NULL; + } + def->heads = 1; return def; } @@ -2858,6 +2864,7 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def) return;
virDomainVideoDefClear(def); + virObjectUnref(def->privateData); VIR_FREE(def); }
@@ -5681,7 +5688,8 @@ virDomainDefPostParseVideo(virDomainDefPtr def,
static int virDomainDefPostParseCommon(virDomainDefPtr def, - struct virDomainDefPostParseDeviceIteratorData *data) + struct virDomainDefPostParseDeviceIteratorData *data, + virDomainXMLOptionPtr xmlopt) { size_t i;
@@ -5716,7 +5724,7 @@ virDomainDefPostParseCommon(virDomainDefPtr def, if (virDomainDefPostParseTimer(def) < 0) return -1;
- if (virDomainDefAddImplicitDevices(def) < 0) + if (virDomainDefAddImplicitDevices(def, xmlopt) < 0) return -1;
if (virDomainDefPostParseVideo(def, data) < 0) @@ -5842,7 +5850,7 @@ virDomainDefPostParse(virDomainDefPtr def, if (virDomainDefPostParseCheckFailure(def, parseFlags, ret) < 0) goto cleanup;
- if ((ret = virDomainDefPostParseCommon(def, &data)) < 0) + if ((ret = virDomainDefPostParseCommon(def, &data, xmlopt)) < 0) goto cleanup;
if (xmlopt->config.assignAddressesCallback) { @@ -15450,7 +15458,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_AUTOFREE(char *) vgamem = NULL; VIR_AUTOFREE(char *) primary = NULL;
- if (!(def = virDomainVideoDefNew())) + if (!(def = virDomainVideoDefNew(xmlopt))) return NULL;
ctxt->node = node; @@ -23771,7 +23779,7 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def) }
static int -virDomainDefAddImplicitVideo(virDomainDefPtr def) +virDomainDefAddImplicitVideo(virDomainDefPtr def, virDomainXMLOptionPtr xmlopt)
newline inbetween the arguments here, and all other similar locations in src/vz there's a usage of virDomainDefAddImplicitDevices which will need xmlopt passed in too. Otherwise: Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole

From: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_domain.c | 38 ++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 12 ++++++++++++ 2 files changed, 50 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 24292b88bf..9437694940 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1309,6 +1309,43 @@ qemuDomainNetworkPrivateDispose(void *obj ATTRIBUTE_UNUSED) } +static virClassPtr qemuDomainVideoPrivateClass; +static void qemuDomainVideoPrivateDispose(void *obj); + + +static int +qemuDomainVideoPrivateOnceInit(void) +{ + if (!VIR_CLASS_NEW(qemuDomainVideoPrivate, virClassForObject())) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(qemuDomainVideoPrivate); + + +static virObjectPtr +qemuDomainVideoPrivateNew(void) +{ + qemuDomainVideoPrivatePtr priv; + + if (qemuDomainVideoPrivateInitialize() < 0) + return NULL; + + if (!(priv = virObjectNew(qemuDomainVideoPrivateClass))) + return NULL; + + return (virObjectPtr) priv; +} + + +static void +qemuDomainVideoPrivateDispose(void *obj ATTRIBUTE_UNUSED) +{ +} + + /* qemuDomainSecretPlainSetup: * @secinfo: Pointer to secret info * @usageType: The virSecretUsageType @@ -3668,6 +3705,7 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { .vsockNew = qemuDomainVsockPrivateNew, .graphicsNew = qemuDomainGraphicsPrivateNew, .networkNew = qemuDomainNetworkPrivateNew, + .videoNew = qemuDomainVideoPrivateNew, .parse = qemuDomainObjPrivateXMLParse, .format = qemuDomainObjPrivateXMLFormat, .getParseOpaque = qemuDomainObjPrivateXMLGetParseOpaque, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 3e87e75c3a..89bd77b3c0 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -501,6 +501,18 @@ struct _qemuDomainVsockPrivate { }; +#define QEMU_DOMAIN_VIDEO_PRIVATE(dev) \ + ((qemuDomainVideoPrivatePtr) (dev)->privateData) + +typedef struct _qemuDomainVideoPrivate qemuDomainVideoPrivate; +typedef qemuDomainVideoPrivate *qemuDomainVideoPrivatePtr; +struct _qemuDomainVideoPrivate { + virObject parent; + + bool tmp_to_remove; +}; + + #define QEMU_DOMAIN_GRAPHICS_PRIVATE(dev) \ ((qemuDomainGraphicsPrivatePtr) (dev)->privateData) -- 2.23.0

On 9/13/19 8:50 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_domain.c | 38 ++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 12 ++++++++++++ 2 files changed, 50 insertions(+)
Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole

From: Marc-André Lureau <marcandre.lureau@redhat.com> Similar to the qemu_tpm.c, add a unit with a few functions to start/stop and setup the cgroup of the external vhost-user-gpu process. See function documentation. The vhost-user connection fd is set on qemuDomainVideoPrivate struct. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_domain.c | 5 +- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_vhost_user_gpu.c | 276 +++++++++++++++++++++++++++++++++ src/qemu/qemu_vhost_user_gpu.h | 49 ++++++ 5 files changed, 332 insertions(+), 2 deletions(-) create mode 100644 src/qemu/qemu_vhost_user_gpu.c create mode 100644 src/qemu/qemu_vhost_user_gpu.h diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 449550e64b..8040bf9366 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -66,6 +66,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_tpm.h \ qemu/qemu_vhost_user.c \ qemu/qemu_vhost_user.h \ + qemu/qemu_vhost_user_gpu.c \ + qemu/qemu_vhost_user_gpu.h \ $(NULL) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9437694940..ab923aa917 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1341,8 +1341,11 @@ qemuDomainVideoPrivateNew(void) static void -qemuDomainVideoPrivateDispose(void *obj ATTRIBUTE_UNUSED) +qemuDomainVideoPrivateDispose(void *obj) { + qemuDomainVideoPrivatePtr priv = obj; + + VIR_FORCE_CLOSE(priv->vhost_user_fd); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 89bd77b3c0..8afb4993d3 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -509,7 +509,7 @@ typedef qemuDomainVideoPrivate *qemuDomainVideoPrivatePtr; struct _qemuDomainVideoPrivate { virObject parent; - bool tmp_to_remove; + int vhost_user_fd; }; diff --git a/src/qemu/qemu_vhost_user_gpu.c b/src/qemu/qemu_vhost_user_gpu.c new file mode 100644 index 0000000000..b259b58434 --- /dev/null +++ b/src/qemu/qemu_vhost_user_gpu.c @@ -0,0 +1,276 @@ +/* + * qemu_vhost_user_gpu.c: QEMU vhost-user GPU support + * + * Copyright (C) 2019 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/>. + */ + +#include <config.h> + +#include "qemu_vhost_user_gpu.h" +#include "qemu_vhost_user.h" +#include "qemu_extdevice.h" + +#include "conf/domain_conf.h" +#include "configmake.h" +#include "vircommand.h" +#include "viralloc.h" +#include "virlog.h" +#include "virutil.h" +#include "virfile.h" +#include "virstring.h" +#include "virtime.h" +#include "virpidfile.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("qemu.vhost_user_gpu"); + + +static char * +qemuVhostUserGPUCreatePidFilename(const char *stateDir, + const char *shortName, + const char *alias) +{ + VIR_AUTOFREE(char *) devicename = NULL; + + if (virAsprintf(&devicename, "%s-%s-vhost-user-gpu", shortName, alias) < 0) + return NULL; + + return virPidFileBuildPath(stateDir, devicename); +} + + +/* + * qemuVhostUserGPUGetPid: + * @binpath: path of executable associated with the pidfile + * @stateDir: the directory where vhost-user-gpu writes the pidfile into + * @shortName: short name of the domain + * @alias: video device alias + * @pid: pointer to pid + * + * Return -errno upon error, or zero on successful reading of the pidfile. + * If the PID was not still alive, zero will be returned, and @pid will be + * set to -1; + */ +static int +qemuVhostUserGPUGetPid(const char *binPath, + const char *stateDir, + const char *shortName, + const char *alias, + pid_t *pid) +{ + VIR_AUTOFREE(char *) pidfile = NULL; + + pidfile = qemuVhostUserGPUCreatePidFilename(stateDir, shortName, alias); + if (!pidfile) + return -ENOMEM; + + return virPidFileReadPathIfAlive(pidfile, pid, binPath); +} + + +int qemuExtVhostUserGPUPrepareDomain(virQEMUDriverPtr driver, + virDomainVideoDefPtr video) +{ + return qemuVhostUserFillDomainGPU(driver, video); +} + + +/* + * qemuExtVhostUserGPUStart: + * @driver: QEMU driver + * @vm: the VM domain + * @video: the video device + * + * Start the external vhost-user-gpu process: + * - open a socketpair for vhost-user communication + * - have the command line built + * - start the external process and sync with it before QEMU start + */ +int qemuExtVhostUserGPUStart(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainVideoDefPtr video) +{ + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOFREE(char *) shortname = NULL; + VIR_AUTOFREE(char *) pidfile = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; + int pair[2] = { -1, -1 }; + int cmdret = 0, rc; + int exitstatus = 0; + pid_t pid; + int ret = -1; + + shortname = virDomainDefGetShortName(vm->def); + if (!shortname) + goto error; + + /* stop any left-over for this VM */ + qemuExtVhostUserGPUStop(driver, vm, video); + + if (!(pidfile = qemuVhostUserGPUCreatePidFilename( + cfg->stateDir, shortname, video->info.alias))) + goto error; + + if (qemuSecuritySetSocketLabel(driver->securityManager, vm->def) < 0) + goto error; + + if (socketpair(AF_UNIX, SOCK_STREAM, 0, pair) < 0) { + virReportSystemError(errno, "%s", _("failed to create socket")); + goto error; + } + + if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0) + goto error; + + cmd = virCommandNew(video->driver->vhost_user_binary); + if (!cmd) + goto error; + + virCommandClearCaps(cmd); + virCommandSetPidFile(cmd, pidfile); + virCommandDaemonize(cmd); + + if (qemuExtDeviceLogCommand(driver, vm, cmd, "vhost-user-gpu") < 0) + goto error; + + virCommandAddArgFormat(cmd, "--fd=%d", pair[0]); + virCommandPassFD(cmd, pair[0], VIR_COMMAND_PASS_FD_CLOSE_PARENT); + pair[0] = -1; + + if (video->accel) { + if (video->accel->accel3d) + virCommandAddArg(cmd, "--virgl"); + + if (video->accel->rendernode) + virCommandAddArgFormat(cmd, "--render-node=%s", video->accel->rendernode); + } + + if (qemuSecurityStartVhostUserGPU(driver, vm, cmd, + &exitstatus, &cmdret) < 0) + goto error; + + if (cmdret < 0 || exitstatus != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not start 'vhost-user-gpu'. exitstatus: %d"), exitstatus); + goto cleanup; + } + + rc = virPidFileReadPath(pidfile, &pid); + if (rc < 0) { + virReportSystemError(-rc, + _("Unable to read vhost-user-gpu pidfile '%s'"), + pidfile); + goto cleanup; + } + + ret = 0; + QEMU_DOMAIN_VIDEO_PRIVATE(video)->vhost_user_fd = pair[1]; + pair[1] = -1; + + cleanup: + VIR_FORCE_CLOSE(pair[0]); + VIR_FORCE_CLOSE(pair[1]); + + return ret; + + error: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("vhost-user-gpu failed to start")); + goto cleanup; +} + + +/* + * qemuExtVhostUserGPUStop: + * + * @driver: QEMU driver + * @vm: the VM domain + * @video: the video device + * + * Check if vhost-user process pidfile is around, kill the process, + * and remove the pidfile. + */ +void qemuExtVhostUserGPUStop(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainVideoDefPtr video) +{ + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOFREE(char *) pidfile = NULL; + VIR_AUTOFREE(char *) shortname = NULL; + virErrorPtr orig_err; + + shortname = virDomainDefGetShortName(vm->def); + if (!(pidfile = qemuVhostUserGPUCreatePidFilename( + cfg->stateDir, shortname, video->info.alias))) { + VIR_WARN("Unable to construct vhost-user-gpu pidfile path"); + return; + } + + virErrorPreserveLast(&orig_err); + if (virPidFileForceCleanupPath(pidfile) < 0) { + VIR_WARN("Unable to kill vhost-user-gpu process"); + } else { + if (unlink(pidfile) < 0 && + errno != ENOENT) { + virReportSystemError(errno, + _("Unable to remove stale pidfile %s"), + pidfile); + } + } + virErrorRestore(&orig_err); +} + + +/* + * qemuExtVhostUserGPUSetupCgroup: + * + * @driver: QEMU driver + * @def: domain definition + * @video: the video device + * @cgroupe: a cgroup + * + * Add the vhost-user-gpu PID to the given cgroup. + */ +int +qemuExtVhostUserGPUSetupCgroup(virQEMUDriverPtr driver, + virDomainDefPtr def, + virDomainVideoDefPtr video, + virCgroupPtr cgroup) +{ + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOFREE(char *) pidfile = NULL; + VIR_AUTOFREE(char *) shortname = NULL; + int rc; + pid_t pid; + + shortname = virDomainDefGetShortName(def); + if (!shortname) + return -1; + + rc = qemuVhostUserGPUGetPid(video->driver->vhost_user_binary, + cfg->stateDir, shortname, video->info.alias, &pid); + if (rc < 0 || (rc == 0 && pid == (pid_t)-1)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not get process id of vhost-user-gpu")); + return -1; + } + if (virCgroupAddProcess(cgroup, pid) < 0) + return -1; + + return 0; +} diff --git a/src/qemu/qemu_vhost_user_gpu.h b/src/qemu/qemu_vhost_user_gpu.h new file mode 100644 index 0000000000..40a8567630 --- /dev/null +++ b/src/qemu/qemu_vhost_user_gpu.h @@ -0,0 +1,49 @@ +/* + * qemu_vhost_user_gpu.h: QEMU vhost-user GPU support + * + * Copyright (C) 2019 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/>. + */ + +#pragma once + +#include "domain_conf.h" +#include "qemu_domain.h" +#include "qemu_security.h" + +int qemuExtVhostUserGPUPrepareDomain(virQEMUDriverPtr driver, + virDomainVideoDefPtr video) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) + ATTRIBUTE_RETURN_CHECK; + +int qemuExtVhostUserGPUStart(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainVideoDefPtr video) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_RETURN_CHECK; + +void qemuExtVhostUserGPUStop(virQEMUDriverPtr driver, + virDomainObjPtr def, + virDomainVideoDefPtr video) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int +qemuExtVhostUserGPUSetupCgroup(virQEMUDriverPtr driver, + virDomainDefPtr def, + virDomainVideoDefPtr video, + virCgroupPtr cgroup) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_RETURN_CHECK; -- 2.23.0

On 9/13/19 8:50 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Similar to the qemu_tpm.c, add a unit with a few functions to start/stop and setup the cgroup of the external vhost-user-gpu process. See function documentation.
The vhost-user connection fd is set on qemuDomainVideoPrivate struct.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_domain.c | 5 +- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_vhost_user_gpu.c | 276 +++++++++++++++++++++++++++++++++ src/qemu/qemu_vhost_user_gpu.h | 49 ++++++ 5 files changed, 332 insertions(+), 2 deletions(-) create mode 100644 src/qemu/qemu_vhost_user_gpu.c create mode 100644 src/qemu/qemu_vhost_user_gpu.h
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 449550e64b..8040bf9366 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -66,6 +66,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_tpm.h \ qemu/qemu_vhost_user.c \ qemu/qemu_vhost_user.h \ + qemu/qemu_vhost_user_gpu.c \ + qemu/qemu_vhost_user_gpu.h \ $(NULL)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9437694940..ab923aa917 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1341,8 +1341,11 @@ qemuDomainVideoPrivateNew(void)
static void -qemuDomainVideoPrivateDispose(void *obj ATTRIBUTE_UNUSED) +qemuDomainVideoPrivateDispose(void *obj) { + qemuDomainVideoPrivatePtr priv = obj; + + VIR_FORCE_CLOSE(priv->vhost_user_fd); }
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 89bd77b3c0..8afb4993d3 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -509,7 +509,7 @@ typedef qemuDomainVideoPrivate *qemuDomainVideoPrivatePtr; struct _qemuDomainVideoPrivate { virObject parent;
- bool tmp_to_remove; + int vhost_user_fd; };
diff --git a/src/qemu/qemu_vhost_user_gpu.c b/src/qemu/qemu_vhost_user_gpu.c new file mode 100644 index 0000000000..b259b58434 --- /dev/null +++ b/src/qemu/qemu_vhost_user_gpu.c @@ -0,0 +1,276 @@ +/* + * qemu_vhost_user_gpu.c: QEMU vhost-user GPU support + * + * Copyright (C) 2019 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/>. + */ + +#include <config.h> + +#include "qemu_vhost_user_gpu.h" +#include "qemu_vhost_user.h" +#include "qemu_extdevice.h" + +#include "conf/domain_conf.h" +#include "configmake.h" +#include "vircommand.h" +#include "viralloc.h" +#include "virlog.h" +#include "virutil.h" +#include "virfile.h" +#include "virstring.h" +#include "virtime.h" +#include "virpidfile.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("qemu.vhost_user_gpu"); + + +static char * +qemuVhostUserGPUCreatePidFilename(const char *stateDir, + const char *shortName, + const char *alias) +{ + VIR_AUTOFREE(char *) devicename = NULL; + + if (virAsprintf(&devicename, "%s-%s-vhost-user-gpu", shortName, alias) < 0) + return NULL; + + return virPidFileBuildPath(stateDir, devicename); +} + + +/* + * qemuVhostUserGPUGetPid: + * @binpath: path of executable associated with the pidfile + * @stateDir: the directory where vhost-user-gpu writes the pidfile into + * @shortName: short name of the domain + * @alias: video device alias + * @pid: pointer to pid + * + * Return -errno upon error, or zero on successful reading of the pidfile. + * If the PID was not still alive, zero will be returned, and @pid will be + * set to -1; + */ +static int +qemuVhostUserGPUGetPid(const char *binPath, + const char *stateDir, + const char *shortName, + const char *alias, + pid_t *pid) +{ + VIR_AUTOFREE(char *) pidfile = NULL; + + pidfile = qemuVhostUserGPUCreatePidFilename(stateDir, shortName, alias); + if (!pidfile) + return -ENOMEM; + + return virPidFileReadPathIfAlive(pidfile, pid, binPath); +} + + +int qemuExtVhostUserGPUPrepareDomain(virQEMUDriverPtr driver, + virDomainVideoDefPtr video) +{ + return qemuVhostUserFillDomainGPU(driver, video); +} + + +/* + * qemuExtVhostUserGPUStart: + * @driver: QEMU driver + * @vm: the VM domain + * @video: the video device + * + * Start the external vhost-user-gpu process: + * - open a socketpair for vhost-user communication + * - have the command line built + * - start the external process and sync with it before QEMU start + */ +int qemuExtVhostUserGPUStart(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainVideoDefPtr video) +{ + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOFREE(char *) shortname = NULL; + VIR_AUTOFREE(char *) pidfile = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; + int pair[2] = { -1, -1 }; + int cmdret = 0, rc; + int exitstatus = 0; + pid_t pid; + int ret = -1; + + shortname = virDomainDefGetShortName(vm->def); + if (!shortname) + goto error; + + /* stop any left-over for this VM */ + qemuExtVhostUserGPUStop(driver, vm, video); + + if (!(pidfile = qemuVhostUserGPUCreatePidFilename( + cfg->stateDir, shortname, video->info.alias))) + goto error; + + if (qemuSecuritySetSocketLabel(driver->securityManager, vm->def) < 0) + goto error; + + if (socketpair(AF_UNIX, SOCK_STREAM, 0, pair) < 0) { + virReportSystemError(errno, "%s", _("failed to create socket")); + goto error; + } + + if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0) + goto error; + + cmd = virCommandNew(video->driver->vhost_user_binary); + if (!cmd) + goto error; + + virCommandClearCaps(cmd); + virCommandSetPidFile(cmd, pidfile); + virCommandDaemonize(cmd); + + if (qemuExtDeviceLogCommand(driver, vm, cmd, "vhost-user-gpu") < 0) + goto error; +
Most, possibly all, of these 'goto error;' usages are overwriting detailed error messages with 'vhost-user-gpu failed to start'. So this needs to be reworked. You can use virGetLastErrorMessage if you wanted to prepend the vhost-user specific bit to the error. Maybe break up this function into chunks, like one to build the Command object
+ virCommandAddArgFormat(cmd, "--fd=%d", pair[0]); + virCommandPassFD(cmd, pair[0], VIR_COMMAND_PASS_FD_CLOSE_PARENT); + pair[0] = -1; + + if (video->accel) { + if (video->accel->accel3d) + virCommandAddArg(cmd, "--virgl"); + + if (video->accel->rendernode) + virCommandAddArgFormat(cmd, "--render-node=%s", video->accel->rendernode); + } + + if (qemuSecurityStartVhostUserGPU(driver, vm, cmd, + &exitstatus, &cmdret) < 0) + goto error; + + if (cmdret < 0 || exitstatus != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not start 'vhost-user-gpu'. exitstatus: %d"), exitstatus); + goto cleanup; + } +
cmdret < 0 will trigger StartVhostUserGPU to return -1, so we will never hit one condition of the below case Otherwise this looks good. Since the series is pretty close otherwise, these bits can be addressed in a follow up patch IMO Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole

Hi On Sat, Sep 21, 2019 at 1:05 AM Cole Robinson <crobinso@redhat.com> wrote:
On 9/13/19 8:50 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Similar to the qemu_tpm.c, add a unit with a few functions to start/stop and setup the cgroup of the external vhost-user-gpu process. See function documentation.
The vhost-user connection fd is set on qemuDomainVideoPrivate struct.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_domain.c | 5 +- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_vhost_user_gpu.c | 276 +++++++++++++++++++++++++++++++++ src/qemu/qemu_vhost_user_gpu.h | 49 ++++++ 5 files changed, 332 insertions(+), 2 deletions(-) create mode 100644 src/qemu/qemu_vhost_user_gpu.c create mode 100644 src/qemu/qemu_vhost_user_gpu.h
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 449550e64b..8040bf9366 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -66,6 +66,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_tpm.h \ qemu/qemu_vhost_user.c \ qemu/qemu_vhost_user.h \ + qemu/qemu_vhost_user_gpu.c \ + qemu/qemu_vhost_user_gpu.h \ $(NULL)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9437694940..ab923aa917 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1341,8 +1341,11 @@ qemuDomainVideoPrivateNew(void)
static void -qemuDomainVideoPrivateDispose(void *obj ATTRIBUTE_UNUSED) +qemuDomainVideoPrivateDispose(void *obj) { + qemuDomainVideoPrivatePtr priv = obj; + + VIR_FORCE_CLOSE(priv->vhost_user_fd); }
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 89bd77b3c0..8afb4993d3 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -509,7 +509,7 @@ typedef qemuDomainVideoPrivate *qemuDomainVideoPrivatePtr; struct _qemuDomainVideoPrivate { virObject parent;
- bool tmp_to_remove; + int vhost_user_fd; };
diff --git a/src/qemu/qemu_vhost_user_gpu.c b/src/qemu/qemu_vhost_user_gpu.c new file mode 100644 index 0000000000..b259b58434 --- /dev/null +++ b/src/qemu/qemu_vhost_user_gpu.c @@ -0,0 +1,276 @@ +/* + * qemu_vhost_user_gpu.c: QEMU vhost-user GPU support + * + * Copyright (C) 2019 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/>. + */ + +#include <config.h> + +#include "qemu_vhost_user_gpu.h" +#include "qemu_vhost_user.h" +#include "qemu_extdevice.h" + +#include "conf/domain_conf.h" +#include "configmake.h" +#include "vircommand.h" +#include "viralloc.h" +#include "virlog.h" +#include "virutil.h" +#include "virfile.h" +#include "virstring.h" +#include "virtime.h" +#include "virpidfile.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("qemu.vhost_user_gpu"); + + +static char * +qemuVhostUserGPUCreatePidFilename(const char *stateDir, + const char *shortName, + const char *alias) +{ + VIR_AUTOFREE(char *) devicename = NULL; + + if (virAsprintf(&devicename, "%s-%s-vhost-user-gpu", shortName, alias) < 0) + return NULL; + + return virPidFileBuildPath(stateDir, devicename); +} + + +/* + * qemuVhostUserGPUGetPid: + * @binpath: path of executable associated with the pidfile + * @stateDir: the directory where vhost-user-gpu writes the pidfile into + * @shortName: short name of the domain + * @alias: video device alias + * @pid: pointer to pid + * + * Return -errno upon error, or zero on successful reading of the pidfile. + * If the PID was not still alive, zero will be returned, and @pid will be + * set to -1; + */ +static int +qemuVhostUserGPUGetPid(const char *binPath, + const char *stateDir, + const char *shortName, + const char *alias, + pid_t *pid) +{ + VIR_AUTOFREE(char *) pidfile = NULL; + + pidfile = qemuVhostUserGPUCreatePidFilename(stateDir, shortName, alias); + if (!pidfile) + return -ENOMEM; + + return virPidFileReadPathIfAlive(pidfile, pid, binPath); +} + + +int qemuExtVhostUserGPUPrepareDomain(virQEMUDriverPtr driver, + virDomainVideoDefPtr video) +{ + return qemuVhostUserFillDomainGPU(driver, video); +} + + +/* + * qemuExtVhostUserGPUStart: + * @driver: QEMU driver + * @vm: the VM domain + * @video: the video device + * + * Start the external vhost-user-gpu process: + * - open a socketpair for vhost-user communication + * - have the command line built + * - start the external process and sync with it before QEMU start + */ +int qemuExtVhostUserGPUStart(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainVideoDefPtr video) +{ + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOFREE(char *) shortname = NULL; + VIR_AUTOFREE(char *) pidfile = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; + int pair[2] = { -1, -1 }; + int cmdret = 0, rc; + int exitstatus = 0; + pid_t pid; + int ret = -1; + + shortname = virDomainDefGetShortName(vm->def); + if (!shortname) + goto error; + + /* stop any left-over for this VM */ + qemuExtVhostUserGPUStop(driver, vm, video); + + if (!(pidfile = qemuVhostUserGPUCreatePidFilename( + cfg->stateDir, shortname, video->info.alias))) + goto error; + + if (qemuSecuritySetSocketLabel(driver->securityManager, vm->def) < 0) + goto error; + + if (socketpair(AF_UNIX, SOCK_STREAM, 0, pair) < 0) { + virReportSystemError(errno, "%s", _("failed to create socket")); + goto error; + } + + if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0) + goto error; + + cmd = virCommandNew(video->driver->vhost_user_binary); + if (!cmd) + goto error; + + virCommandClearCaps(cmd); + virCommandSetPidFile(cmd, pidfile); + virCommandDaemonize(cmd); + + if (qemuExtDeviceLogCommand(driver, vm, cmd, "vhost-user-gpu") < 0) + goto error; +
Most, possibly all, of these 'goto error;' usages are overwriting detailed error messages with 'vhost-user-gpu failed to start'. So this needs to be reworked. You can use virGetLastErrorMessage if you wanted to prepend the vhost-user specific bit to the error. Maybe break up this function into chunks, like one to build the Command object
It doesn't "overwrite" the detailed error, it merely adds another virReportError(). Or am I missing something? I'll leave that for now.
+ virCommandAddArgFormat(cmd, "--fd=%d", pair[0]); + virCommandPassFD(cmd, pair[0], VIR_COMMAND_PASS_FD_CLOSE_PARENT); + pair[0] = -1; + + if (video->accel) { + if (video->accel->accel3d) + virCommandAddArg(cmd, "--virgl"); + + if (video->accel->rendernode) + virCommandAddArgFormat(cmd, "--render-node=%s", video->accel->rendernode); + } + + if (qemuSecurityStartVhostUserGPU(driver, vm, cmd, + &exitstatus, &cmdret) < 0) + goto error; + + if (cmdret < 0 || exitstatus != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not start 'vhost-user-gpu'. exitstatus: %d"), exitstatus); + goto cleanup; + } +
cmdret < 0 will trigger StartVhostUserGPU to return -1, so we will never hit one condition of the below case
Otherwise this looks good. Since the series is pretty close otherwise, these bits can be addressed in a follow up patch IMO
Reviewed-by: Cole Robinson <crobinso@redhat.com>
- Cole
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Marc-André Lureau

On Mon, Sep 23, 2019 at 02:16:37PM +0400, Marc-André Lureau wrote:
Hi
On Sat, Sep 21, 2019 at 1:05 AM Cole Robinson <crobinso@redhat.com> wrote:
On 9/13/19 8:50 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Similar to the qemu_tpm.c, add a unit with a few functions to start/stop and setup the cgroup of the external vhost-user-gpu process. See function documentation.
The vhost-user connection fd is set on qemuDomainVideoPrivate struct.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_domain.c | 5 +- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_vhost_user_gpu.c | 276 +++++++++++++++++++++++++++++++++ src/qemu/qemu_vhost_user_gpu.h | 49 ++++++ 5 files changed, 332 insertions(+), 2 deletions(-) create mode 100644 src/qemu/qemu_vhost_user_gpu.c create mode 100644 src/qemu/qemu_vhost_user_gpu.h
[...]
+ virCommandClearCaps(cmd); + virCommandSetPidFile(cmd, pidfile); + virCommandDaemonize(cmd); + + if (qemuExtDeviceLogCommand(driver, vm, cmd, "vhost-user-gpu") < 0) + goto error; +
Most, possibly all, of these 'goto error;' usages are overwriting detailed error messages with 'vhost-user-gpu failed to start'. So this needs to be reworked. You can use virGetLastErrorMessage if you wanted to prepend the vhost-user specific bit to the error. Maybe break up this function into chunks, like one to build the Command object
It doesn't "overwrite" the detailed error, it merely adds another virReportError(). Or am I missing something?
virReportError does two things: * logs an error into the configured log * sets the thread-local error variable The error variable can only store one error and this is what will get propagated to the API users. All previous errors logged via virReportError can only be found in the log. So ideally [0] one error path would only set one error. And vice-versa, once you log an error via virReportError, resetting it via virResetLastError will not unlog it, only reset the local error pointer. This function is intended to be used on public APIs entry points to make sure we don't have a leftover error from previous APIs. Jano
I'll leave that for now.
[0] Ideally to match this design. It might be sensible to report more than one error to the API users, but we do not have the infrastructure for that.

On Fri, Sep 13, 2019 at 04:50:51PM +0400, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Similar to the qemu_tpm.c, add a unit with a few functions to start/stop and setup the cgroup of the external vhost-user-gpu process. See function documentation.
The vhost-user connection fd is set on qemuDomainVideoPrivate struct.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_domain.c | 5 +- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_vhost_user_gpu.c | 276 +++++++++++++++++++++++++++++++++ src/qemu/qemu_vhost_user_gpu.h | 49 ++++++ 5 files changed, 332 insertions(+), 2 deletions(-) create mode 100644 src/qemu/qemu_vhost_user_gpu.c create mode 100644 src/qemu/qemu_vhost_user_gpu.h
+/* + * qemuExtVhostUserGPUSetupCgroup: + * + * @driver: QEMU driver + * @def: domain definition + * @video: the video device + * @cgroupe: a cgroup + * + * Add the vhost-user-gpu PID to the given cgroup. + */ +int +qemuExtVhostUserGPUSetupCgroup(virQEMUDriverPtr driver, + virDomainDefPtr def, + virDomainVideoDefPtr video, + virCgroupPtr cgroup) +{ + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOFREE(char *) pidfile = NULL;
Clang fails to compile this: qemu/qemu_vhost_user_gpu.c:256:26: error: unused variable 'pidfile' [-Werror,-Wunused-variable] VIR_AUTOFREE(char *) pidfile = NULL; ^ 1 error generated.
+ VIR_AUTOFREE(char *) shortname = NULL; + int rc; + pid_t pid; +
Jano

From: Marc-André Lureau <marcandre.lureau@redhat.com> Learn to override the paths to the program to execute (vhost-user helpers are executed to check for runtime capabilities). Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- tests/virfilewrapper.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/virfilewrapper.c b/tests/virfilewrapper.c index 160cd571e0..3d3f319f2c 100644 --- a/tests/virfilewrapper.c +++ b/tests/virfilewrapper.c @@ -44,6 +44,8 @@ static FILE *(*real_fopen)(const char *path, const char *mode); static int (*real_access)(const char *path, int mode); static int (*real_mkdir)(const char *path, mode_t mode); static DIR *(*real_opendir)(const char *path); +static int (*real_execv)(const char *path, char *const argv[]); +static int (*real_execve)(const char *path, char *const argv[], char *const envp[]); static void init_syms(void) { @@ -55,6 +57,8 @@ static void init_syms(void) VIR_MOCK_REAL_INIT(mkdir); VIR_MOCK_REAL_INIT(open); VIR_MOCK_REAL_INIT(opendir); + VIR_MOCK_REAL_INIT(execv); + VIR_MOCK_REAL_INIT(execve); } @@ -191,4 +195,22 @@ DIR *opendir(const char *path) return real_opendir(newpath ? newpath : path); } +int execv(const char *path, char *const argv[]) +{ + VIR_AUTOFREE(char *) newpath = NULL; + + PATH_OVERRIDE(newpath, path); + + return real_execv(newpath ? newpath : path, argv); +} + +int execve(const char *path, char *const argv[], char *const envp[]) +{ + VIR_AUTOFREE(char *) newpath = NULL; + + PATH_OVERRIDE(newpath, path); + + return real_execve(newpath ? newpath : path, argv, envp); +} + #endif -- 2.23.0

From: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- tests/qemuxml2argvtest.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index f9eb74e7ed..b31034be46 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -689,6 +689,16 @@ mymain(void) virFileWrapperAddPrefix("/home/user/.config/qemu/firmware", abs_srcdir "/qemufirmwaredata/home/user/.config/qemu/firmware"); + virFileWrapperAddPrefix(SYSCONFDIR "/qemu/vhost-user", + abs_srcdir "/qemuvhostuserdata/etc/qemu/vhost-user"); + virFileWrapperAddPrefix(PREFIX "/share/qemu/vhost-user", + abs_srcdir "/qemuvhostuserdata/usr/share/qemu/vhost-user"); + virFileWrapperAddPrefix("/home/user/.config/qemu/vhost-user", + abs_srcdir "/qemuvhostuserdata/home/user/.config/qemu/vhost-user"); + + virFileWrapperAddPrefix("/usr/libexec/qemu/vhost-user", + abs_srcdir "/qemuvhostuserdata/usr/libexec/qemu/vhost-user"); + /** * The following set of macros allows testing of XML -> argv conversion with a * real set of capabilities gathered from a real qemu copy. It is desired to use -- 2.23.0

From: Marc-André Lureau <marcandre.lureau@redhat.com> Call qemuExtVhostUserGPUPrepareDomain() to fill the domain with the location of the vhost-user binary to start. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_extdevice.c | 29 +++++++++++++++++++++++++++++ src/qemu/qemu_extdevice.h | 5 +++++ src/qemu/qemu_process.c | 4 ++++ 3 files changed, 38 insertions(+) diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 409e2d82ee..7855fa6770 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -21,6 +21,7 @@ #include <config.h> #include "qemu_extdevice.h" +#include "qemu_vhost_user_gpu.h" #include "qemu_domain.h" #include "qemu_tpm.h" #include "qemu_slirp.h" @@ -78,6 +79,34 @@ qemuExtDevicesInitPaths(virQEMUDriverPtr driver, } +/* + * qemuExtDevicesPrepareDomain: + * + * @driver: QEMU driver + * @vm: domain + * + * Code that modifies live XML of a domain which is about to start. + */ +int +qemuExtDevicesPrepareDomain(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + int ret = 0; + size_t i; + + for (i = 0; i < vm->def->nvideos; i++) { + virDomainVideoDefPtr video = vm->def->videos[i]; + + if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) { + if ((ret = qemuExtVhostUserGPUPrepareDomain(driver, video)) < 0) + break; + } + } + + return ret; +} + + /* * qemuExtDevicesPrepareHost: * diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index f29317498b..a035c9f638 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -30,6 +30,11 @@ int qemuExtDeviceLogCommand(virQEMUDriverPtr driver, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; +int qemuExtDevicesPrepareDomain(virQEMUDriverPtr driver, + virDomainObjPtr vm) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) + ATTRIBUTE_RETURN_CHECK; + int qemuExtDevicesPrepareHost(virQEMUDriverPtr driver, virDomainObjPtr vm) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 463b783966..491f374938 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6367,6 +6367,10 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver, if (qemuFirmwareFillDomain(driver, vm, flags) < 0) goto cleanup; + VIR_DEBUG("Preparing external devices"); + if (qemuExtDevicesPrepareDomain(driver, vm) < 0) + goto cleanup; + for (i = 0; i < vm->def->nchannels; i++) { if (qemuDomainPrepareChannel(vm->def->channels[i], priv->channelTargetDir) < 0) -- 2.23.0

From: Marc-André Lureau <marcandre.lureau@redhat.com> Each vhost-user-gpu needs its own helper gpu process. Start/stop them, and apply the emulator cgroup controller. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_extdevice.c | 45 +++++++++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 7855fa6770..abcaa9fca2 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -162,6 +162,16 @@ qemuExtDevicesStart(virQEMUDriverPtr driver, if (qemuExtDevicesInitPaths(driver, vm->def) < 0) return -1; + for (i = 0; i < vm->def->nvideos; i++) { + virDomainVideoDefPtr video = vm->def->videos[i]; + + if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) { + ret = qemuExtVhostUserGPUStart(driver, vm, video); + if (ret < 0) + return ret; + } + } + if (vm->def->tpm) ret = qemuExtTPMStart(driver, vm, incomingMigration); @@ -185,10 +195,17 @@ qemuExtDevicesStop(virQEMUDriverPtr driver, virDomainDefPtr def = vm->def; size_t i; - if (qemuExtDevicesInitPaths(driver, vm->def) < 0) + if (qemuExtDevicesInitPaths(driver, def) < 0) return; - if (vm->def->tpm) + for (i = 0; i < def->nvideos; i++) { + virDomainVideoDefPtr video = def->videos[i]; + + if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) + qemuExtVhostUserGPUStop(driver, vm, video); + } + + if (def->tpm) qemuExtTPMStop(driver, vm); for (i = 0; i < def->nnets; i++) { @@ -204,6 +221,13 @@ qemuExtDevicesStop(virQEMUDriverPtr driver, bool qemuExtDevicesHasDevice(virDomainDefPtr def) { + size_t i; + + for (i = 0; i < def->nvideos; i++) { + if (def->videos[i]->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) + return true; + } + if (def->tpm && def->tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) return true; @@ -216,10 +240,19 @@ qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver, virDomainDefPtr def, virCgroupPtr cgroup) { - int ret = 0; + size_t i; - if (def->tpm) - ret = qemuExtTPMSetupCgroup(driver, def, cgroup); + for (i = 0; i < def->nvideos; i++) { + virDomainVideoDefPtr video = def->videos[i]; - return ret; + if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER && + qemuExtVhostUserGPUSetupCgroup(driver, def, video, cgroup) < 0) + return -1; + } + + if (def->tpm && + qemuExtTPMSetupCgroup(driver, def, cgroup) < 0) + return -1; + + return 0; } -- 2.23.0

From: Marc-André Lureau <marcandre.lureau@redhat.com> For each vhost-user GPUs, - build a socket chardev, and pass the vhost-user socket to it - build a vhost-user video device and associate it with the chardev Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_command.c | 56 +++++++++++++++++++--- tests/qemuxml2argvdata/virtio-options.args | 4 +- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0a9110f3fa..3b87952bfb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4526,16 +4526,23 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, virQEMUCapsPtr qemuCaps) { VIR_AUTOCLEAN(virBuffer) buf = VIR_BUFFER_INITIALIZER; - const char *model; + const char *model = NULL; /* We try to chose the best model for primary video device by preferring * model with VGA compatibility mode. For some video devices on some * architectures there might not be such model so fallback to one * without VGA compatibility mode. */ - if (video->primary && qemuDomainSupportsVideoVga(video, qemuCaps)) - model = qemuDeviceVideoTypeToString(video->type); - else - model = qemuDeviceVideoSecondaryTypeToString(video->type); + if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) { + if (video->primary && qemuDomainSupportsVideoVga(video, qemuCaps)) + model = "vhost-user-vga"; + else + model = "vhost-user-gpu"; + } else { + if (video->primary && qemuDomainSupportsVideoVga(video, qemuCaps)) + model = qemuDeviceVideoTypeToString(video->type); + else + model = qemuDeviceVideoSecondaryTypeToString(video->type); + } if (!model || STREQ(model, "")) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -4544,8 +4551,8 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, return NULL; } - if (STREQ(model, "virtio-gpu")) { - if (qemuBuildVirtioDevStr(&buf, "virtio-gpu", qemuCaps, + if (STREQ(model, "virtio-gpu") || STREQ(model, "vhost-user-gpu")) { + if (qemuBuildVirtioDevStr(&buf, model, qemuCaps, VIR_DOMAIN_DEVICE_VIDEO, video) < 0) { return NULL; } @@ -4588,6 +4595,10 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, if (video->heads) virBufferAsprintf(&buf, ",max_outputs=%u", video->heads); } + } else if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) { + if (video->heads) + virBufferAsprintf(&buf, ",max_outputs=%u", video->heads); + virBufferAsprintf(&buf, ",chardev=chr-vu-%s", video->info.alias); } else if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS)) { if (video->heads) @@ -4699,6 +4710,23 @@ qemuBuildVgaVideoCommand(virCommandPtr cmd, } +static char * +qemuBuildVhostUserChardevStr(const char *alias, + int *fd, + virCommandPtr cmd) +{ + char *chardev = NULL; + + if (virAsprintf(&chardev, "socket,id=chr-vu-%s,fd=%d", alias, *fd) < 0) + return NULL; + + virCommandPassFD(cmd, *fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + *fd = -1; + + return chardev; +} + + static int qemuBuildVideoCommandLine(virCommandPtr cmd, const virDomainDef *def, @@ -4706,6 +4734,20 @@ qemuBuildVideoCommandLine(virCommandPtr cmd, { size_t i; + for (i = 0; i < def->nvideos; i++) { + VIR_AUTOFREE(char *) chardev = NULL; + virDomainVideoDefPtr video = def->videos[i]; + + if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) { + if (!(chardev = qemuBuildVhostUserChardevStr(video->info.alias, + &QEMU_DOMAIN_VIDEO_PRIVATE(video)->vhost_user_fd, + cmd))) + return -1; + + virCommandAddArgList(cmd, "-chardev", chardev, NULL); + } + } + for (i = 0; i < def->nvideos; i++) { VIR_AUTOFREE(char *) str = NULL; virDomainVideoDefPtr video = def->videos[i]; diff --git a/tests/qemuxml2argvdata/virtio-options.args b/tests/qemuxml2argvdata/virtio-options.args index 92bce8283c..79216a5503 100644 --- a/tests/qemuxml2argvdata/virtio-options.args +++ b/tests/qemuxml2argvdata/virtio-options.args @@ -49,7 +49,9 @@ ats=on \ ats=on \ -device virtio-input-host-pci,id=input3,evdev=/dev/input/event1234,bus=pci.0,\ addr=0x12,iommu_platform=on,ats=on \ --device virtio-gpu-pci,id=video0,bus=pci.0,addr=0x2,iommu_platform=on,ats=on \ +-chardev socket,id=chr-vu-video0,fd=0 \ +-device vhost-user-gpu-pci,id=video0,max_outputs=1,chardev=chr-vu-video0,\ +bus=pci.0,addr=0x2,iommu_platform=on,ats=on \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xc,iommu_platform=on,\ ats=on \ -object rng-random,id=objrng0,filename=/dev/random \ -- 2.23.0

From: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ...host-user-gpu-secondary.x86_64-latest.args | 43 +++++++++++++++++ .../vhost-user-gpu-secondary.xml | 46 +++++++++++++++++++ .../vhost-user-vga.x86_64-latest.args | 40 ++++++++++++++++ tests/qemuxml2argvdata/vhost-user-vga.xml | 42 +++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 5 files changed, 174 insertions(+) create mode 100644 tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml create mode 100644 tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/vhost-user-vga.xml diff --git a/tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args new file mode 100644 index 0000000000..58f49595e7 --- /dev/null +++ b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args @@ -0,0 +1,43 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-object memory-backend-memfd,id=ram-node0,share=yes,size=224395264 \ +-numa node,nodeid=0,cpus=0,memdev=ram-node0 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ +-chardev socket,id=chr-vu-video0,fd=0 \ +-chardev socket,id=chr-vu-video1,fd=0 \ +-device vhost-user-vga,id=video0,max_outputs=1,chardev=chr-vu-video0,bus=pci.0,\ +addr=0x2 \ +-device vhost-user-gpu-pci,id=video1,max_outputs=1,chardev=chr-vu-video1,\ +bus=pci.0,addr=0x4 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml new file mode 100644 index 0000000000..bb1f345532 --- /dev/null +++ b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml @@ -0,0 +1,46 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <memoryBacking> + <access mode='shared'/> + <source type='memfd'/> + </memoryBacking> + <vcpu placement='static'>1</vcpu> + <cpu> + <numa> + <cell id='0' cpus='0' memory='219136' unit='KiB'/> + </numa> + </cpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + <video> + <driver name='vhostuser'/> + <model type='virtio' heads='1'/> + </video> + <video> + <driver name='vhostuser'/> + <model type='virtio' heads='1'/> + </video> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args b/tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args new file mode 100644 index 0000000000..6640d86fa5 --- /dev/null +++ b/tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args @@ -0,0 +1,40 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-object memory-backend-memfd,id=ram-node0,share=yes,size=224395264 \ +-numa node,nodeid=0,cpus=0,memdev=ram-node0 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ +-chardev socket,id=chr-vu-video0,fd=0 \ +-device vhost-user-vga,id=video0,max_outputs=1,chardev=chr-vu-video0,bus=pci.0,\ +addr=0x2 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/vhost-user-vga.xml b/tests/qemuxml2argvdata/vhost-user-vga.xml new file mode 100644 index 0000000000..baa0242725 --- /dev/null +++ b/tests/qemuxml2argvdata/vhost-user-vga.xml @@ -0,0 +1,42 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <memoryBacking> + <access mode='shared'/> + <source type='memfd'/> + </memoryBacking> + <vcpu placement='static'>1</vcpu> + <cpu> + <numa> + <cell id='0' cpus='0' memory='219136' unit='KiB'/> + </numa> + </cpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + <video> + <driver name='vhostuser'/> + <model type='virtio' heads='1'/> + </video> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b31034be46..81132bd5b4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -3018,6 +3018,9 @@ mymain(void) DO_TEST_CAPS_LATEST("os-firmware-efi-secboot"); DO_TEST_CAPS_ARCH_LATEST("aarch64-os-firmware-efi", "aarch64"); + DO_TEST_CAPS_LATEST("vhost-user-vga"); + DO_TEST_CAPS_LATEST("vhost-user-gpu-secondary"); + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakerootdir); -- 2.23.0

On 9/13/19 8:50 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ...host-user-gpu-secondary.x86_64-latest.args | 43 +++++++++++++++++ .../vhost-user-gpu-secondary.xml | 46 +++++++++++++++++++ .../vhost-user-vga.x86_64-latest.args | 40 ++++++++++++++++ tests/qemuxml2argvdata/vhost-user-vga.xml | 42 +++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 5 files changed, 174 insertions(+) create mode 100644 tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml create mode 100644 tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/vhost-user-vga.xml
For 15-20: Reviewed-by: Cole Robinson <crobinso@redhat.com> git log says you have commit access, so I think this series is fine to push with: - patch #2 split out and changed+submitted separately - Jano's comments addressed - the cgroup crasher fixed. just ignore my comment about the function rename - Send a follow up patch to fix the error reporting in qemu_vhost_user_gpu.c As for testing the rendernode allocation, virHostGetDRMRenderNode is mocked in tests/qemuxml2argvmock.c, and the test case ./tests/qemuxml2argvdata/graphics-spice-gl-auto-rendernode.xml triggers that mocking, so maybe something similar can be done for vhost-user - Cole

On 9/13/19 8:50 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Hi,
This series adds support for running virtio GPUs in seperate processes, thanks to vhost-user backend.
Some quick notes. I attempted to run this with fedora 31 qemu. The qemu provided 50-qemu-gpu.json needs the trailing comma removed or libvirt fails to parse it. I sent a qemu-devel patch to fix that. If model type='virtio' without driver name='vhostuser', VM startup crashes libvirt at: qemuSetupDevicesCgroup (vm=0x7fff9838b160) at qemu/qemu_cgroup.c:826 if (qemuSetupVideoAccelCgroup(vm, vm->def->videos[i]->accel) < 0) I didn't look into it any more than that. I will check more tomorrow Thanks, Cole
participants (4)
-
Cole Robinson
-
Ján Tomko
-
Marc-André Lureau
-
marcandre.lureau@redhat.com