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

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

[David Teigland]

Here's a patch for review that changes fenced to use the logsys api I
posted earlier.  It also includes some other minor changes related to ccs
and cman setup.  Untested, and can't be commited until the logsys api is
official.


commit 1ccaf002a5989a22cb14fde4dfc3d13260f6316f
Author: David Teigland <teigland@redhat.com>
Date:   Wed Jun 25 15:56:34 2008 -0500

    fenced: use logsys
    
    - Setup ccs connection once at the start and keep it open.
    - Read logging configuration from ccs.
    - Replace calls to syslog with calls to logsys.
    - Direct debug statements to logsys.
    - cman setup uses cman_is_active
    - cman setup retries cman_init and cman_is_active
    
    Signed-off-by: David Teigland <teigland@redhat.com>

diff --git a/fence/fenced/Makefile b/fence/fenced/Makefile
index 1e9bbc9..61ec989 100644
--- a/fence/fenced/Makefile
+++ b/fence/fenced/Makefile
@@ -15,7 +15,8 @@ OBJS=	config.o \
 	group.o \
 	main.o \
 	member_cman.o \
-	recover.o
+	recover.o \
+	logging.o
 
 CFLAGS += -D_FILE_OFFSET_BITS=64
 CFLAGS += -I${ccsincdir} -I${cmanincdir} -I${fenceincdir} -I${openaisincdir}
diff --git a/fence/fenced/config.c b/fence/fenced/config.c
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;
 	}
-	return cd;
+
+	if (cd < 0)
+		return cd;
+
+	ccs_handle = cd;
+	return 0;
+}
+
+void read_ccs_name(char *path, char *name)
+{
+	char *str;
+	int error;
+
+	error = ccs_get(ccs_handle, path, &str);
+	if (error || !str)
+		return;
+
+	strcpy(name, str);
+
+	free(str);
+}
+
+void read_ccs_yesno(char *path, int *yes, int *no)
+{
+	char *str;
+	int error;
+
+	*yes = 0;
+	*no = 0;
+
+	error = ccs_get(ccs_handle, path, &str);
+	if (error || !str)
+		return;
+
+	if (!strcmp(str, "yes"))
+		*yes = 1;
+
+	else if (!strcmp(str, "no"))
+		*no = 1;
+
+	free(str);
 }
 
