Add ability to retain specified fds open in RunCmd

Passing tap devices to KVM as file descriptors requires that the respective
file decriptors remain open during utils.RunCmd execution. To this direction,
we add a “noclose_fds” keyword argument to utils.RunCmd, accepting a list of
file descriptors to keep open. The actual fd handling is implemented in
_RunCmdPipe and _RunCmdFile using subprocess.Popen's “preexec_fn”[1],
since subprocess.Popen provides no other way to selectively handle fds.

A small modification is also made to test/ganeti.utils_unittest.py to comply
with _RunCmdPipe's new API and a new unit test is added to test the selective
fd retention functionality.

[1] “If preexec_fn is set to a callable object, this object will be called in
     the child process just before the child is executed. (Unix only)”
    Subprocess documentation

Signed-off-by: Apollon Oikonomopoulos <apollon@noc.grnet.gr>
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

diff --git a/lib/utils/process.py b/lib/utils/process.py
index 5fbc8fe..be7d774 100644 (file)
--- a/lib/utils/process.py
+++ b/lib/utils/process.py
@@ -141,7 +141,7 @@ def _BuildCmdEnvironment(env, reset):
 
 
 def RunCmd(cmd, env=None, output=None, cwd="/", reset_env=False,
-           interactive=False, timeout=None):
+           interactive=False, timeout=None, noclose_fds=None):
   """Execute a (shell) command.
 
   The command should not read from its standard input, as it will be
@@ -166,6 +166,9 @@ def RunCmd(cmd, env=None, output=None, cwd="/", reset_env=False,
   @type timeout: int
   @param timeout: If not None, timeout in seconds until child process gets
                   killed
+  @type noclose_fds: list
+  @param noclose_fds: list of additional (fd >=3) file descriptors to leave
+                      open for the child process
   @rtype: L{RunResult}
   @return: RunResult instance
   @raise errors.ProgrammerError: if we call this when forks are disabled
@@ -196,10 +199,11 @@ def RunCmd(cmd, env=None, output=None, cwd="/", reset_env=False,
   try:
     if output is None:
       out, err, status, timeout_action = _RunCmdPipe(cmd, cmd_env, shell, cwd,
-                                                     interactive, timeout)
+                                                     interactive, timeout,
+                                                     noclose_fds)
     else:
       timeout_action = _TIMEOUT_NONE
-      status = _RunCmdFile(cmd, cmd_env, shell, output, cwd)
+      status = _RunCmdFile(cmd, cmd_env, shell, output, cwd, noclose_fds)
       out = err = ""
   except OSError, err:
     if err.errno == errno.ENOENT:
@@ -463,7 +467,7 @@ def _WaitForProcess(child, timeout):
     pass
 
 
-def _RunCmdPipe(cmd, env, via_shell, cwd, interactive, timeout,
+def _RunCmdPipe(cmd, env, via_shell, cwd, interactive, timeout, noclose_fds,
                 _linger_timeout=constants.CHILD_LINGER_TIMEOUT):
   """Run a command and return its output.
 
@@ -479,6 +483,9 @@ def _RunCmdPipe(cmd, env, via_shell, cwd, interactive, timeout,
   @param interactive: Run command interactive (without piping)
   @type timeout: int
   @param timeout: Timeout after the programm gets terminated
+  @type noclose_fds: list
+  @param noclose_fds: list of additional (fd >=3) file descriptors to leave
+                      open for the child process
   @rtype: tuple
   @return: (out, err, status)
 
@@ -492,12 +499,20 @@ def _RunCmdPipe(cmd, env, via_shell, cwd, interactive, timeout,
   if interactive:
     stderr = stdout = stdin = None
 
+  if noclose_fds:
+    preexec_fn = lambda: CloseFDs(noclose_fds)
+    close_fds = False
+  else:
+    preexec_fn = None
+    close_fds = True
+
   child = subprocess.Popen(cmd, shell=via_shell,
                            stderr=stderr,
                            stdout=stdout,
                            stdin=stdin,
-                           close_fds=True, env=env,
-                           cwd=cwd)
+                           close_fds=close_fds, env=env,
+                           cwd=cwd,
+                           preexec_fn=preexec_fn)
 
   out = StringIO()
   err = StringIO()
@@ -592,7 +607,7 @@ def _RunCmdPipe(cmd, env, via_shell, cwd, interactive, timeout,
   return out, err, status, timeout_action
 
 
-def _RunCmdFile(cmd, env, via_shell, output, cwd):
+def _RunCmdFile(cmd, env, via_shell, output, cwd, noclose_fds):
   """Run a command and save its output to a file.
 
   @type  cmd: string or list
@@ -605,18 +620,30 @@ def _RunCmdFile(cmd, env, via_shell, output, cwd):
   @param output: the filename in which to save the output
   @type cwd: string
   @param cwd: the working directory for the program
+  @type noclose_fds: list
+  @param noclose_fds: list of additional (fd >=3) file descriptors to leave
+                      open for the child process
   @rtype: int
   @return: the exit status
 
   """
   fh = open(output, "a")
+
+  if noclose_fds:
+    preexec_fn = lambda: CloseFDs(noclose_fds + [fh.fileno()])
+    close_fds = False
+  else:
+    preexec_fn = None
+    close_fds = True
+
   try:
     child = subprocess.Popen(cmd, shell=via_shell,
                              stderr=subprocess.STDOUT,
                              stdout=fh,
                              stdin=subprocess.PIPE,
-                             close_fds=True, env=env,
-                             cwd=cwd)
+                             close_fds=close_fds, env=env,
+                             cwd=cwd,
+                             preexec_fn=preexec_fn)
 
     child.stdin.close()
     status = child.wait()

diff --git a/test/ganeti.utils.process_unittest.py b/test/ganeti.utils.process_unittest.py
index 6f4c15a..39e8485 100755 (executable)
--- a/test/ganeti.utils.process_unittest.py
+++ b/test/ganeti.utils.process_unittest.py
@@ -225,7 +225,7 @@ class TestRunCmd(testutils.GanetiTestCase):
     timeout = 0.2
     (out, err, status, ta) = \
       utils.process._RunCmdPipe(cmd, {}, False, "/", False,
-                                timeout, _linger_timeout=0.2)
+                                timeout, None, _linger_timeout=0.2)
     self.assert_(status < 0)
     self.assertEqual(-status, signal.SIGKILL)
 
@@ -308,6 +308,22 @@ class TestRunCmd(testutils.GanetiTestCase):
     self.assertRaises(errors.ProgrammerError, utils.RunCmd, ["true"],
                       output="/dev/null", interactive=True)
 
+  def testNocloseFds(self):
+    """Test selective fd retention (noclose_fds)"""
+    temp = open(self.fname, "r+")
+    try:
+      temp.write("test")
+      temp.seek(0)
+      cmd = "read -u %d; echo $REPLY" % temp.fileno()
+      result = utils.RunCmd(["/bin/bash", "-c", cmd])
+      self.assertEqual(result.stdout.strip(), "")
+      temp.seek(0)
+      result = utils.RunCmd(["/bin/bash", "-c", cmd],
+                            noclose_fds=[temp.fileno()])
+      self.assertEqual(result.stdout.strip(), "test")
+    finally:
+      temp.close()
+
 
 class TestRunParts(testutils.GanetiTestCase):
   """Testing case for the RunParts function"""