ctdb-logging: New option CTDB_LOGGING, remove CTDB_LOGFILE, CTDB_SYSLOG
authorMartin Schwenke <martin@meltin.net>
Mon, 11 Aug 2014 07:07:41 +0000 (17:07 +1000)
committerAmitay Isaacs <amitay@samba.org>
Tue, 28 Oct 2014 04:42:04 +0000 (05:42 +0100)
Remove --logfile and --syslog daemon options and replace with
--logging.

Modularise and clean up logging initialisation code.  The
initialisation API includes an app_name argument that is currently
unused - this will be used in extensions to the syslog backend.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>
15 files changed:
ctdb/config/ctdb.sysconfig
ctdb/config/ctdbd_wrapper
ctdb/config/functions
ctdb/doc/ctdbd.1.xml
ctdb/doc/ctdbd.conf.5.xml
ctdb/doc/examples/cluster.conf
ctdb/doc/examples/natgw.conf
ctdb/include/ctdb_client.h
ctdb/include/ctdb_logging.h
ctdb/server/ctdb_logging.c
ctdb/server/ctdb_logging_file.c
ctdb/server/ctdb_logging_syslog.c
ctdb/server/ctdbd.c
ctdb/tests/eventscripts/scripts/local.sh
ctdb/tests/simple/scripts/local_daemons.bash

index db34c07ef06365f3629cb6c260046d12b937a0f5..b44e478b9377b05612b6005ab605bec973455147 100644 (file)
@@ -23,8 +23,7 @@ CTDB_PUBLIC_ADDRESSES=/etc/ctdb/public_addresses
 # ulimit -n 10000
 
 # Default is to use the log file below instead of syslog.
-# CTDB_LOGFILE=/var/log/log.ctdb
-# CTDB_SYSLOG=no
+# CTDB_LOGGING=file:/var/log/log.ctdb
 
 # Default log level is ERR.  NOTICE is a little more verbose.
 CTDB_DEBUGLEVEL=NOTICE
index c2c5c1a3e6a8a277c6dc26193bbae9e41885a2eb..331ee43ba0d44b7d1a755f3aa13bf9201e6413f7 100755 (executable)
@@ -111,7 +111,7 @@ build_ctdb_options ()
     maybe_set "--pidfile"                "$pidfile"
 
     # build up ctdb_options variable from optional parameters
-    maybe_set "--logfile"                "$CTDB_LOGFILE"
+    maybe_set "--logging"                "$CTDB_LOGGING"
     maybe_set "--nlist"                  "$CTDB_NODES"
     maybe_set "--socket"                 "$CTDB_SOCKET"
     maybe_set "--public-addresses"       "$CTDB_PUBLIC_ADDRESSES"
@@ -129,7 +129,6 @@ build_ctdb_options ()
     maybe_set "--no-lmaster"             "$CTDB_CAPABILITY_LMASTER"   "no"
     maybe_set "--lvs --single-public-ip" "$CTDB_LVS_PUBLIC_IP"
     maybe_set "--script-log-level"       "$CTDB_SCRIPT_LOG_LEVEL"
-    maybe_set "--syslog"                 "$CTDB_SYSLOG"               "yes"
     maybe_set "--max-persistent-check-errors" "$CTDB_MAX_PERSISTENT_CHECK_ERRORS"
 }
 
@@ -187,9 +186,14 @@ start()
        ctdb_options="${ctdb_options} --valgrinding"
     fi
 
-    if [ "$CTDB_SYSLOG" != "yes" ] ; then
-       logger -t ctdbd "CTDB is being run without syslog enabled.  Logs will be in ${CTDB_LOGFILE:-/var/log/log.ctdb}"
-    fi
+    case "$CTDB_LOGGING" in
+       syslog) : ;;
+       file:*)
+           logger -t ctdbd "CTDB is being run without syslog enabled.  Logs will be in ${CTDB_LOGGING#file:}"
+           ;;
+       *)
+           logger -t ctdbd "CTDB is being run without syslog enabled.  Logs will be in log.ctdb"
+    esac
 
     eval "$ctdbd" "$ctdb_options" || return 1
 
