From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Dake Date: Wed, 25 Jun 2008 09:12:50 -0700 Subject: [Cluster-devel] logsys in fenced In-Reply-To: <20080625144343.GA18958@redhat.com> References: <20080625042547.24195.qmail@sourceware.org> <20080625144343.GA18958@redhat.com> Message-ID: <1214410370.3645.18.camel@balance> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Wed, 2008-06-25 at 09:43 -0500, David Teigland wrote: > > commit 95a5c6b13294742956b13070ebc4f4513278255f > > Author: Fabio M. Di Nitto > > Date: Wed Jun 25 06:24:11 2008 +0200 > > > > [FENCE] fenced: separate concept of fork and debugging > > > > allow fenced to fork when debugging is set from the configuration > > or the system will hang at boot. > > > > Signed-off-by: Fabio M. Di Nitto > > > > commit da704715c606c9c01637ae53d79f8dec6a8b0389 > > Author: Fabio M. Di Nitto > > Date: Wed Jun 25 05:19:35 2008 +0200 > > > > [FENCE] Allow fenced to configure logsys > > > > Signed-off-by: Fabio M. Di Nitto > > > > commit 18e085596bb8844f74689a92662f2e5e9166836b > > Author: Fabio M. Di Nitto > > Date: Wed Jun 25 04:49:41 2008 +0200 > > > > [FENCE] Move logsys configuration calls where they belong > > > > Signed-off-by: Fabio M. Di Nitto > > > > commit c54c56c5a09f98547ceda3bc5fa9afa28b354480 > > Author: Fabio M. Di Nitto > > Date: Wed Jun 25 04:23:20 2008 +0200 > > > > [FENCE] Make fenced ready to load logsys config > > > > Signed-off-by: Fabio M. Di Nitto > > > > commit cf4c7ebac813b0b607acf6cf74bbdddfc8cfb12a > > Author: Fabio M. Di Nitto > > Date: Tue Jun 24 14:34:35 2008 +0200 > > > > [FENCE] Start porting fenced to logsys > > > > Signed-off-by: Fabio M. Di Nitto > > OK, I'm fine with replacing the use of syslog with logsys, but this goes > beyond that. Here are the problems I see: > > > . Leave log_debug() unchanged, and leave the meaning/effect of -D unchanged. > syslog/logsys are about logging to files. The debug "logging" I use is > about logging to either an in-memory buffer or to stderr; syslog/logsys > are not relevant to that. > > > . Change log_error() to use logsys instead of syslog, i.e. don't change > the existing log_error() call sites. > > #define log_error(fmt, args...) \ > do { \ > log_debug(fmt, ##args); \ > - syslog(LOG_ERR, fmt, ##args); \ > + log_printf(LOG_ERR, fmt, ##args); \ > } while (0) > > > . Finally, one gripe with logsys itself. Here's syslog initialization: > > openlog("fenced", LOG_PID, LOG_DAEMON); > > Compare with logsys initialization: > > LOGSYS_DECLARE_SYSTEM (NULL, > LOG_MODE_OUTPUT_STDERR | LOG_MODE_OUTPUT_SYSLOG_THREADED | > LOG_MODE_OUTPUT_FILE | LOG_MODE_BUFFER_BEFORE_CONFIG, > LOGDIR "/fenced.log", > SYSLOGFACILITY); > LOGSYS_DECLARE_SUBSYS ("FENCED", LOG_LEVEL_INFO); > > ... > > logsys_config_mode_set(LOG_MODE_OUTPUT_STDERR | > LOG_MODE_OUTPUT_SYSLOG_THREADED | LOG_MODE_OUTPUT_FILE | > LOG_MODE_FLUSH_AFTER_CONFIG); > The declaration of the logging system is up to individual choice. If we were to make it one macro "LOGSYS_DECLARE_SYSTEM()" without any arguments, the user of logsys would have no choice. While openlog may be simple, it also offers very little functionality. It simply "opens a log". It doesn't let you declare whether you want to output to stderr, or also if you want threaded mode of operation, or whether you should wait for fork to execute your first logging output. Because it doesn't support these features, it can be a simple little api "start logging to file X". Logsys offers a variety of features and isn't simply an api to log to syslog, but a full blown logging system to support a variety of goals all with nonblocking operation. I suggest reading the man page to understand what each of those options mean. I think once you do, you will see that none of them can be removed, and there is no way to have an "openlog" api that also supports the features of logsys. > ... and at the start of every other file: > > LOGSYS_DECLARE_SUBSYS ("FENCED", LOG_LEVEL_INFO); > > This really gets out of hand. I'd like to see the initialization wrapped > into a single function call, and the macros at the start of every file > unnecessary. > > The necessity for the DECLARE_SUBSYS macro is to support subsystems. Looking at the macro we see how this is implemented: #define LOGSYS_DECLARE_SUBSYS(subsys,priority) \ static unsigned int logsys_subsys_id __attribute__((unused)); \ __attribute__ ((constructor)) static void logsys_subsys_init (void) \ { \ logsys_subsys_id = \ _logsys_subsys_create ((subsys), (priority)); \ } So what exactly does this macro do? It creates a local static variable that any logging function can use to tell the logsys system which subsystem it belongs to. This is absolutely required. The _logsys_subsys_create maps a character string to this 32 bit identifier. There is no other way to support subsystem logging in a particular file. The reason is log_printf needs to know which subsystem it should log under for the particular invocation. This is done via the logsys_subsys_id static variable in each file. If you can propose an alternative that would do exactly the same thing, I'm all ears. I really don't see the big issue with adding one line of code to each file to support subsystem logging.