'max log size' and logfile rotations in Samba 4.0

[Andrew Bartlett]

I've been looking into the behaviour of our logfile rotation and
re-opening code.

I have a few patches.  The first simply fixes up killall -HUP to do the
right thing on the 'samba' binary, by not renaming logfiles, just
re-opening them (logrotate has done the rename).

The issue here was that samba4 so far does not have the 'max log size'
parameter, and so it is implicitly 0 in the debug system.

This lead me to look into what would happen if we had already merged the
'max log size' parameter.  This turns out to be quite a problem, and
within the Samba process I've seen with lsof us have some children
atteched to the log.samba.old, others attached to an already deleted
log.samba.old, and one process attached to the correct log.samba

The complexity of this all has lead me to wonder if we should be doing
this at all?  Shouldn't we leave log rotation to tools like logrotate?

By moving logs to log.xxx.old, and then removing them, we loose
information at the time we need it most, and at least in the samba
binary (this is what I was working on), I can't even be assured that we
are logging to either log.samba or log.samba.old!  

Furthermore, the code to detect and then rename logs is inherently racy,
using fstat() followed by a path-based rename, and I can't see a
sensible way to make it less racy without imposing large penalties or
great complexity on our logging code.

		(void)reopen_logs_internal();
		if (state.fd > 2 && (fstat(state.fd, &st) == 0
				     && st.st_size > maxlog)) {
			char *name = NULL;
			
			if (asprintf(&name, "%s.old", state.debugf ) < 0) {
				return;
			}
			(void)rename(state.debugf, name);
			
			if (!reopen_logs_internal()) {
				/* We failed to reopen a log - continue using the old name. */
				(void)rename(name, state.debugf);
			}
			SAFE_FREE(name);
		}

I would for example like only one process to possibly rename a log, and
for other processes to simply be told to reopen it - but the idea of
sending signals or messages from deep in our logging code seems
impractical. 

Other ugly details - like only checking log size every 100 messages,
then having overrides that set the current count of messages to 100
(such as the one that tires to fire every 60 seconds from our event
loop) to force a check only complicate things further.  The remaining
patches here attempt to connect this into the samba binary, but don't
work correctly. 

Sadly even with the efforts I put in the last year or two, there isn't a
clear direction or design here, just a series of patches on patches.  

Please let me know what you think, particularly about removing 'max log
size', and any other thoughts on how we can tidy this up enough to have
consistent behaviour across all of Samba.

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-debug-Do-not-constantly-rename-logs-when-max-log-siz.patch
Type: text/x-patch
Size: 2038 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120611/c910bb60/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-lib-param-Add-max-log-size-and-timestamp-logs-parame.patch
Type: text/x-patch
Size: 2125 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120611/c910bb60/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-s3-lib-Convert-lib-events.c-to-modern-tevent-names.patch
Type: text/x-patch
Size: 3309 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120611/c910bb60/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-lib-util-Move-event_add_idle-to-the-top-level.patch
Type: text/x-patch
Size: 8874 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120611/c910bb60/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-s4-smbd-Add-idle-handler-to-check-for-over-size-logs.patch
Type: text/x-patch
Size: 6396 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120611/c910bb60/attachment-0004.bin>