[libvirt] [PATCH v5 00/20] Hi,
From: Marc-André Lureau <marcandre.lureau@redhat.com> 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 v5: addressing v4 reviews - replaced "util: ignore EACCESS in virDirOpenIfExists" with more specific "qemu-interop: ignore non-readable directories" - use virFileSanitizePath & virBufferEscapeString on rendernode path - fix ->accel NULL crash, replace qemuSetupVideoAccelCgroup with qemuSetupVideoCgroup - fix src/vz virDomainDefAddImplicitVideo usage - few indent changes, rebased, added r-b tags 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 qemu-interop: ignore non-readable directories 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 | 85 +++- src/conf/domain_conf.h | 22 +- src/qemu/Makefile.inc.am | 6 + src/qemu/qemu_capabilities.c | 6 + src/qemu/qemu_capabilities.h | 4 + src/qemu/qemu_cgroup.c | 28 ++ 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 | 189 ++++++++ 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 | 275 ++++++++++++ src/qemu/qemu_vhost_user_gpu.h | 49 ++ src/vz/vz_sdk.c | 2 +- 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, 1828 insertions(+), 206 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> Reviewed-by: Ján Tomko <jtomko@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
From: Marc-André Lureau <marcandre.lureau@redhat.com> Whether a directory is missing or is not readable doesn't make much difference when populating the available configs. This removes errors when firmare or vhost-user config is looked up from a user session libvirt if /etc/libvirt is not readable for ex. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_interop_config.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c index 1f39d4b576..f3c5d2e083 100644 --- a/src/qemu/qemu_interop_config.c +++ b/src/qemu/qemu_interop_config.c @@ -41,8 +41,14 @@ qemuBuildFileList(virHashTablePtr files, const char *dir) int rc; int ret = -1; - if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) + if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) { + /* silently ignore unreadable directories */ + if (virLastErrorIsSystemErrno(EACCES)) { + virResetLastError(); + return 0; + } return -1; + } if (rc == 0) return 0; -- 2.23.0
On Mon, Sep 23, 2019 at 02:44:25PM +0400, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Whether a directory is missing or is not readable doesn't make much difference when populating the available configs.
This removes errors when firmare or vhost-user config is looked up from a user session libvirt if /etc/libvirt is not readable for ex.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_interop_config.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c index 1f39d4b576..f3c5d2e083 100644 --- a/src/qemu/qemu_interop_config.c +++ b/src/qemu/qemu_interop_config.c @@ -41,8 +41,14 @@ qemuBuildFileList(virHashTablePtr files, const char *dir) int rc; int ret = -1;
- if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) + if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) { + /* silently ignore unreadable directories */
This will not be silent - virDirOpenIfExists already logged an error. You can call virFileExists upfront - which is just a wrapper to access and then take virDirOpenIfExists errors seriously - if someone changes the permissions in the meantime, they deserve the error.
+ if (virLastErrorIsSystemErrno(EACCES)) { + virResetLastError(); + return 0; + } return -1; + }
With that fixed: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Hi On Mon, Sep 23, 2019 at 4:35 PM Ján Tomko <jtomko@redhat.com> wrote:
On Mon, Sep 23, 2019 at 02:44:25PM +0400, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Whether a directory is missing or is not readable doesn't make much difference when populating the available configs.
This removes errors when firmare or vhost-user config is looked up from a user session libvirt if /etc/libvirt is not readable for ex.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_interop_config.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c index 1f39d4b576..f3c5d2e083 100644 --- a/src/qemu/qemu_interop_config.c +++ b/src/qemu/qemu_interop_config.c @@ -41,8 +41,14 @@ qemuBuildFileList(virHashTablePtr files, const char *dir) int rc; int ret = -1;
- if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) + if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) { + /* silently ignore unreadable directories */
This will not be silent - virDirOpenIfExists already logged an error.
Right, let's drop the comment.
You can call virFileExists upfront - which is just a wrapper to access and then take virDirOpenIfExists errors seriously - if someone changes the permissions in the meantime, they deserve the error.
That won't work the desired way, since virFileExists() is true even when you don't have permissions to read it.
+ if (virLastErrorIsSystemErrno(EACCES)) { + virResetLastError(); + return 0; + } return -1; + }
With that fixed: Reviewed-by: Ján Tomko <jtomko@redhat.com>
I see several ways forward, if any of the directory doesn't have readable permissions : 1. keep behaviour before this patch: fail entirely. This is unnecessarily strict imho 2. use this patch (remove the comment): log an error, but browse other directories 3. change the code further so that no error is logged To me, 2 is a good compromise. Does your r-b tag holds with the comment removed only? thanks
On Tue, Sep 24, 2019 at 10:12:19AM +0400, Marc-André Lureau wrote:
Hi
On Mon, Sep 23, 2019 at 4:35 PM Ján Tomko <jtomko@redhat.com> wrote:
On Mon, Sep 23, 2019 at 02:44:25PM +0400, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Whether a directory is missing or is not readable doesn't make much difference when populating the available configs.
This removes errors when firmare or vhost-user config is looked up from a user session libvirt if /etc/libvirt is not readable for ex.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_interop_config.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c index 1f39d4b576..f3c5d2e083 100644 --- a/src/qemu/qemu_interop_config.c +++ b/src/qemu/qemu_interop_config.c @@ -41,8 +41,14 @@ qemuBuildFileList(virHashTablePtr files, const char *dir) int rc; int ret = -1;
- if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) + if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) { + /* silently ignore unreadable directories */
This will not be silent - virDirOpenIfExists already logged an error.
Right, let's drop the comment.
You can call virFileExists upfront - which is just a wrapper to access and then take virDirOpenIfExists errors seriously - if someone changes the permissions in the meantime, they deserve the error.
That won't work the desired way, since virFileExists() is true even when you don't have permissions to read it.
+ if (virLastErrorIsSystemErrno(EACCES)) { + virResetLastError(); + return 0; + } return -1; + }
With that fixed: Reviewed-by: Ján Tomko <jtomko@redhat.com>
I see several ways forward, if any of the directory doesn't have readable permissions :
1. keep behaviour before this patch: fail entirely. This is unnecessarily strict imho 2. use this patch (remove the comment): log an error, but browse other directories 3. change the code further so that no error is logged
To me, 2 is a good compromise.
Does your r-b tag holds with the comment removed only?
Logging errors that are not treated as such pollutes logs and misleads users. I assume the only possible path here is QEMU_ETC_LOCATION - the user should have access to $HOME and preventing access to /usr/share does not sound like a reasonable system. What would be the benefits of making /etc/qemu private? If it's a misconfiguration, we don't have to be lenient here and can go with #1 Otherwise, making virFileAccessibleAs(etcLocation, R_OK | X_OK, geteuid(), getegid()) a condition for if (qemuBuildFileList(files, etcLocation) < 0) return -1; seems to be the least effort to silence unaccessible /etc/qemu while still erroring out on the required cases. Jano
thanks
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hi On Tue, Sep 24, 2019 at 2:08 PM Ján Tomko <jtomko@redhat.com> wrote:
On Tue, Sep 24, 2019 at 10:12:19AM +0400, Marc-André Lureau wrote:
Hi
On Mon, Sep 23, 2019 at 4:35 PM Ján Tomko <jtomko@redhat.com> wrote:
On Mon, Sep 23, 2019 at 02:44:25PM +0400, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Whether a directory is missing or is not readable doesn't make much difference when populating the available configs.
This removes errors when firmare or vhost-user config is looked up from a user session libvirt if /etc/libvirt is not readable for ex.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_interop_config.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c index 1f39d4b576..f3c5d2e083 100644 --- a/src/qemu/qemu_interop_config.c +++ b/src/qemu/qemu_interop_config.c @@ -41,8 +41,14 @@ qemuBuildFileList(virHashTablePtr files, const char *dir) int rc; int ret = -1;
- if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) + if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) { + /* silently ignore unreadable directories */
This will not be silent - virDirOpenIfExists already logged an error.
Right, let's drop the comment.
You can call virFileExists upfront - which is just a wrapper to access and then take virDirOpenIfExists errors seriously - if someone changes the permissions in the meantime, they deserve the error.
That won't work the desired way, since virFileExists() is true even when you don't have permissions to read it.
+ if (virLastErrorIsSystemErrno(EACCES)) { + virResetLastError(); + return 0; + } return -1; + }
With that fixed: Reviewed-by: Ján Tomko <jtomko@redhat.com>
I see several ways forward, if any of the directory doesn't have readable permissions :
1. keep behaviour before this patch: fail entirely. This is unnecessarily strict imho 2. use this patch (remove the comment): log an error, but browse other directories 3. change the code further so that no error is logged
To me, 2 is a good compromise.
Does your r-b tag holds with the comment removed only?
Logging errors that are not treated as such pollutes logs and misleads users.
It's treated as an error: it logs an error, but it's not fatal.
I assume the only possible path here is QEMU_ETC_LOCATION - the user should have access to $HOME and preventing access to /usr/share does not sound like a reasonable system.
What would be the benefits of making /etc/qemu private? If it's a misconfiguration, we don't have to be lenient here and can go with #1
It could be a misconfiguration or an administrator policy. In both cases, it's not a good idea to prevent the user from starting the VM with the user configs.
Otherwise, making virFileAccessibleAs(etcLocation, R_OK | X_OK, geteuid(), getegid()) a condition for if (qemuBuildFileList(files, etcLocation) < 0) return -1; seems to be the least effort to silence unaccessible /etc/qemu while still erroring out on the required cases.
Sounds okay to me, but the user would have no clear clue that something unusual happened (that system config are not readable).
Jano
thanks
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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> Reviewed-by: Ján Tomko <jtomko@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 76aaa63f57..92f30c6f7f 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", @@ -26493,13 +26529,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 57ce5b95dc..0288795abe 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 */ @@ -3423,6 +3434,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
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> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- docs/formatdomain.html.in | 6 ++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 15 ++++++++++++++- src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/virtio-options.xml | 2 +- 5 files changed, 27 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 92f30c6f7f..333a703fff 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,9 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node) def->accel2d = val; } + if (rendernode) + def->rendernode = virFileSanitizePath(rendernode); + cleanup: return def; } @@ -26506,6 +26518,7 @@ virDomainVideoAccelDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " accel2d='%s'", virTristateBoolTypeToString(def->accel2d)); } + virBufferEscapeString(buf, " rendernode='%s'", def->rendernode); virBufferAddLit(buf, "/>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0288795abe..45843e4d25 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
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 | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index ecd96efb0a..740a1b33dc 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -503,6 +503,29 @@ qemuSetupGraphicsCgroup(virDomainObjPtr vm, } +static int +qemuSetupVideoCgroup(virDomainObjPtr vm, + virDomainVideoDefPtr def) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainVideoAccelDefPtr accel = def->accel; + int ret; + + if (!accel) + return 0; + + if (!accel->rendernode || + !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) + return 0; + + ret = virCgroupAllowDevicePath(priv->cgroup, accel->rendernode, + VIR_CGROUP_DEVICE_RW, false); + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", accel->rendernode, + "rw", ret); + return ret; +} + + static int qemuSetupBlkioCgroup(virDomainObjPtr vm) { @@ -803,6 +826,11 @@ qemuSetupDevicesCgroup(virDomainObjPtr vm) goto cleanup; } + for (i = 0; i < vm->def->nvideos; i++) { + if (qemuSetupVideoCgroup(vm, vm->def->videos[i]) < 0) + goto cleanup; + } + for (i = 0; i < vm->def->ninputs; i++) { if (qemuSetupInputCgroup(vm, vm->def->inputs[i]) < 0) goto cleanup; -- 2.23.0
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 43f01bf2ba..0936a1d7db 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -538,6 +538,10 @@ VIR_ENUM_IMPL(virQEMUCaps, "bochs-display", "migration-file-drop-cache", "dbus-vmstate", + "vhost-user-gpu", + + /* 340 */ + "vhost-user-vga", ); @@ -1129,6 +1133,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 fe80fb5391..56beae726f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -519,6 +519,10 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_DEVICE_BOCHS_DISPLAY, /* -device bochs-display */ QEMU_CAPS_MIGRATION_FILE_DROP_CACHE, /* migration with disk cache on is safe for type='file' disks */ 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 f4583d7fe7..5d4540b3f7 100644 --- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml @@ -209,6 +209,8 @@ <flag name='canonical-cpu-features'/> <flag name='bochs-display'/> <flag name='migration-file-drop-cache'/> + <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 ad4db7e881..dd2e7f3518 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -12777,9 +12777,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> Reviewed-by: Cole Robinson <crobinso@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 0b2afe6841..a92e9617cc 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 a8fd371674..0b8ea13f58 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2892,8 +2892,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
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> Reviewed-by: Cole Robinson <crobinso@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 90e090c72e..3f127dad1f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4542,9 +4542,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
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> --- 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 333a703fff..e7ec9e972d 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 45843e4d25..0143efddbe 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 e1bcd10c7c..56f4027cfa 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -290,6 +290,7 @@ test_programs += qemuxml2argvtest qemuxml2xmltest \ qemumigparamstest \ qemusecuritytest \ qemufirmwaretest \ + qemuvhostusertest \ $(NULL) test_helpers += qemucapsprobe test_libraries += libqemumonitortestutils.la \ @@ -692,6 +693,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 \ @@ -706,6 +714,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
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> Reviewed-by: Cole Robinson <crobinso@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
From: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 26 +++++++++++++++++--------- src/conf/domain_conf.h | 8 ++++++-- src/vz/vz_sdk.c | 2 +- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e7ec9e972d..6dea670257 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) { @@ -15451,7 +15459,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; @@ -23743,7 +23751,7 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def) } static int -virDomainDefAddImplicitVideo(virDomainDefPtr def) +virDomainDefAddImplicitVideo(virDomainDefPtr def, virDomainXMLOptionPtr xmlopt) { int ret = -1; virDomainVideoDefPtr video = NULL; @@ -23753,7 +23761,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) @@ -23766,7 +23774,7 @@ virDomainDefAddImplicitVideo(virDomainDefPtr def) } int -virDomainDefAddImplicitDevices(virDomainDefPtr def) +virDomainDefAddImplicitDevices(virDomainDefPtr def, virDomainXMLOptionPtr xmlopt) { if (virDomainDefAddConsoleCompat(def) < 0) return -1; @@ -23774,7 +23782,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 0143efddbe..cff33f6682 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); @@ -3063,7 +3066,8 @@ 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); diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index c6e3398620..7ac4888ecb 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1928,7 +1928,7 @@ prlsdkLoadDomain(vzDriverPtr driver, if (prlsdkGetDomainState(dom, sdkdom, &domainState) < 0) goto error; - if (!IS_CT(def) && virDomainDefAddImplicitDevices(def) < 0) + if (!IS_CT(def) && virDomainDefAddImplicitDevices(def, driver->xmlopt) < 0) goto error; if (def->ngraphics > 0) { -- 2.23.0
From: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Cole Robinson <crobinso@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 dd2e7f3518..5962b4fb33 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 f53ea146e1..94c22c82ef 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
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> Reviewed-by: Cole Robinson <crobinso@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 | 275 +++++++++++++++++++++++++++++++++ src/qemu/qemu_vhost_user_gpu.h | 49 ++++++ 5 files changed, 331 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 5962b4fb33..2aa2164953 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 94c22c82ef..6c490bd9d8 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..1d6fdcb5e9 --- /dev/null +++ b/src/qemu/qemu_vhost_user_gpu.c @@ -0,0 +1,275 @@ +/* + * 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 *) 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
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> Reviewed-by: Cole Robinson <crobinso@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> Reviewed-by: Cole Robinson <crobinso@redhat.com> --- tests/qemuxml2argvtest.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 0b8ea13f58..dd38e57c2b 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -687,6 +687,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> Reviewed-by: Cole Robinson <crobinso@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 a92e9617cc..a50cd54393 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> Reviewed-by: Cole Robinson <crobinso@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> Reviewed-by: Cole Robinson <crobinso@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 3f127dad1f..77470a6037 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4513,16 +4513,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, @@ -4531,8 +4538,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; } @@ -4575,6 +4582,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) @@ -4686,6 +4697,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, @@ -4693,6 +4721,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> Reviewed-by: Cole Robinson <crobinso@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 dd38e57c2b..5bbac1c8b8 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -3016,6 +3016,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/23/19 6:44 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
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
I dropped patch #2 since there's discussion on it. Patch 10 needed a tests/Makefile.am adjustment to fix RPM build. Pushed now with those changes Thanks, Cole
participants (4)
-
Cole Robinson -
Ján Tomko -
Marc-André Lureau -
marcandre.lureau＠redhat.com