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

Daniel P. Berrange berrange at redhat.com
Thu Aug 21 13:58:30 UTC 2008


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....

 lxc_driver.c      |   17 ++++++++++++++---
 openvz_driver.c   |    6 ++++--
 qemu_driver.c     |   26 +++++++++++++++++++-------
 storage_backend.c |    6 ++++--
 util.c            |    8 ++++++--
 util.h            |    7 +++++++
 6 files changed, 54 insertions(+), 16 deletions(-)

Daniel

Index: src/lxc_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/lxc_driver.c,v
retrieving revision 1.23
diff -u -p -r1.23 lxc_driver.c
--- src/lxc_driver.c	20 Aug 2008 20:55:32 -0000	1.23
+++ src/lxc_driver.c	21 Aug 2008 13:52:25 -0000
@@ -615,6 +615,8 @@ static int lxcControllerStart(virConnect
     const char **largv = NULL;
     pid_t child;
     int status;
+    char *keepfd = NULL;
+    char appPtyStr[30];
 
 #define ADD_ARG_SPACE                                                   \
     do { \
@@ -638,11 +640,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++) {
@@ -652,10 +656,15 @@ 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,
+    if (VIR_ALLOC_N(keepfd, VIR_EXEC_FDSET_SIZE()) < 0)
+        goto no_memory;
+
+    VIR_EXEC_FDSET_ON(keepfd, appPty);
+
+    if (virExec(conn, largv, NULL, keepfd, &child,
                 vm->stdin_fd, &vm->stdout_fd, &vm->stderr_fd,
                 VIR_EXEC_NONE) < 0)
         goto cleanup;
@@ -686,6 +695,8 @@ static int lxcControllerStart(virConnect
     ret = 0;
 
 cleanup:
+    VIR_FREE(keepfd);
+
     for (i = 0 ; i < largc ; i++)
         VIR_FREE(largv[i]);
 
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	21 Aug 2008 13:52:25 -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	21 Aug 2008 13:52:26 -0000
@@ -847,6 +847,7 @@ static int qemudStartVMDaemon(virConnect
     int *tapfds = NULL;
     int ntapfds = 0;
     int qemuCmdFlags;
+    char *keepfd = NULL;
 
     if (virDomainIsActive(vm)) {
         qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
@@ -950,12 +951,22 @@ static int qemudStartVMDaemon(virConnect
     vm->stdout_fd = -1;
     vm->stderr_fd = -1;
 
-    ret = virExec(conn, argv, NULL, &vm->pid,
-                  vm->stdin_fd, &vm->stdout_fd, &vm->stderr_fd,
-                  VIR_EXEC_NONBLOCK);
-    if (ret == 0) {
-        vm->def->id = driver->nextvmid++;
-        vm->state = migrateFrom ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING;
+    if (VIR_ALLOC_N(keepfd, VIR_EXEC_FDSET_SIZE()) < 0) {
+        ret = -1;
+        qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL);
+    } else {
+        for (i = 0 ; i < ntapfds ; i++)
+            VIR_EXEC_FDSET_ON(keepfd, tapfds[i]);
+
+        ret = virExec(conn, argv, NULL, keepfd, &vm->pid,
+                      vm->stdin_fd, &vm->stdout_fd, &vm->stderr_fd,
+                      VIR_EXEC_NONBLOCK);
+        if (ret == 0) {
+            vm->def->id = driver->nextvmid++;
+            vm->state = migrateFrom ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING;
+        }
+
+        VIR_FREE(keepfd);
     }
 
     for (i = 0 ; argv[i] ; i++)
@@ -1219,7 +1230,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	21 Aug 2008 13:52:26 -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	21 Aug 2008 13:52:26 -0000
@@ -132,6 +132,7 @@ int
 virExec(virConnectPtr conn,
         const char *const*argv,
         const char *const*envp,
+        const char *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 ||
+             !VIR_EXEC_FDSET_ISON(keepfd, i)))
             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	21 Aug 2008 13:52:26 -0000
@@ -33,9 +33,16 @@ enum {
     VIR_EXEC_DAEMON = (1 << 1),
 };
 
+#define VIR_EXEC_FDSET_SIZE() (sysconf(_SC_OPEN_MAX)/8)
+#define VIR_EXEC_FDSET_CLEAR(set) memset((set), 0, VIR_EXEC_FDSET_SIZE())
+#define VIR_EXEC_FDSET_ON(set, fd) (set[(fd)/8] |= (1 << ((fd)%8)))
+#define VIR_EXEC_FDSET_OFF(set, fd) (set[(fd)/8] &= ~(1 << ((fd)%8)))
+#define VIR_EXEC_FDSET_ISON(set, fd) (set[(fd)/8] & (1 << ((fd)%8)))
+
 int virExec(virConnectPtr conn,
             const char *const*argv,
             const char *const*envp,
+            const char *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 :|




More information about the libvir-list mailing list