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