[PATCHES] CTDB - fix PID file problem

[Martin Schwenke]

On Wed, 2 Jul 2014 12:36:33 +0200, Sumit Bose <sbose at redhat.com> wrote:

> On Wed, Jul 02, 2014 at 07:56:43PM +1000, Martin Schwenke wrote:
> > 2 patches:
> > 
> > * ctdb-daemon: Check PID in ctdb_remove_pidfile(), not unreliable flag
> > 
> >   If something unexpectedly uses fork() then an exiting child will
> >   remove the PID file while the main daemon is still running.  The real
> >   test is whether the current process has the PID of the main CTDB
> >   daemon.
> > 
> > * ctdb-daemon: Remove ctdb->ctdbd_pid, just use ctdbd_pid
> 
> Wouldn't it be nicer to remove the global variable?
> 
> > 
> >   The global variable is needed anyway so just use it.
> 
> Why? I have found 4 places where ctdbd_pid is used. One is the
> assignment, at two other place the ctdb_context is already available.
> The remaining one is in ctdb_tevent_trace() where the ctdb_context must
> be given as private data in tevent_set_trace_callback() to make
> ctdb->ctdbd_pid available.

... and an extra use is introduced in the 1st patch.  We need a
reliable way of checking whether the main ctdbd is exiting so that we
can remove the PID file.  The current method is unreliable (e.g. if a
library call unexpectedly forks a child that then exits then the PID
file is mistakenly removed). So, we need to compare against the main
ctdbd PID.  We remove the PID file in an atexit() handler, which needs a
global variable.

If there's a better way of doing the 1st patch then I'm happy to drop
the 2nd patch.  :-)

Personally, I don't understand what people have against global
variables.  If 90% of the functions need a "struct ctdb_context" that
is never freed during the lifetime of the daemon, then you've basically
got a global variable anyway...  ;-)

peace & happiness,
martin