From 5c4d37f9fe4d894ed0f8bd8fd7e5c5b87abbe104 Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Tue, 5 Oct 2010 14:30:40 +0200
Subject: [PATCH] Use only one version of WritePidFile
This patch merges the pid file handling used for ganeti-* daemons and
impexp daemons. The latter version is used, since it's more reliable:
uses locked pid files as opposed to checking 'live' processes.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>
---
lib/daemon.py | 2 +-
lib/utils.py | 35 +++++++++++++++--------------------
test/ganeti.utils_unittest.py | 11 ++++++++---
3 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/lib/daemon.py b/lib/daemon.py
index 9df926200..e765614cf 100644
--- a/lib/daemon.py
+++ b/lib/daemon.py
@@ -621,7 +621,7 @@ def GenericMain(daemon_name, optionparser, check_fn, exec_fn,
utils.CloseFDs()
utils.Daemonize(logfile=constants.DAEMONS_LOGFILES[daemon_name])
- utils.WritePidFile(daemon_name)
+ utils.WritePidFile(utils.DaemonPidFileName(daemon_name))
try:
utils.SetupLogging(logfile=constants.DAEMONS_LOGFILES[daemon_name],
debug=options.debug,
diff --git a/lib/utils.py b/lib/utils.py
index 85ea39344..9e04d980e 100644
--- a/lib/utils.py
+++ b/lib/utils.py
@@ -408,20 +408,7 @@ def _StartDaemonChild(errpipe_read, errpipe_write,
# Open PID file
if pidfile:
- try:
- # TODO: Atomic replace with another locked file instead of writing into
- # it after creating
- fd_pidfile = os.open(pidfile, os.O_WRONLY | 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.
- LockFile(fd_pidfile)
-
- os.write(fd_pidfile, "%d\n" % os.getpid())
- except Exception, err:
- raise Exception("Creating and locking PID file failed: %s" % err)
+ fd_pidfile = WritePidFile(pidfile)
# Keeping the file open to hold the lock
noclose_fds.append(fd_pidfile)
@@ -2217,23 +2204,31 @@ def StopDaemon(name):
return True
-def WritePidFile(name):
+def WritePidFile(pidfile):
"""Write the current process pidfile.
The file will be written to L{constants.RUN_GANETI_DIR}I{/name.pid}
@type name: str
@param name: the daemon name to use
+ @param pid: if passed, will be used instead of getpid()
@raise errors.GenericError: if the pid file already exists and
points to a live process
"""
- pid = os.getpid()
- pidfilename = DaemonPidFileName(name)
- if IsProcessAlive(ReadPidFile(pidfilename)):
- raise errors.GenericError("%s contains a live process" % pidfilename)
+ # 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)
+
+ # 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.
+ LockFile(fd_pidfile)
+
+ os.write(fd_pidfile, "%d\n" % os.getpid())
- WriteFile(pidfilename, data="%d\n" % pid)
+ return fd_pidfile
def RemovePidFile(name):
diff --git a/test/ganeti.utils_unittest.py b/test/ganeti.utils_unittest.py
index 731d749ef..78d747834 100755
--- a/test/ganeti.utils_unittest.py
+++ b/test/ganeti.utils_unittest.py
@@ -177,13 +177,15 @@ class TestPidFileFunctions(unittest.TestCase):
def testPidFileFunctions(self):
pid_file = self.f_dpn('test')
- utils.WritePidFile('test')
+ fd = utils.WritePidFile(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(errors.GenericError, utils.WritePidFile, 'test')
+ self.failUnlessRaises(errors.LockError, utils.WritePidFile,
+ self.f_dpn('test'))
+ os.close(fd)
utils.RemovePidFile('test')
self.failIf(os.path.exists(pid_file),
"PID file should not exist anymore")
@@ -194,6 +196,9 @@ class TestPidFileFunctions(unittest.TestCase):
fh.close()
self.failUnlessEqual(utils.ReadPidFile(pid_file), 0,
"ReadPidFile should return 0 for invalid pid file")
+ # but now, even with the file existing, we should be able to lock it
+ fd = utils.WritePidFile(self.f_dpn('test'))
+ os.close(fd)
utils.RemovePidFile('test')
self.failIf(os.path.exists(pid_file),
"PID file should not exist anymore")
@@ -203,7 +208,7 @@ class TestPidFileFunctions(unittest.TestCase):
r_fd, w_fd = os.pipe()
new_pid = os.fork()
if new_pid == 0: #child
- utils.WritePidFile('child')
+ utils.WritePidFile(self.f_dpn('child'))
os.write(w_fd, 'a')
signal.pause()
os._exit(0)
--
GitLab