[CTDB] Some patches
Martin Schwenke
martin at meltin.net
Wed Jun 12 04:40:20 MDT 2013
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
More information about the samba-technical
mailing list