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@xxxxxxxxxxxxxxx>
> ---
> 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 :|
