Nir Soffer has uploaded a new change for review.
Change subject: utils: Raise detectable error if termination fail
......................................................................
utils: Raise detectable error if termination fail
If terminating process failed because of unexpected error (for example
Popen.poll() raised), we use to log the exception and return silently.
This is a disastrous behavior for storage, when we want to ensure that a
process was terminated. If we could not terminate the process, a storage
job running the process must stay in running state to prevent engine
from starting the same operation on another machine.
Typically when we cannot handle an error we should not handle it. But
in this special case, letting the caller handle the error is not useful
since it does not have any context.
To make it possible to handle this critical error, we raise now a new
TerminatingFallure exception with process pid and the original error.
Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c
Signed-off-by: Nir Soffer <nsof...@redhat.com>
---
M lib/vdsm/utils.py
M tests/utilsTests.py
2 files changed, 19 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/25/65325/1
diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py
index e5c28b8..ccc13f3 100644
--- a/lib/vdsm/utils.py
+++ b/lib/vdsm/utils.py
@@ -68,6 +68,18 @@
_THP_STATE_PATH = '/sys/kernel/mm/redhat_transparent_hugepage/enabled'
+class TerminatingFailure(Exception):
+
+ msg = "Failed to terminate process {self.pid}: {self.error}"
+
+ def __init__(self, pid, error):
+ self.pid = pid
+ self.error = error
+
+ def __str__(self):
+ return msg.format(self=self)
+
+
class IOCLASS:
REALTIME = 1
BEST_EFFORT = 2
@@ -739,8 +751,8 @@
logging.debug('Terminating process pid=%d' % proc.pid)
proc.kill()
proc.wait()
- except Exception:
- logging.exception('Failed to kill process %d' % proc.pid)
+ except Exception as e:
+ raise TerminatingFailure(proc.pid, e)
def get_selinux_enforce_mode():
diff --git a/tests/utilsTests.py b/tests/utilsTests.py
index 6ff85d4..e3a184c 100644
--- a/tests/utilsTests.py
+++ b/tests/utilsTests.py
@@ -106,9 +106,12 @@
raise FakeKillError("fake kill exception")
self.proc.kill = fake_kill
- with utils.terminating(self.proc):
- self.assertIsNone(self.proc.poll())
+ with self.assertRaises(utils.TerminatingFailure) as e:
+ with utils.terminating(self.proc):
+ self.assertIsNone(self.proc.poll())
+ self.assertEqual(e.exception.pid, self.proc.pid)
+ self.assertEqual(type(e.exception.error), FakeKillError)
self.assertIsNone(self.proc.returncode)
--
To view, visit https://gerrit.ovirt.org/65325
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsof...@redhat.com>
_______________________________________________
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
