[PATCH] Rework the daemonization sequence

[Iustin Pop]

This is a backport of commit 2258 on the 2.0 branch (actually trunk) to
the 1.2 branch:

  The current fork+close fds sequence has deficiencies which are hard to
  work around:
    - logging can start logging before we fork (e.g. if we need to emit
      messages related to master checking), and thus use FDs which we
      can't track nicely
    - the queue locks the queue file, and again this fd needs to be kept
      open which is hard from the main loop (and this error is currently
      hidden by the fact that we don't log it)

  Given the above, it's much simpler, in case we will fork later, to close
  file descriptors right at the beginning of the program, and in Daemonize
  only close/reopen the stdin/out/err fds.

  In addition, we also close() the handlers we remove in SetupLogging so
  that the cleanup is more thorough.

  Reviewed-by: imsnah

The reason for the backport is that in certain cases, Twisted 8.1 will
open extra file descriptors at reactor instantiation, so our closing of
the filedescriptors is too late. The patch attempts to close the
filedescripts earlier (which might be or not early enough).
---
 daemons/ganeti-noded |    4 ++
 daemons/ganeti-rapi  |    1 +
 lib/utils.py         |   78 ++++++++++++++++++++++++++++++++++++-------------
 3 files changed, 62 insertions(+), 21 deletions(-)

diff --git a/daemons/ganeti-noded b/daemons/ganeti-noded
index 58c3804..49ed263 100755
--- a/daemons/ganeti-noded
+++ b/daemons/ganeti-noded
@@ -589,6 +589,10 @@ def main():
   """
   options, args = ParseOptions()
   utils.debug = options.debug
+
+  if options.fork:
+    utils.CloseFDs()
+
   for fname in (constants.SSL_CERT_FILE,):
     if not os.path.isfile(fname):
       print "config %s not there, will not run." % fname
diff --git a/daemons/ganeti-rapi b/daemons/ganeti-rapi
index fa1d41f..b379e11 100755
--- a/daemons/ganeti-rapi
+++ b/daemons/ganeti-rapi
@@ -86,6 +86,7 @@ def main():
   """
   options, args = ParseOptions()
   if options.fork:
+    utils.CloseFDs()
     utils.Daemonize(logfile=constants.LOG_RAPISERVER)
   RESTHTTPServer.start(options)
   sys.exit(0)
diff --git a/lib/utils.py b/lib/utils.py
index 083fd2d..7387781 100644
--- a/lib/utils.py
+++ b/lib/utils.py
@@ -1119,15 +1119,33 @@ def TestDelay(duration):
   return True
 
 
-def Daemonize(logfile, noclose_fds=None):
-  """Daemonize the current process.
+def _CloseFDNoErr(fd, retries=5):
+  """Close a file descriptor ignoring errors.
 
-  This detaches the current process from the controlling terminal and
-  runs it in the background as a daemon.
+  @type fd: int
+  @param fd: the file descriptor
+  @type retries: int
+  @param retries: how many retries to make, in case we get any
+      other error than EBADF
+
+  """
+  try:
+    os.close(fd)
+  except OSError, err:
+    if err.errno != errno.EBADF:
+      if retries > 0:
+        _CloseFDNoErr(fd, retries - 1)
+    # else either it's closed already or we're out of retries, so we
+    # ignore this and go on
+
+
+def CloseFDs(noclose_fds=None):
+  """Close file descriptors.
+
+  This closes all file descriptors above 2 (i.e. except
+  stdin/out/err).
 
   """
-  UMASK = 077
-  WORKDIR = "/"
   # Default maximum for the number of available file descriptors.
   if 'SC_OPEN_MAX' in os.sysconf_names:
     try:
@@ -1138,6 +1156,31 @@ def Daemonize(logfile, noclose_fds=None):
       MAXFD = 1024
   else:
     MAXFD = 1024
+  maxfd = resource.getrlimit(resource.RLIMIT_NOFILE)[1]
+  if (maxfd == resource.RLIM_INFINITY):
+    maxfd = MAXFD
+
+  # Iterate through and close all file descriptors (except the standard ones)
+  for fd in range(3, maxfd):
+    if noclose_fds and fd in noclose_fds:
+      continue
+    _CloseFDNoErr(fd)
+
+
+def Daemonize(logfile):
+  """Daemonize the current process.
+
+  This detaches the current process from the controlling terminal and
+  runs it in the background as a daemon.
+
+  @type logfile: str
+  @param logfile: the logfile to which we should redirect stdout/stderr
+  @rtype: int
+  @returns: the value zero
+
+  """
+  UMASK = 077
+  WORKDIR = "/"
 
   # this might fail
   pid = os.fork()
@@ -1153,22 +1196,15 @@ def Daemonize(logfile, noclose_fds=None):
       os._exit(0) # Exit parent (the first child) of the second child.
   else:
     os._exit(0) # Exit parent of the first child.
-  maxfd = resource.getrlimit(resource.RLIMIT_NOFILE)[1]
-  if (maxfd == resource.RLIM_INFINITY):
-    maxfd = MAXFD
 
-  # Iterate through and close all file descriptors.
-  for fd in range(0, maxfd):
-    if noclose_fds and fd in noclose_fds:
-      continue
-    try:
-      os.close(fd)
-    except OSError: # ERROR, fd wasn't open to begin with (ignored)
-      pass
-  os.open(logfile, os.O_RDWR|os.O_CREAT|os.O_APPEND, 0600)
-  # Duplicate standard input to standard output and standard error.
-  os.dup2(0, 1)     # standard output (1)
-  os.dup2(0, 2)     # standard error (2)
+  for fd in range(3):
+    _CloseFDNoErr(fd)
+  i = os.open("/dev/null", os.O_RDONLY) # stdin
+  assert i == 0, "Can't close/reopen stdin"
+  i = os.open(logfile, os.O_WRONLY|os.O_CREAT|os.O_APPEND, 0600) # stdout
+  assert i == 1, "Can't close/reopen stdout"
+  # Duplicate standard output to standard error.
+  os.dup2(1, 2)
   return 0
 
 
-- 
1.5.6.5


--oyUTqETQ0mS9luUI--