[CTDB] Some patches

[Sumit Bose]

On Wed, Jun 12, 2013 at 08:40:20PM +1000, Martin Schwenke wrote:
> On Fri, 7 Jun 2013 18:26:23 +1000, Amitay Isaacs <amitay at gmail.com>
> wrote:
> 
> > > > On Fri, Jun 7, 2013 at 6:54 AM, Mathieu Parent <math.parent at gmail.com
> 
> > > > > There are also patches on the Fedora side:
> > > > > http://pkgs.fedoraproject.org/cgit/ctdb.git/tree/, please take a look.
> 
> > Martin, can you look at the patches for init script and systemd changes?
> 
> Most of the initscript patches look good, but need some work to get
> them to apply against the master branch.
> 
> * 0001-Extract-some-init-functions-into-a-separate-file.patch
> 
>   This one is completely OK.
> 
>   The only thing to consider is whether we want to redo this as a much
>   smaller patch after we move the TDB checks into the "init" event in
>   config/events.d/00.ctdb...  if we decide to go ahead with that.
>   Amitay and I have discussed doing this because it would make the
>   initscript much simpler to understand.  This checking is really
>   internal to CTDB and doesn't need to be in the initscript.
> 
>   It is now possible to move the TDB checks to the "init" event
>   without causing confusion because the initscript now waits until
>   CTDB has successfully passed through the "init" and "setup" events
>   before declaring success.  The initscript now will fail if CTDB
>   exits due to either of those events failing.  The justification for
>   doing this was due to complaints that the initscript would succeed
>   but milliseconds later the "setup" event would fail, so ctdbd would
>   not be running as advertised by the initscript.
> 
>   We can similarly move the call to drop_all_public_ips().  That
>   function has been moved to the functions file anyway so that it can
>   be called from ctdb-crash-cleanup.sh.  We can just call it from
>   00.ctdb::init.
> 
> * 0002-Add-systemd-support.patch
> 
>   I'm a little confused by the churn in config/init_functions.  This
>   patch seems to include:
> 
>   - Some whitespace fixes, possibly needed due to mangling in the
>     previous patch.
> 
>   - Some more recent improvements incorporated from the master
>     branch.  I think they would be better in a separate patch, although
>     if we're applying this patch on top of master then I guess these
>     changes will go away anyway...  :-)
> 
>   I think this patch should be minimised to just add the systemd
>   support.
> 
> * clean-up-systemd-integration.patch
> 
>   2) Removes the extraneous single quotes around the values in
>      build_ctdb_options(). ctdbd doesn't know how to strip those out,
>      so they shouldn't be added at all.
> 
>   This potentially breaks the initscript if any of the variables are
>   assigned values containing spaces.  See
>   9a9b36149042d4d8f455959582ec5c882162266a.  There are a couple of
>   variables in ctdb.sysconfig that might have spaces.  Adding the
>   quotes does mean that the arguments have to be passed to ctdbd under
>   a call to eval.  I'm not sure if this is the done thing in a systemd
>   script but I think that's what needs to happen.
> 
>   3) removes the bogus PIDFile= line from ctdb.service. systemd
>      expects the daemon to create that file, but ctdb doesn't do so.
> 
>   ctdbd now creates a PID file, so we should take this out and set
>   this up correctly.
> 
> * ctdb-no_default_runlevel.patch
> 
>   I don't understand why this is done.  However, there are many things
>   about initscript magic for different distributions that I don't
>   understand.  :-)
> 
> I'm happy to do the rework.  Alternatively, I can do the move of TDB
> checking and IP dropping to 00.ctdb::init and then ask Sumit to redo
> the patches on top of that.
> 
> Whatever works...  :-)

Thank you for checking the patches. I would prefer if as many checks and
tasks as possible can be moved to 00.ctdb::init. I think this will help
to make supporting the two init systems much easier. Please ping me,
when the changes are done so that I can redo the patches.

bye,
Sumit

> 
> peace & happiness,
> martin