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@xxxxxxxxxx>
---
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@xxxxxxxxxx> Jano
Attachment:
signature.asc
Description: PGP signature
