[Cluster-devel] fenced logsys/cman/ccs setup

[Fabio M. Di Nitto]

Hi David,

I didn't test the patch directly, but I have a few comments.

> index 56b7b0e..f2a8372 100644
> --- a/fence/fenced/config.c
> +++ b/fence/fenced/config.c
> @@ -1,7 +1,9 @@
> #include "fd.h"
> #include "ccs.h"
>
> -static int open_ccs(void)
> +static int ccs_handle;
> +
> +int setup_ccs(void)
> {
> 	int i = 0, cd;
>
> @@ -9,18 +11,62 @@ static int open_ccs(void)
> 		sleep(1);
> 		if (++i > 9 && !(i % 10))
> 			log_error("connect to ccs error %d, "
> -				  "check ccsd or cluster status", cd);
> +				  "check cluster status", cd);
> +
> +		/* FIXME: do we want this infinite? */
> +		if (i > 10)
> +			break;

I think we want this to be infinite (and consistent across the board) or 
configurable the same way other daemons/tools do so that users can decide 
how long to wait.

> @@ -122,7 +165,6 @@ int read_ccs(struct fd *fd)
>
> 	log_debug("added %d nodes from ccs", count);
>  out:
> -	ccs_disconnect(cd);
> 	return 0;
> }

I don't see any call to ccs_disconnect around. We still need to invoke it 
on exit to close the connection to the objdb when we exit and release 
aisexec resources.

> +#define DEFAULT_FACILITY	LOG_DAEMON

You can either do:

#define DEFAULT_FACILITY SYSLOGFACILITY

or just use SYSLOGFACILITY instead.

SYSLOGFACILITY is defined by the build system as default build option and 
can be set by packagers according to distro rules/best practice/etc.

> +#define DEFAULT_PRIORITY	LOG_LEVEL_ERROR

I don't have a DEFAULT_PRIORITY in the build system (maybe i can add it) 
but it should be LOG_LEVEL_INFO. At least this is the best practise we 
used so far.

> +#define DEFAULT_FILE		NULL

#define DEFAULT_FILE LOGDIR "/fenced.log"

LOGDIR is set by the build system (same reasons as SYSLOGFACILITY). We 
want files by default consistently across the board.

> +#define LEVEL_PATH "/cluster/logging/logger_subsys[@subsys=\"FENCED\"]/@syslog_level"
> +#define DEBUG_PATH "/cluster/logging/logger_subsys[@subsys=\"FENCED\"]/@debug"

I am kind of curious.. why do you add defines for some queries but not for 
others?

> +
> +/* Read cluster.conf settings and convert them into logsys values.
> +   If no cluster.conf setting exists, the default that was used in
> +   logsys_init() is used.
> +
> +   mode from
> +   "/cluster/logging/@to_stderr"
> +   "/cluster/logging/@to_syslog"
> +   "/cluster/logging/@to_file"
> +
> +   facility from
> +   "/cluster/logging/@syslog_facility"
> +
> +   priority from
> +   "/cluster/logging/logger_subsys[@subsys=\"prog_name\"]/@syslog_level"
> +
> +   file from
> +   "/cluster/logging/@filename"
> +
> +   debug from
> +   "/cluster/logging/logger_subsys[@subsys=\"prog_name\"]/@debug"
> +*/

This logic is almost ok.

You need to check for:

/cluster/logging/@debug - global debug on/off switch.

the other daemons respects globla debug setting if no subsystem debug 
setting is available.

You also want to allow debugging to be set via cmdline and envvar.

This allows a great control of debugging.

The use cases are:

- set /cluster/logging/@debug to on:
   all daemons across the whole cluster are in debug mode (and logging
   debug info). You don't need to add X subsystem lines to achieve this.

- /cluster/logging/logger_subsys[@subsys=\"prog_name\"]/@debug:
   users either want to debug only this system or disable debugging only
   for this subsystem across the whole cluster when master debug is on.

- cmdline debug:
   allows the user to enable debugging manually only on that specific
   daemon on that specific node.

- envvar debug on (see for example CMAN_DEBUG or CCSD_DEBUG or QDISK_...):
   these are easy to set within init scripts and /etc/defaults/... without
   changing the daemon invokation in the init script itself that is not
   always trivial and it is not a recommended practise.
   the use case is similar (but more flexible) to the cmdline debug.
   It allows the users to enable debugging for one specific node.

cmdline and envvar debugging overrides whatever is in the config.

Fabio

--
I'm going to make him an offer he can't refuse.