[Cluster-devel] logsys in fenced

[Steven Dake <sdake@redhat.com>]

On Wed, 2008-06-25 at 09:43 -0500, David Teigland wrote:
> > commit 95a5c6b13294742956b13070ebc4f4513278255f
> > Author: Fabio M. Di Nitto <fdinitto@redhat.com>
> > 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 <fdinitto@redhat.com>
> > 
> > commit da704715c606c9c01637ae53d79f8dec6a8b0389
> > Author: Fabio M. Di Nitto <fdinitto@redhat.com>
> > Date:   Wed Jun 25 05:19:35 2008 +0200
> > 
> >     [FENCE] Allow fenced to configure logsys
> >     
> >     Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
> > 
> > commit 18e085596bb8844f74689a92662f2e5e9166836b
> > Author: Fabio M. Di Nitto <fdinitto@redhat.com>
> > Date:   Wed Jun 25 04:49:41 2008 +0200
> > 
> >     [FENCE] Move logsys configuration calls where they belong
> >     
> >     Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
> > 
> > commit c54c56c5a09f98547ceda3bc5fa9afa28b354480
> > Author: Fabio M. Di Nitto <fdinitto@redhat.com>
> > Date:   Wed Jun 25 04:23:20 2008 +0200
> > 
> >     [FENCE] Make fenced ready to load logsys config
> >     
> >     Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
> > 
> > commit cf4c7ebac813b0b607acf6cf74bbdddfc8cfb12a
> > Author: Fabio M. Di Nitto <fdinitto@redhat.com>
> > Date:   Tue Jun 24 14:34:35 2008 +0200
> > 
> >     [FENCE] Start porting fenced to logsys
> >     
> >     Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
> 
> 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.