Bug 731371

Summary: Patch for several oft-requested features of logrotate
Product: [Fedora] Fedora Reporter: Jim Keir <jimkeir>
Component: logrotateAssignee: Jan Kaluža <jkaluza>
Status: CLOSED EOL QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 19CC: jkaluza
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-02-18 13:36:55 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Zipfile containing patches
none
Updated patch for logrotate.c - cleaner none

Description Jim Keir 2011-08-17 13:08:37 UTC
The attached patch (against 3.7.9) adds:

- "createolddir" parameter, to allow creation of missing olddirs using the same uid/gid/mode as the source dir
- "minage" parameter, to keep logfiles in the original location for a minimum number of days.
- "duplicateok" parameter, to allow catch-all rules at the end of a config file which specify a wildcard which matches the same files as more specific entries further up the file. The specific entries are processed first, with anything that's left being caught by the catch-all.
- Automatic removal of lines from the state file where the log no longer exists.
- Fix for (probably libc) bug in mmap and 0-byte files. If a 0-byte file is include on Linux, logrotate crashes when it access (mmap'd area + 1) even though mmap is requested to reserve (length+2) bytes.
- The first time it is run, it assumes that the last-modified time on the file is the last rotated time. This immediately gets rid of any really old logs, and hopefully stops people asking why it seems to be doing nothing at all.
- Is now able to backup files across different filesystems if necessary.
- Some minor bugfixes for HPUX and Cygwin

We have many logfiles with generated names (i.e. including the date, user and/or PID) which need to be maintained. The attached patches allow this to happen.

Tested on HPUX and Linux.

Comment 1 Jim Keir 2011-08-17 13:09:17 UTC
Created attachment 518674 [details]
Zipfile containing patches

Comment 2 Jan Kaluža 2011-08-17 14:16:47 UTC
Thanks for the patches. I've just read comment so far and some of your changes are already implemented. I will check the patches and write my ideas here when I'll have time.

Comment 3 Jan Kaluža 2011-08-18 10:50:36 UTC
(In reply to comment #0)
> The attached patch (against 3.7.9) adds:
> 
I've checked your patches and I have some ideas to share:

> - "createolddir" parameter, to allow creation of missing olddirs using the same
> uid/gid/mode as the source dir

I think I'll add this feature without any config directive, so it will create olddir automatically. There will be probably problem with selinux, because it won't allow logrotate to do mkdir by default, so it has to be added into selinux-policy too.

> - "minage" parameter, to keep logfiles in the original location for a minimum
> number of days.

I don't see how this feature is useful. If you rotation per day is too much for you, you can rotate weekly. What's the real problem you're trying to fix here?

> - "duplicateok" parameter, to allow catch-all rules at the end of a config file
> which specify a wildcard which matches the same files as more specific entries
> further up the file. The specific entries are processed first, with anything
> that's left being caught by the catch-all.

I will re-read your implementation again, but so far it looks like you presume wildcard-rules will be the last ones in the last config file. This is not user-friendly and brings more problems when two duplicates are defined by mistake.

To be honest, I'm not sure I'm fan of this feature, but I probably can imagine it like checking wildcard configs in the end of config parsing when all non-wildcard configs are already parsed. That could be useful.

> - Automatic removal of lines from the state file where the log no longer
> exists.

Some people tend to check state file to check what happend to log file if it's not there, but I think this is not normal use-case. I think I'll accept this one too.

> - Fix for (probably libc) bug in mmap and 0-byte files. If a 0-byte file is
> include on Linux, logrotate crashes when it access (mmap'd area + 1) even
> though mmap is requested to reserve (length+2) bytes.

This is fixed in logrotate-3.8.0.

> - The first time it is run, it assumes that the last-modified time on the file
> is the last rotated time. This immediately gets rid of any really old logs, and
> hopefully stops people asking why it seems to be doing nothing at all.

Hm, this doesn't sound bad. I will give it a try and see. I think it's logical that logrotate should rotate log when it's executed and the log is not in database yet (unless "size" is used and the condition is not true, ...)

> - Is now able to backup files across different filesystems if necessary.

This way can lose messages during rotation. Although, I don't know how to do that differently, but I think this is the reason why it never been supported.

> - Some minor bugfixes for HPUX and Cygwin

Will accept it.

> We have many logfiles with generated names (i.e. including the date, user
> and/or PID) which need to be maintained. The attached patches allow this to
> happen.
> 
> Tested on HPUX and Linux.

Comment 4 Jim Keir 2011-08-18 12:06:22 UTC
Hi,

> > - "createolddir" parameter, to allow creation of missing olddirs using the same
> > uid/gid/mode as the source dir
> I think I'll add this feature without any config directive,

Not a problem for me, I was just trying to keep the default behaviour the same as in previous versions.

> > - "minage" parameter, to keep logfiles in the original location for a minimum
> > number of days.
> I don't see how this feature is useful. If you rotation per day is too much for
> you, you can rotate weekly. What's the real problem you're trying to fix here?

The problem is that things never get rotated out of existence, it's not to do with the amount of time between rotations.

Say I have a logfile called "proc_123_JK_4431.log". It's a one-off, the name includes a UID and PID. Logrotate runs once, whether it's a day or a week or whatever, moving it to "_oldlogs/proc_123_JK_4431.log.1.gz". All fine so far. However, since that logfile name is highly unlikely to be re-used, the .1.gz will stay on disk forever because it will never get rotated out.

So, in order to have those files cleaned up after a while I need to be able to define a rule which removes them at the first pass - rotate 0 - but doesn't do it for a set amount of time. "maxage" only works if the logfile is to be rotated, which is not the case here.

Having "rotate 0" without this "minage" command limits me to keeping things for exactly one day/week/month. We need to keep these files for at least three months, so "rotate 0" is out unless I can also say "but not for 90 days". "rotate (anything other than 0)" is also out because the file never re-appears, leaving the first backup in place forever.

> > - "duplicateok" parameter, to allow catch-all rules at the end of a config file
> I will re-read your implementation again, but so far it looks like you presume
> wildcard-rules will be the last ones in the last config file.

Yes, I am presuming that. However, with this feature off by default then the existing behaviour is unchanged - accidental duplicates will still cause a failure. Only people who have RTFM'd and added this parameter to their config will be affected.

> > - Automatic removal of lines from the state file where the log no longer
> > exists.
> 
> Some people tend to check state file to check what happend to log file if it's
> not there, but I think this is not normal use-case. I think I'll accept this
> one too.

It could easily be modified to only delete the entry if the file doesn't exist, *and* the last-rotated time is older than, say, a month?

> > - Is now able to backup files across different filesystems if necessary.
> 
> This way can lose messages during rotation. Although, I don't know how to do
> that differently, but I think this is the reason why it never been supported.

Hmm - OK, fair point, although copyTruncate does something very similar ;) In non-copying mode if it has to go across filesystems it could maybe rename the original to a temp file in the same directory, allowing the normal continuance of the logging, and then copy/unlink the tempfile.

I should probably have just re-used the existing copyTruncate code rather than having a separate copy routine. If I get time I'll take another poke at these and send another patch.

Cheers,
Jim

Comment 5 Jan Kaluža 2011-08-18 12:29:16 UTC
(In reply to comment #4)
> Hi,
> 
> > > - "duplicateok" parameter, to allow catch-all rules at the end of a config file
> > I will re-read your implementation again, but so far it looks like you presume
> > wildcard-rules will be the last ones in the last config file.
> 
> Yes, I am presuming that. However, with this feature off by default then the
> existing behaviour is unchanged - accidental duplicates will still cause a
> failure. Only people who have RTFM'd and added this parameter to their config
> will be affected.

I just don't think it works as people would expect. It looks to me like "half-done" feature. It works, but only in some very specific circumstances which does not happen for most of users. I will try to write patch which does what I suggested. This will work for you anyway, but should be more flexible and error-prone.

> Hmm - OK, fair point, although copyTruncate does something very similar ;) In
> non-copying mode if it has to go across filesystems it could maybe rename the
> original to a temp file in the same directory, allowing the normal continuance
> of the logging, and then copy/unlink the tempfile.

> I should probably have just re-used the existing copyTruncate code rather than
> having a separate copy routine. If I get time I'll take another poke at these
> and send another patch.

Copytruncate has this behaviour mentioned in manpage and its use is logged. But you're right, since we already have this in logrotate, I would probably not mind accepting this if it'll use copytruncate and print some info what is it doing. I will think about some option to turn it on explicitly.

Comment 6 Jim Keir 2011-08-18 14:16:27 UTC
OK, I've changed the copy across devices section to use copytruncate and issue a warning, and the delay removing unused entries from the state file for 30 days. New patch (to replace the previous one for logrotate.c) attached.

Your implementation of the duplicates-allowed rule would be better, but I don't have time to do it and test it thoroughly right now I'm afraid!

Comment 7 Jim Keir 2011-08-18 14:17:12 UTC
Created attachment 518877 [details]
Updated patch for logrotate.c - cleaner

Comment 8 Fedora End Of Life 2013-04-03 19:25:26 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 19 development cycle.
Changing version to '19'.

(As we did not run this process for some time, it could affect also pre-Fedora 19 development
cycle bugs. We are very sorry. It will help us with cleanup during Fedora 19 End Of Life. Thank you.)

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora19

Comment 9 Fedora End Of Life 2015-01-09 21:52:10 UTC
This message is a notice that Fedora 19 is now at end of life. Fedora 
has stopped maintaining and issuing updates for Fedora 19. It is 
Fedora's policy to close all bug reports from releases that are no 
longer maintained. Approximately 4 (four) weeks from now this bug will
be closed as EOL if it remains open with a Fedora 'version' of '19'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 19 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Comment 10 Fedora End Of Life 2015-02-18 13:36:55 UTC
Fedora 19 changed to end-of-life (EOL) status on 2015-01-06. Fedora 19 is
no longer maintained, which means that it will not receive any further
security or bug fix updates. As a result we are closing this bug.

If you can reproduce this bug against a currently maintained version of
Fedora please feel free to reopen this bug against that version. If you
are unable to reopen this bug, please file a new report against the
current release. If you experience problems, please add a comment to this
bug.

Thank you for reporting this bug and we are sorry it could not be fixed.