diff --git a/lib/utils.py b/lib/utils.py index a9cdfc0b873551a82a82183f65b6cdb5031bad2b..a9b71b5fbb3c0161d298dc0ead40a5f0027b8588 100644 --- a/lib/utils.py +++ b/lib/utils.py @@ -268,9 +268,13 @@ def IsProcessAlive(pid): Returns: true or false, depending on if the pid exists or not - Remarks: zombie processes treated as not alive + Remarks: zombie processes treated as not alive, and giving a pid <= + 0 makes the function to return False. """ + if pid <= 0: + return False + try: f = open("/proc/%d/status" % pid) except IOError, err: @@ -290,29 +294,30 @@ def IsProcessAlive(pid): return alive -def IsPidFileAlive(pidfile): - """Check whether the given pidfile points to a live process. +def ReadPidFile(pidfile): + """Read the pid from a file. - @param pidfile: Path to a file containing the pid to be checked - @type pidfile: string (filename) + @param pidfile: Path to a file containing the pid to be checked + @type pidfile: string (filename) + @return: The process id, if the file exista and contains a valid PID, + otherwise 0 + @rtype: int """ try: pf = open(pidfile, 'r') - except EnvironmentError, open_err: - if open_err.errno == errno.ENOENT: - return False - else: - raise errors.GenericError("Cannot open file %s. Error: %s" % - (pidfile, str(open_err))) + except EnvironmentError, err: + if err.errno != errno.ENOENT: + logging.exception("Can't read pid file?!") + return 0 try: pid = int(pf.read()) - except ValueError: - raise errors.GenericError("Invalid pid string in %s" % - (pidfile,)) + except ValueError, err: + logging.info("Can't parse pid file contents", exc_info=err) + return 0 - return IsProcessAlive(pid) + return pid def MatchNameComponent(key, name_list): @@ -1066,7 +1071,7 @@ def WritePidFile(name): """ pid = os.getpid() pidfilename = _DaemonPidFileName(name) - if IsPidFileAlive(pidfilename): + if IsProcessAlive(ReadPidFile(pidfilename)): raise errors.GenericError("%s contains a live process" % pidfilename) WriteFile(pidfilename, data="%d\n" % pid) diff --git a/test/ganeti.utils_unittest.py b/test/ganeti.utils_unittest.py index 7a9c64f5cefd0f8ac49bacc6f45c4416cb18e2cf..23a8b6b75fb52f27e31bf70b18895131d66a80b0 100755 --- a/test/ganeti.utils_unittest.py +++ b/test/ganeti.utils_unittest.py @@ -43,12 +43,13 @@ from ganeti.utils import IsProcessAlive, RunCmd, \ ParseUnit, AddAuthorizedKey, RemoveAuthorizedKey, \ ShellQuote, ShellQuoteArgs, TcpPing, ListVisibleFiles, \ SetEtcHostsEntry, RemoveEtcHostsEntry, FirstFree -from ganeti.errors import LockError, UnitParseError +from ganeti.errors import LockError, UnitParseError, GenericError def _ChildHandler(signal, stack): global _ChildFlag _ChildFlag = True + class TestIsProcessAlive(unittest.TestCase): """Testing case for IsProcessAlive""" @@ -99,8 +100,9 @@ class TestIsProcessAlive(unittest.TestCase): self.assert_(not IsProcessAlive(self.pid_non_existing), "noexisting process detected") + class TestPidFileFunctions(unittest.TestCase): - """Tests for WritePidFile, RemovePidFile and IsPidFileAlive""" + """Tests for WritePidFile, RemovePidFile and ReadPidFile""" def setUp(self): self.dir = tempfile.mkdtemp() @@ -108,13 +110,31 @@ class TestPidFileFunctions(unittest.TestCase): utils._DaemonPidFileName = self.f_dpn def testPidFileFunctions(self): + pid_file = self.f_dpn('test') utils.WritePidFile('test') - self.assert_(os.path.exists(self.f_dpn('test'))) - self.assert_(utils.IsPidFileAlive(self.f_dpn('test'))) + self.failUnless(os.path.exists(pid_file), + "PID file should have been created") + read_pid = utils.ReadPidFile(pid_file) + self.failUnlessEqual(read_pid, os.getpid()) + self.failUnless(utils.IsProcessAlive(read_pid)) + self.failUnlessRaises(GenericError, utils.WritePidFile, 'test') + utils.RemovePidFile('test') + self.failIf(os.path.exists(pid_file), + "PID file should not exist anymore") + self.failUnlessEqual(utils.ReadPidFile(pid_file), 0, + "ReadPidFile should return 0 for missing pid file") + fh = open(pid_file, "w") + fh.write("blah\n") + fh.close() + self.failUnlessEqual(utils.ReadPidFile(pid_file), 0, + "ReadPidFile should return 0 for invalid pid file") utils.RemovePidFile('test') - self.assert_(not os.path.exists(self.f_dpn('test'))) + self.failIf(os.path.exists(pid_file), + "PID file should not exist anymore") def tearDown(self): + for name in os.listdir(self.dir): + os.unlink(os.path.join(self.dir, name)) os.rmdir(self.dir)