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>

diff --git a/lib/daemon.py b/lib/daemon.py
index 9df9262..e765614 100644 (file)
--- 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 85ea393..9e04d98 100644 (file)
--- 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 731d749..78d7478 100755 (executable)
--- 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)