From b6522276819f6e63892d411d6ed9a74a09dbccdb Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Thu, 8 Dec 2011 15:33:31 +0100
Subject: [PATCH] utils.io.WritePidFile: Improve error reporting
If the PID file is already locked by another process, try to read
the content and report it as part of the error message.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
---
lib/errors.py | 6 ++++++
lib/utils/io.py | 31 +++++++++++++++++++++++++------
test/ganeti.utils.io_unittest.py | 19 ++++++++++++++++++-
3 files changed, 49 insertions(+), 7 deletions(-)
diff --git a/lib/errors.py b/lib/errors.py
index 181288618..7319cd9f1 100644
--- a/lib/errors.py
+++ b/lib/errors.py
@@ -82,6 +82,12 @@ class LockError(GenericError):
pass
+class PidFileLockError(LockError):
+ """PID file is already locked by another process.
+
+ """
+
+
class HypervisorError(GenericError):
"""Hypervisor-related exception.
diff --git a/lib/utils/io.py b/lib/utils/io.py
index 80dc1f0d3..43f744937 100644
--- a/lib/utils/io.py
+++ b/lib/utils/io.py
@@ -701,13 +701,24 @@ def ReadPidFile(pidfile):
logging.exception("Can't read pid file")
return 0
+ return _ParsePidFileContents(raw_data)
+
+
+def _ParsePidFileContents(data):
+ """Tries to extract a process ID from a PID file's content.
+
+ @type data: string
+ @rtype: int
+ @return: Zero if nothing could be read, PID otherwise
+
+ """
try:
- pid = int(raw_data)
- except (TypeError, ValueError), err:
+ pid = int(data)
+ except (TypeError, ValueError):
logging.info("Can't parse pid file contents", exc_info=True)
return 0
-
- return pid
+ else:
+ return pid
def ReadLockedPidFile(path):
@@ -834,13 +845,21 @@ def WritePidFile(pidfile):
"""
# We don't rename nor truncate the file to not drop locks under
# existing processes
- fd_pidfile = os.open(pidfile, os.O_WRONLY | os.O_CREAT, 0600)
+ fd_pidfile = os.open(pidfile, os.O_RDWR | os.O_CREAT, 0600)
# Lock the PID file (and fail if not possible to do so). Any code
# wanting to send a signal to the daemon should try to lock the PID
# file before reading it. If acquiring the lock succeeds, the daemon is
# no longer running and the signal should not be sent.
- filelock.LockFile(fd_pidfile)
+ try:
+ filelock.LockFile(fd_pidfile)
+ except errors.LockError:
+ msg = ["PID file '%s' is already locked by another process" % pidfile]
+ # Try to read PID file
+ pid = _ParsePidFileContents(os.read(fd_pidfile, 100))
+ if pid > 0:
+ msg.append(", PID read from file is %s" % pid)
+ raise errors.PidFileLockError("".join(msg))
os.write(fd_pidfile, "%d\n" % os.getpid())
diff --git a/test/ganeti.utils.io_unittest.py b/test/ganeti.utils.io_unittest.py
index 46bb009d7..53f1c8151 100755
--- a/test/ganeti.utils.io_unittest.py
+++ b/test/ganeti.utils.io_unittest.py
@@ -711,7 +711,7 @@ class TestPidFileFunctions(unittest.TestCase):
read_pid = utils.ReadPidFile(pid_file)
self.failUnlessEqual(read_pid, os.getpid())
self.failUnless(utils.IsProcessAlive(read_pid))
- self.failUnlessRaises(errors.LockError, utils.WritePidFile,
+ self.failUnlessRaises(errors.PidFileLockError, utils.WritePidFile,
self.f_dpn('test'))
os.close(fd)
utils.RemoveFile(self.f_dpn("test"))
@@ -747,11 +747,28 @@ class TestPidFileFunctions(unittest.TestCase):
read_pid = utils.ReadPidFile(pid_file)
self.failUnlessEqual(read_pid, new_pid)
self.failUnless(utils.IsProcessAlive(new_pid))
+
+ # Try writing to locked file
+ try:
+ utils.WritePidFile(pid_file)
+ except errors.PidFileLockError, err:
+ errmsg = str(err)
+ self.assertTrue(errmsg.endswith(" %s" % new_pid),
+ msg=("Error message ('%s') didn't contain correct"
+ " PID (%s)" % (errmsg, new_pid)))
+ else:
+ self.fail("Writing to locked file didn't fail")
+
utils.KillProcess(new_pid, waitpid=True)
self.failIf(utils.IsProcessAlive(new_pid))
utils.RemoveFile(self.f_dpn('child'))
self.failUnlessRaises(errors.ProgrammerError, utils.KillProcess, 0)
+ def testExceptionType(self):
+ # Make sure the PID lock error is a subclass of LockError in case some code
+ # depends on it
+ self.assertTrue(issubclass(errors.PidFileLockError, errors.LockError))
+
def tearDown(self):
shutil.rmtree(self.dir)
--
GitLab