From 7b0bf9cdd2656adbbbc1fa888a6df77fbe2f42a5 Mon Sep 17 00:00:00 2001 From: Apollon Oikonomopoulos <apollon@noc.grnet.gr> Date: Mon, 10 Jan 2011 18:05:26 +0200 Subject: [PATCH] Add ability to retain specified fds open in RunCmd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- lib/utils/process.py | 45 +++++++++++++++++++++------ test/ganeti.utils.process_unittest.py | 18 ++++++++++- 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/lib/utils/process.py b/lib/utils/process.py index 5fbc8fe3f..be7d77496 100644 --- 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 6f4c15a41..39e84858f 100755 --- 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""" -- GitLab