diff mbox

[1/1] busybox: Make S01logging source a default file.

Message ID 1456483852-12830-1-git-send-email-nicolas.cavallari@green-communications.fr
State Superseded
Headers show

Commit Message

Nicolas Cavallari Feb. 26, 2016, 10:50 a.m. UTC
The default syslog parameters are to keep only 200-400 KiB of logs,
which is very few if there is a spammy daemon on the system, or a daemon
that fails and then spams errors that hides the original problem.

Make S01logging source a /etc/default/logging file where these
parameters can be overridden.

Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
---
 package/busybox/S01logging | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Luca Ceresoli March 1, 2016, 10:47 p.m. UTC | #1
Hi Nicolas,

On 26/02/2016 11:50, Nicolas Cavallari wrote:
> The default syslog parameters are to keep only 200-400 KiB of logs,
> which is very few if there is a spammy daemon on the system, or a daemon
> that fails and then spams errors that hides the original problem.
> 
> Make S01logging source a /etc/default/logging file where these
> parameters can be overridden.
> 
> Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> ---
>  package/busybox/S01logging | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/package/busybox/S01logging b/package/busybox/S01logging
> index a72c69d..af7bdff 100644
> --- a/package/busybox/S01logging
> +++ b/package/busybox/S01logging
> @@ -3,10 +3,14 @@
>  # Start logging
>  #
>  
> +SYSLOGD_ARGS=-n
> +KLOGD_ARGS=-n
> +[ -e /etc/default/logging ] && . /etc/default/logging

Instead of '-e', we typically use '-r'.

Other than that, looks good. Could you send v2 with this change?

>  start() {
>  	printf "Starting logging: "
> -	start-stop-daemon -b -S -q -m -p /var/run/syslogd.pid --exec /sbin/syslogd -- -n
> -	start-stop-daemon -b -S -q -m -p /var/run/klogd.pid --exec /sbin/klogd -- -n
> +	start-stop-daemon -b -S -q -m -p /var/run/syslogd.pid --exec /sbin/syslogd -- $SYSLOGD_ARGS
> +	start-stop-daemon -b -S -q -m -p /var/run/klogd.pid --exec /sbin/klogd -- $KLOGD_ARGS

Unrelated to your patch, but passing '-n' (Run in foreground) looks
strange since we are daemonizing those processes anyway. Do you know any
good reason for this?
Arnout Vandecappelle March 2, 2016, 6:20 p.m. UTC | #2
On 03/01/16 23:47, Luca Ceresoli wrote:
> Hi Nicolas,
> 
> On 26/02/2016 11:50, Nicolas Cavallari wrote:
>> The default syslog parameters are to keep only 200-400 KiB of logs,
>> which is very few if there is a spammy daemon on the system, or a daemon
>> that fails and then spams errors that hides the original problem.
>>
>> Make S01logging source a /etc/default/logging file where these
>> parameters can be overridden.
>>
>> Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
>> ---
>>  package/busybox/S01logging | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/package/busybox/S01logging b/package/busybox/S01logging
>> index a72c69d..af7bdff 100644
>> --- a/package/busybox/S01logging
>> +++ b/package/busybox/S01logging
>> @@ -3,10 +3,14 @@
>>  # Start logging
>>  #
>>  
>> +SYSLOGD_ARGS=-n
>> +KLOGD_ARGS=-n
>> +[ -e /etc/default/logging ] && . /etc/default/logging
> 
> Instead of '-e', we typically use '-r'.
> 
> Other than that, looks good. Could you send v2 with this change?
> 
>>  start() {
>>  	printf "Starting logging: "
>> -	start-stop-daemon -b -S -q -m -p /var/run/syslogd.pid --exec /sbin/syslogd -- -n
>> -	start-stop-daemon -b -S -q -m -p /var/run/klogd.pid --exec /sbin/klogd -- -n
>> +	start-stop-daemon -b -S -q -m -p /var/run/syslogd.pid --exec /sbin/syslogd -- $SYSLOGD_ARGS
>> +	start-stop-daemon -b -S -q -m -p /var/run/klogd.pid --exec /sbin/klogd -- $KLOGD_ARGS
> 
> Unrelated to your patch, but passing '-n' (Run in foreground) looks
> strange since we are daemonizing those processes anyway. Do you know any
> good reason for this?

 I do: these daemons don't create a PID file, and if they daemonize and
start-stop-daemon creates the PID file, the PID logged in there will be wrong
(it will be the PID of the exited parent).

 Regards,
 Arnout
Luca Ceresoli March 3, 2016, 1:23 p.m. UTC | #3
Dear Arnout,

On 02/03/2016 19:20, Arnout Vandecappelle wrote:
> On 03/01/16 23:47, Luca Ceresoli wrote:
[...]
>> Unrelated to your patch, but passing '-n' (Run in foreground) looks
>> strange since we are daemonizing those processes anyway. Do you know any
>> good reason for this?
> 
>  I do: these daemons don't create a PID file, and if they daemonize and
> start-stop-daemon creates the PID file, the PID logged in there will be wrong
> (it will be the PID of the exited parent).

Good point. Thanks.
diff mbox

Patch

diff --git a/package/busybox/S01logging b/package/busybox/S01logging
index a72c69d..af7bdff 100644
--- a/package/busybox/S01logging
+++ b/package/busybox/S01logging
@@ -3,10 +3,14 @@ 
 # Start logging
 #
 
+SYSLOGD_ARGS=-n
+KLOGD_ARGS=-n
+[ -e /etc/default/logging ] && . /etc/default/logging
+
 start() {
 	printf "Starting logging: "
-	start-stop-daemon -b -S -q -m -p /var/run/syslogd.pid --exec /sbin/syslogd -- -n
-	start-stop-daemon -b -S -q -m -p /var/run/klogd.pid --exec /sbin/klogd -- -n
+	start-stop-daemon -b -S -q -m -p /var/run/syslogd.pid --exec /sbin/syslogd -- $SYSLOGD_ARGS
+	start-stop-daemon -b -S -q -m -p /var/run/klogd.pid --exec /sbin/klogd -- $KLOGD_ARGS
 	echo "OK"
 }