10:58 a.m.
Currently, virCommand implementation uses FD_ macros from
sys/select.h. However, those cannot handle more opened files
than FD_SETSIZE. Therefore switch to generalized implementation
based on array of integers.
---
diff to v1:
- Eric's review included
src/util/command.c | 113 ++++++++++++++++++++++++++++++++++++++++------------
1 files changed, 87 insertions(+), 26 deletions(-)
diff --git a/src/util/command.c b/src/util/command.c
index bdaa88b..41fb020 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -75,9 +75,10 @@ struct _virCommand {
char *pwd;
- /* XXX Use int[] if we ever need to support more than FD_SETSIZE fd's. */
- fd_set preserve; /* FDs to pass to child. */
- fd_set transfer; /* FDs to close in parent. */
+ int *preserve; /* FDs to pass to child. */
+ int preserve_size;
+ int *transfer; /* FDs to close in parent. */
+ int transfer_size;
unsigned int flags;
@@ -130,6 +131,65 @@ static int virClearCapabilities(void)
}
# endif
+/*
+ * virCommandFDIsSet:
+ * @fd: FD to test
+ * @set: the set
+ * @set_size: actual size of @set
+ *
+ * Check if FD is already in @set or not.
+ *
+ * Returns true if @set contains @fd,
+ * false otherwise.
+ */
+static bool
+virCommandFDIsSet(int fd,
+ const int *set,
+ int set_size)
+{
+ int i = 0;
+
+ while (i < set_size)
+ if (set[i++] == fd)
+ return true;
+
+ return false;
+}
+
+/*
+ * virCommandFDSet:
+ * @fd: FD to be put into @set
+ * @set: the set
+ * @set_size: actual size of @set
+ *
+ * This is practically generalized implementation
+ * of FD_SET() as we do not want to be limited
+ * by FD_SETSIZE.
+ *
+ * Returns: 0 on success,
+ * -1 on usage error,
+ * ENOMEM on OOM
+ */
+static int
+virCommandFDSet(int fd,
+ int **set,
+ int *set_size)
+{
+ if (fd < 0 || !set || !set_size)
+ return -1;
+
+ if (virCommandFDIsSet(fd, *set, *set_size))
+ return 0;
+
+ if (VIR_REALLOC_N(*set, *set_size + 1) < 0) {
+ return ENOMEM;
+ }
+
+ (*set)[*set_size] = fd;
+ (*set_size)++;
+
+ return 0;
+}
/**
* virFork:
@@ -303,7 +363,8 @@ prepareStdFd(int fd, int std)
static int
virExecWithHook(const char *const*argv,
const char *const*envp,
- const fd_set *keepfd,
+ const int *keepfd,
+ int keepfd_size,
pid_t *retpid,
int infd, int *outfd, int *errfd,
unsigned int flags,
@@ -430,7 +491,7 @@ virExecWithHook(const char *const*argv,
for (i = 3; i < openmax; i++) {
if (i == infd || i == childout || i == childerr)
continue;
- if (!keepfd || i >= FD_SETSIZE || !FD_ISSET(i, keepfd)) {
+ if (!keepfd || !virCommandFDIsSet(i, keepfd, keepfd_size)) {
tmpfd = i;
VIR_FORCE_CLOSE(tmpfd);
} else if (virSetInherit(i, true) < 0) {
@@ -619,7 +680,8 @@ virRun(const char *const *argv ATTRIBUTE_UNUSED,
static int
virExecWithHook(const char *const*argv ATTRIBUTE_UNUSED,
const char *const*envp ATTRIBUTE_UNUSED,
- const fd_set *keepfd ATTRIBUTE_UNUSED,
+ const int *keepfd ATTRIBUTE_UNUSED,
+ int keepfd_size ATTRIBUTE_UNUSED,
pid_t *retpid ATTRIBUTE_UNUSED,
int infd ATTRIBUTE_UNUSED,
int *outfd ATTRIBUTE_UNUSED,
@@ -687,8 +749,6 @@ virCommandNewArgs(const char *const*args)
cmd->handshakeNotify[0] = -1;
cmd->handshakeNotify[1] = -1;
- FD_ZERO(&cmd->preserve);
- FD_ZERO(&cmd->transfer);
cmd->infd = cmd->outfd = cmd->errfd = -1;
cmd->inpipe = -1;
cmd->pid = -1;
@@ -736,19 +796,21 @@ virCommandNewArgList(const char *binary, ...)
static bool
virCommandKeepFD(virCommandPtr cmd, int fd, bool transfer)
{
+ int ret = 0;
+
if (!cmd)
return fd > STDERR_FILENO;
- if (fd <= STDERR_FILENO || FD_SETSIZE <= fd) {
+ if (fd <= STDERR_FILENO ||
+ (ret = virCommandFDSet(fd, &cmd->preserve, &cmd->preserve_size))
||
+ (transfer && (ret = virCommandFDSet(fd, &cmd->transfer,
+ &cmd->transfer_size)))) {
if (!cmd->has_error)
- cmd->has_error = -1;
+ cmd->has_error = ret ? : -1 ;
VIR_DEBUG("cannot preserve %d", fd);
- return fd > STDERR_FILENO;
+ return true;
}
- FD_SET(fd, &cmd->preserve);
- if (transfer)
- FD_SET(fd, &cmd->transfer);
return false;
}
@@ -2082,7 +2144,8 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
ret = virExecWithHook((const char *const *)cmd->args,
(const char *const *)cmd->env,
- &cmd->preserve,
+ cmd->preserve,
+ cmd->preserve_size,
&cmd->pid,
cmd->infd,
cmd->outfdptr,
@@ -2095,13 +2158,11 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
VIR_DEBUG("Command result %d, with PID %d",
ret, (int)cmd->pid);
- for (i = STDERR_FILENO + 1; i < FD_SETSIZE; i++) {
- if (FD_ISSET(i, &cmd->transfer)) {
- int tmpfd = i;
- VIR_FORCE_CLOSE(tmpfd);
- FD_CLR(i, &cmd->transfer);
- }
+ for (i = 0; i < cmd->transfer_size; i++) {
+ VIR_FORCE_CLOSE(cmd->transfer[i]);
}
+ cmd->transfer_size = 0;
+ VIR_FREE(cmd->transfer);
if (ret == 0 && pid)
*pid = cmd->pid;
@@ -2466,11 +2527,8 @@ virCommandFree(virCommandPtr cmd)
if (!cmd)
return;
- for (i = STDERR_FILENO + 1; i < FD_SETSIZE; i++) {
- if (FD_ISSET(i, &cmd->transfer)) {
- int tmpfd = i;
- VIR_FORCE_CLOSE(tmpfd);
- }
+ for (i = 0; i < cmd->transfer_size; i++) {
+ VIR_FORCE_CLOSE(cmd->transfer[i]);
}
VIR_FREE(cmd->inbuf);
@@ -2500,5 +2558,8 @@ virCommandFree(virCommandPtr cmd)
if (cmd->reap)
virCommandAbort(cmd);
+ VIR_FREE(cmd->transfer);
+ VIR_FREE(cmd->preserve);
+
VIR_FREE(cmd);
}
--
1.7.3.4