[libvirt PATCH] src: use closefrom() for mass closing of FDs
Ján Tomko
jtomko at redhat.com
Sun Feb 9 01:37:43 UTC 2020
On Fri, Feb 07, 2020 at 04:03:51PM +0000, Daniel P. Berrangé wrote:
>On FreeBSD 12 the default ulimit settings allow for 100,000
>open file descriptors. As a result spawning processes in
>libvirt is abominably slow. Fortunately FreeBSD has long
>since provided a good solution in the form of closefrom(),
>which closes all FDs equal to or larger than the specified
>parameter.
>
>Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
>---
> src/util/vircommand.c | 60 ++++++++++++++++++++++++++++++++++++++++---
> tests/testutils.c | 9 +++++++
> 2 files changed, 66 insertions(+), 3 deletions(-)
>
>diff --git a/src/util/vircommand.c b/src/util/vircommand.c
>index 904a3023c5..764fb2fe43 100644
>--- a/src/util/vircommand.c
>+++ b/src/util/vircommand.c
>@@ -494,6 +494,59 @@ virCommandMassCloseGetFDsGeneric(virCommandPtr cmd G_GNUC_UNUSED,
> }
> # endif /* !__linux__ */
>
>+# ifdef __FreeBSD__
>+
>+static int
>+virCommandMassClose(virCommandPtr cmd,
>+ int childin,
>+ int childout,
>+ int childerr)
>+{
>+ int lastfd = -1;
>+ int fd = -1;
>+
>+ /*
>+ * Two phases of closing.
>+ *
>+ * The first (inefficient) phase iterates over FDs,
>+ * preserving certain FDs we need to pass down, and
>+ * closing others. The number of iterations is bounded
>+ * to the number of the biggest FD we need to preserve.
>+ *
>+ * The second (speedy) phase uses closefrom() to cull
>+ * all remaining FDs in the process.
>+ *
>+ * Usually the first phase will be fairly quick only
>+ * processing a handful of low FD numbers, and thus using
>+ * closefrom() is a massive win for high ulimit() NFILES
>+ * values.
>+ */
>+ lastfd = MAX(lastfd, childin);
>+ lastfd = MAX(lastfd, childout);
>+ lastfd = MAX(lastfd, childerr);
>+
>+ while (fd < cmd->npassfd)
>+ lastfd = MAX(lastfd, cmd->passfd[fd].fd);
This accesses cmd->passfd[-1] and never increments fd.
Please use a for loop with 'i' for readability.
>+
>+ for (fd = 0; fd <= lastfd; fd++) {
>+ if (fd == childin || fd == childout || fd == childerr)
>+ continue;
>+ if (!virCommandFDIsSet(cmd, fd)) {
>+ int tmpfd = fd;
>+ VIR_MASS_CLOSE(tmpfd);
>+ } else if (virSetInherit(fd, true) < 0) {
>+ virReportSystemError(errno, _("failed to preserve fd %d"), fd);
>+ return -1;
>+ }
>+ }
>+
>+ closefrom(lastfd + 1);
>+
>+ return 0;
>+}
>+
>+# else /* ! __FreeBSD__ */
>+
> static int
> virCommandMassClose(virCommandPtr cmd,
> int childin,
>@@ -520,13 +573,13 @@ virCommandMassClose(virCommandPtr cmd,
> if (!(fds = virBitmapNew(openmax)))
> return -1;
>
>-# ifdef __linux__
>+# ifdef __linux__
> if (virCommandMassCloseGetFDsLinux(cmd, fds) < 0)
> return -1;
>-# else
>+# else
> if (virCommandMassCloseGetFDsGeneric(cmd, fds) < 0)
> return -1;
>-# endif
>+# endif
>
> fd = virBitmapNextSetBit(fds, 2);
> for (; fd >= 0; fd = virBitmapNextSetBit(fds, fd)) {
>@@ -544,6 +597,7 @@ virCommandMassClose(virCommandPtr cmd,
> return 0;
> }
>
>+# endif /* ! __FreeBSD__ */
>
> /*
> * virExec:
>diff --git a/tests/testutils.c b/tests/testutils.c
>index 7b9a5ea05b..662203d707 100644
>--- a/tests/testutils.c
>+++ b/tests/testutils.c
>@@ -333,8 +333,10 @@ static
> void virTestCaptureProgramExecChild(const char *const argv[],
> int pipefd)
> {
I think this whole function should be dropped:
https://www.redhat.com/archives/libvir-list/2020-February/msg00361.html
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/20200209/32cfcb28/attachment-0001.sig>
More information about the libvir-list
mailing list