[PATCH 09/27] qemu: Introduce helper functions for passing FDs to qemu

[Ján Tomko]

On a Wednesday in 2022, Peter Krempa wrote:
>The existing helpers we have are very clumsy and there's no integration
>with the monitor.
>
>This patch introduces new helpers to bridge the gap and simplify handing

*handling

>of fdsets and classic FD passing when generating commandline/hotplug
>arguments.
>
>Signed-off-by: Peter Krempa <pkrempa at redhat.com>
>---
> po/POTFILES.in       |   1 +
> src/qemu/meson.build |   1 +
> src/qemu/qemu_fd.c   | 296 +++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_fd.h   |  56 ++++++++
> 4 files changed, 354 insertions(+)
> create mode 100644 src/qemu/qemu_fd.c
> create mode 100644 src/qemu/qemu_fd.h
>
>diff --git a/po/POTFILES.in b/po/POTFILES.in
>index bf0a3b3529..1fd3afdd6f 100644
>--- a/po/POTFILES.in
>+++ b/po/POTFILES.in
>@@ -164,6 +164,7 @@
> @SRCDIR at src/qemu/qemu_domainjob.c
> @SRCDIR at src/qemu/qemu_driver.c
> @SRCDIR at src/qemu/qemu_extdevice.c
>+ at SRCDIR@src/qemu/qemu_fd.c
> @SRCDIR at src/qemu/qemu_firmware.c
> @SRCDIR at src/qemu/qemu_hostdev.c
> @SRCDIR at src/qemu/qemu_hotplug.c
>diff --git a/src/qemu/meson.build b/src/qemu/meson.build
>index 3ea084cff8..39f0f615cc 100644
>--- a/src/qemu/meson.build
>+++ b/src/qemu/meson.build
>@@ -15,6 +15,7 @@ qemu_driver_sources = [
>   'qemu_domainjob.c',
>   'qemu_driver.c',
>   'qemu_extdevice.c',
>+  'qemu_fd.c',
>   'qemu_firmware.c',
>   'qemu_hostdev.c',
>   'qemu_hotplug.c',
>diff --git a/src/qemu/qemu_fd.c b/src/qemu/qemu_fd.c
>new file mode 100644
>index 0000000000..4052323cb0
>--- /dev/null
>+++ b/src/qemu/qemu_fd.c
>@@ -0,0 +1,296 @@
>+/*
>+ * qemu_fd.c: QEMU fd and fdpass passing helpers
>+ *
>+ * 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/>.

I believe that for new code, all we need is:
SPDX-License-Identifier: LGPL-2.1-or-later

(which is shorter and easier to read, but will be inconsistent)

>+ */
>+
>+#include <config.h>
>+
>+#include "qemu_fd.h"
>+#include "qemu_domain.h"
>+
>+#include "viralloc.h"
>+#include "virfile.h"
>+#include "virlog.h"


>+/**
>+ * qemuFDPassNew:
>+ * @prefix: prefix used for naming the passed FDs
>+ * @dompriv: qemu domain private data
>+ * @useFDSet: Always use FD sets (/dev/fdset/N) for this instance
>+ *
>+ * Create a new helper object for passing FDs to qemu. If @useFDSet is false
>+ * the older approach of directly using FD number on the commandline and 'getfd'
>+ * QMP command is used. Otherwise '-add-fd' and /dev/fdset/N is used to pass the
>+ * FD.
>+ *

I dislike using bools in helpers, can this be turned into a flag?

Alternatively, to discourage the older approach you can create a thin
qemuFDPassNewLegacy wrapper and leave qemuFDPassNew without the comment.

(Can we even convince QEMU to use fdset for sockets? Or is that not
worth pursuing?)

>+ * Non-test uses must pass a valid @dompriv.
>+ *
>+ * @prefix is used for naming the FD if needed and is later referenced when
>+ * removing the FDSet via monitor.
>+ */
>+qemuFDPass *
>+qemuFDPassNew(const char *prefix,
>+              void *dompriv,
>+              bool useFDSet)
>+{
>+    qemuDomainObjPrivate *priv = dompriv;
>+    qemuFDPass *fdpass = g_new0(qemuFDPass, 1);
>+
>+    fdpass->prefix = g_strdup(prefix);
>+    fdpass->useFDSet = useFDSet;
>+
>+    if (fdpass->useFDSet && priv)
>+        fdpass->fdSetID = qemuDomainFDSetIdNew(priv);
>+
>+    return fdpass;
>+}
>+
>+
>+/**
>+ * qemuFDPassAddFD:
>+ * @fdpass: The fd passing helper struct
>+ * @fd: File descriptor to pass
>+ * @suffix: Name suffix for the file descriptor name
>+ *
>+ * Adds @fd to be passed to qemu when transferring @fdpass to qemu.

QEMU

>When @fdpass
>+ * is configured to use FD set mode, multiple file descriptors can be passed by
>+ * calling this function repeatedly.
>+ *
>+ * @suffix is used to build the name of the file descriptor by concatenating
>+ * it with @prefix passed to qemuFDPassNew. @suffix may be NULL, in which case
>+ * it's considered to be an empty string.
>+ *
>+ * Returns 0 on success, -1 on error (when attempting to pass multiple FDs) using
>+ * the 'direct' method.
>+ */
>+int
>+qemuFDPassAddFD(qemuFDPass *fdpass,
>+                int *fd,
>+                const char *suffix)
>+{
>+    struct qemuFDPassFD newfd = { .fd = *fd };
>+
>+    if (newfd.fd < 0) {
>+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>+                       _("invalid file descriptor"));
>+        return -1;
>+    }
>+
>+    if (fdpass->nfds >= 1) {
>+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>+                       _("direct FD passing supports only 1 file descriptor"));

This check conflicts with the comment above saying multiple FDs are
allowed with fdsets. Also, please s/1/one/

>+        return -1;
>+    }
>+
>+    *fd = -1;
>+
>+    newfd.opaque = g_strdup_printf("%s%s", fdpass->prefix, NULLSTR_EMPTY(suffix));
>+
>+    VIR_APPEND_ELEMENT(fdpass->fds, fdpass->nfds, newfd);
>+
>+    return 0;
>+}

Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20220210/fa60a15a/attachment-0001.sig>