On Wed, Aug 19, 2020 at 12:03:41PM +0200, Natanael Copa wrote:
Add a portable generic implementation of virMassClose as fallback on non-FreeBSD and non-glibc.
This implementation uses poll(2) to look for open files to keep performance reasonable while not using any mallocs.
The patch isn't avoiding malloc - the virReportSystemError calls all trigger mallocs in the error reporting code, as well as triggering the logging code which mallocs. The idea of using poll() as a fallback still makes sense as a general feature though.
This solves a deadlock with musl libc.
Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> --- src/util/vircommand.c | 76 +++++++++++++++++++++++++++++++++---------- 1 file changed, 58 insertions(+), 18 deletions(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 17e5bb00d3..06579cfb44 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -443,7 +443,7 @@ virExecCommon(virCommandPtr cmd, gid_t *groups, int ngroups) return 0; }
-# ifdef __linux__ +# if defined(__linux__) && defined(__GLIBC__) /* On Linux, we can utilize procfs and read the table of opened * FDs and selectively close only those FDs we don't want to pass * onto child process (well, the one we will exec soon since this @@ -482,17 +482,7 @@ virCommandMassCloseGetFDsLinux(virCommandPtr cmd G_GNUC_UNUSED, VIR_DIR_CLOSE(dp); return ret; } - -# else /* !__linux__ */ - -static int -virCommandMassCloseGetFDsGeneric(virCommandPtr cmd G_GNUC_UNUSED, - virBitmapPtr fds) -{ - virBitmapSetAll(fds); - return 0; -} -# endif /* !__linux__ */ +# endif /* __linux__ && __GLIBC__ */
# ifdef __FreeBSD__
@@ -546,7 +536,7 @@ virCommandMassClose(virCommandPtr cmd, return 0; }
-# else /* ! __FreeBSD__ */ +# elif defined(__GLIBC_) /* ! __FreeBSD__ */
static int virCommandMassClose(virCommandPtr cmd, @@ -574,13 +564,8 @@ virCommandMassClose(virCommandPtr cmd, if (!(fds = virBitmapNew(openmax))) return -1;
-# ifdef __linux__ if (virCommandMassCloseGetFDsLinux(cmd, fds) < 0) return -1; -# else - if (virCommandMassCloseGetFDsGeneric(cmd, fds) < 0) - return -1; -# endif
fd = virBitmapNextSetBit(fds, 2); for (; fd >= 0; fd = virBitmapNextSetBit(fds, fd)) { @@ -598,6 +583,61 @@ virCommandMassClose(virCommandPtr cmd, return 0; }
+#else /* ! __FreeBSD__ && ! __GLIBC__ */ +static int +virCommandMassClose(virCommandPtr cmd, + int childin, + int childout, + int childerr) +{ + static struct pollfd pfds[1024]; + int fd = 0; + int i, total; + int max_fd = sysconf(_SC_OPEN_MAX); + + if (max_fd < 0) { + virReportSystemError(errno, "%s", _("sysconf(_SC_OPEN_MAX) failed")); + return -1; + } + + total = max_fd - fd; + for (i = 0; i < (total < 1024 ? total : 1024); i++) + pfds[i].events = 0; + + while (fd < max_fd) { + int nfds, r = 0; + + total = max_fd - fd; + nfds = total < 1024 ? total : 1024; + + for (i = 0; i < nfds; i++) + pfds[i].fd = fd + i; + + do { + r = poll(pfds, nfds, 0); + } while (r == -1 && errno == EINTR); + + if (r < 0) { + virReportSystemError(errno, "%s", _("poll() failed")); + return -1; + } + + for (i = 0; i < nfds; i++) + if (pfds[i].revents != POLLNVAL) { + if (pfds[i].fd == childin || pfds[i].fd == childout || pfds[i].fd == childerr) + continue; + if (!virCommandFDIsSet(cmd, pfds[i].fd)) { + VIR_MASS_CLOSE(pfds[i].fd); + } else if (virSetInherit(pfds[i].fd, true) < 0) { + virReportSystemError(errno, _("failed to preserve fd %d"), pfds[i].fd); + return -1; + } + } + fd += nfds; + } + return 0; +} + # endif /* ! __FreeBSD__ */
/* -- 2.28.0
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|