-static void read_ccs_int(int cd, char *path, int *config_val)
+void read_ccs_int(char *path, int *config_val)
 {
 	char *str;
 	int val;
 	int error;
 
-	error = ccs_get(cd, path, &str);
+	error = ccs_get(ccs_handle, path, &str);
 	if (error || !str)
 		return;
 
@@ -48,11 +94,8 @@ int read_ccs(struct fd *fd)
 {
 	char path[256];
 	char *str;
-	int error, cd, i = 0, count = 0;
-
-	cd = open_ccs();
-	if (cd < 0)
-		return cd;
+	int error, i = 0, count = 0;
+	int cd = ccs_handle;
 
 	/* Our own nodename must be in cluster.conf before we're allowed to
 	   join the fence domain and then mount gfs; other nodes need this to
@@ -122,7 +165,6 @@ int read_ccs(struct fd *fd)
 
 	log_debug("added %d nodes from ccs", count);
  out:
-	ccs_disconnect(cd);
 	return 0;
 }
 
diff --git a/fence/fenced/fd.h b/fence/fenced/fd.h
index 5ef1756..c74da54 100644
--- a/fence/fenced/fd.h
+++ b/fence/fenced/fd.h
@@ -10,7 +10,6 @@
 #include <errno.h>
 #include <string.h>
 #include <stdint.h>
-#include <syslog.h>
 #include <time.h>
 #include <sched.h>
 #include <sys/ioctl.h>
@@ -24,6 +23,7 @@
 
 #include <openais/saAis.h>
 #include <openais/cpg.h>
+#include <openais/service/logsys.h>
 
 #include "list.h"
 #include "linux_endian.h"
@@ -58,6 +58,7 @@
 #define GROUP_LIBCPG            3
 
 extern int daemon_debug_opt;
+extern int daemon_debug_logsys;
 extern int daemon_quit;
 extern struct list_head domains;
 extern int cman_quorate;
@@ -74,14 +75,17 @@ extern void daemon_dump_save(void);
 #define log_debug(fmt, args...) \
 do { \
 	snprintf(daemon_debug_buf, 255, "%ld " fmt "\n", time(NULL), ##args); \
-	if (daemon_debug_opt) fprintf(stderr, "%s", daemon_debug_buf); \
 	daemon_dump_save(); \
+	if (daemon_debug_opt) \
+		fprintf(stderr, "%s", daemon_debug_buf); \
+	if (daemon_debug_logsys) \
+		log_printf(LOG_DEBUG, "%s", daemon_debug_buf); \
 } while (0)
 
 #define log_error(fmt, args...) \
 do { \
 	log_debug(fmt, ##args); \
-	syslog(LOG_ERR, fmt, ##args); \
+	log_printf(LOG_ERR, fmt, ##args); \
 } while (0)
 
 /* config option defaults */
@@ -210,6 +214,10 @@ struct fd {
 
 /* config.c */
 
+int setup_ccs(void);
+void read_ccs_name(char *path, char *name);
+void read_ccs_yesno(char *path, int *yes, int *no);
+void read_ccs_int(char *path, int *config_val);
 int read_ccs(struct fd *fd);
 
 /* cpg.c */
@@ -266,5 +274,10 @@ void delay_fencing(struct fd *fd, int node_join);
 void defer_fencing(struct fd *fd);
 void fence_victims(struct fd *fd);
 
+/* logging.c */
+
+int init_logging(void);
+int setup_logging(int *prog_debug);
+
 #endif				/*  __FD_DOT_H__  */
 
diff --git a/fence/fenced/logging.c b/fence/fenced/logging.c
new file mode 100644
index 0000000..6d09839
--- /dev/null
+++ b/fence/fenced/logging.c
@@ -0,0 +1,153 @@
+#include "fd.h"
+
+#define DEFAULT_MODE		LOG_MODE_OUTPUT_SYSLOG_THREADED
+#define DEFAULT_FACILITY	LOG_DAEMON
+#define DEFAULT_PRIORITY	LOG_LEVEL_ERROR
+#define DEFAULT_FILE		NULL
+
+#define LEVEL_PATH "/cluster/logging/logger_subsys[@subsys=\"FENCED\"]/@syslog_level"
+#define DEBUG_PATH "/cluster/logging/logger_subsys[@subsys=\"FENCED\"]/@debug"
+
+/* 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"
+*/
+
+static int read_ccs_logging(int *mode, int *facility, int *priority, char *file,
+			    int *debug)
+{
+	char name[PATH_MAX];
+	int val, y, n;
+	int m = 0, f = 0, p = 0;
+
+	/*
+	 * defaults
+	 */
+
+	*mode = DEFAULT_MODE;
+	*facility = DEFAULT_FACILITY;
+	*priority = DEFAULT_PRIORITY;
+	if (DEFAULT_FILE)
+		strcpy(file, DEFAULT_FILE);
+
+	/*
+	 * mode
+	 */
+
+	read_ccs_yesno("/cluster/logging/@to_stderr", &y, &n);
+	if (y)
+		m |= LOG_MODE_OUTPUT_STDERR;
+
+	read_ccs_yesno("/cluster/logging/@to_syslog", &y, &n);
+	if (y)
+		m |= LOG_MODE_OUTPUT_SYSLOG_THREADED;
+
+	read_ccs_yesno("/cluster/logging/@to_file", &y, &n);
+	if (y)
+		m |= LOG_MODE_OUTPUT_FILE;
+
+	if (!m)
+		goto facil;
+
+	*mode = m;
+
+	/*
+	 * facility
+	 */
+ facil:
+	memset(name, 0, sizeof(name));
+	read_ccs_name("/cluster/logging/@syslog_facility", name);
+
+	if (!name[0])
+		goto prior;
+
+	f = logsys_facility_get(name);
+	if (f < 0)
+		goto prior;
+
+	*facility = f;
+
+	/*
+	 * priority
+	 */
+ prior:
+	read_ccs_int(LEVEL_PATH, &val);
+	if (!val)
+		goto dofile;
+
+	p = logsys_priority_id_get(val);
+	if (p < 0)
+		goto dofile;
+
+	*priority = p;
+
+	/*
+	 * file
+	 */
+ dofile:
+	memset(name, 0, sizeof(name));
+	read_ccs_name("/cluster/logging/@filename", name);
+
+	if (!name[0])
+		goto deb;
+
+	strcpy(file, name);
+
+	/*
+	 * debug
+	 */
+  deb:
+	memset(name, 0, sizeof(name));
+	read_ccs_name("/cluster/logging/@debug", name);
+
+	if (!strcmp(name, "on"))
+		*debug = 1;
+
+	memset(name, 0, sizeof(name));
+	read_ccs_name(DEBUG_PATH, name);
+
+	if (!strcmp(name, "on"))
+		*debug = 1;
+
+	return 0;
+}
+
+/* initial settings until we can read cluster.conf logging settings from ccs */
+
+void init_logging(void)
+{
+	logsys_init("fenced", LOG_MODE_OUTPUT_SYSLOG_THREADED, LOG_DAEMON,
+		    LOG_LEVEL_ERROR, NULL);
+}
+
+/* this function is also called when we get a cman config-update event */
+
+void setup_logging(int *prog_debug)
+{
+	int mode, facility, priority;
+	char file[PATH_MAX];
+
+	/* The debug setting is special, it's used by the program
+	   and not used to configure logsys. */
+
+	read_ccs_logging(&mode, &facility, &priority, file, prog_debug);
+	logsys_conf("fenced", mode, facility, priority, file);
+}
+
diff --git a/fence/fenced/main.c b/fence/fenced/main.c
index 83ca075..0a8c10c 100644
--- a/fence/fenced/main.c
+++ b/fence/fenced/main.c
@@ -619,6 +619,12 @@ static int loop(void)
 		goto out;
 	client_add(rv, process_cman, cluster_dead);
 
+	rv = setup_ccs();
+	if (rv < 0)
+		goto out;
+
+	setup_logging(&daemon_debug_logsys);
+
 	group_mode = GROUP_LIBCPG;
 
 	if (comline.groupd_compat) {
@@ -852,6 +858,8 @@ int main(int argc, char **argv)
 	comline.override_time = DEFAULT_OVERRIDE_TIME;
 	comline.override_path = strdup(DEFAULT_OVERRIDE_PATH);
 
+	init_logging();
+
 	read_arguments(argc, argv);
 
 	lockfile();
@@ -863,7 +871,6 @@ int main(int argc, char **argv)
 		}
 		umask(0);
 	}
-	openlog("fenced", LOG_PID, LOG_DAEMON);
 	signal(SIGTERM, sigterm_handler);
 
 	set_oom_adj(-16);
@@ -888,6 +895,7 @@ void daemon_dump_save(void)
 }
 
 int daemon_debug_opt;
+int daemon_debug_logsys;
 int daemon_quit;
 struct list_head domains;
 int cman_quorate;
diff --git a/fence/fenced/member_cman.c b/fence/fenced/member_cman.c
index e85fcb6..1cc409a 100644
--- a/fence/fenced/member_cman.c
+++ b/fence/fenced/member_cman.c
@@ -122,6 +122,10 @@ static void cman_callback(cman_handle_t h, void *private, int reason, int arg)
 		if (!quorate && cman_quorate && (group_mode == GROUP_LIBCPG))
 			process_fd_changes();
 		break;
+
+	case CMAN_REASON_CONFIG_UPDATE:
+		setup_logging(&daemon_debug_logsys);
+		break;
 	}
 }
 
