10:38 a.m.
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Merge the virCommandPreserveFD / virCommandTransferFD methods
into a single virCommandPasFD method, and use a new
VIR_COMMAND_PASS_FD_CLOSE_PARENT to indicate their difference
in behaviour
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/fdstream.c | 3 +-
src/libvirt_private.syms | 3 +-
src/lxc/lxc_process.c | 6 +-
src/qemu/qemu_command.c | 16 +++--
src/uml/uml_conf.c | 3 +-
src/util/vircommand.c | 159 ++++++++++++++++++++++-------------------------
src/util/vircommand.h | 13 ++--
tests/commandtest.c | 5 +-
8 files changed, 105 insertions(+), 103 deletions(-)
diff --git a/src/fdstream.c b/src/fdstream.c
index 2ee9bd8..10c7c2a 100644
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -648,7 +648,8 @@ virFDStreamOpenFileInternal(virStreamPtr st,
path,
NULL);
virCommandAddArgFormat(cmd, "%llu", length);
- virCommandTransferFD(cmd, fd);
+ virCommandPassFD(cmd, fd,
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
virCommandAddArgFormat(cmd, "%d", fd);
if ((oflags & O_ACCMODE) == O_RDONLY) {
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0ab7632..a9e702c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1228,7 +1228,7 @@ virCommandNew;
virCommandNewArgList;
virCommandNewArgs;
virCommandNonblockingFDs;
-virCommandPreserveFD;
+virCommandPassFD;
virCommandRequireHandshake;
virCommandRun;
virCommandRunAsync;
@@ -1249,7 +1249,6 @@ virCommandSetSELinuxLabel;
virCommandSetUID;
virCommandSetWorkingDirectory;
virCommandToString;
-virCommandTransferFD;
virCommandWait;
virCommandWriteArgLog;
virFork;
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index dd908b8..54eb728 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -853,13 +853,13 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
for (i = 0; i < nttyFDs; i++) {
virCommandAddArg(cmd, "--console");
virCommandAddArgFormat(cmd, "%d", ttyFDs[i]);
- virCommandPreserveFD(cmd, ttyFDs[i]);
+ virCommandPassFD(cmd, ttyFDs[i], 0);
}
for (i = 0; i < nfiles; i++) {
virCommandAddArg(cmd, "--passfd");
virCommandAddArgFormat(cmd, "%d", files[i]);
- virCommandPreserveFD(cmd, files[i], 0);
+ virCommandPassFD(cmd, files[i], 0);
}
virCommandAddArgPair(cmd, "--security",
@@ -873,7 +873,7 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
virCommandAddArgList(cmd, "--veth", veths[i], NULL);
}
- virCommandPreserveFD(cmd, handshakefd);
+ virCommandPassFD(cmd, handshakefd, 0);
return cmd;
cleanup:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 879aed8..4f126e7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -245,7 +245,8 @@ static int qemuCreateInBridgePortWithHelper(virQEMUDriverConfigPtr
cfg,
virCommandAddArgFormat(cmd, "--use-vnet");
virCommandAddArgFormat(cmd, "--br=%s", brname);
virCommandAddArgFormat(cmd, "--fd=%d", pair[1]);
- virCommandTransferFD(cmd, pair[1]);
+ virCommandPassFD(cmd, pair[1],
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
virCommandClearCaps(cmd);
#ifdef CAP_NET_ADMIN
virCommandAllowCap(cmd, CAP_NET_ADMIN);
@@ -6524,13 +6525,15 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
}
for (i = 0; i < tapfdSize; i++) {
- virCommandTransferFD(cmd, tapfd[i]);
+ virCommandPassFD(cmd, tapfd[i],
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
if (virAsprintf(&tapfdName[i], "%d", tapfd[i]) < 0)
goto cleanup;
}
for (i = 0; i < vhostfdSize; i++) {
- virCommandTransferFD(cmd, vhostfd[i]);
+ virCommandPassFD(cmd, vhostfd[i],
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
if (virAsprintf(&vhostfdName[i], "%d", vhostfd[i]) < 0)
goto cleanup;
}
@@ -8301,7 +8304,8 @@ qemuBuildCommandLine(virConnectPtr conn,
goto error;
}
- virCommandTransferFD(cmd, configfd);
+ virCommandPassFD(cmd, configfd,
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
}
}
virCommandAddArg(cmd, "-device");
@@ -8367,7 +8371,7 @@ qemuBuildCommandLine(virConnectPtr conn,
} else if (STREQ(migrateFrom, "stdio")) {
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) {
virCommandAddArgFormat(cmd, "fd:%d", migrateFd);
- virCommandPreserveFD(cmd, migrateFd);
+ virCommandPassFD(cmd, migrateFd, 0);
} else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_EXEC)) {
virCommandAddArg(cmd, "exec:cat");
virCommandSetInputFD(cmd, migrateFd);
@@ -8396,7 +8400,7 @@ qemuBuildCommandLine(virConnectPtr conn,
goto error;
}
virCommandAddArg(cmd, migrateFrom);
- virCommandPreserveFD(cmd, migrateFd);
+ virCommandPassFD(cmd, migrateFd, 0);
} else if (STRPREFIX(migrateFrom, "unix")) {
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_UNIX)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index 18cff50..0f2f38e 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -332,7 +332,8 @@ umlBuildCommandLineChr(virDomainChrDefPtr def,
VIR_FORCE_CLOSE(fd_out);
return NULL;
}
- virCommandTransferFD(cmd, fd_out);
+ virCommandPassFD(cmd, fd_out,
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
}
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 3879504..00ff69a 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -64,6 +64,14 @@ enum {
VIR_EXEC_ASYNC_IO = (1 << 4),
};
+typedef struct _virCommandFD virCommandFD;
+typedef virCommandFD *virCommandFDPtr;
+
+struct _virCommandFD {
+ int fd;
+ unsigned int flags;
+};
+
struct _virCommand {
int has_error; /* ENOMEM on allocation failure, -1 for anything else. */
@@ -77,10 +85,8 @@ struct _virCommand {
char *pwd;
- int *preserve; /* FDs to pass to child. */
- int preserve_size;
- int *transfer; /* FDs to close in parent. */
- int transfer_size;
+ size_t npassfd;
+ virCommandFDPtr passfd;
unsigned int flags;
@@ -135,14 +141,15 @@ struct _virCommand {
* false otherwise.
*/
static bool
-virCommandFDIsSet(int fd,
- const int *set,
- int set_size)
+virCommandFDIsSet(virCommandPtr cmd,
+ int fd)
{
size_t i = 0;
+ if (!cmd)
+ return false;
- while (i < set_size)
- if (set[i++] == fd)
+ while (i < cmd->npassfd)
+ if (cmd->passfd[i++].fd == fd)
return true;
return false;
@@ -163,22 +170,21 @@ virCommandFDIsSet(int fd,
* ENOMEM on OOM
*/
static int
-virCommandFDSet(int fd,
- int **set,
- int *set_size)
+virCommandFDSet(virCommandPtr cmd,
+ int fd,
+ unsigned int flags)
{
- if (fd < 0 || !set || !set_size)
+ if (!cmd || fd < 0)
return -1;
- if (virCommandFDIsSet(fd, *set, *set_size))
+ if (virCommandFDIsSet(cmd, fd))
return 0;
- if (VIR_REALLOC_N(*set, *set_size + 1) < 0) {
+ if (VIR_EXPAND_N(cmd->passfd, cmd->npassfd, 1) < 0)
return ENOMEM;
- }
- (*set)[*set_size] = fd;
- (*set_size)++;
+ cmd->passfd[cmd->npassfd - 1].fd = fd;
+ cmd->passfd[cmd->npassfd - 1].flags = flags;
return 0;
}
@@ -525,8 +531,7 @@ virExec(virCommandPtr cmd)
for (fd = 3; fd < openmax; fd++) {
if (fd == childin || fd == childout || fd == childerr)
continue;
- if (!cmd->preserve ||
- !virCommandFDIsSet(fd, cmd->preserve, cmd->preserve_size)) {
+ if (!virCommandFDIsSet(cmd, fd)) {
tmpfd = fd;
VIR_MASS_CLOSE(tmpfd);
} else if (virSetInherit(fd, true) < 0) {
@@ -882,67 +887,51 @@ virCommandNewVAList(const char *binary, va_list list)
}
-/*
- * Preserve the specified file descriptor in the child, instead of
- * closing it. FD must not be one of the three standard streams. If
- * transfer is true, then fd will be closed in the parent after a call
- * to Run/RunAsync/Free, otherwise caller is still responsible for fd.
- * Returns true if a transferring caller should close FD now, and
- * false if the transfer is successfully recorded.
- */
-static bool
-virCommandKeepFD(virCommandPtr cmd, int fd, bool transfer)
-{
- int ret = 0;
-
- if (!cmd)
- return fd > STDERR_FILENO;
-
- 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 = ret ? ret : -1;
- VIR_DEBUG("cannot preserve %d", fd);
- return fd > STDERR_FILENO;
- }
-
- return false;
-}
-
-/**
- * virCommandPreserveFD:
- * @cmd: the command to modify
- * @fd: fd to mark for inheritance into child
- *
- * Preserve the specified file descriptor
- * in the child, instead of closing it on exec.
- * The parent is still responsible for managing fd.
- */
-void
-virCommandPreserveFD(virCommandPtr cmd, int fd)
-{
- virCommandKeepFD(cmd, fd, false);
-}
+#define VIR_COMMAND_MAYBE_CLOSE_FD(fd, flags) \
+ if ((fd > STDERR_FILENO) && \
+ (flags & VIR_COMMAND_PASS_FD_CLOSE_PARENT)) \
+ VIR_FORCE_CLOSE(fd)
/**
- * virCommandTransferFD:
+ * virCommandPassFD:
* @cmd: the command to modify
* @fd: fd to reassign to the child
+ * @flags: the flags
*
- * Transfer the specified file descriptor
- * to the child, instead of closing it on exec.
- * The parent should no longer use fd, and the parent's copy will
- * be automatically closed no later than during Run/RunAsync/Free.
+ * Transfer the specified file descriptor to the child, instead
+ * of closing it on exec. @fd must not be one of the three
+ * standard streams.
+ *
+ * If the flag VIR_COMMAND_PASS_FD_CLOSE_PARENT is set then fd will
+ * be closed in the parent no later than Run/RunAsync/Free. The parent
+ * should cease using the @fd when this call completes
*/
void
-virCommandTransferFD(virCommandPtr cmd, int fd)
+virCommandPassFD(virCommandPtr cmd, int fd, unsigned int flags)
{
- if (virCommandKeepFD(cmd, fd, true))
- VIR_FORCE_CLOSE(fd);
-}
+ int ret = 0;
+
+ if (!cmd) {
+ VIR_COMMAND_MAYBE_CLOSE_FD(fd, flags);
+ return;
+ }
+
+ if (fd <= STDERR_FILENO) {
+ VIR_DEBUG("invalid fd %d", fd);
+ VIR_COMMAND_MAYBE_CLOSE_FD(fd, flags);
+ if (!cmd->has_error)
+ cmd->has_error = -1;
+ return;
+ }
+ if ((ret = virCommandFDSet(cmd, fd, flags)) != 0) {
+ if (!cmd->has_error)
+ cmd->has_error = ret;
+ VIR_DEBUG("cannot preserve %d", fd);
+ VIR_COMMAND_MAYBE_CLOSE_FD(fd, flags);
+ return;
+ }
+}
/**
* virCommandSetPidFile:
@@ -2252,11 +2241,12 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
VIR_DEBUG("Command result %d, with PID %d",
ret, (int)cmd->pid);
- for (i = 0; i < cmd->transfer_size; i++) {
- VIR_FORCE_CLOSE(cmd->transfer[i]);
+ for (i = 0; i < cmd->npassfd; i++) {
+ if (cmd->passfd[i].flags & VIR_COMMAND_PASS_FD_CLOSE_PARENT)
+ VIR_FORCE_CLOSE(cmd->passfd[i].fd);
}
- cmd->transfer_size = 0;
- VIR_FREE(cmd->transfer);
+ cmd->npassfd = 0;
+ VIR_FREE(cmd->passfd);
if (ret == 0 && pid)
*pid = cmd->pid;
@@ -2431,8 +2421,10 @@ void virCommandRequireHandshake(virCommandPtr cmd)
"keep handshake wait=%d notify=%d",
cmd->handshakeWait[1], cmd->handshakeNotify[0],
cmd->handshakeWait[0], cmd->handshakeNotify[1]);
- virCommandTransferFD(cmd, cmd->handshakeWait[1]);
- virCommandTransferFD(cmd, cmd->handshakeNotify[0]);
+ virCommandPassFD(cmd, cmd->handshakeWait[1],
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+ virCommandPassFD(cmd, cmd->handshakeNotify[0],
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
cmd->handshake = true;
}
@@ -2555,9 +2547,12 @@ virCommandFree(virCommandPtr cmd)
if (!cmd)
return;
- for (i = 0; i < cmd->transfer_size; i++) {
- VIR_FORCE_CLOSE(cmd->transfer[i]);
+ for (i = 0; i < cmd->npassfd; i++) {
+ if (cmd->passfd[i].flags & VIR_COMMAND_PASS_FD_CLOSE_PARENT)
+ VIR_FORCE_CLOSE(cmd->passfd[i].fd);
}
+ cmd->npassfd = 0;
+ VIR_FREE(cmd->passfd);
if (cmd->asyncioThread) {
virThreadJoin(cmd->asyncioThread);
@@ -2579,7 +2574,7 @@ virCommandFree(virCommandPtr cmd)
if (cmd->handshake) {
/* The other 2 fds in these arrays are closed
- * due to use with virCommandTransferFD
+ * due to use with virCommandPassFD
*/
VIR_FORCE_CLOSE(cmd->handshakeWait[0]);
VIR_FORCE_CLOSE(cmd->handshakeNotify[1]);
@@ -2590,8 +2585,6 @@ virCommandFree(virCommandPtr cmd)
if (cmd->reap)
virCommandAbort(cmd);
- VIR_FREE(cmd->transfer);
- VIR_FREE(cmd->preserve);
#if defined(WITH_SECDRIVER_SELINUX)
VIR_FREE(cmd->seLinuxLabel);
#endif
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index a402139..c619e06 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -51,11 +51,14 @@ virCommandPtr virCommandNewVAList(const char *binary, va_list list)
* delayed until the Run/RunAsync methods
*/
-void virCommandPreserveFD(virCommandPtr cmd,
- int fd);
-
-void virCommandTransferFD(virCommandPtr cmd,
- int fd);
+enum {
+ /* Close the FD in the parent */
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT = (1 << 0),
+};
+
+void virCommandPassFD(virCommandPtr cmd,
+ int fd,
+ unsigned int flags);
void virCommandSetPidFile(virCommandPtr cmd,
const char *pidfile) ATTRIBUTE_NONNULL(2);
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 62f0277..eeb6d1e 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -194,8 +194,9 @@ static int test3(const void *unused ATTRIBUTE_UNUSED)
int newfd3 = dup(STDERR_FILENO);
int ret = -1;
- virCommandPreserveFD(cmd, newfd1);
- virCommandTransferFD(cmd, newfd3);
+ virCommandPassFD(cmd, newfd1, 0);
+ virCommandPassFD(cmd, newfd3,
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
if (virCommandRun(cmd, NULL) < 0) {
virErrorPtr err = virGetLastError();
--
1.8.1.4