From 09b727836fce66a69dd517c0d78334101a896c3b Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Fri, 9 Nov 2012 16:12:22 +0100
Subject: [PATCH] RunCmd: Expose "postfork" callback
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The β€œ_postfork_fn” parameter was only used for tests until now. To
implement a good locking scheme, remote commands must also make use of
this callback to release a lock when the command was successfully
started (but did not yet finish).

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>
---
 lib/utils/process.py                  | 24 +++++++++++++-----------
 test/ganeti.utils.process_unittest.py |  8 ++++----
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/lib/utils/process.py b/lib/utils/process.py
index 243151f32..cc1c9ed06 100644
--- a/lib/utils/process.py
+++ b/lib/utils/process.py
@@ -142,7 +142,7 @@ def _BuildCmdEnvironment(env, reset):
 
 def RunCmd(cmd, env=None, output=None, cwd="/", reset_env=False,
            interactive=False, timeout=None, noclose_fds=None,
-           input_fd=None, _postfork_fn=None):
+           input_fd=None, postfork_fn=None):
   """Execute a (shell) command.
 
   The command should not read from its standard input, as it will be
@@ -172,7 +172,8 @@ def RunCmd(cmd, env=None, output=None, cwd="/", reset_env=False,
                       open for the child process
   @type input_fd: C{file}-like object or numeric file descriptor
   @param input_fd: File descriptor for process' standard input
-  @param _postfork_fn: Callback run after fork but before timeout (unittest)
+  @type postfork_fn: Callable receiving PID as parameter
+  @param postfork_fn: Callback run after fork but before timeout
   @rtype: L{RunResult}
   @return: RunResult instance
   @raise errors.ProgrammerError: if we call this when forks are disabled
@@ -211,10 +212,11 @@ def RunCmd(cmd, env=None, output=None, cwd="/", reset_env=False,
       out, err, status, timeout_action = _RunCmdPipe(cmd, cmd_env, shell, cwd,
                                                      interactive, timeout,
                                                      noclose_fds, input_fd,
-                                                     _postfork_fn=_postfork_fn)
+                                                     postfork_fn=postfork_fn)
     else:
-      assert _postfork_fn is None, \
-          "_postfork_fn not supported if output provided"
+      if postfork_fn:
+        raise errors.ProgrammerError("postfork_fn is not supported if output"
+                                     " should be captured")
       assert input_fd is None
       timeout_action = _TIMEOUT_NONE
       status = _RunCmdFile(cmd, cmd_env, shell, output, cwd, noclose_fds)
@@ -490,9 +492,8 @@ def _WaitForProcess(child, timeout):
 
 
 def _RunCmdPipe(cmd, env, via_shell, cwd, interactive, timeout, noclose_fds,
-                input_fd,
-                _linger_timeout=constants.CHILD_LINGER_TIMEOUT,
-                _postfork_fn=None):
+                input_fd, postfork_fn=None,
+                _linger_timeout=constants.CHILD_LINGER_TIMEOUT):
   """Run a command and return its output.
 
   @type  cmd: string or list
@@ -512,7 +513,8 @@ def _RunCmdPipe(cmd, env, via_shell, cwd, interactive, timeout, noclose_fds,
                       open for the child process
   @type input_fd: C{file}-like object or numeric file descriptor
   @param input_fd: File descriptor for process' standard input
-  @param _postfork_fn: Function run after fork but before timeout (unittest)
+  @type postfork_fn: Callable receiving PID as parameter
+  @param postfork_fn: Function run after fork but before timeout
   @rtype: tuple
   @return: (out, err, status)
 
@@ -548,8 +550,8 @@ def _RunCmdPipe(cmd, env, via_shell, cwd, interactive, timeout, noclose_fds,
                            cwd=cwd,
                            preexec_fn=preexec_fn)
 
-  if _postfork_fn:
-    _postfork_fn(child.pid)
+  if postfork_fn:
+    postfork_fn(child.pid)
 
   out = StringIO()
   err = StringIO()
diff --git a/test/ganeti.utils.process_unittest.py b/test/ganeti.utils.process_unittest.py
index 322d74235..f3fbce66a 100755
--- a/test/ganeti.utils.process_unittest.py
+++ b/test/ganeti.utils.process_unittest.py
@@ -155,7 +155,7 @@ class TestIsProcessHandlingSignal(unittest.TestCase):
 
 
 class _PostforkProcessReadyHelper:
-  """A helper to use with _postfork_fn in RunCmd.
+  """A helper to use with C{postfork_fn} in RunCmd.
 
   It makes sure a process has reached a certain state by reading from a fifo.
 
@@ -264,7 +264,7 @@ class TestRunCmd(testutils.GanetiTestCase):
            (self.proc_ready_helper.write_fd, self.fifo_file))
     result = utils.RunCmd(["/bin/sh", "-c", cmd], timeout=0.2,
                           noclose_fds=[self.proc_ready_helper.write_fd],
-                          _postfork_fn=self.proc_ready_helper.Ready)
+                          postfork_fn=self.proc_ready_helper.Ready)
     self.assertEqual(result.exit_code, 0)
 
   def testTimeoutKill(self):
@@ -276,7 +276,7 @@ class TestRunCmd(testutils.GanetiTestCase):
                                 timeout, [self.proc_ready_helper.write_fd],
                                 None,
                                 _linger_timeout=0.2,
-                                _postfork_fn=self.proc_ready_helper.Ready)
+                                postfork_fn=self.proc_ready_helper.Ready)
     self.assert_(status < 0)
     self.assertEqual(-status, signal.SIGKILL)
 
@@ -285,7 +285,7 @@ class TestRunCmd(testutils.GanetiTestCase):
            (self.proc_ready_helper.write_fd, self.fifo_file))
     result = utils.RunCmd(["/bin/sh", "-c", cmd], timeout=0.2,
                           noclose_fds=[self.proc_ready_helper.write_fd],
-                          _postfork_fn=self.proc_ready_helper.Ready)
+                          postfork_fn=self.proc_ready_helper.Ready)
     self.assert_(result.failed)
     self.assertEqual(result.stdout, "sigtermed\n")
 
-- 
GitLab