[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 6/6] Merge virCommandPreserveFD / virCommandTransferFD



From: "Daniel P. Berrange" <berrange 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 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]