On Thu, Feb 10, 2022 at 18:35:44 +0100, Ján Tomko wrote:
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
[...]
+/** + * 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?
Well I dislike flags because it makes the code of the function harder to read as opposed to making the call itself easier to read, so I try to avoid them if it's feasible in a given situation.
Alternatively, to discourage the older approach you can create a thin qemuFDPassNewLegacy wrapper and leave qemuFDPassNew without the comment.
I think I'll go with this one.
(Can we even convince QEMU to use fdset for sockets? Or is that not worth pursuing?)
I actually have a qemu patch prepared which makes the code paths using direct FD passing or the 'getfd' QMP command to accept fdsets too, but I still have to test it. The idea is to hide the complexity in these wrappers and then just use fdsets.
+ * 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/
oops, the idea was to do that check only when the legacy mode is used. Changes in this series actually don't use the multi-fd mode yet.