@@ -140,10 +144,28 @@ int setup_cman(void)
 {
 	cman_node_t node;
 	int rv, fd;
+	int init = 0, active = 0;
 
+ retry_init:
 	ch = cman_init(NULL);
 	if (!ch) {
-		log_error("cman_init error %p %d", ch, errno);
+		if (init++ < 2) {
+			sleep(1);
+			goto retry_init;
+		}
+		log_error("cman_init error %d", errno);
+		return -ENOTCONN;
+	}
+
+ retry_active:
+	rv = cman_is_active(ch);
+	if (!rv) {
+		if (active++ < 2) {
+			sleep(1);
+			goto retry_active;
+		}
+		log_error("cman_is_active error %d", errno);
+		cman_finish(ch);
 		return -ENOTCONN;
 	}
 
diff --git a/fence/fenced/recover.c b/fence/fenced/recover.c
index 21bf735..e3bf7e1 100644
--- a/fence/fenced/recover.c
+++ b/fence/fenced/recover.c
@@ -116,7 +116,7 @@ static int check_override(int ofd, char *nodename, int timeout)
 
 	ret = select(ofd + 1, &rfds, NULL, NULL, &tv);
 	if (ret < 0) {
-		syslog(LOG_ERR, "select: %s\n", strerror(errno));
+		log_printf(LOG_ERR, "select: %s\n", strerror(errno));
 		return -1;
 	}
 
@@ -126,7 +126,7 @@ static int check_override(int ofd, char *nodename, int timeout)
 	memset(buf, 0, sizeof(buf));
 	ret = read(ofd, buf, sizeof(buf) - 1);
 	if (ret < 0) {
-		syslog(LOG_ERR, "read: %s\n", strerror(errno));
+		log_printf(LOG_ERR, "read: %s\n", strerror(errno));
 		return -1;
 	}
 
@@ -212,8 +212,8 @@ void delay_fencing(struct fd *fd, int node_join)
 		  (int) (last.tv_sec - first.tv_sec), victim_count);
  out:
 	list_for_each_entry(node, &fd->victims, list) {
-		syslog(LOG_INFO, "%s not a cluster member after %d sec %s",
-		       node->name, delay, delay_type);
+		log_printf(LOG_INFO, "%s not a cluster member after %d sec %s",
+		           node->name, delay, delay_type);
 	}
 }
 
