init scripts: rewrite S01logging

Continuing our effort to improve daemon startup scripts, this series focuses on S01logging, which starts the logging daemon. Common features are:

- Indent with tabs, not spaces.
- Implement start, stop, restart and reload as functions.
- Use start-stop-daemon.
- Correctly detect and report start/stop/restart/reload errors.
- Support a configuration file at /etc/default (example files for each package are provided in individual patches).
- Support a configuration variable that completely disables the service and issues a warning message on any invocation.

The configuration files are provided mostly as examples for init script authors but they also contain information about options that cannot be used when running in background.

All files implement the following FSM:

                                                  +---------+
             +-------stop--------+   +----(1s)----| stopped |
             |                   |   |            |   (*)   |
             |                   |   |            +---------+
             v                   |   v                 ^
      +---------+             +---------+              |
      |         |             |         |----restart---+
      | STOPPED |----start--->| STARTED |
      |         |             |         |----reload----+
      +---------+             +---------+              |
                                     ^                 |
                                     |                 |
                                     +-----------------+

* "stopped" is an intermediary state that transitions to STARTED after one second.

Attempts to do invalid transitions (e.g. start from STARTED state) will fail. That's why we don't pass -o (--oknodo) to start-stop-daemon. This changes the script behavior, in some cases, while in other cases just reports errors that were ignored previously.

The "restart" transition is implemented as "stop, sleep 1, start", so restarting from STOPPED state is possible, although an error message is shown because "stop" fails.

The "reload" transition semantics is "reload the configuration and keep running", when possible, otherwise it is the same as "restart" (in this series, only syslog-ng supports a true "reload" operation).

