Possible bug in ctdb

[Martin Schwenke]

On Fri, 26 Feb 2021 17:22:56 +1100, Amitay Isaacs <amitay at gmail.com>
wrote:

> On Fri, Feb 26, 2021 at 2:57 PM Martin Schwenke <martin at meltin.net> wrote:

> > On Thu, 25 Feb 2021 13:05:52 +0100, Ralph Boehme <slow at samba.org> wrote:
> >  
> > > I noticed the following if condition in check_static_boolean_change()
> > > (defined twice):
> > >
> > >      if (mode == CONF_MODE_RELOAD || CONF_MODE_API)
> > >
> > > This will always evaluate to true, I guess that is not intended and the
> > > fix would be
> > >
> > >      if (mode == CONF_MODE_RELOAD || mode == CONF_MODE_API)
> > >
> > > (WIP attached, lacking bugnumber).  
> >
> > Yes, obviously a bug.  My bug.  Fix looks sane.
> >
> > Note that this code is actually a no-op and it just logs a warning.
> > CTDB doesn't currently support reloading the configuration at run
> > time... but the config system does. When reloading is implemented it
> > will flag that after a config reload we don't look at the new value of
> > the variable that points to that config value, so there is no change the
> > daemon's behaviour even if that config setting is changed.  There are
> > just some things that you can't (or don't want to ;-) change at
> > run-time.
> >
> > For consistency I'd almost like to see that condition coded as:
> >
> >   if (conf_maybe_updating(mode)) {
> >
> > although perhaps we should just write it as:
> >
> >   if (mode != CONF_MODE_LOAD) {
> >
> > since that catches the other cases consistently.
> >
> > Let's see what Amitay says.  :-)
> >  
> 
> Let's do mode != CONF_MODE_LOAD...

Sold!

Ralph, do you want to update your patch?  In addition to fixing that,
it is probably worth changing other similar checks (which currently
just check for CONF_MODE_RELOAD).  If not, then I'm happy to do it.

I have some patches somewhere to move all of the config stuff into a
single directory.  Amitay and I decided that dispersing the config
stuff doesn't make total sense because options don't necessarily map to
subsystems - some affect overall behaviour.  So we'll be able to get
rid of some of the duplication later on too.

Since it doesn't have a real impact, we probably don't need to backport
it.  If we can see a misleading message in a log somewhere (e.g.
systemd journal) then it *might* be worth backporting...

Sound good?

peace & happiness,
martin