@@ -227,7 +227,7 @@ void defer_fencing(struct fd *fd)
 	master_name = nodeid_to_name(fd->master);
 
 	log_debug("defer fencing to %d %s", fd->master, master_name);
-	syslog(LOG_INFO, "fencing deferred to %s", master_name);
+	log_printf(LOG_INFO, "fencing deferred to %s", master_name);
 }
 
 void fence_victims(struct fd *fd)
@@ -258,13 +258,13 @@ void fence_victims(struct fd *fd)
 		}
 
 		log_debug("fencing node %s", node->name);
-		syslog(LOG_INFO, "fencing node \"%s\"", node->name);
+		log_printf(LOG_INFO, "fencing node \"%s\"", node->name);
 
 		query_unlock();
 		error = fence_node(node->name);
 		query_lock();
 
-		syslog(LOG_INFO, "fence \"%s\" %s", node->name,
+		log_printf(LOG_INFO, "fence \"%s\" %s", node->name,
 		       error ? "failed" : "success");
 
 		if (!error) {
@@ -286,8 +286,8 @@ void fence_victims(struct fd *fd)
 		override = open_override(comline.override_path);
 		if (check_override(override, node->name,
 				   comline.override_time) > 0) {
-			syslog(LOG_WARNING, "fence \"%s\" overridden by "
-			       "administrator intervention", node->name);
+			log_printf(LOG_WARNING, "fence \"%s\" overridden by "
+				   "administrator intervention", node->name);
 			victim_done(fd, node->nodeid, VIC_DONE_OVERRIDE);
 			list_del(&node->list);
 			free(node);

[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.

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

[David Teigland]

On Wed, Jul 02, 2008 at 06:31:10AM +0200, Fabio M. Di Nitto wrote:
> >@@ -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.

OK, I think the following logic supports that:

- During startup, a program first connects to cman and waits for it to be
  ready (this should be a consistent and finite retry period).

- Then the program connects to ccs.  I think we can assume that since cman
  is running, the ccs connection will work within a short time.  So,
  it's safe to just put an infinite loop around the ccs_connect.


> >@@ -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.

OK, I need to rework the exit path for the daemon to close/cleanup things.
I've just been lazy in the past because an exit will almost always clean
everything up automatically anyway.


> >+#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.

OK, that works for fenced.  What about things that aren't a daemon,
though?  Perhaps they'd like a different default?


> >+#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.

OK, just to be clear, this setting is used:
- during startup before reading the user preferences in cluster.conf
- during running if the user has set no preference in cluster.conf

and the defaults should assume that the majority of people will never add
logging preferences to cluster.conf.  (Both because we picked good
defaults, and because they're lazy to look up and write the xml.)


> >+#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.

I think that by default we should probably have all logging go to
/var/log/messages like it has in the past.  Or, if we're daring, maybe
have it all go to /var/log/cluster.log.  But, I really doubt that people
want all programs to log to different files.


> >+#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?

I'm still in limbo about what coding style I like there; in this case it's
if the line goes over 80 chars.


> >+/* 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.

OK, so subsystem setting has priority over global setting.


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

ok

> 
> - /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.

ok

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

This is taking it too far for me (overengineering).  You'll need a new
debugging system to debug your complicated debugging system.

There two debug schemes:

1. for users/customers:  uses logsys, configured in cluster.conf
2. for me:  does not use logsys, controlled via command line

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

[David Teigland]

On Wed, Jul 02, 2008 at 10:49:05AM -0500, David Teigland wrote:
> > #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.
> 
> I think that by default we should probably have all logging go to
> /var/log/messages like it has in the past.  Or, if we're daring, maybe
> have it all go to /var/log/cluster.log.  But, I really doubt that people
> want all programs to log to different files.

Thinking more about this, debug logs would definately make sense in
per-program log files like fenced.log.  I was thinking about the error
messages we're switching from syslog to logsys.  So, how do we tell logsys
to use /var/log/messages for errors and a separate file for debug output?

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

[Fabio M. Di Nitto]

On Wed, 2 Jul 2008, David Teigland wrote:

> On Wed, Jul 02, 2008 at 06:31:10AM +0200, Fabio M. Di Nitto wrote:
>>> @@ -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.
>
> OK, I think the following logic supports that:
>
> - During startup, a program first connects to cman and waits for it to be
>  ready (this should be a consistent and finite retry period).

Aggreed. Some daemons get a timeout setting from the tools and stuff like 
that. We can review that after we start standardizing around.

> - Then the program connects to ccs.  I think we can assume that since cman
>  is running, the ccs connection will work within a short time.  So,
>  it's safe to just put an infinite loop around the ccs_connect.

If we loop in cman and wait for is_available, i doubt there is any need to 
loop in ccs at all. ccs plugin is initialized before cman within the 
aisexec execution path.

>>> +#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.
>
> OK, that works for fenced.  What about things that aren't a daemon,
> though?  Perhaps they'd like a different default?

This is a build time setting that's usually set by the distro. I don't see 
the use case to have them logging at different facility.

>
>>> +#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.
>
> OK, just to be clear, this setting is used:
> - during startup before reading the user preferences in cluster.conf
> - during running if the user has set no preference in cluster.conf

This is right.

>
> and the defaults should assume that the majority of people will never add
> logging preferences to cluster.conf.  (Both because we picked good
> defaults, and because they're lazy to look up and write the xml.)

Indeed. So for now let's set it to LOG_LEVEL_INFO and I will make it a 
build option for the user to decide what is the built-in default.

>
>>> +#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.
>
> I think that by default we should probably have all logging go to
> /var/log/messages like it has in the past.  Or, if we're daring, maybe
> have it all go to /var/log/cluster.log.  But, I really doubt that people
> want all programs to log to different files.

I will answer this in the other email.

>>> +#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?
>
> I'm still in limbo about what coding style I like there; in this case it's
> if the line goes over 80 chars.

ehe 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.
>
> OK, so subsystem setting has priority over global setting.

Exactly.

>>
>> - 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.
>
> This is taking it too far for me (overengineering).  You'll need a new
> debugging system to debug your complicated debugging system.

It is much simpler than you think, really. If debug is set by cmdline (the 
way you usually do it as developer) or envvar (very useful for init 
scripts that we never use directly, but production users do), you don't 
even need to query ccs for those info :)

> There two debug schemes:
>
> 1. for users/customers:  uses logsys, configured in cluster.conf

^^^ this is always cluster wide. Doesn't allow you to do debugging on a 
single node.

> 2. for me:  does not use logsys, controlled via command line

2 doesn't exist really. It's for "us" all developers ;) and some of us 
would like to use logsys.

Fabio

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

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

[Fabio M. Di Nitto]

On Wed, 2 Jul 2008, David Teigland wrote:

> On Wed, Jul 02, 2008 at 10:49:05AM -0500, David Teigland wrote:
>>> #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.
>>
>> I think that by default we should probably have all logging go to
>> /var/log/messages like it has in the past.  Or, if we're daring, maybe
>> have it all go to /var/log/cluster.log.  But, I really doubt that people
>> want all programs to log to different files.
>
> Thinking more about this, debug logs would definately make sense in
> per-program log files like fenced.log.

Right :)

> I was thinking about the error
> messages we're switching from syslog to logsys.  So, how do we tell logsys
> to use /var/log/messages for errors and a separate file for debug output?

Why do you need this? Remember that you are setting a syslog facility. 
syslog doesn't necessarely send info locally and a certain facility can be 
directed to a specific file (even remote).

The normal /var/log/cluster/fenced.log will collect everything locally (if 
enabled).

Fabio

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

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

[David Teigland]

On Wed, Jul 02, 2008 at 08:22:54PM +0200, Fabio M. Di Nitto wrote:
> >I was thinking about the error
> >messages we're switching from syslog to logsys.  So, how do we tell logsys
> >to use /var/log/messages for errors and a separate file for debug output?
> 
> Why do you need this? Remember that you are setting a syslog facility. 
> syslog doesn't necessarely send info locally and a certain facility can be 
> directed to a specific file (even remote).
> 
> The normal /var/log/cluster/fenced.log will collect everything locally (if 
> enabled).

OK, I wasn't properly thinking about the modes.

Because our default mode is LOG_MODE_OUTPUT_SYSLOG_THREADED, the file
won't be used by default, and error messages will still go to
/var/log/messages (by logsys using syslog).

If someone edits cluster.conf and just adds cluster/logging/to_file="yes",
without setting cluster/logging/filename, then all log_printf's will go to
the default filename (in addition to other places if other modes are set).
This is the only occasion when the default filename will be used.

Next, to verify how setting/unsetting the modes works.

By default we have LOG_MODE_OUTPUT_SYSLOG_THREADED.
- errors go to /var/log/messages
- debug goes nowhere
- (if debug was set, debug would go to /var/log/messages)

If someone sets to_file="yes", it adds LOG_MODE_OUTPUT_FILE, resulting
in LOG_MODE_OUTPUT_SYSLOG_THREADED | LOG_MODE_OUTPUT_FILE.
- errors go to both /var/log/messages and /var/log/fenced.log
- debug goes nowhere
- (if debug was set, debug would go to both /var/log/messages and
  /var/log/fenced.log)

If someone sets to_file="yes" to_syslog="no", it adds LOG_MODE_OUTPUT_FILE,
removes LOG_MODE_OUTPUT_SYSLOG_THREADED, leaving just LOG_MODE_OUTPUT_FILE.
- errors go to /var/log/fenced.log
- debug goes nowhere
- (if debug was set, debug would go to /var/log/fenced.log)


Say there's a normal setup where no logging is configured, and errors are
going to /var/log/messages.  Then a person wants to turn on debugging and
collect the debug messages in /var/log/fenced.log.  What combination of
to_file, to_syslog, filename, and debug will allow that?  I can't find
any, and I think that would be one of the most common things people would
want.

I don't want to call all this confusing, because I can't offer any better
suggestions, but...  (And we haven't added command line options or
environment variables to the picture yet.)

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

[Fabio M. Di Nitto]

On Wed, 2 Jul 2008, David Teigland wrote:

> On Wed, Jul 02, 2008 at 08:22:54PM +0200, Fabio M. Di Nitto wrote:
>>> I was thinking about the error
>>> messages we're switching from syslog to logsys.  So, how do we tell logsys
>>> to use /var/log/messages for errors and a separate file for debug output?
>>
>> Why do you need this? Remember that you are setting a syslog facility.
>> syslog doesn't necessarely send info locally and a certain facility can be
>> directed to a specific file (even remote).
>>
>> The normal /var/log/cluster/fenced.log will collect everything locally (if
>> enabled).
>
> OK, I wasn't properly thinking about the modes.
>
> Because our default mode is LOG_MODE_OUTPUT_SYSLOG_THREADED, the file
> won't be used by default, and error messages will still go to
> /var/log/messages (by logsys using syslog).

The defaults in the other daemons are LOG_MODE_OUTPUT_SYSLOG_THREADED | 
LOG_MODE_OUTPUT_FILE | LOG_MODE_OUTPUT_STDERR. Better catch them all than 
none. I might have missed the email where you said that 
LOG_MODE_OUTPUT_SYSLOG_THREADED should be the only default, but i think 
it's better to spit out a few more messages if we can't configure 
according to user whishes than hiding.

> If someone edits cluster.conf and just adds cluster/logging/to_file="yes",
> without setting cluster/logging/filename, then all log_printf's will go to
> the default filename (in addition to other places if other modes are set).
> This is the only occasion when the default filename will be used.

Not if we use the built-in defaults like i did for the other bits.

> Next, to verify how setting/unsetting the modes works.
>
> By default we have LOG_MODE_OUTPUT_SYSLOG_THREADED.
> - errors go to /var/log/messages
> - debug goes nowhere
> - (if debug was set, debug would go to /var/log/messages)

it might not be /var/log/messages. This is very distro specific.

> If someone sets to_file="yes", it adds LOG_MODE_OUTPUT_FILE, resulting
> in LOG_MODE_OUTPUT_SYSLOG_THREADED | LOG_MODE_OUTPUT_FILE.
> - errors go to both /var/log/messages and /var/log/fenced.log
> - debug goes nowhere
> - (if debug was set, debug would go to both /var/log/messages and
>  /var/log/fenced.log)

Right.

> If someone sets to_file="yes" to_syslog="no", it adds LOG_MODE_OUTPUT_FILE,
> removes LOG_MODE_OUTPUT_SYSLOG_THREADED, leaving just LOG_MODE_OUTPUT_FILE.
> - errors go to /var/log/fenced.log
> - debug goes nowhere
> - (if debug was set, debug would go to /var/log/fenced.log)

Right.

> Say there's a normal setup where no logging is configured, and errors are
> going to /var/log/messages.

Well this only make sense if by default we log only to syslog.

>  Then a person wants to turn on debugging and
> collect the debug messages in /var/log/fenced.log.  What combination of
> to_file, to_syslog, filename, and debug will allow that?  I can't find
> any, and I think that would be one of the most common things people would
> want.

When you enable debugging in other daemons, the messages go everywhere. 
Not just in one specific file.

> I don't want to call all this confusing, because I can't offer any better
> suggestions, but...

I think what you are trying to tell me is that you would like to see debug 
messages going to a specific file rather than to the default 
LOG_MODE_OUTPUT. If i understand you right, this is not an option right 
now and personally i don't see the need of it.

> (And we haven't added command line options or
> environment variables to the picture yet.)

I think you misunderstood this cmdlin + envvar thing. I am not saying we 
need to be able to configure _everything_.. not at all. Only the 
equivalent of debug=on/off.

something like this basically (very pseudo code):

parse_args()..
   -D debug = 1;

if (getenv(SYSTEM_DEBUG))
   debug = 1;

then later in get_logsys_config_data(int *debug)

if (!*debug) {
    check ccs etc...
}

very simple.. i didn't plan nor think to add all config options.. not at 
all... sorry if that was not clear from my side.

Fabio

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