[CTDB] Some patches

[Martin Schwenke]

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...  :-)

peace & happiness,
martin