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

Re: [libvirt] PATCH: Allow specific FDs to be kept open in virExec()



On Thu, Aug 21, 2008 at 02:58:30PM +0100, Daniel P. Berrange wrote:
> With my recent patches to virExec(), all FDs except stdin/out/err are closed
> before the child is exec'd to prevent accidental leaks. Of course I forgot
> the one key place where we need to propagate FDs... The TAP devices passed
> to QEMU.
> 
> This patch adds a 'keepfd' parameter to virExec() which allows the caller
> to specify a bitset of file descriptors to keep open, and updates the 
> callers to use this as required. The QEMU driver specifies any TAP fds
> and the LXC driver specifies its PTY this way removing a previous hack.
> 
> QEMU networking works again with fix....

Changed to use fd_set from sys/select

Index: src/lxc_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/lxc_driver.c,v
retrieving revision 1.24
diff -u -p -r1.24 lxc_driver.c
--- src/lxc_driver.c	22 Aug 2008 15:35:37 -0000	1.24
+++ src/lxc_driver.c	26 Aug 2008 09:11:42 -0000
@@ -621,6 +621,10 @@ static int lxcControllerStart(virConnect
     const char **largv = NULL;
     pid_t child;
     int status;
+    fd_set keepfd;
+    char appPtyStr[30];
+
+    FD_ZERO(&keepfd);
 
 #define ADD_ARG_SPACE                                                   \
     do { \
@@ -644,11 +648,13 @@ static int lxcControllerStart(virConnect
             goto no_memory;                                             \
     } while (0)
 
+    snprintf(appPtyStr, sizeof(appPtyStr), "%d", appPty);
+
     ADD_ARG_LIT(vm->def->emulator);
     ADD_ARG_LIT("--name");
     ADD_ARG_LIT(vm->def->name);
     ADD_ARG_LIT("--console");
-    ADD_ARG_LIT("0");  /* Passing console master PTY as FD 0 */
+    ADD_ARG_LIT(appPtyStr);
     ADD_ARG_LIT("--background");
 
     for (i = 0 ; i < nveths ; i++) {
@@ -658,10 +664,12 @@ static int lxcControllerStart(virConnect
 
     ADD_ARG(NULL);
 
-    vm->stdin_fd = appPty; /* Passing console master PTY as FD 0 */
+    vm->stdin_fd = -1;
     vm->stdout_fd = vm->stderr_fd = logfd;
 
-    if (virExec(conn, largv, NULL, &child,
+    FD_SET(appPty, &keepfd);
+
+    if (virExec(conn, largv, NULL, &keepfd, &child,
                 vm->stdin_fd, &vm->stdout_fd, &vm->stderr_fd,
                 VIR_EXEC_NONE) < 0)
         goto cleanup;
Index: src/openvz_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/openvz_driver.c,v
retrieving revision 1.43
diff -u -p -r1.43 openvz_driver.c
--- src/openvz_driver.c	20 Aug 2008 20:48:36 -0000	1.43
+++ src/openvz_driver.c	26 Aug 2008 09:11:43 -0000
@@ -807,7 +807,8 @@ static int openvzListDomains(virConnectP
     char *endptr;
     const char *cmd[] = {VZLIST, "-ovpsid", "-H" , NULL};
 
-    ret = virExec(conn, cmd, NULL, &pid, -1, &outfd, &errfd, VIR_EXEC_NONE);
+    ret = virExec(conn, cmd, NULL, NULL,
+                  &pid, -1, &outfd, &errfd, VIR_EXEC_NONE);
     if(ret == -1) {
         openvzError(conn, VIR_ERR_INTERNAL_ERROR,
                _("Could not exec %s"), VZLIST);
@@ -844,7 +845,8 @@ static int openvzListDefinedDomains(virC
     const char *cmd[] = {VZLIST, "-ovpsid", "-H", "-S", NULL};
 
     /* the -S options lists only stopped domains */
-    ret = virExec(conn, cmd, NULL, &pid, -1, &outfd, &errfd, VIR_EXEC_NONE);
+    ret = virExec(conn, cmd, NULL, NULL,
+                  &pid, -1, &outfd, &errfd, VIR_EXEC_NONE);
     if(ret == -1) {
         openvzError(conn, VIR_ERR_INTERNAL_ERROR,
                _("Could not exec %s"), VZLIST);
Index: src/qemu_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/qemu_driver.c,v
retrieving revision 1.111
diff -u -p -r1.111 qemu_driver.c
--- src/qemu_driver.c	20 Aug 2008 20:48:36 -0000	1.111
+++ src/qemu_driver.c	26 Aug 2008 09:11:45 -0000
@@ -847,6 +847,9 @@ static int qemudStartVMDaemon(virConnect
     int *tapfds = NULL;
     int ntapfds = 0;
     int qemuCmdFlags;
+    fd_set keepfd;
+
+    FD_ZERO(&keepfd);
 
     if (virDomainIsActive(vm)) {
         qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
@@ -950,7 +953,10 @@ static int qemudStartVMDaemon(virConnect
     vm->stdout_fd = -1;
     vm->stderr_fd = -1;
 
-    ret = virExec(conn, argv, NULL, &vm->pid,
+    for (i = 0 ; i < ntapfds ; i++)
+        FD_SET(tapfds[i], &keepfd);
+
+    ret = virExec(conn, argv, NULL, &keepfd, &vm->pid,
                   vm->stdin_fd, &vm->stdout_fd, &vm->stderr_fd,
                   VIR_EXEC_NONBLOCK);
     if (ret == 0) {
@@ -1219,7 +1225,8 @@ dhcpStartDhcpDaemon(virConnectPtr conn,
     if (qemudBuildDnsmasqArgv(conn, network, &argv) < 0)
         return -1;
 
-    ret = virExec(conn, argv, NULL, &network->dnsmasqPid, -1, NULL, NULL, VIR_EXEC_NONBLOCK);
+    ret = virExec(conn, argv, NULL, NULL,
+                  &network->dnsmasqPid, -1, NULL, NULL, VIR_EXEC_NONBLOCK);
 
     for (i = 0; argv[i]; i++)
         VIR_FREE(argv[i]);
Index: src/storage_backend.c
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend.c,v
retrieving revision 1.19
diff -u -p -r1.19 storage_backend.c
--- src/storage_backend.c	20 Aug 2008 09:24:14 -0000	1.19
+++ src/storage_backend.c	26 Aug 2008 09:11:46 -0000
@@ -403,7 +403,8 @@ virStorageBackendRunProgRegex(virConnect
 
 
     /* Run the program and capture its output */
-    if (virExec(conn, prog, NULL, &child, -1, &fd, NULL, VIR_EXEC_NONE) < 0) {
+    if (virExec(conn, prog, NULL, NULL,
+                &child, -1, &fd, NULL, VIR_EXEC_NONE) < 0) {
         goto cleanup;
     }
 
@@ -537,7 +538,8 @@ virStorageBackendRunProgNul(virConnectPt
         v[i] = NULL;
 
     /* Run the program and capture its output */
-    if (virExec(conn, prog, NULL, &child, -1, &fd, NULL, VIR_EXEC_NONE) < 0) {
+    if (virExec(conn, prog, NULL, NULL,
+                &child, -1, &fd, NULL, VIR_EXEC_NONE) < 0) {
         goto cleanup;
     }
 
Index: src/util.c
===================================================================
RCS file: /data/cvs/libvirt/src/util.c,v
retrieving revision 1.53
diff -u -p -r1.53 util.c
--- src/util.c	20 Aug 2008 19:42:36 -0000	1.53
+++ src/util.c	26 Aug 2008 09:11:47 -0000
@@ -132,6 +132,7 @@ int
 virExec(virConnectPtr conn,
         const char *const*argv,
         const char *const*envp,
+        const fd_set *keepfd,
         int *retpid,
         int infd, int *outfd, int *errfd,
         int flags) {
@@ -293,7 +294,9 @@ virExec(virConnectPtr conn,
         if (i != infd &&
             i != null &&
             i != childout &&
-            i != childerr)
+            i != childerr &&
+            (!keepfd ||
+             !FD_ISSET(i, keepfd)))
             close(i);
 
     if (flags & VIR_EXEC_DAEMON) {
@@ -403,7 +406,8 @@ virRun(virConnectPtr conn,
        int *status) {
     int childpid, exitstatus, ret;
 
-    if ((ret = virExec(conn, argv, NULL, &childpid, -1, NULL, NULL, VIR_EXEC_NONE)) < 0)
+    if ((ret = virExec(conn, argv, NULL, NULL,
+                       &childpid, -1, NULL, NULL, VIR_EXEC_NONE)) < 0)
         return ret;
 
     while ((ret = waitpid(childpid, &exitstatus, 0) == -1) && errno == EINTR);
Index: src/util.h
===================================================================
RCS file: /data/cvs/libvirt/src/util.h,v
retrieving revision 1.25
diff -u -p -r1.25 util.h
--- src/util.h	20 Aug 2008 19:42:36 -0000	1.25
+++ src/util.h	26 Aug 2008 09:11:47 -0000
@@ -26,6 +26,7 @@
 
 #include "util-lib.h"
 #include "verify.h"
+#include <sys/select.h>
 
 enum {
     VIR_EXEC_NONE   = 0,
@@ -36,6 +37,7 @@ enum {
 int virExec(virConnectPtr conn,
             const char *const*argv,
             const char *const*envp,
+            const fd_set *keepfd,
             int *retpid,
             int infd,
             int *outfd,


-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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