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