index 3f2ccee5ee50e7e646527e86c03185609bb1ec96..5c9497dc520a97ed69ba0660e611ddc778f57100 100755 (executable)
@@ -86,17 +86,25 @@ script_log ()
 {
     _tag="$1" ; shift
 
-    if [ "$CTDB_SYSLOG" = "yes" ] ; then
-       logger -t "ctdbd: ${_tag}" $*
-    else
-       {
-           if [ -n "$*" ] ; then
-               echo "$*"
+    case "$CTDB_LOGGING" in
+       file:*|"")
+           if [ -n "$CTDB_LOGGING" ] ; then
+               _file="${CTDB_LOGGING#file:}"
            else
-               cat
+               _file="/var/log/log.ctdb"
            fi
-       } >>"${CTDB_LOGFILE:-/var/log/log.ctdb}"
-    fi
+           {
+               if [ -n "$*" ] ; then
+                   echo "$*"
+               else
+                   cat
+               fi
+           } >>"$_file"
+           ;;
+       *)
+           logger -t "ctdbd: ${_tag}" $*
+           ;;
+    esac
 }
 
 # When things are run in the background in an eventscript then logging
index 20f2d8bb85b094148b19c7936cc2d77e71fe6a95..e700bf293ee8190da620fe7564fdee4b12abdb79 100644 (file)
        <listitem>
          <para>
            This option sets the debug level to DEBUGLEVEL, which
-           controls what will be written to the logfile. The default is
-           0 which will only log important events and errors. A larger
-           number will provide additional logging.
+           controls what will be written by the logging
+           subsystem. The default is 0 which will only log important
+           events and errors. A larger number will provide additional
+           logging.
          </para>
          <para>
            See the <citetitle>DEBUG LEVELS</citetitle> section in
       </varlistentry>
 
       <varlistentry>
-       <term>--logfile=<parameter>FILENAME</parameter></term>
+       <term>--logging=<parameter>STRING</parameter></term>
        <listitem>
          <para>
-           FILENAME where ctdbd will write its log. This is usually
-           <filename>/var/log/log.ctdb</filename>.
-         </para>
+           STRING specifies where ctdbd will write its log. The
+           default is file:<filename>/var/log/log.ctdb</filename> or
+           similar - the prefix may differ depending on how CTDB was
+           built.
+         </para>
+         <para>
+           Valid values are:
+         </para>
+         <variablelist>
+           <varlistentry>
+             <term>file:<parameter>FILENAME</parameter></term>
+             <listitem>
+               <para>
+                 FILENAME where ctdbd will write its log. This is usually
+                 <filename>/var/log/log.ctdb</filename>.
+               </para>
+             </listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>syslog</term>
+             <listitem>
+               <para>
+                 CTDB will log to syslog
+               </para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
        </listitem>
       </varlistentry>
 
index e316abb4124b5decaaf46cd6c727deb62b618340..898688dc12c9c11da489fe4354e37e3559c92a79 100644 (file)
       </varlistentry>
 
       <varlistentry>
-       <term>CTDB_LOGFILE=<parameter>FILENAME</parameter></term>
+       <term>CTDB_LOGGING=<parameter>STRING</parameter></term>
        <listitem>
          <para>
-           Defaults to <filename>/var/log/log.ctdb</filename>.
-           Corresponds to <option>--logfile</option>.  See also
-           <citetitle>CTDB_SYSLOG</citetitle>.
+           STRING specifies where ctdbd will write its log. The
+           default is file:<filename>/var/log/log.ctdb</filename> or
+           similar - the prefix may differ depending on how CTDB was
+           built.  Corresponds to <option>--logging</option>.
          </para>
