[Pkg-libvirt-commits] [libguestfs] 52/266: Revert "launch: Close file descriptors after fork (RHBZ#1123007)."

[Hilko Bengen]

This is an automated email from the git hooks/post-receive script.

bengen pushed a commit to annotated tag debian/1%1.27.35-1
in repository libguestfs.

commit 286f116691c8aad4ac8bf0d044e9f6ca8d5b3356
Author: Richard W.M. Jones <rjones at redhat.com>
Date:   Tue Jul 29 22:20:37 2014 +0100

    Revert "launch: Close file descriptors after fork (RHBZ#1123007)."
    
    This attempted fix for RHBZ#1123007 causes the qemu command line to be
    lost when verbose mode is enabled.  Since this is essential for
    debugging many problems, I am reverting the patch.
    
    This reverts commit 115fcc34325f965ac3723683e4462fc667dcd254.
---
 src/guestfs-internal-frontend.h | 16 +---------------
 src/launch-direct.c             | 17 ++++++++---------
 src/launch-uml.c                | 13 ++++++++-----
 3 files changed, 17 insertions(+), 29 deletions(-)

diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h
index 1e59ad1..acd7be5 100644
--- a/src/guestfs-internal-frontend.h
+++ b/src/guestfs-internal-frontend.h
@@ -1,5 +1,5 @@
 /* libguestfs
- * Copyright (C) 2013-2014 Red Hat Inc.
+ * Copyright (C) 2013 Red Hat Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -164,18 +164,4 @@ extern GUESTFS_DLL_PUBLIC int guestfs___add_libvirt_dom (guestfs_h *g, virDomain
 #  define program_name "libguestfs"
 #endif
 
-/* Close all file descriptors matching the condition. */
-#define close_file_descriptors(cond) do {                               \
-    int max_fd = sysconf (_SC_OPEN_MAX);                                \
-    int fd;                                                             \
-    if (max_fd == -1)                                                   \
-      max_fd = 1024;                                                    \
-    if (max_fd > 65536)                                                 \
-      max_fd = 65536;          /* bound the amount of work we do here */ \
-    for (fd = 0; fd < max_fd; ++fd) {                                   \
-      if (cond)                                                         \
-        close (fd);                                                     \
-    }                                                                   \
-  } while (0)
-
 #endif /* GUESTFS_INTERNAL_FRONTEND_H_ */
diff --git a/src/launch-direct.c b/src/launch-direct.c
index 9654e04..3de869b 100644
--- a/src/launch-direct.c
+++ b/src/launch-direct.c
@@ -717,13 +717,6 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
         goto dup_failed;
 
       close (sv[1]);
-
-      /* Close any other file descriptors that we don't want to pass
-       * to qemu.  This prevents file descriptors which didn't have
-       * O_CLOEXEC set properly from leaking into the subprocess.  See
-       * RHBZ#1123007.
-       */
-      close_file_descriptors (fd >= 2);
     }
 
     /* Dump the command line (after setting up stderr above). */
@@ -754,7 +747,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   if (g->recovery_proc) {
     r = fork ();
     if (r == 0) {
-      int i;
+      int i, fd, max_fd;
       struct sigaction sa;
       pid_t qemu_pid = data->pid;
       pid_t parent_pid = getppid ();
@@ -774,7 +767,13 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
       /* Close all other file descriptors.  This ensures that we don't
        * hold open (eg) pipes from the parent process.
        */
-      close_file_descriptors (1);
+      max_fd = sysconf (_SC_OPEN_MAX);
+      if (max_fd == -1)
+        max_fd = 1024;
+      if (max_fd > 65536)
+        max_fd = 65536; /* bound the amount of work we do here */
+      for (fd = 0; fd < max_fd; ++fd)
+        close (fd);
 
       /* It would be nice to be able to put this in the same process
        * group as qemu (ie. setpgid (0, qemu_pid)).  However this is
diff --git a/src/launch-uml.c b/src/launch-uml.c
index 88c684b..2a6ddaf 100644
--- a/src/launch-uml.c
+++ b/src/launch-uml.c
@@ -333,9 +333,6 @@ launch_uml (guestfs_h *g, void *datav, const char *arg)
         goto dup_failed;
 
       close (csv[1]);
-
-      /* RHBZ#1123007 */
-      close_file_descriptors (fd >= 2 && fd != dsv[1]);
     }
 
     /* Dump the command line (after setting up stderr above). */
@@ -363,7 +360,7 @@ launch_uml (guestfs_h *g, void *datav, const char *arg)
   if (g->recovery_proc) {
     r = fork ();
     if (r == 0) {
-      int i;
+      int i, fd, max_fd;
       struct sigaction sa;
       pid_t vmlinux_pid = data->pid;
       pid_t parent_pid = getppid ();
@@ -383,7 +380,13 @@ launch_uml (guestfs_h *g, void *datav, const char *arg)
       /* Close all other file descriptors.  This ensures that we don't
        * hold open (eg) pipes from the parent process.
        */
-      close_file_descriptors (1);
+      max_fd = sysconf (_SC_OPEN_MAX);
+      if (max_fd == -1)
+        max_fd = 1024;
+      if (max_fd > 65536)
+        max_fd = 65536; /* bound the amount of work we do here */
+      for (fd = 0; fd < max_fd; ++fd)
+        close (fd);
 
       /* It would be nice to be able to put this in the same process
        * group as vmlinux (ie. setpgid (0, vmlinux_pid)).  However

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-libvirt/libguestfs.git