The scripts and configuration files checked with shellcheck v0.5.0:

  $ shellcheck -s sh package/*/{S01logging,etc.default.logging}

Carlos Santos (8):
busybox: update S01logging
busybox: add logging configuration file
rsyslog: update S01logging
rsyslog: add logging configuration file
sysklogd: update S01logging
sysklogd: add logging configuration file
syslog-ng: update S01logging
syslog-ng: add logging configuration file

 package/busybox/S01logging            | 82 +++++++++++++++---------
 package/busybox/busybox.mk            |  2 +
 package/busybox/etc.default.logging   | 17 +++++
 package/rsyslog/S01logging            | 68 +++++++++++++-------
 package/rsyslog/etc.default.logging   | 24 +++++++
 package/rsyslog/rsyslog.mk            |  2 +
 package/sysklogd/S01logging           | 90 +++++++++++++++++++++------
 package/sysklogd/etc.default.logging  | 33 ++++++++++
 package/sysklogd/sysklogd.mk          |  2 +
 package/syslog-ng/S01logging          | 73 +++++++++++++++-------
 package/syslog-ng/etc.default.logging | 21 +++++++
 package/syslog-ng/syslog-ng.mk        |  2 +
 12 files changed, 323 insertions(+), 93 deletions(-)
 create mode 100644 package/busybox/etc.default.logging
 create mode 100644 package/rsyslog/etc.default.logging
 create mode 100644 package/sysklogd/etc.default.logging
 create mode 100644 package/syslog-ng/etc.default.logging

2.17.1

Reformat and fix syslogd/klogd startup script for better quality and code style:

- Detect and report start/stop errors (previous version ignored them and always reported OK).
- Use a separate function for restart.
- Implement reload as restart.
- Support a configuration variable that completely disables the service and issues a warning message on any invocation.

---
Changes v1->v2
- Implement suggestions made by Nicolas Cavallari and Arnout Vandecappelle

---
 package/busybox/S01logging | 82 +++++++++++++++++++++++++-------------
 1 file changed, 54 insertions(+), 28 deletions(-)

index fcb3e7d236..ddd27bba9a 100644
@@ -1,40 +1,66 @@
 #!/bin/sh
-#
-# Start logging
-#
 
-SYSLOGD_ARGS=-n
-KLOGD_ARGS=-n
-[ -r /etc/default/logging ] && . /etc/default/logging
+DAEMON="logging"
+SPIDFILE="/var/run/syslogd.pid"
+KPIDFILE="/var/run/klogd.pid"
 
+SYSLOGD_ARGS=""
+KLOGD_ARGS=""
+ENABLED="yes"
+
+# shellcheck source=/dev/null
+[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
+
+if [ "$ENABLED" != "yes" ]; then
+	printf '%s is disabled\n' "$DAEMON"
+	exit 0
+fi
+
+# BusyBox' syslogd and klogd do not create pidfiles, so use "-m" to instruct
+# start-stop-daemon to create them. This also means that we must pass "-n" to
+# sylogd and klogd in the command line.
 start() {
-	printf "Starting logging: "
-	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"
+	printf 'Starting %s: ' "$DAEMON"
+	status=0
+	# shellcheck disable=SC2086 # we need the word splitting
+	start-stop-daemon -b -S -q -m -p "$SPIDFILE" -x /sbin/syslogd -- -n $SYSLOGD_ARGS || status=$?
+	start-stop-daemon -b -S -q -m -p "$KPIDFILE" -x /sbin/klogd -- -n $KLOGD_ARGS || status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
 }
 
 stop() {
-	printf "Stopping logging: "
-	start-stop-daemon -K -q -p /var/run/syslogd.pid
-	start-stop-daemon -K -q -p /var/run/klogd.pid
-	echo "OK"
+	printf 'Stopping %s: ' "$DAEMON"
+	status=0
+	start-stop-daemon -K -q -p "$SPIDFILE" || status=$?
+	[ "$status" -eq 0 ] && rm -f "$SPIDFILE"
+	start-stop-daemon -K -q -p "$KPIDFILE" || status=$?
+	[ "$status" -eq 0 ] && rm -f "$KPIDFILE"
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
 }
 
-case "$1" in
-  start)
-	start
-	;;
-  stop)
-	stop
-	;;
-  restart|reload)
+restart() {
 	stop
+	sleep 1
 	start
-	;;
-  *)
-	echo "Usage: $0 {start|stop|restart|reload}"
-	exit 1
-esac
+}
 
-exit $?
+case "$1" in
+        start|stop|restart)
+		"$1";;
+	reload)
+		# Restart, since there is no true "reload" feature.
+		restart;;
+        *)
+                echo "Usage: $0 {start|stop|restart|reload}"
+                exit 1
+esac

2.17.1

Carlos,

Hi Carlos,

Thank you again for this series. I still have some feedback, which I discussed
as well with the others here at the Buildroot meeting, and we generally agree on these aspects.

Let me start with repeating that we appreciate very much this work.

Thomas mentioned that he would prefer every init script to be called the same
as the executable it starts (which is usually the same as the package). There is general agreement about that aspect.

Also he mentioned that it is better to split into two separate scripts when two
daemons are started (i.e., klogd). There's agreement on that as well.

Still, these changes can be done in follow-up patches, so it's OK if you keep
it as S01logging for the time being (if you want to do the split right away, that's OK as well). However, the names of the default files would be better to already reflect the daemon name rather than just /etc/default/logging.

Note also that with the splitting, we can improve the situation for busybox a
little. Now, it is possible that klogd is NOT selected in busybox, but it is still started. With a separate init script, it would be possible to install it only when klogd is enabled (like is done now for syslogd).

	start-stop-daemon -b -S -q -m -p "$SPIDFILE" -x /sbin/syslogd \
		-- -n $SYSLOGD_ARGS || status=$?

Provide a template to help users to customize syslogd and klogd without editting the startup script.

syslogd options worth to configure are remote logging (-R), rotation (-s, -b) and minimal priority level (-l).

klogd minimal priority level (-c) can be configured too, preventing non-critical kernel messages from appearing on the console.

This file is also useful as an example for init script authors.

---
Changes v1->v2
- Implement suggestions made by Nicolas Cavallari and Arnout Vandecappelle Changes v2->v3
- Add reference to documentation, as suggested by Matt Weber.

---
 package/busybox/busybox.mk          |  2 ++
 package/busybox/etc.default.logging | 17 +++++++++++++++++
 2 files changed, 19 insertions(+)
 create mode 100644 package/busybox/etc.default.logging

index 757086592f..91131c0012 100644
@@ -252,6 +252,8 @@ define BUSYBOX_INSTALL_LOGGING_SCRIPT
 	then \
 		$(INSTALL) -m 0755 -D package/busybox/S01logging \
 			$(TARGET_DIR)/etc/init.d/S01logging; \
+		$(INSTALL) -m 0644 -D package/busybox/etc.default.logging \
+			$(TARGET_DIR)/etc/default/logging; \
 	fi
 endef
 

new file mode 100644
index 0000000000..5e52eff985
@@ -0,0 +1,17 @@
+#
+# /etc/default/logging (busybox version)
+#
+# Online documentation of syslogd and klogd is available at
+#
+#    https://busybox.net/downloads/BusyBox.html
+#
+
+# Use SYSLOGD_ARGS to pass additional arguments to syslogd (e.g. for log
+# rotation).
+# SYSLOGD_ARGS="" # (default value)
+
+# Use KLOGD_ARGS to pass additional arguments to klogd.
+# KLOGD_ARGS="" # (default value)
+
+# Uncomment the line below to disable this service
+# ENABLED="no"

2.17.1

Carlos,

Tested the disable option in the config and the init script picked it up as expected.

We also discussed this, and we agreed in the end that it is not very useful to
have a default defaults file. It needlessly consumes space on the target, the user anyway probably has to consult documentation to know which options to use, and there is (usually) only one option that can be set in the file:
<DAEMON>_ARGS. In addition, we can't expect all package authors to submit something that is of such a high quality as this example...

Regards,
Arnout

OK, I will drop the default files from the series.

Carlos Santos (Casantos) - DATACOM, P&D
?Marched towards the enemy, spear upright, armed with the certainty
that only the ignorant can have.? ? Epitaph of a volunteer

Reformat and fix rsyslog startup script for better quality and code style:

- Indent with tabs, not spaces.
- Support a configuration file at /etc/default (an example file will be added in forthcomming patch).
- Support a configuration variable that completely disables the service and issues a warning message on any invocation.

---
Changes v1->v2
- Implement suggestions made by Nicolas Cavallari and Arnout Vandecappelle Changes v2->v3
- Include /etc/default/logging, not /etc/default/$DAEMON.

---
 package/rsyslog/S01logging | 68 +++++++++++++++++++++++++-------------
 1 file changed, 45 insertions(+), 23 deletions(-)

index 8e4a59c2d5..5764780467 100644
@@ -1,36 +1,58 @@
 #!/bin/sh
 
+DAEMON="rsyslogd"
+PIDFILE="/var/run/$DAEMON.pid"
+
+RSYSLOGD_ARGS=""
+ENABLED="yes"
+
+# shellcheck source=/dev/null
+[ -r "/etc/default/logging" ] && . "/etc/default/logging"
+
+if [ "$ENABLED" != "yes" ]; then
+	printf '%s is disabled\n' "$DAEMON"
+	exit 0
+fi
+
 start() {
-  printf "Starting rsyslog daemon: "
-  start-stop-daemon -S -q -p /var/run/rsyslogd.pid --exec /usr/sbin/rsyslogd
-  [ $? = 0 ] && echo "OK" || echo "FAIL"
+	printf 'Starting %s: ' "$DAEMON"
+	# shellcheck disable=SC2086 # we need the word splitting
+	start-stop-daemon -S -q -p "$PIDFILE" -x /usr/sbin/rsyslogd -- $RSYSLOGD_ARGS
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
 }
 
 stop() {
-  printf "Stopping rsyslog daemon: "
-  start-stop-daemon -K -q -p /var/run/rsyslogd.pid
-  [ $? = 0 ] && echo "OK" || echo "FAIL"
+	printf 'Stopping %s: ' "$DAEMON"
+	start-stop-daemon -K -q -p "$PIDFILE"
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
 }
 
 restart() {
-  stop
-  sleep 1
-  start
+	stop
+	sleep 1
+	start
 }
 
 case "$1" in
-  start)
-    start
-    ;;
-  stop)
-    stop
-    ;;
-  restart|reload)
-    restart
-    ;;
-  *)
-    echo "Usage: $0 {start|stop|restart}"
-    exit 1
+        start|stop|restart)
+		"$1";;
+	reload)
+		# Restart, since there is no true "reload" feature (does not
+		# reconfigure/restart on SIGHUP, just closes all open files).
+		restart;;
+        *)
+                echo "Usage: $0 {start|stop|restart|reload}"
+                exit 1
 esac
-
-exit $?

2.17.1

Carlos,

Provide a template to help users to customize rsyslog without editting the startup script. Also warn about options that must not be used.

Add instructions on how to configure debugging, since it a bit tricky when rsyslogd runs in background, requiring a separete log file.

This file is also useful as an example for init script authors.

---
Changes v1->v2
- Document that "-n" command line option must not be used.
- Small improvements in comments.
Changes v2->v3
- Add reference to documentation, as suggested by Matt Weber.

---
 package/rsyslog/etc.default.logging | 24 ++++++++++++++++++++++++
 package/rsyslog/rsyslog.mk          |  2 ++
 2 files changed, 26 insertions(+)
 create mode 100644 package/rsyslog/etc.default.logging

new file mode 100644
index 0000000000..12223eba21
@@ -0,0 +1,24 @@
+#
+# /etc/default/logging (rsyslog version)
+#
+# Online documentation of rsyslog is available at
+#
+#    https://www.rsyslog.com/doc/
+#
+# Command line options are mostly undocumented in recent rsyslog versions, so
+# avoid using them. Only "-n" and "-d" are briefly described when rsyslogd is
+# invoked with the "-h" option. Other options are described on old versions of
+# the rsyslogd(8) manual page but they are certainly outdated.
+#
+
+# Use RSYSLOGD_ARGS to pass additional arguments to rsyslogd.
+# - Do NOT use "-i pid_file", "-n", nor "-v", since they will break the
+#   startup script.
+# - Use "-d" to enable debug (see below). Warning: log files may be HUGE!
+# RSYSLOGD_ARGS="" # (default value)
+
+# You will need this along with "-d".
+# RSYSLOG_DEBUGLOG="/var/log/rsyslogd"; export RSYSLOG_DEBUGLOG
+
+# Uncomment the line below to disable this service
+# ENABLED="no"

index 61e08ba765..c80d0696e2 100644
@@ -74,6 +74,8 @@ endif
 define RSYSLOG_INSTALL_INIT_SYSV
 	$(INSTALL) -m 0755 -D package/rsyslog/S01logging \
 		$(TARGET_DIR)/etc/init.d/S01logging
+	$(INSTALL) -m 0644 -D package/rsyslog/etc.default.logging \
+		$(TARGET_DIR)/etc/default/logging
 endef
 
 # The rsyslog.service is installed by rsyslog, but the link is not created

2.17.1

Carlos,

Similar to the busybox config, I disabled the logger and the init script reflected the change.

Reformat and fix syslogd/klogd startup script for better quality and code style:

- Implement start, stop, restart and reload as functions, like in other S01logging scripts, using start-stop-daemon.
- Indent with tabs, not spaces.
- Detect and report start/stop errors (previous version ignored them and always reported OK).
- Support a configuration file at /etc/default (an example file will be added in forthcomming patch).
- Support a configuration variable that completely disables the service and issues a warning message on any invocation.
- Do not kill syslogd in "reload". Send a SIGHUP signal, instructing it to perform a re-initialization. Also do not kill klogd. Send a signal (default 0, which does nothing). Users can configure this signal in /etc/default/logging to either SIGUSR1 or SIGUSR2.

---
Changes v1->v2
- Implement suggestions made by Arnout Vandecappelle Changes v2->v3
- Include /etc/default/logging, not /etc/default/$DAEMON.

---
 package/sysklogd/S01logging | 90 ++++++++++++++++++++++++++++---------
 1 file changed, 70 insertions(+), 20 deletions(-)

index 1cbfe869fa..9b6e879d8a 100644
@@ -1,25 +1,75 @@
 #!/bin/sh
 
-case "$1" in
-	start)
-		printf "Starting logging: "
-		/sbin/syslogd -m 0
-		/sbin/klogd
+DAEMON="sysklogd"
+SPIDFILE="/var/run/syslogd.pid"
+KPIDFILE="/var/run/klogd.pid"
+
+SYSLOGD_ARGS="-m 0"
+KLOGD_ARGS=""
+KLOGD_RELOAD="0"
+ENABLED="yes"
+
+# shellcheck source=/dev/null
+[ -r "/etc/default/logging" ] && . "/etc/default/logging"
+
+if [ "$ENABLED" != "yes" ]; then
+	printf '%s is disabled\n' "$DAEMON"
+	exit 0
+fi
+
+start() {
+	printf 'Starting %s: ' "$DAEMON"
+	status=0
+	# shellcheck disable=SC2086 # we need the word splitting
+	start-stop-daemon -S -q -p "$SPIDFILE" -x /sbin/syslogd -- $SYSLOGD_ARGS || status=$?
+	start-stop-daemon -S -q -p "$KPIDFILE" -x /sbin/klogd -- $KLOGD_ARGS || status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
+stop() {
+	printf 'Stopping %s: ' "$DAEMON"
+	status=0
+	start-stop-daemon -K -q -p "$SPIDFILE" || status=$?
+	start-stop-daemon -K -q -p "$KPIDFILE" || status=$?
+	if [ "$status" -eq 0 ]; then
 		echo "OK"
-		;;
-	stop)
-		printf "Stopping logging: "
-		[ -f /var/run/klogd.pid ] && kill `cat /var/run/klogd.pid`
-		[ -f /var/run/syslogd.pid ] && kill `cat /var/run/syslogd.pid`
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
+restart() {
+	stop
+	sleep 1
+	start
+}
+
+# SIGHUP makes syslogd reload its configuration
+# SIGUSR1 makes klogd reload kernel module symbols
+# SIGUSR2 makes klogd reload static kernel symbols and kernel module symbols
+reload() {
+	printf 'Reloading %s: ' "$DAEMON"
+	status=0
+	start-stop-daemon -K -s HUP -q -p "$SPIDFILE" || status=$?
+	start-stop-daemon -K -s "$KLOGD_RELOAD" -q -p "$KPIDFILE" || status=$?
+	if [ "$status" -eq 0 ]; then
 		echo "OK"
-		;;
-	restart|reload)
-		$0 stop
-		$0 start
-		;;
-	*)
-		echo "Usage: $0 {start|stop|restart}"
-		exit 1
-esac
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
 
-exit $?
+case "$1" in
+        start|stop|restart|reload)
+                "$1";;
+        *)
+                echo "Usage: $0 {start|stop|restart|reload}"
+                exit 1
+esac

2.17.1

Provide a template to help users to customize syslogd and klogd without editting the startup script. Also warn about options that must not be used.

syslogd options worth to configure are mark interval (-m), remote log support (-r) and domain list (-s).

Give example of how to configure klogd re-initialization signal.

---
Changes v1->v2:
- Document that the "-n" command line option must not be used.
- Going against the request from Arnout Vandecappelle, keep the klogd reload configuration. Add a comment explainig why it is still useful. BTW, I work on a project which requires dynamic installation of kernel modules.
Changes v2->v3
- Add reference to documentation, as suggested by Matt Weber.

---
 package/sysklogd/etc.default.logging | 33 ++++++++++++++++++++++++++++
 package/sysklogd/sysklogd.mk         |  2 ++
 2 files changed, 35 insertions(+)
 create mode 100644 package/sysklogd/etc.default.logging

new file mode 100644
index 0000000000..a655e7ad42
@@ -0,0 +1,33 @@
+#
+# /etc/default/logging (sysklogd version)
+#
+# Documentation is available in the klogd.8 and sysklogd.8 manual pages
+# distributed along with the source code.
+#
+
+# Use SYSLOGD_ARGS to pass additional arguments to syslogd.
+# - Do NOT use "-n", "-v", since they will break the startup script.
+# - Do NOT use "-d", since debugging does not work when running in background.
+# SYSLOGD_ARGS="-m 0" # (default value)
+
+# Use KLOGD_ARGS to pass additional arguments to klogd.
+# - Do NOT use "-n", "-o" nor "-v", since they will break the startup script.
+# - Do NOT use "-d", since debugging does not work when running in background.
+# KLOGD_ARGS="-m 0" # (default value)
+
+# KLOGD_RELOAD selects the "S01logging reload" behavior.
+# - "USR1" will cause the kernel module symbols to be reloaded.
+# - "USR2" will cause both the static kernel symbols and the kernel module
+#   symbols to be reloaded.
+# - "0" will do nothing.
+# - Do not use any other value here. Use kill(1) for fine-grained control of
+#   klogd, as documented in its manual page.
+#
+# Most users will never need to reload klogd in buildroot context because they
+# are not going to dynamically install kernel modules. In the rare cases that
+# this does happen, the user should be smart enough to send a SIGUSR1/2 but
+# let's keep this configurable for documentation purposes.
+# KLOGD_RELOAD="0" # (default value)
+
+# Uncomment the line below to disable this service
+# ENABLED="no"

index c4f064c10b..b7e67ffee3 100644
@@ -25,6 +25,8 @@ endef
 define SYSKLOGD_INSTALL_INIT_SYSV
 	$(INSTALL) -m 755 -D package/sysklogd/S01logging \
 		$(TARGET_DIR)/etc/init.d/S01logging
+	$(INSTALL) -m 0644 -D package/sysklogd/etc.default.logging \
+		$(TARGET_DIR)/etc/default/logging
 endef
 
 define SYSKLOGD_INSTALL_INIT_SYSTEMD

2.17.1

Reformat and fix syslog-ng startup script for better quality and code style:

- Indent with tabs, not spaces.
- Do not kill syslog-ng in "reload". Send a SIGHUP signal, instructing it to perform a re-initialization.
- Support a configuration file at /etc/default (an example file will be added in forthcomming patch).
- Support a configuration variable that completely disables the service and issues a warning message on any invocation.

---
Changes v1->v2:
- Implement changes suggested by Arnout Vandecappelle. Changes v2->v3:
- Include /etc/default/logging, not /etc/default/$DAEMON.
- Remove stray 'g' spotted by Chris Packham.

---
 package/syslog-ng/S01logging | 73 +++++++++++++++++++++++++-----------
 1 file changed, 51 insertions(+), 22 deletions(-)

index d7c899a1e3..255ca16660 100644
@@ -1,17 +1,42 @@
 #!/bin/sh
 
+DAEMON="syslog-ng"
+PIDFILE="/var/run/$DAEMON.pid"
+
+SYSLOG_NG_ARGS=""
+ENABLED="yes"
+
+# shellcheck source=/dev/null
+[ -r "/etc/default/logging" ] && . "/etc/default/logging"
+
+if [ "$ENABLED" != "yes" ]; then
+	printf '%s is disabled\n' "$DAEMON"
+	exit 0
+fi
+
 start() {
-	printf "Starting syslog-ng daemon: "
-	start-stop-daemon -S -q -p /var/run/syslog-ng.pid \
-		-x /usr/sbin/syslog-ng -- --pidfile /var/run/syslog-ng.pid
-	[ $? = 0 ] && echo "OK" || echo "FAIL"
+	printf 'Starting %s: ' "$DAEMON"
+	# shellcheck disable=SC2086 # we need the word splitting
+	start-stop-daemon -S -q -p "$PIDFILE" -x /usr/sbin/syslog-ng -- $SYSLOG_NG_ARGS
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
 }
 
 stop() {
-	printf "Stopping syslog-ng daemon: "
-	start-stop-daemon -K -q -p /var/run/syslog-ng.pid \
-		-x /usr/sbin/syslog-ng
-	[ $? = 0 ] && echo "OK" || echo "FAIL"
+	printf 'Stopping %s: ' "$DAEMON"
+	start-stop-daemon -K -q -p "$PIDFILE"
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
 }
 
 restart() {

@@ -20,19 +45,23 @@ restart() {
 	start
 }
 
+# SIGHUP makes syslog-ng reload its configuration
+reload() {
+	printf 'Stopping %s: ' "$DAEMON"
+	start-stop-daemon -K -s HUP -q -p "$PIDFILE"
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
 case "$1" in
-	start)
-		start
-		;;
-	stop)
-		stop
-		;;
-	restart|reload)
-		restart
-		;;
-	*)
-		echo "Usage: $0 {start|stop|restart}"
-		exit 1
+        start|stop|restart|reload)
+                "$1";;
+        *)
+                echo "Usage: $0 {start|stop|restart|reload}"
+                exit 1
 esac
-
-exit $?

2.17.1

Provide a template to help users to customize syslog-ng without editting the startup script. Mostly warn about options that must not be used.

This file is also useful as an example for init script authors.

---
Changes v1->v2
- Document that -F/--foreground must not be used
Changes v2->v3:
- Add reference to documentation, as suggested by Matt Weber.
- Fix copy/paste error spotted by Chris Packham (s/rsyslog/syslog-ng/).

---
 package/syslog-ng/etc.default.logging | 21 +++++++++++++++++++++
 package/syslog-ng/syslog-ng.mk        |  2 ++
 2 files changed, 23 insertions(+)
 create mode 100644 package/syslog-ng/etc.default.logging

new file mode 100644
index 0000000000..f0f4d52b02
@@ -0,0 +1,21 @@
+#
+# /etc/default/logging (syslog-ng version)
+#
+# Documentation is available in the syslog-ng.8 and syslog-ng.conf.5 manual
+# pages distributed along with the source code, in the doc/man subdirectory.
+#
+# Online documentation of syslog-ng is available at
+#
+#    https://www.syslog-ng.com/technical-documents/
+#
+
+# Use SYSLOG_NG_ARGS to pass additional arguments to syslog-ng.
+# - Do NOT use "-F/--foreground", "--help"/"-h", "--version"/"-V", "--stderr"/"-e",
+#   "--syntax-only"/"-s", or "--process-mode=<background|safe-background>", since
+#   they will break the startup script.
+# - Do NOT use "--debug"/"-d", since debugging does not work when running in
+#   background.
+# SYSLOG_NG_ARGS="" # (default value)
+
+# Uncomment the line below to disable this service
+# ENABLED="no"

index 793fea0972..a837dad841 100644
@@ -95,6 +95,8 @@ endif
 define SYSLOG_NG_INSTALL_INIT_SYSV
 	$(INSTALL) -m 0755 -D package/syslog-ng/S01logging \
 		$(TARGET_DIR)/etc/init.d/S01logging
+	$(INSTALL) -m 0644 -D package/syslog-ng/etc.default.logging \
+		$(TARGET_DIR)/etc/default/logging
 endef
 
 # By default syslog-ng installs a number of sample configuration

2.17.1

Hello,

Thanks a lot for working on this, much appreciated. Overall, I am happy with the proposal you're making and the consistency between init scripts that your proposal is bringing.

The only thing that I'm not entirely sure about is specific to these logging init scripts:

(1) Do we really want that Busybox, syslog-ng, rsyslog, sysklogd all install a file of the same name, S01logging ? Isn't this contradicting our goal of not having one package overwrite files installed by another package ?

Wouldn't it make more sense to install S01syslog-ng, S01rsyslog, S01sysklogd ? This way, we also have a mapping between the init script name and the daemon/package being started ?

(2) For the Busybox case, do we want to keep a single init script that starts both klogd and syslogd ? Or should we have one init script per daemon, so that again the init script name matches the daemon being started ? So S01klogd, S01syslogd ?

Note: the above points are really proposals/discussion items. Do not rework your series according to those comments until there has been some discussion/agreement with other developers.

Another point, but which can definitely be handled separately, is documenting in the manual what the init scripts should look like, by at least pointing to the "good" init scripts, so that people know which ones are good to use as a template.

Thanks a lot!

Thomas

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

I don't know why it was decided to use the same name for all packages. Looks like the original intention was to prevent multiple logging damons from running at the same time.

Perhaps we should add an item to the "System configuration" menu allowing the user to choose a logging provider, just like the "Init system" item, and convert rsyslog/sysklogd/syslog-ng to virtual packages.

There is also sysklogd, from which Busyox borrowed the code (or at least the idea) a long time ago. I think we should keep klogd and syslogd tied to each other, since klogd does not make much sense without syslogd.

It's already in my endless to-do list. :-)

Carlos Santos (Casantos) - DATACOM, P&D
?Marched towards the enemy, spear upright, armed with the certainty
that only the ignorant can have.? ? Epitaph of a volunteer

Hello,

Yes, but why? If we go down this route, then all web servers should install S50httpd, and not S50lighttpd, S50apache, etc. Dropbear and OpenSSH should not install S50dropbear and S50sshd, etc.

I don't think we want to do that, because then we need to do this for "what mail server do you want?", "what http server do you want?", "what SSH server do you want?", etc.

If you enable two SSH servers, the second to start will fail. If you enable two HTTP servers, the second to start will fail, etc. Similarly, if you enable two logging daemons, it will not work nicely.

I don't think we should solve that problem, and just leave it up to the user to do a configuration that makes sense. If you enable two SSH servers or two logging daemons, your configuration doesn't make sense, and it should be fixed.

Tied to each other doesn't necessarily mean they should be handled by a common init script. busybox.mk can install S01klogd S02syslogd, and we keep this one daemon == one init script logic ?

Otherwise, assuming we stop using the common S01logging name, what would be the name of the init script installed by Busybox for its logging daemons ?

Thomas

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Did I ever suggest adding menus to select HTTP and SSH servers? Let's stick to what was said, please.

The problem here is Busybox. If we name each init script after the corresponding daemon and select syslog-ng we will have both S01syslogd and S01syslog-ng installed unless we have some configuration and/or put some logic like this in busybox.mk:

ifeq ($(BR2_PACKAGE_SYSKLOGD)$(BR2_PACKAGE_RSYSLOG)$(BR2_PACKAGE_SYSLOG_NG),) define BUSYBOX_INSTALL_LOGGING_SCRIPT

        if grep -q CONFIG_SYSLOGD=y $(@D)/.config; \
        then \
                $(INSTALL) -m 0755 -D package/busybox/S01syslogd \
                        $(TARGET_DIR)/etc/init.d/S01syslogd; \
        fi; \
        if grep -q CONFIG_KLOGD=y $(@D)/.config; \
        then \
                $(INSTALL) -m 0755 -D package/busybox/S02klogd \
                        $(TARGET_DIR)/etc/init.d/S02klogd; \
        fi

endef
endif

Is that OK for you?

Carlos Santos (Casantos) - DATACOM, P&D
?Marched towards the enemy, spear upright, armed with the certainty
that only the ignorant can have.? ? Epitaph of a volunteer

Hello Carlos,

I'm not sure why you felt the need to become angry here, there really was no such need. I was just commenting on why I thought adding a menu for selecting the logging solution was IMO not appropriate, because it was in my opinion a direction that could lead to doing the same for tons of other packages that have conflicting functionality. So yes, the comparison with HTTP and SSH servers made sense: I was not implying *you* were suggesting to add menus for HTTP and SSH servers, but rather that if we start adding more and more system configuration menus to select between potentially conflicting packages, we may have people proposing patches to select HTTP/SSH servers.

E-mail communication probably didn't communicate my feeling very well here. Because overall, I am *really* happy with you taking the lead on this init script cleanup discussion. It's long overdue, and I'm very happy that someone like you has decided to do something about it, knowing that doing something about it includes getting through the usual bikeshedding that the Buildroot community is so good at.

So please take my comments in this discussion as nice comments from someone who is really happy to see you help cleaning up our init scripts! :-)

To me, this is exactly like what happens if you enable both Dropbear and OpenSSH. It installs two init scripts with different names, even if in practice, only one SSH server will start. We don't handle this "conflict" for the SSH servers, I don't see why we should have some special thing to handle it for logging daemons.

Yes, I think this downside is OK for me, because it's a downside we already have for lots of other packages in Buildroot, and fixing it requires way too much complexity compared to the benefits.

Of course, this is only my opinion, and other BR developers can share their possibly different view on this.

Thanks!

Thomas

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[snip]

There is one big difference with the HTTP daemon, however: even if busybox
httpd is enabled, we don't install an init script for it. However, S01logging is installed unconditionally, even if the user didn't consciously choose for busybox as its logging provider.

[Note: there are three other init scripts that are installed implicitly by
busybox: mdev (which does have an explicit option), watchdog (but no non-busybox equivalent exists for it) and telnet (but we have removed the standalone telnetd package).

Note 2: a somewhat similar situation exists with S40networking, which (probably?) conflicts with network-manager and connman.]

In that sense, it does make sense to offer an option to choose between the
logging providers.

That said, in general we prefer to do things implicitly by selecting a package
rather than having explicit choices for various system-level tools. Which indeed leads us to:

Note that this bit was already there, but it also explicitly checked that
S01logging didn't exist. So the complexity of handling the conflict was already there.

Note also that it is no longer needed to add rsyslog and syslog-ng to the
busybox dependencies: they were only there because they install the S01logging script which conflicts with the one installed by busybox. So in a way, doing it this way is cleaner than what we had before.

So yes, I agree with Thomas.

However, as I mentioned before, I see this as completely orthogonal to the
S01logging cleanup, because this series is in fact mostly about standardizing the format of the init scripts. So Carlos, if you prefer to keep it as S01logging, that's fine.

Thanks for your work on this!

Regards,
Arnout

Hello,

Well, my point was not simply for Busybox HTTP server, but for HTTP servers in general. We have S50apache and S50lighttpd for example, and possibly others as well. We have S50dropbear and S50sshd.

So the problem doesn't exist today for the Busybox HTTP server, because we don't have an init script for it (but we may add one at some point in the future). But it already exists for tons of other daemons/services.

No, see above :-)

On this, I totally agree, it is independent from Carlos work.

Best regards,

Thomas

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com