Hello Greg Padgett,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/45885

to review the following change.

Change subject: supervdsm: don't let _runAs return early due to EINTR
......................................................................

supervdsm: don't let _runAs return early due to EINTR

When _runAs executes a command, it polls a pipe to determine status of
that command.  The poll has a timeout, also allowing the child to be
killed if execution takes too long.  If a stray signal is received by
the parent process, it may cause the poll to return early, prematurely
ending child command execution ending in unintentional failure.

This is particularly prevalant when subsequent _runAs executions are
performed.  Termination of the first will result in a SIGCHLD which
interrupts polling for the second, as exposed by commit 777c36d6.

Because poll will return early without an exception if the low-level
call is interrupted, NoIntrCall() cannot be used and thus a new helper
function is provided that retries based on the time elapsed since the
call started.

PEP-475 will handle this in Python 3.5, but in the meantime this
workaround is needed.

Change-Id: I25af73a8fe67e7bc434f60a7d8492e33dc3b2ffa
Backport-To: 3.6
Bug-Url: https://bugzilla.redhat.com/1259310
Signed-off-by: Greg Padgett <[email protected]>
Reviewed-On: https://gerrit.ovirt.org/45752
Continuous-Integration: Jenkins CI
Reviewed-by: Nir Soffer <[email protected]>
Tested-by: Greg Padgett <[email protected]>
---
M vdsm/supervdsmServer
1 file changed, 21 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/85/45885/1

diff --git a/vdsm/supervdsmServer b/vdsm/supervdsmServer
index 978fc18..c5766a3 100755
--- a/vdsm/supervdsmServer
+++ b/vdsm/supervdsmServer
@@ -118,6 +118,26 @@
     return wrapper
 
 
+def safe_poll(mp_connection, timeout):
+    """
+    This is a workaround until we get the PEP-475 fix for EINTR.  It
+    ensures that a multiprocessing.connection.poll() will not return
+    before the timeout due to an interruption.
+
+    Returns True if there is any data to read from the pipe or if the
+    pipe was closed.  Returns False if the timeout expired.
+    """
+    deadline = utils.monotonic_time() + timeout
+    remaining = timeout
+
+    while not mp_connection.poll(remaining):
+        remaining = deadline - utils.monotonic_time()
+        if remaining <= 0:
+            return False
+
+    return True
+
+
 class _SuperVdsm(object):
 
     UDEV_WITH_RELOAD_VERSION = 181
@@ -246,7 +266,7 @@
 
         needReaping = True
         try:
-            if not pipe.poll(RUN_AS_TIMEOUT):
+            if not safe_poll(pipe, RUN_AS_TIMEOUT):
                 try:
 
                     os.kill(proc.pid, signal.SIGKILL)


-- 
To view, visit https://gerrit.ovirt.org/45885
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I25af73a8fe67e7bc434f60a7d8492e33dc3b2ffa
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Greg Padgett <[email protected]>
Gerrit-Reviewer: Greg Padgett <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to