On 15:44 Tue 11 Jan     , Guido Trotter wrote:
> On Tue, Jan 11, 2011 at 3:40 PM, Apollon Oikonomopoulos
> <[email protected]> wrote:
> > On 15:39 Tue 11 Jan     , Michael Hanselmann wrote:
> >> Am 10. Januar 2011 16:05 schrieb Apollon Oikonomopoulos 
> >> <[email protected]>:
> >> > 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
> >>
> >> LGTM, but this will need a rebase after the utils split. Unless you
> >> want to do it, I can do it tomorrow.
> >>
> >> Michael
> >
> > No, I'll do it since I also have to rebase the rest of the series anyway
> > ;-)
> >
> 
> I can do it as well as part of pushing: it's just applying the same
> code change to another file, so...
> 
> Guido

Done already :)
--

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 <[email protected]>
---
 lib/utils/__init__.py         |   43 ++++++++++++++++++++++++++++++++--------
 test/ganeti.utils_unittest.py |   17 +++++++++++++++-
 2 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/lib/utils/__init__.py b/lib/utils/__init__.py
index 582b4ef..6f408ee 100644
--- a/lib/utils/__init__.py
+++ b/lib/utils/__init__.py
@@ -171,7 +171,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
@@ -196,6 +196,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
@@ -226,10 +229,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:
@@ -491,7 +495,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.
 
@@ -507,6 +511,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)
 
@@ -520,12 +527,19 @@ 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()
@@ -618,7 +632,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
@@ -631,18 +645,29 @@ 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_unittest.py b/test/ganeti.utils_unittest.py
index 1d58057..433068c 100755
--- a/test/ganeti.utils_unittest.py
+++ b/test/ganeti.utils_unittest.py
@@ -235,7 +235,7 @@ class TestRunCmd(testutils.GanetiTestCase):
     cmd = ["/bin/sh", "-c", "trap '' TERM; read < %s" % self.fifo_file]
     timeout = 0.2
     out, err, status, ta = utils._RunCmdPipe(cmd, {}, False, "/", False,
-                                             timeout, _linger_timeout=0.2)
+                                             timeout, None, 
_linger_timeout=0.2)
     self.assert_(status < 0)
     self.assertEqual(-status, signal.SIGKILL)
 
@@ -314,6 +314,21 @@ class TestRunCmd(testutils.GanetiTestCase):
     self.assertRaises(errors.ProgrammerError, 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 = RunCmd(["/bin/bash", "-c", cmd])
+      self.assertEqual(result.stdout.strip(), "")
+      temp.seek(0)
+      result = 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"""
-- 
1.7.1

Reply via email to