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 <pkre...@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@src/qemu/qemu_domainjob.c
@SRCDIR@src/qemu/qemu_driver.c
@SRCDIR@src/qemu/qemu_extdevice.c
+@SRCDIR@src/qemu/qemu_fd.c
@SRCDIR@src/qemu/qemu_firmware.c
@SRCDIR@src/qemu/qemu_hostdev.c
@SRCDIR@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 <jto...@redhat.com>

Jano

Attachment: signature.asc
Description: PGP signature

Reply via email to