+         <para>
+           Valid values are:
+         </para>
+         <variablelist>
+           <varlistentry>
+             <term>file:<parameter>FILENAME</parameter></term>
+             <listitem>
+               <para>
+                 FILENAME where ctdbd will write its log. This is usually
+                 <filename>/var/log/log.ctdb</filename>.
+               </para>
+             </listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>syslog</term>
+             <listitem>
+               <para>
+                 CTDB will log to syslog
+               </para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
        </listitem>
       </varlistentry>
 
        </listitem>
       </varlistentry>
 
-      <varlistentry>
-       <term>CTDB_SYSLOG=yes|no</term>
-       <listitem>
-         <para>
-           Default is no.  Corresponds to <option>--syslog</option>.
-         </para>
-       </listitem>
-      </varlistentry>
-
       <varlistentry>
        <term>CTDB_TRANSPORT=tcp|infiniband</term>
        <listitem>
index 871468e341bb88aed95f1e2b681324d49aa63332..8da982c53f243c076d6d76a7b881ee1f6457953b 100644 (file)
@@ -68,7 +68,7 @@ CTDB_NODES=/etc/ctdb/nodes
 CTDB_PUBLIC_ADDRESSES=/etc/ctdb/public_addresses
 
 # Enable logging to syslog
-CTDB_SYSLOG=yes
+CTDB_LOGGING=syslog
 
 # Default log level
 CTDB_DEBUGLEVEL=NOTICE
index 2e3a3ea702b42edc494d8758ff23154dc0367fd4..ce8f04113ef33d542b94d68b2a75cd811e44a5cd 100644 (file)
@@ -70,7 +70,7 @@ CTDB_NODES=/etc/ctdb/nodes
 CTDB_PUBLIC_ADDRESSES=/etc/ctdb/public_addresses
 
 # Enable logging to syslog
-CTDB_SYSLOG=yes
+CTDB_LOGGING=syslog
 
 # Default log level
 CTDB_DEBUGLEVEL=NOTICE
