[PATCHES] Logging to multiple debug backends

Martin Schwenke martin at meltin.net
Fri Mar 13 21:52:05 MDT 2015


On Fri, 13 Mar 2015 11:57:13 -0700, Christof Schmitt <cs at samba.org>
wrote:

> The attached patches add support for multiple debug backends. Today's
> code only has the notion of logging to a file, and optionally logging to
> syslog through the 'syslog' and 'syslog only' parameters.  With the
> patches applied, this becomes more flexible. Multiple logging backends
> can be configured through the new 'logging' parameter, e.g.:
> 
> logging = lttng:10 file:5 syslog:0 
> 
> Additional backends are the new interface to the systemd journal that
> can log more attributes for each message, and for gpfs. The existing
> config parameters 'syslog' and 'syslog only' are marked as 'deprecated'
> for now. Once the 'logging' parameter has been  around for a while, the
> previous parameters could be removed.
> 
> Any review and comments would be appreciated.

I've browsed through most of this and it looks very nice.

Can I please offer the following potential improvements?

The debug code is very heavily used so I would prematurely
optimise.  :-)  Instead of using an array of all possible backends in
debug_set_backends() and debug_backends_log(), I would suggest using a
doubly-linked list, and inserting the backends in reverse-sorted order
of priority.  Then you only need to traverse the list as long as
msg_level <= log_level.  This makes debug_set_backends() more expensive
but makes debug_backends_log() cheaper.

On top of this change, since you wouldn't need to reset the log level
for all backends, you could also tweak the reload() API.  I think that
if the reload() function had access to the backend structure (e.g.
"self") then it could just be passed the new log_level, do the right
magic (since it knows whether it was active before and knows the old log
level) and then update the log level to the new one. This would probably
involve moving the list of (previously) active backends to a temporary
variable, putting each continuing/new one into the active list and
calling reload().  Any leftovers could just have -1 passed to reload()
as the log level and would close/clean-up.

Given that this would mean passing the log level to reload(), you might
as well pass the token.  That also gives you the possibly of passing
options to the backend.  Changing the log level separator to '@', you
could do things like "syslog:nonblocking at 1 file:/var/log/log.smbd at 3".
In CTDB I implemented some of the option handling by making reload()
set the callback or, in this case, backend.log.  If we can also do
option handling then we can move some extra features from the CTDB
syslog code into debug.c.

All of the above would allow you to drop new_log_level from struct
debug_backend and reload would look like:

  void (*reload)(struct debug_backend *self, char *tok, const char *prog_name);

Crazy?  :-)

peace & happiness,
martin


More information about the samba-technical mailing list