[CTDB] Some patches
sbose at redhat.com
Wed Jun 12 05:14:41 MDT 2013
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>
> > > > 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
> * 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
> * 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.
> peace & happiness,
More information about the samba-technical