index ac235139e3501f64556ecd2131092458fe273c1e..18b74b255faeb6c3c1354e164cfe6ad4da190c16 100644 (file)
@@ -367,8 +367,6 @@ uint32_t *ctdb_get_connected_nodes(struct ctdb_context *ctdb,
 
 int ctdb_statistics_reset(struct ctdb_context *ctdb, uint32_t destnode);
 
-int ctdb_set_logfile(TALLOC_CTX *mem_ctx, const char *logfile, bool use_syslog);
-
 typedef int (*ctdb_traverse_func)(struct ctdb_context *, TDB_DATA, TDB_DATA, void *);
 int ctdb_traverse(struct ctdb_db_context *ctdb_db, ctdb_traverse_func fn, void *private_data);
 
index 8f53c6b5e9e3123b75e32683c7e6d250cd3319b4..c419e85d77b157778b48db90c14e520531f8f303 100644 (file)
@@ -43,7 +43,12 @@ const char *get_debug_by_level(int32_t level);
 bool parse_debug(const char *str, int32_t *level);
 void print_debug_levels(FILE *stream);
 
-int ctdb_log_setup_syslog(void);
-int ctdb_log_setup_file(TALLOC_CTX *mem_ctx, const char *f);
+bool ctdb_logging_init(TALLOC_CTX *mem_ctx, const char *logging);
+typedef int (*ctdb_log_setup_fn_t)(TALLOC_CTX *mem_ctx,
+                                  const char *logging,
+                                  const char *app_name);
+void ctdb_log_register_backend(const char *prefix, ctdb_log_setup_fn_t init);
+void ctdb_log_init_file(void);
+void ctdb_log_init_syslog(void);
 
 #endif /* _CTDB_LOGGING_H_ */
index bc47dd273feaf3ddec054417eb28c39901e80741..85dbfcfcc006094b7df6ce1664d42939f25248e3 100644 (file)
 #include "system/time.h"
 #include "system/filesys.h"
 #include "lib/util/debug.h"
+#include "lib/util/dlinklist.h"
+
+struct ctdb_log_backend {
+       struct ctdb_log_backend *prev, *next;
+       const char *prefix;
+       ctdb_log_setup_fn_t setup;
+};
 
 struct ctdb_log_state {
        const char *prefix;
@@ -32,16 +39,34 @@ struct ctdb_log_state {
        uint16_t buf_used;
        void (*logfn)(const char *, uint16_t, void *);
        void *logfn_private;
+       struct ctdb_log_backend *backends;
 };
 
-/* we need this global to keep the DEBUG() syntax */
+/* Used by ctdb_set_child_logging() */
 static struct ctdb_log_state *log_state;
 
-/*
-  choose the logfile location
-*/
-int ctdb_set_logfile(TALLOC_CTX *mem_ctx, const char *logfile, bool use_syslog)
+void ctdb_log_register_backend(const char *prefix, ctdb_log_setup_fn_t setup)
 {
+       struct ctdb_log_backend *b;
+
+       b = talloc_zero(log_state, struct ctdb_log_backend);
+       if (b == NULL) {
+               printf("Failed to register backend \"%s\" - no memory\n",
+                      prefix);
+               return;
+       }
+
+       b->prefix = prefix;
+       b->setup = setup;
+
+       DLIST_ADD_END(log_state->backends, b, NULL);
+}
+
+
+/* Initialise logging */
+bool ctdb_logging_init(TALLOC_CTX *mem_ctx, const char *logging)
+{
+       struct ctdb_log_backend *b;
        int ret;
 
        log_state = talloc_zero(mem_ctx, struct ctdb_log_state);
@@ -50,22 +75,26 @@ int ctdb_set_logfile(TALLOC_CTX *mem_ctx, const char *logfile, bool use_syslog)
                abort();
        }
 
-       if (use_syslog) {
-               ret = ctdb_log_setup_syslog();
-               if (ret != 0) {
-                       printf("Setup of syslog logging failed with \"%s\"\n",
-                              strerror(ret));
-                       abort();
-               }
-       } else {
-               ret = ctdb_log_setup_file(mem_ctx, logfile);
-               if (ret != 0) {
-                       printf("Setup of file logging failed with \"%s\"\n",
-                              strerror(ret));
-                       abort();
+       ctdb_log_init_file();
+       ctdb_log_init_syslog();
+
+       for (b = log_state->backends; b != NULL; b = b->next) {
+               size_t l = strlen(b->prefix);
+               /* Exact match with prefix or prefix followed by ':' */
+               if (strncmp(b->prefix, logging, l) == 0 &&
+                   (logging[l] == '\0' || logging[l] == ':')) {
+                       ret = b->setup(mem_ctx, logging, "ctdbd");
+                       if (ret == 0) {
+                               return true;
+                       }
+                       printf("Log init for \"%s\" failed with \"%s\"\n",
+                              logging, strerror(ret));
+                       return false;
                }
        }
-       return 0;
+
+       printf("Unable to find log backend for \"%s\"\n", logging);
+       return false;
 }
 
 /* Note that do_debug always uses the global log state. */
index 28b72231d9d3e679ec9333d4eb1c059bfe88033a..f931b4a8b83073ff80be3813ffd52c324d681320 100644 (file)
@@ -24,6 +24,8 @@
 #include "system/filesys.h"
 #include "lib/util/time_basic.h"
 
+#define CTDB_LOG_FILE_PREFIX "file"
+
 struct file_state {
        int fd;
 };
@@ -64,9 +66,19 @@ static int file_state_destructor(struct file_state *state)
        return 0;
 }
 
-int ctdb_log_setup_file(TALLOC_CTX *mem_ctx, const char *logfile)
+static int ctdb_log_setup_file(TALLOC_CTX *mem_ctx,
+                              const char *logging,
+                              const char *app_name)
 {
        struct file_state *state;
+       const char *logfile;
+       size_t l;
+
+       l = strlen(CTDB_LOG_FILE_PREFIX);
+       if (logging[l] != ':') {
+               return EINVAL;
+       }
+       logfile = &logging[0] + l + 1;
 
        state = talloc_zero(mem_ctx, struct file_state);
        if (state == NULL) {
@@ -94,3 +106,8 @@ int ctdb_log_setup_file(TALLOC_CTX *mem_ctx, const char *logfile)
 
        return 0;
 }
+
+void ctdb_log_init_file(void)
+{
+       ctdb_log_register_backend(CTDB_LOG_FILE_PREFIX, ctdb_log_setup_file);
+}
index 382c7916e5a206068ee361113e83bfb0a49efc4d..58093375f2d0a19da17b4294e44a5d769ec19900 100644 (file)
@@ -54,8 +54,15 @@ static void ctdb_log_to_syslog(void *private_ptr, int dbglevel, const char *s)
               "%s%s", debug_extra, s);
 }
 
-int ctdb_log_setup_syslog(void)
+static int ctdb_log_setup_syslog(TALLOC_CTX *mem_ctx,
+                                const char *logging,
+                                const char *app_name)
 {
        debug_set_callback(NULL, ctdb_log_to_syslog);
        return 0;
 }
+
+void ctdb_log_init_syslog(void)
+{
+       ctdb_log_register_backend("syslog", ctdb_log_setup_syslog);
+}
index 16a647ac48cd4821f6699913e5d30837cb1f6998..ec285c043cce433d0257b5e6f390bbe24f59ca2e 100644 (file)
@@ -33,7 +33,7 @@ static struct {
        const char *public_address_list;
        const char *event_script_dir;
        const char *notification_script;
-       const char *logfile;
+       const char *logging;
        const char *recovery_lock_file;
        const char *db_dir;
        const char *db_dir_persistent;
@@ -42,7 +42,6 @@ static struct {
        const char *single_public_ip;
        int         valgrinding;
        int         nosetsched;
-       int         use_syslog;
        int         start_as_disabled;
        int         start_as_stopped;
        int         no_lmaster;
@@ -56,7 +55,7 @@ static struct {
        .public_address_list = NULL,
        .transport = "tcp",
        .event_script_dir = NULL,
-       .logfile = LOGDIR "/log.ctdb",
+       .logging = "file:" LOGDIR "/log.ctdb",
        .db_dir = CTDB_VARDIR,
        .db_dir_persistent = CTDB_VARDIR "/persistent",
        .db_dir_state = CTDB_VARDIR "/state",
@@ -111,7 +110,7 @@ int main(int argc, const char *argv[])
                { "public-interface", 0, POPT_ARG_STRING, &options.public_interface, 0, "public interface", "interface"},
                { "single-public-ip", 0, POPT_ARG_STRING, &options.single_public_ip, 0, "single public ip", "ip-address"},
                { "event-script-dir", 0, POPT_ARG_STRING, &options.event_script_dir, 0, "event script directory", "dirname" },
-               { "logfile", 0, POPT_ARG_STRING, &options.logfile, 0, "log file location", "filename" },
+               { "logging", 0, POPT_ARG_STRING, &options.logging, 0, "logging method to be used", NULL },
                { "nlist", 0, POPT_ARG_STRING, &options.nlist, 0, "node list file", "filename" },
                { "notification-script", 0, POPT_ARG_STRING, &options.notification_script, 0, "notification script", "filename" },
                { "listen", 0, POPT_ARG_STRING, &options.myaddress, 0, "address to listen on", "address" },
@@ -123,7 +122,6 @@ int main(int argc, const char *argv[])
                { "pidfile", 0, POPT_ARG_STRING, &ctdbd_pidfile, 0, "location of PID file", "filename" },
                { "valgrinding", 0, POPT_ARG_NONE, &options.valgrinding, 0, "disable setscheduler SCHED_FIFO call, use mmap for tdbs", NULL },
                { "nosetsched", 0, POPT_ARG_NONE, &options.nosetsched, 0, "disable setscheduler SCHED_FIFO call, use mmap for tdbs", NULL },
-               { "syslog", 0, POPT_ARG_NONE, &options.use_syslog, 0, "log messages to syslog", NULL },
                { "start-as-disabled", 0, POPT_ARG_NONE, &options.start_as_disabled, 0, "Node starts in disabled state", NULL },
                { "start-as-stopped", 0, POPT_ARG_NONE, &options.start_as_stopped, 0, "Node starts in stopped state", NULL },
                { "no-lmaster", 0, POPT_ARG_NONE, &options.no_lmaster, 0, "disable lmaster role on this node", NULL },
@@ -175,10 +173,7 @@ int main(int argc, const char *argv[])
 
        script_log_level = options.script_log_level;
 
-       ret = ctdb_set_logfile(ctdb, options.logfile, options.use_syslog);
-       if (ret == -1) {
-               printf("ctdb_set_logfile to %s failed - %s\n", 
-                      options.use_syslog?"syslog":options.logfile, ctdb_errstr(ctdb));
+       if (!ctdb_logging_init(ctdb, options.logging)) {
                exit(1);
        }
 
index 8f31154c02189aac3187855e47b0b0eb4897a6f1..d14e42ec84758e8c195e4a43e6a4170aa7666c3f 100644 (file)
@@ -23,8 +23,9 @@ fi
 mkdir -p "$EVENTSCRIPTS_TESTS_VAR_DIR"
 export CTDB_VARDIR="$EVENTSCRIPTS_TESTS_VAR_DIR/ctdb"
 
-export CTDB_LOGFILE="${EVENTSCRIPTS_TESTS_VAR_DIR}/log.ctdb"
-touch "$CTDB_LOGFILE" || die "Unable to create CTDB_LOGFILE=$CTDB_LOGFILE"
+export CTDB_LOGGING="file:${EVENTSCRIPTS_TESTS_VAR_DIR}/log.ctdb"
+touch "${CTDB_LOGGING#file:}" || \
+    die "Unable to setup logging for \"$CTDB_LOGGING\""
 
 if [ -d "${TEST_SUBDIR}/etc" ] ; then    
     cp -a "${TEST_SUBDIR}/etc" "$EVENTSCRIPTS_TESTS_VAR_DIR"
index a227a5d1f276eafeebaae2cbf69790232d83542a..58d679a5c7ee09fc8ae6855b2e3793a2ec681de8 100644 (file)
@@ -97,7 +97,7 @@ daemons_start_1 ()
     fi
 
     local node_ip=$(sed -n -e "$(($pnn + 1))p" "$CTDB_NODES")
-    local ctdb_options="--sloppy-start --reclock=${TEST_VAR_DIR}/rec.lock --nlist $CTDB_NODES --nopublicipcheck --listen=${node_ip} --event-script-dir=${TEST_VAR_DIR}/events.d --logfile=${TEST_VAR_DIR}/daemon.${pnn}.log -d 3 --dbdir=${TEST_VAR_DIR}/test.db --dbdir-persistent=${TEST_VAR_DIR}/test.db/persistent --dbdir-state=${TEST_VAR_DIR}/test.db/state --nosetsched"
+    local ctdb_options="--sloppy-start --reclock=${TEST_VAR_DIR}/rec.lock --nlist $CTDB_NODES --nopublicipcheck --listen=${node_ip} --event-script-dir=${TEST_VAR_DIR}/events.d --logging=file:${TEST_VAR_DIR}/daemon.${pnn}.log -d 3 --dbdir=${TEST_VAR_DIR}/test.db --dbdir-persistent=${TEST_VAR_DIR}/test.db/persistent --dbdir-state=${TEST_VAR_DIR}/test.db/state --nosetsched"
 
     if [ $pnn -eq $no_public_ips ] ; then
        ctdb_options="$ctdb_options --public-addresses=/dev/null"