Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

310.accounting: Verify process accounting is active before log rotation. #648

Closed
wants to merge 1 commit into from

Conversation

jgrafton
Copy link
Contributor

Check if process accounting is active in the periodic script. If not, exit the periodic script indicating process accounting is not active instead of attempting to rotate the logs. onerotate_log turns accounting back on regardless of what accounting_enable in rc.conf is set to.

PR: 267464

Check if process accounting is active in the periodic script.  If not,
exit the periodic script indicating process accounting is not active
instead of attempting to rotate the logs.  onerotate_log turns accounting
back on regardless of what accounting_enable in rc.conf is set to.

PR: 267464
@bsdimp
Copy link
Member

bsdimp commented Feb 18, 2023

This looks good to me.

@bsdimp
Copy link
Member

bsdimp commented Feb 28, 2023

https://reviews.freebsd.org/D37434 has another 'looks good' but wanted more review.

Copy link
Contributor

@igalic igalic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the description in the Phabricator review much better, but regardless, 👍

@bsdimp
Copy link
Member

bsdimp commented Feb 28, 2023

OK. I'll push this in... But next time, if you have a review open, please add a line like

Differential Revision: https://reviews.freebsd.org/D37434

since it saves me a step, which can really add up for these small commits.

@bsdimp bsdimp closed this Feb 28, 2023
freebsd-git pushed a commit that referenced this pull request Feb 28, 2023
This corrects a bug in which the daily periodic script '310.accounting'
attempts to rotate logs via /etc/rc.d/accounting by calling
onerotate_logs function. The rotate logs function turns accounting back
on regardless of what acccounting_enable is set to in /etc/rc.conf. This
is due to checkyesno always returning YES since rotate logs is called
with the 'one' prefix.

In effect, accounting will always be turned back on once a day even if
it is disabled and stopped by hand. The fix was simple, just check if
accounting is before rotating logs and if it is, don't attempt the
rotate.

PR: 267464
Reviewed by: imp, hps (lgtm, not approval), Mina Galić
Pull Request: #648
Differential Revision: https://reviews.freebsd.org/D37434
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Mar 29, 2023
This corrects a bug in which the daily periodic script '310.accounting'
attempts to rotate logs via /etc/rc.d/accounting by calling
onerotate_logs function. The rotate logs function turns accounting back
on regardless of what acccounting_enable is set to in /etc/rc.conf. This
is due to checkyesno always returning YES since rotate logs is called
with the 'one' prefix.

In effect, accounting will always be turned back on once a day even if
it is disabled and stopped by hand. The fix was simple, just check if
accounting is before rotating logs and if it is, don't attempt the
rotate.

PR: 267464
Reviewed by: imp, hps (lgtm, not approval), Mina Galić
Pull Request: freebsd/freebsd-src#648
Differential Revision: https://reviews.freebsd.org/D37434
@emaste emaste added the merged label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants