[Buildroot] [PATCH v4 0/4] init scripts: rewrite S01logging

[Buildroot] [PATCH v4 0/4] init scripts: rewrite S01logging

[Carlos Santos]

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.
- Use one init script per daemom. Name the script accordding to the
  daemon it starts (e.g. S01syslogd, S02klogd).

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, sysklogd and syslog-ng support true "reload" operations).

The scripts were checked with shellcheck v0.5.0:

  $ shellcheck package/*/S0*log*

Carlos Santos (4):
  busybox: rewrite logging init script
  rsyslog: rewrite init script
  sysklogd: rewrite init script
  syslog-ng: rewrite init script

 package/busybox/S01logging     | 40 ---------------------
 package/busybox/S01syslogd     | 56 +++++++++++++++++++++++++++++
 package/busybox/S02klogd       | 56 +++++++++++++++++++++++++++++
 package/busybox/busybox.mk     | 19 +++++-----
 package/rsyslog/S01logging     | 36 -------------------
 package/rsyslog/S01rsyslogd    | 53 +++++++++++++++++++++++++++
 package/rsyslog/rsyslog.mk     |  4 +--
 package/sysklogd/S01logging    | 25 -------------
 package/sysklogd/S01syslogd    | 62 ++++++++++++++++++++++++++++++++
 package/sysklogd/S02klogd      | 65 ++++++++++++++++++++++++++++++++++
 package/sysklogd/sysklogd.mk   |  6 ++--
 package/syslog-ng/S01logging   | 38 --------------------
 package/syslog-ng/S01syslog-ng | 62 ++++++++++++++++++++++++++++++++
 package/syslog-ng/syslog-ng.mk |  4 +--
 14 files changed, 373 insertions(+), 153 deletions(-)
 delete mode 100644 package/busybox/S01logging
 create mode 100644 package/busybox/S01syslogd
 create mode 100644 package/busybox/S02klogd
 delete mode 100644 package/rsyslog/S01logging
 create mode 100644 package/rsyslog/S01rsyslogd
 delete mode 100644 package/sysklogd/S01logging
 create mode 100644 package/sysklogd/S01syslogd
 create mode 100644 package/sysklogd/S02klogd
 delete mode 100644 package/syslog-ng/S01logging
 create mode 100644 package/syslog-ng/S01syslog-ng

-- 
2.17.1

[Buildroot] [PATCH v4 1/4] busybox: rewrite logging init script

[Carlos Santos]

- Split S01logging into S01syslogd and S02klogd. Install them only if no
  other syslog package is selected and the corresponding daemons are
  selected in the Busybox configuration.
- Support /etc/default/$DAEMON configuration files.
- Detect and report start/stop errors (previous version ignored them and
  always reported OK).
- Use a separate function for restart.
- Implement reload as restart.

Signed-off-by: Carlos Santos <casantos@datacom.com.br>
---
Changes v1->v2
- Implement suggestions made by Nicolas Cavallari and Arnout Vandecappelle
Changes v2->v3
- None, just series update
Changes v3-v4
- Follow the decisions taken at the Buildroot meeting.
- Leave the daemon args themselves on a separate line, as suggested by Arnout
  Vandecappelle.
- Drop Matt Weber's Reviewed-by, since there were too many changes since then.
- Use a less fancy commit message :-)
---
 package/busybox/S01logging | 40 ---------------------------
 package/busybox/S01syslogd | 56 ++++++++++++++++++++++++++++++++++++++
 package/busybox/S02klogd   | 56 ++++++++++++++++++++++++++++++++++++++
 package/busybox/busybox.mk | 19 +++++++------
 4 files changed, 123 insertions(+), 48 deletions(-)
 delete mode 100644 package/busybox/S01logging
 create mode 100644 package/busybox/S01syslogd
 create mode 100644 package/busybox/S02klogd

diff --git a/package/busybox/S01logging b/package/busybox/S01logging
deleted file mode 100644
index fcb3e7d236..0000000000
--- a/package/busybox/S01logging
+++ /dev/null
@@ -1,40 +0,0 @@
-#!/bin/sh
-#
-# Start logging
-#
-
-SYSLOGD_ARGS=-n
-KLOGD_ARGS=-n
-[ -r /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 -- $SYSLOGD_ARGS
-	start-stop-daemon -b -S -q -m -p /var/run/klogd.pid --exec /sbin/klogd -- $KLOGD_ARGS
-	echo "OK"
-}
-
-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"
-}
-
-case "$1" in
-  start)
-	start
-	;;
-  stop)
-	stop
-	;;
-  restart|reload)
-	stop
-	start
-	;;
-  *)
-	echo "Usage: $0 {start|stop|restart|reload}"
-	exit 1
-esac
-
-exit $?
diff --git a/package/busybox/S01syslogd b/package/busybox/S01syslogd
new file mode 100644
index 0000000000..66bc4e609b
--- /dev/null
+++ b/package/busybox/S01syslogd
@@ -0,0 +1,56 @@
+#!/bin/sh
+
+DAEMON="syslogd"
+PIDFILE="/var/run/$DAEMON.pid"
+
+SYSLOGD_ARGS=""
+
+# shellcheck source=/dev/null
+[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
+
+# 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 %s: ' "$DAEMON"
+	# shellcheck disable=SC2086 # we need the word splitting
+	start-stop-daemon -b -m -S -q -p "$PIDFILE" -x "/sbin/$DAEMON" \
+		-- -n $SYSLOGD_ARGS
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
+stop() {
+	printf 'Stopping %s: ' "$DAEMON"
+	start-stop-daemon -K -q -p "$PIDFILE"
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		rm -f "$PIDFILE"
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
+restart() {
+	stop
+	sleep 1
+	start
+}
+
+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
diff --git a/package/busybox/S02klogd b/package/busybox/S02klogd
new file mode 100644
index 0000000000..1d2684148a
--- /dev/null
+++ b/package/busybox/S02klogd
@@ -0,0 +1,56 @@
+#!/bin/sh
+
+DAEMON="klogd"
+PIDFILE="/var/run/$DAEMON.pid"
+
+KLOGD_ARGS=""
+
+# shellcheck source=/dev/null
+[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
+
+# 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 %s: ' "$DAEMON"
+	# shellcheck disable=SC2086 # we need the word splitting
+	start-stop-daemon -b -m -S -q -p "$PIDFILE" -x "/sbin/$DAEMON" \
+		-- -n $KLOGD_ARGS
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
+stop() {
+	printf 'Stopping %s: ' "$DAEMON"
+	start-stop-daemon -K -q -p "$PIDFILE"
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		rm -f "$PIDFILE"
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
+restart() {
+	stop
+	sleep 1
+	start
+}
+
+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
diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index 757086592f..028ca1905c 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -55,10 +55,7 @@ BUSYBOX_DEPENDENCIES = \
 	$(if $(BR2_PACKAGE_PCIUTILS),pciutils) \
 	$(if $(BR2_PACKAGE_PROCPS_NG),procps-ng) \
 	$(if $(BR2_PACKAGE_PSMISC),psmisc) \
-	$(if $(BR2_PACKAGE_RSYSLOGD),rsyslog) \
 	$(if $(BR2_PACKAGE_START_STOP_DAEMON),start-stop-daemon) \
-	$(if $(BR2_PACKAGE_SYSKLOGD),sysklogd) \
-	$(if $(BR2_PACKAGE_SYSLOG_NG),syslog-ng) \
 	$(if $(BR2_PACKAGE_SYSTEMD),systemd) \
 	$(if $(BR2_PACKAGE_SYSVINIT),sysvinit) \
 	$(if $(BR2_PACKAGE_TAR),tar) \
@@ -245,15 +242,21 @@ define BUSYBOX_INSTALL_INDIVIDUAL_BINARIES
 endef
 endif
 
-# Only install our own if no other package already did.
+# Only install our logging scripts if no other package does it.
+ifeq ($(BR2_PACKAGE_SYSKLOGD)$(BR2_PACKAGE_RSYSLOG)$(BR2_PACKAGE_SYSLOG_NG),)
 define BUSYBOX_INSTALL_LOGGING_SCRIPT
-	if grep -q CONFIG_SYSLOGD=y $(@D)/.config && \
-		[ ! -e $(TARGET_DIR)/etc/init.d/S01logging ]; \
+	if grep -q CONFIG_SYSLOGD=y $(@D)/.config; \
 	then \
-		$(INSTALL) -m 0755 -D package/busybox/S01logging \
-			$(TARGET_DIR)/etc/init.d/S01logging; \
+		$(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
 
 ifeq ($(BR2_INIT_BUSYBOX),y)
 define BUSYBOX_INSTALL_INITTAB
-- 
2.17.1

[Buildroot] [PATCH v4 2/4] rsyslog: rewrite init script

[Carlos Santos]

- Rename it to S01rsyslogd to make every init script be called the same
  as the executable it starts.
- Support a /etc/default/rsyslogd configuration file.
- Indent with tabs, not spaces.

Signed-off-by: Carlos Santos <casantos@datacom.com.br>
---
Changes v1->v2
- Implement suggestions made by Nicolas Cavallari and Arnout Vandecappelle
Changes v2->v3
- Include /etc/default/logging, not /etc/default/$DAEMON.
Changes v3-v4
- Follow the decisions taken at the Buildroot meeting.
- Leave the daemon args themselves on a separate line, as suggested by
  Arnout Vandecappelle.
- Use a less fancy commit message :-)
---
 package/rsyslog/S01logging  | 36 -------------------------
 package/rsyslog/S01rsyslogd | 53 +++++++++++++++++++++++++++++++++++++
 package/rsyslog/rsyslog.mk  |  4 +--
 3 files changed, 55 insertions(+), 38 deletions(-)
 delete mode 100644 package/rsyslog/S01logging
 create mode 100644 package/rsyslog/S01rsyslogd

diff --git a/package/rsyslog/S01logging b/package/rsyslog/S01logging
deleted file mode 100644
index 8e4a59c2d5..0000000000
--- a/package/rsyslog/S01logging
+++ /dev/null
@@ -1,36 +0,0 @@
-#!/bin/sh
-
-start() {
-  printf "Starting rsyslog daemon: "
-  start-stop-daemon -S -q -p /var/run/rsyslogd.pid --exec /usr/sbin/rsyslogd
-  [ $? = 0 ] && echo "OK" || echo "FAIL"
-}
-
-stop() {
-  printf "Stopping rsyslog daemon: "
-  start-stop-daemon -K -q -p /var/run/rsyslogd.pid
-  [ $? = 0 ] && echo "OK" || echo "FAIL"
-}
-
-restart() {
-  stop
-  sleep 1
-  start
-}
-
-case "$1" in
-  start)
-    start
-    ;;
-  stop)
-    stop
-    ;;
-  restart|reload)
-    restart
-    ;;
-  *)
-    echo "Usage: $0 {start|stop|restart}"
-    exit 1
-esac
-
-exit $?
diff --git a/package/rsyslog/S01rsyslogd b/package/rsyslog/S01rsyslogd
new file mode 100644
index 0000000000..e9922d932d
--- /dev/null
+++ b/package/rsyslog/S01rsyslogd
@@ -0,0 +1,53 @@
+#!/bin/sh
+
+DAEMON="rsyslogd"
+PIDFILE="/var/run/$DAEMON.pid"
+
+RSYSLOGD_ARGS=""
+
+# shellcheck source=/dev/null
+[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
+
+start() {
+	printf 'Starting %s: ' "$DAEMON"
+	# shellcheck disable=SC2086 # we need the word splitting
+	start-stop-daemon -S -q -p "$PIDFILE" -x "/usr/sbin/$DAEMON" \
+		-- $RSYSLOGD_ARGS
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
+stop() {
+	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
+}
+
+case "$1" in
+        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
diff --git a/package/rsyslog/rsyslog.mk b/package/rsyslog/rsyslog.mk
index 61e08ba765..fcd476cee3 100644
--- a/package/rsyslog/rsyslog.mk
+++ b/package/rsyslog/rsyslog.mk
@@ -72,8 +72,8 @@ RSYSLOG_CONF_OPTS += \
 endif
 
 define RSYSLOG_INSTALL_INIT_SYSV
-	$(INSTALL) -m 0755 -D package/rsyslog/S01logging \
-		$(TARGET_DIR)/etc/init.d/S01logging
+	$(INSTALL) -m 0755 -D package/rsyslog/S01rsyslogd \
+		$(TARGET_DIR)/etc/init.d/S01rsyslogd
 endef
 
 # The rsyslog.service is installed by rsyslog, but the link is not created
-- 
2.17.1

[Buildroot] [PATCH v4 3/4] sysklogd: rewrite init script

[Carlos Santos]

- Split it into S01syslogd and S02klogd to make every init script be
  called the same as the executable it starts.
- Implement start, stop, restart and reload as functions, like in other
  init 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 /etc/default/$DAEMON configuration files.
- Do not kill syslogd in "reload". Send a SIGHUP signal, instructing it
  to perform a re-initialization.
- Do not kill klogd in "reload". Send a signal (default 0, which does
  nothing).  Users can configure this signal in /etc/default/klogd to
  either SIGUSR1 or SIGUSR2.

Signed-off-by: Carlos Santos <casantos@datacom.com.br>
---
Changes v1->v2
- Implement suggestions made by Arnout Vandecappelle
Changes v2->v3
- Include /etc/default/logging, not /etc/default/$DAEMON.
Changes v3-v4
- Follow the decisions taken at the Buildroot meeting.
- Leave the daemon args themselves on a separate line, as suggested by
  Arnout Vandecappelle.
- Use a less fancy commit message :-)
---
 package/sysklogd/S01logging  | 25 --------------
 package/sysklogd/S01syslogd  | 62 ++++++++++++++++++++++++++++++++++
 package/sysklogd/S02klogd    | 65 ++++++++++++++++++++++++++++++++++++
 package/sysklogd/sysklogd.mk |  6 ++--
 4 files changed, 131 insertions(+), 27 deletions(-)
 delete mode 100644 package/sysklogd/S01logging
 create mode 100644 package/sysklogd/S01syslogd
 create mode 100644 package/sysklogd/S02klogd

diff --git a/package/sysklogd/S01logging b/package/sysklogd/S01logging
deleted file mode 100644
index 1cbfe869fa..0000000000
--- a/package/sysklogd/S01logging
+++ /dev/null
@@ -1,25 +0,0 @@
-#!/bin/sh
-
-case "$1" in
-	start)
-		printf "Starting logging: "
-		/sbin/syslogd -m 0
-		/sbin/klogd
-		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`
-		echo "OK"
-		;;
-	restart|reload)
-		$0 stop
-		$0 start
-		;;
-	*)
-		echo "Usage: $0 {start|stop|restart}"
-		exit 1
-esac
-
-exit $?
diff --git a/package/sysklogd/S01syslogd b/package/sysklogd/S01syslogd
new file mode 100644
index 0000000000..d0951f0235
--- /dev/null
+++ b/package/sysklogd/S01syslogd
@@ -0,0 +1,62 @@
+#!/bin/sh
+
+DAEMON="syslogd"
+PIDFILE="/var/run/$DAEMON.pid"
+
+SYSLOGD_ARGS="-m 0"
+
+# shellcheck source=/dev/null
+[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
+
+start() {
+	printf 'Starting %s: ' "$DAEMON"
+	# shellcheck disable=SC2086 # we need the word splitting
+	start-stop-daemon -S -q -p "$PIDFILE" -x "/sbin/$DAEMON" \
+		-- $SYSLOGD_ARGS
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
+stop() {
+	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
+}
+
+# SIGHUP makes syslogd reload its configuration
+reload() {
+	printf 'Reloading %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|stop|restart|reload)
+                "$1";;
+        *)
+                echo "Usage: $0 {start|stop|restart|reload}"
+                exit 1
+esac
diff --git a/package/sysklogd/S02klogd b/package/sysklogd/S02klogd
new file mode 100644
index 0000000000..93f39e1f0e
--- /dev/null
+++ b/package/sysklogd/S02klogd
@@ -0,0 +1,65 @@
+#!/bin/sh
+
+DAEMON="klogd"
+PIDFILE="/var/run/$DAEMON.pid"
+
+KLOGD_ARGS=""
+
+KLOGD_RELOAD="0"
+
+# shellcheck source=/dev/null
+[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
+
+start() {
+	printf 'Starting %s: ' "$DAEMON"
+	# shellcheck disable=SC2086 # we need the word splitting
+	start-stop-daemon -S -q -p "$PIDFILE" -x "/sbin/$DAEMON" \
+		-- $KLOGD_ARGS
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
+stop() {
+	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
+}
+
+# SIGUSR1 makes klogd reload kernel module symbols
+# SIGUSR2 makes klogd reload static kernel symbols and kernel module symbols
+reload() {
+	printf 'Reloading %s: ' "$DAEMON"
+	start-stop-daemon -K -s "$KLOGD_RELOAD" -q -p "$PIDFILE"
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
+case "$1" in
+        start|stop|restart|reload)
+                "$1";;
+        *)
+                echo "Usage: $0 {start|stop|restart|reload}"
+                exit 1
+esac
diff --git a/package/sysklogd/sysklogd.mk b/package/sysklogd/sysklogd.mk
index c4f064c10b..976438c110 100644
--- a/package/sysklogd/sysklogd.mk
+++ b/package/sysklogd/sysklogd.mk
@@ -23,8 +23,10 @@ define SYSKLOGD_INSTALL_TARGET_CMDS
 endef
 
 define SYSKLOGD_INSTALL_INIT_SYSV
-	$(INSTALL) -m 755 -D package/sysklogd/S01logging \
-		$(TARGET_DIR)/etc/init.d/S01logging
+	$(INSTALL) -m 755 -D package/sysklogd/S01syslogd \
+		$(TARGET_DIR)/etc/init.d/S01syslogd
+	$(INSTALL) -m 755 -D package/sysklogd/S02klogd \
+		$(TARGET_DIR)/etc/init.d/S02klogd
 endef
 
 define SYSKLOGD_INSTALL_INIT_SYSTEMD
-- 
2.17.1

[Buildroot] [PATCH v4 4/4] syslog-ng: rewrite init script

[Carlos Santos]

- Rename it to S01syslog-ng to make every init script be called the same
  as the executable it starts.
- 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 /etc/default/syslog-ng configuration file.

Signed-off-by: Carlos Santos <casantos@datacom.com.br>
---
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.
Changes v3-v4:
- Follow the decisions taken at the Buildroot meeting.
- Leave the daemon args themselves on a separate line, as suggested by
  Arnout Vandecappelle.
- Use a less fancy commit message :-)
---
 package/syslog-ng/S01logging   | 38 ---------------------
 package/syslog-ng/S01syslog-ng | 62 ++++++++++++++++++++++++++++++++++
 package/syslog-ng/syslog-ng.mk |  4 +--
 3 files changed, 64 insertions(+), 40 deletions(-)
 delete mode 100644 package/syslog-ng/S01logging
 create mode 100644 package/syslog-ng/S01syslog-ng

diff --git a/package/syslog-ng/S01logging b/package/syslog-ng/S01logging
deleted file mode 100644
index d7c899a1e3..0000000000
--- a/package/syslog-ng/S01logging
+++ /dev/null
@@ -1,38 +0,0 @@
-#!/bin/sh
-
-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"
-}
-
-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"
-}
-
-restart() {
-	stop
-	sleep 1
-	start
-}
-
-case "$1" in
-	start)
-		start
-		;;
-	stop)
-		stop
-		;;
-	restart|reload)
-		restart
-		;;
-	*)
-		echo "Usage: $0 {start|stop|restart}"
-		exit 1
-esac
-
-exit $?
diff --git a/package/syslog-ng/S01syslog-ng b/package/syslog-ng/S01syslog-ng
new file mode 100644
index 0000000000..1d74bf8839
--- /dev/null
+++ b/package/syslog-ng/S01syslog-ng
@@ -0,0 +1,62 @@
+#!/bin/sh
+
+DAEMON="syslog-ng"
+PIDFILE="/var/run/$DAEMON.pid"
+
+SYSLOG_NG_ARGS=""
+
+# shellcheck source=/dev/null
+[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
+
+start() {
+	printf 'Starting %s: ' "$DAEMON"
+	# shellcheck disable=SC2086 # we need the word splitting
+	start-stop-daemon -S -q -p "$PIDFILE" -x "/usr/sbin/$DAEMON" \
+		-- $SYSLOG_NG_ARGS
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
+stop() {
+	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
+}
+
+# SIGHUP makes syslog-ng reload its configuration
+reload() {
+	printf 'Reloading %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|stop|restart|reload)
+                "$1";;
+        *)
+                echo "Usage: $0 {start|stop|restart|reload}"
+                exit 1
+esac
diff --git a/package/syslog-ng/syslog-ng.mk b/package/syslog-ng/syslog-ng.mk
index 793fea0972..29e284a4bf 100644
--- a/package/syslog-ng/syslog-ng.mk
+++ b/package/syslog-ng/syslog-ng.mk
@@ -93,8 +93,8 @@ SYSLOG_NG_CONF_OPTS += --disable-systemd
 endif
 
 define SYSLOG_NG_INSTALL_INIT_SYSV
-	$(INSTALL) -m 0755 -D package/syslog-ng/S01logging \
-		$(TARGET_DIR)/etc/init.d/S01logging
+	$(INSTALL) -m 0755 -D package/syslog-ng/S01syslog-ng \
+		$(TARGET_DIR)/etc/init.d/S01syslog-ng
 endef
 
 # By default syslog-ng installs a number of sample configuration
-- 
2.17.1

[Buildroot] [PATCH v4 1/4] busybox: rewrite logging init script

[Matthew Weber]

Carlos,

On Sat, Nov 3, 2018 at 1:24 PM Carlos Santos <casantos@datacom.com.br> wrote:
>
> - Split S01logging into S01syslogd and S02klogd. Install them only if no
>   other syslog package is selected and the corresponding daemons are
>   selected in the Busybox configuration.
> - Support /etc/default/$DAEMON configuration files.
> - Detect and report start/stop errors (previous version ignored them and
>   always reported OK).
> - Use a separate function for restart.
> - Implement reload as restart.
>
> Signed-off-by: Carlos Santos <casantos@datacom.com.br>
> ---
> Changes v1->v2
> - Implement suggestions made by Nicolas Cavallari and Arnout Vandecappelle
> Changes v2->v3
> - None, just series update
> Changes v3-v4
> - Follow the decisions taken at the Buildroot meeting.
> - Leave the daemon args themselves on a separate line, as suggested by Arnout
>   Vandecappelle.
> - Drop Matt Weber's Reviewed-by, since there were too many changes since then.
> - Use a less fancy commit message :-)
> ---
>  package/busybox/S01logging | 40 ---------------------------
>  package/busybox/S01syslogd | 56 ++++++++++++++++++++++++++++++++++++++
>  package/busybox/S02klogd   | 56 ++++++++++++++++++++++++++++++++++++++
>  package/busybox/busybox.mk | 19 +++++++------
>  4 files changed, 123 insertions(+), 48 deletions(-)
>  delete mode 100644 package/busybox/S01logging
>  create mode 100644 package/busybox/S01syslogd
>  create mode 100644 package/busybox/S02klogd
>
> diff --git a/package/busybox/S01logging b/package/busybox/S01logging
> deleted file mode 100644
> index fcb3e7d236..0000000000
> --- a/package/busybox/S01logging
> +++ /dev/null

I can see this naming change will cause a number of people to end up
debugging their targets as a custom S01logging will be copied over and
then the installs of the new files with different names below.  Is
there any thoughts on making this a visible or obvious change so users
don't get caught up?


> @@ -1,40 +0,0 @@
> -#!/bin/sh
> -#
> -# Start logging
> -#
> -
> -SYSLOGD_ARGS=-n
> -KLOGD_ARGS=-n
> -[ -r /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 -- $SYSLOGD_ARGS
> -       start-stop-daemon -b -S -q -m -p /var/run/klogd.pid --exec /sbin/klogd -- $KLOGD_ARGS
> -       echo "OK"
> -}
> -
> -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"
> -}
> -
> -case "$1" in
> -  start)
> -       start
> -       ;;
> -  stop)
> -       stop
> -       ;;
> -  restart|reload)
> -       stop
> -       start
> -       ;;
> -  *)
> -       echo "Usage: $0 {start|stop|restart|reload}"
> -       exit 1
> -esac
> -
> -exit $?
> diff --git a/package/busybox/S01syslogd b/package/busybox/S01syslogd
> new file mode 100644
> index 0000000000..66bc4e609b
> --- /dev/null
> +++ b/package/busybox/S01syslogd
> @@ -0,0 +1,56 @@
> +#!/bin/sh
> +
> +DAEMON="syslogd"
> +PIDFILE="/var/run/$DAEMON.pid"
> +
> +SYSLOGD_ARGS=""
> +
> +# shellcheck source=/dev/null
> +[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
> +
> +# 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.

sylogd -> syslogd

Since this script only handles syslogd now, assuming maybe the text
should only reference that daemon? (nit)

> +start() {
> +       printf 'Starting %s: ' "$DAEMON"
> +       # shellcheck disable=SC2086 # we need the word splitting
> +       start-stop-daemon -b -m -S -q -p "$PIDFILE" -x "/sbin/$DAEMON" \
> +               -- -n $SYSLOGD_ARGS
> +       status=$?
> +       if [ "$status" -eq 0 ]; then
> +               echo "OK"
> +       else
> +               echo "FAIL"
> +       fi
> +       return "$status"
> +}
> +
> +stop() {
> +       printf 'Stopping %s: ' "$DAEMON"
> +       start-stop-daemon -K -q -p "$PIDFILE"
> +       status=$?
> +       if [ "$status" -eq 0 ]; then
> +               rm -f "$PIDFILE"
> +               echo "OK"
> +       else
> +               echo "FAIL"
> +       fi
> +       return "$status"
> +}
> +
> +restart() {
> +       stop
> +       sleep 1
> +       start
> +}
> +
> +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
> diff --git a/package/busybox/S02klogd b/package/busybox/S02klogd
> new file mode 100644
> index 0000000000..1d2684148a
> --- /dev/null
> +++ b/package/busybox/S02klogd
> @@ -0,0 +1,56 @@
> +#!/bin/sh
> +
> +DAEMON="klogd"
> +PIDFILE="/var/run/$DAEMON.pid"
> +
> +KLOGD_ARGS=""
> +
> +# shellcheck source=/dev/null
> +[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
> +
> +# 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.

Same spelling and nit mentioned above.

> +start() {
> +       printf 'Starting %s: ' "$DAEMON"
> +       # shellcheck disable=SC2086 # we need the word splitting
> +       start-stop-daemon -b -m -S -q -p "$PIDFILE" -x "/sbin/$DAEMON" \
> +               -- -n $KLOGD_ARGS
> +       status=$?
> +       if [ "$status" -eq 0 ]; then
> +               echo "OK"
> +       else
> +               echo "FAIL"
> +       fi
> +       return "$status"
> +}
> +
> +stop() {
> +       printf 'Stopping %s: ' "$DAEMON"
> +       start-stop-daemon -K -q -p "$PIDFILE"
> +       status=$?
> +       if [ "$status" -eq 0 ]; then
> +               rm -f "$PIDFILE"
> +               echo "OK"
> +       else
> +               echo "FAIL"
> +       fi
> +       return "$status"
> +}
> +
> +restart() {

What's the impact of someone restarting syslogd without stoping klogd first?

> +       stop
> +       sleep 1
> +       start
> +}
> +
> +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
> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
> index 757086592f..028ca1905c 100644
> --- a/package/busybox/busybox.mk
> +++ b/package/busybox/busybox.mk
> @@ -55,10 +55,7 @@ BUSYBOX_DEPENDENCIES = \
>         $(if $(BR2_PACKAGE_PCIUTILS),pciutils) \
>         $(if $(BR2_PACKAGE_PROCPS_NG),procps-ng) \
>         $(if $(BR2_PACKAGE_PSMISC),psmisc) \
> -       $(if $(BR2_PACKAGE_RSYSLOGD),rsyslog) \
>         $(if $(BR2_PACKAGE_START_STOP_DAEMON),start-stop-daemon) \
> -       $(if $(BR2_PACKAGE_SYSKLOGD),sysklogd) \
> -       $(if $(BR2_PACKAGE_SYSLOG_NG),syslog-ng) \
>         $(if $(BR2_PACKAGE_SYSTEMD),systemd) \
>         $(if $(BR2_PACKAGE_SYSVINIT),sysvinit) \
>         $(if $(BR2_PACKAGE_TAR),tar) \
> @@ -245,15 +242,21 @@ define BUSYBOX_INSTALL_INDIVIDUAL_BINARIES
>  endef
>  endif
>
> -# Only install our own if no other package already did.
> +# Only install our logging scripts if no other package does it.
> +ifeq ($(BR2_PACKAGE_SYSKLOGD)$(BR2_PACKAGE_RSYSLOG)$(BR2_PACKAGE_SYSLOG_NG),)
>  define BUSYBOX_INSTALL_LOGGING_SCRIPT
> -       if grep -q CONFIG_SYSLOGD=y $(@D)/.config && \
> -               [ ! -e $(TARGET_DIR)/etc/init.d/S01logging ]; \
> +       if grep -q CONFIG_SYSLOGD=y $(@D)/.config; \
>         then \
> -               $(INSTALL) -m 0755 -D package/busybox/S01logging \
> -                       $(TARGET_DIR)/etc/init.d/S01logging; \
> +               $(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
>
>  ifeq ($(BR2_INIT_BUSYBOX),y)
>  define BUSYBOX_INSTALL_INITTAB
> --
> 2.17.1
>

Reviewed-by: Matt Weber <matthew.weber@rockwellcollins.com>

[Buildroot] [PATCH v4 2/4] rsyslog: rewrite init script

[Matthew Weber]

Carlos,

On Sat, Nov 3, 2018 at 1:24 PM Carlos Santos <casantos@datacom.com.br> wrote:
>
> - Rename it to S01rsyslogd to make every init script be called the same
>   as the executable it starts.
> - Support a /etc/default/rsyslogd configuration file.
> - Indent with tabs, not spaces.
>
> Signed-off-by: Carlos Santos <casantos@datacom.com.br>
> ---
> Changes v1->v2
> - Implement suggestions made by Nicolas Cavallari and Arnout Vandecappelle
> Changes v2->v3
> - Include /etc/default/logging, not /etc/default/$DAEMON.
> Changes v3-v4
> - Follow the decisions taken at the Buildroot meeting.
> - Leave the daemon args themselves on a separate line, as suggested by
>   Arnout Vandecappelle.
> - Use a less fancy commit message :-)
> ---
>  package/rsyslog/S01logging  | 36 -------------------------
>  package/rsyslog/S01rsyslogd | 53 +++++++++++++++++++++++++++++++++++++
>  package/rsyslog/rsyslog.mk  |  4 +--
>  3 files changed, 55 insertions(+), 38 deletions(-)
>  delete mode 100644 package/rsyslog/S01logging
>  create mode 100644 package/rsyslog/S01rsyslogd
>
> diff --git a/package/rsyslog/S01logging b/package/rsyslog/S01logging
> deleted file mode 100644
> index 8e4a59c2d5..0000000000
> --- a/package/rsyslog/S01logging
> +++ /dev/null
> @@ -1,36 +0,0 @@
> -#!/bin/sh
> -
> -start() {
> -  printf "Starting rsyslog daemon: "
> -  start-stop-daemon -S -q -p /var/run/rsyslogd.pid --exec /usr/sbin/rsyslogd
> -  [ $? = 0 ] && echo "OK" || echo "FAIL"
> -}
> -
> -stop() {
> -  printf "Stopping rsyslog daemon: "
> -  start-stop-daemon -K -q -p /var/run/rsyslogd.pid
> -  [ $? = 0 ] && echo "OK" || echo "FAIL"
> -}
> -
> -restart() {
> -  stop
> -  sleep 1
> -  start
> -}
> -
> -case "$1" in
> -  start)
> -    start
> -    ;;
> -  stop)
> -    stop
> -    ;;
> -  restart|reload)
> -    restart
> -    ;;
> -  *)
> -    echo "Usage: $0 {start|stop|restart}"
> -    exit 1
> -esac
> -
> -exit $?
> diff --git a/package/rsyslog/S01rsyslogd b/package/rsyslog/S01rsyslogd
> new file mode 100644
> index 0000000000..e9922d932d
> --- /dev/null
> +++ b/package/rsyslog/S01rsyslogd
> @@ -0,0 +1,53 @@
> +#!/bin/sh
> +
> +DAEMON="rsyslogd"
> +PIDFILE="/var/run/$DAEMON.pid"
> +
> +RSYSLOGD_ARGS=""
> +
> +# shellcheck source=/dev/null
> +[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
> +
> +start() {
> +       printf 'Starting %s: ' "$DAEMON"
> +       # shellcheck disable=SC2086 # we need the word splitting
> +       start-stop-daemon -S -q -p "$PIDFILE" -x "/usr/sbin/$DAEMON" \
> +               -- $RSYSLOGD_ARGS
> +       status=$?
> +       if [ "$status" -eq 0 ]; then
> +               echo "OK"
> +       else
> +               echo "FAIL"
> +       fi
> +       return "$status"
> +}
> +
> +stop() {
> +       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
> +}
> +
> +case "$1" in
> +        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
> diff --git a/package/rsyslog/rsyslog.mk b/package/rsyslog/rsyslog.mk
> index 61e08ba765..fcd476cee3 100644
> --- a/package/rsyslog/rsyslog.mk
> +++ b/package/rsyslog/rsyslog.mk
> @@ -72,8 +72,8 @@ RSYSLOG_CONF_OPTS += \
>  endif
>
>  define RSYSLOG_INSTALL_INIT_SYSV
> -       $(INSTALL) -m 0755 -D package/rsyslog/S01logging \
> -               $(TARGET_DIR)/etc/init.d/S01logging
> +       $(INSTALL) -m 0755 -D package/rsyslog/S01rsyslogd \
> +               $(TARGET_DIR)/etc/init.d/S01rsyslogd

Reviewed-by: Matt Weber <matthew.weber@rockwellcollins.com>

[Buildroot] [PATCH v4 3/4] sysklogd: rewrite init script

[Matthew Weber]

Carlos,

On Sat, Nov 3, 2018 at 1:24 PM Carlos Santos <casantos@datacom.com.br> wrote:
>
> - Split it into S01syslogd and S02klogd to make every init script be
>   called the same as the executable it starts.
> - Implement start, stop, restart and reload as functions, like in other
>   init 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 /etc/default/$DAEMON configuration files.
> - Do not kill syslogd in "reload". Send a SIGHUP signal, instructing it
>   to perform a re-initialization.
> - Do not kill klogd in "reload". Send a signal (default 0, which does
>   nothing).  Users can configure this signal in /etc/default/klogd to
>   either SIGUSR1 or SIGUSR2.
>
> Signed-off-by: Carlos Santos <casantos@datacom.com.br>
> ---
> Changes v1->v2
> - Implement suggestions made by Arnout Vandecappelle
> Changes v2->v3
> - Include /etc/default/logging, not /etc/default/$DAEMON.
> Changes v3-v4
> - Follow the decisions taken at the Buildroot meeting.
> - Leave the daemon args themselves on a separate line, as suggested by
>   Arnout Vandecappelle.
> - Use a less fancy commit message :-)
> ---
>  package/sysklogd/S01logging  | 25 --------------
>  package/sysklogd/S01syslogd  | 62 ++++++++++++++++++++++++++++++++++
>  package/sysklogd/S02klogd    | 65 ++++++++++++++++++++++++++++++++++++
>  package/sysklogd/sysklogd.mk |  6 ++--
>  4 files changed, 131 insertions(+), 27 deletions(-)
>  delete mode 100644 package/sysklogd/S01logging
>  create mode 100644 package/sysklogd/S01syslogd
>  create mode 100644 package/sysklogd/S02klogd
>
> diff --git a/package/sysklogd/S01logging b/package/sysklogd/S01logging
> deleted file mode 100644
> index 1cbfe869fa..0000000000
> --- a/package/sysklogd/S01logging
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -#!/bin/sh
> -
> -case "$1" in
> -       start)
> -               printf "Starting logging: "
> -               /sbin/syslogd -m 0
> -               /sbin/klogd
> -               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`
> -               echo "OK"
> -               ;;
> -       restart|reload)
> -               $0 stop
> -               $0 start
> -               ;;
> -       *)
> -               echo "Usage: $0 {start|stop|restart}"
> -               exit 1
> -esac
> -
> -exit $?
> diff --git a/package/sysklogd/S01syslogd b/package/sysklogd/S01syslogd
> new file mode 100644
> index 0000000000..d0951f0235
> --- /dev/null
> +++ b/package/sysklogd/S01syslogd
> @@ -0,0 +1,62 @@
> +#!/bin/sh
> +
> +DAEMON="syslogd"
> +PIDFILE="/var/run/$DAEMON.pid"
> +
> +SYSLOGD_ARGS="-m 0"
> +
> +# shellcheck source=/dev/null
> +[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
> +
> +start() {
> +       printf 'Starting %s: ' "$DAEMON"
> +       # shellcheck disable=SC2086 # we need the word splitting
> +       start-stop-daemon -S -q -p "$PIDFILE" -x "/sbin/$DAEMON" \
> +               -- $SYSLOGD_ARGS
> +       status=$?
> +       if [ "$status" -eq 0 ]; then
> +               echo "OK"
> +       else
> +               echo "FAIL"
> +       fi
> +       return "$status"
> +}
> +
> +stop() {
> +       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

Similar question to the busybox syslog vs klog.  What happens if you
restart the syslog daemon with klogd not stopped?  Do we need to
handle that case with stopping it or printing a warning?

> +       sleep 1
> +       start
> +}
> +
> +# SIGHUP makes syslogd reload its configuration
> +reload() {
> +       printf 'Reloading %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|stop|restart|reload)
> +                "$1";;
> +        *)
> +                echo "Usage: $0 {start|stop|restart|reload}"
> +                exit 1
> +esac
> diff --git a/package/sysklogd/S02klogd b/package/sysklogd/S02klogd
> new file mode 100644
> index 0000000000..93f39e1f0e
> --- /dev/null
> +++ b/package/sysklogd/S02klogd
> @@ -0,0 +1,65 @@
> +#!/bin/sh
> +
> +DAEMON="klogd"
> +PIDFILE="/var/run/$DAEMON.pid"
> +
> +KLOGD_ARGS=""
> +
> +KLOGD_RELOAD="0"
> +
> +# shellcheck source=/dev/null
> +[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
> +
> +start() {
> +       printf 'Starting %s: ' "$DAEMON"
> +       # shellcheck disable=SC2086 # we need the word splitting
> +       start-stop-daemon -S -q -p "$PIDFILE" -x "/sbin/$DAEMON" \
> +               -- $KLOGD_ARGS
> +       status=$?
> +       if [ "$status" -eq 0 ]; then
> +               echo "OK"
> +       else
> +               echo "FAIL"
> +       fi
> +       return "$status"
> +}
> +
> +stop() {
> +       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
> +}
> +
> +# SIGUSR1 makes klogd reload kernel module symbols
> +# SIGUSR2 makes klogd reload static kernel symbols and kernel module symbols
> +reload() {
> +       printf 'Reloading %s: ' "$DAEMON"
> +       start-stop-daemon -K -s "$KLOGD_RELOAD" -q -p "$PIDFILE"
> +       status=$?
> +       if [ "$status" -eq 0 ]; then
> +               echo "OK"
> +       else
> +               echo "FAIL"
> +       fi
> +       return "$status"
> +}
> +
> +case "$1" in
> +        start|stop|restart|reload)
> +                "$1";;
> +        *)
> +                echo "Usage: $0 {start|stop|restart|reload}"
> +                exit 1
> +esac
> diff --git a/package/sysklogd/sysklogd.mk b/package/sysklogd/sysklogd.mk
> index c4f064c10b..976438c110 100644
> --- a/package/sysklogd/sysklogd.mk
> +++ b/package/sysklogd/sysklogd.mk
> @@ -23,8 +23,10 @@ define SYSKLOGD_INSTALL_TARGET_CMDS
>  endef
>
>  define SYSKLOGD_INSTALL_INIT_SYSV
> -       $(INSTALL) -m 755 -D package/sysklogd/S01logging \
> -               $(TARGET_DIR)/etc/init.d/S01logging
> +       $(INSTALL) -m 755 -D package/sysklogd/S01syslogd \
> +               $(TARGET_DIR)/etc/init.d/S01syslogd
> +       $(INSTALL) -m 755 -D package/sysklogd/S02klogd \
> +               $(TARGET_DIR)/etc/init.d/S02klogd

So by default the user may take a busybox based target and get the
logging daemons from busybox's default config.  Then they choose to go
enable one of the other logging options but don't understand they need
to make a custom busybox config.  For sysklogd, the install would just
copy over the init scripts busybox installed, however do we need to
handle this case with rsyslog(S02klogd would be left in the target
from busybox) and syslog-ng(both S01/S02 would be left) ?  Maybe a
check for existance and exit with warning that the busybox
configuration needs to have logging disabled....

Reviewed-by: Matt Weber <matthew.weber@rockwellcollins.com>

[Buildroot] [PATCH v4 3/4] sysklogd: rewrite init script

[Arnout Vandecappelle]

On 05/11/18 15:07, Matthew Weber wrote:
> So by default the user may take a busybox based target and get the
> logging daemons from busybox's default config.  Then they choose to go
> enable one of the other logging options but don't understand they need
> to make a custom busybox config.  For sysklogd, the install would just
> copy over the init scripts busybox installed, however do we need to
> handle this case with rsyslog(S02klogd would be left in the target
> from busybox) and syslog-ng(both S01/S02 would be left) ?  Maybe a
> check for existance and exit with warning that the busybox
> configuration needs to have logging disabled....

 That is handled in busybox.mk:

+# Only install our logging scripts if no other package does it.
+ifeq ($(BR2_PACKAGE_SYSKLOGD)$(BR2_PACKAGE_RSYSLOG)$(BR2_PACKAGE_SYSLOG_NG),)
 define BUSYBOX_INSTALL_LOGGING_SCRIPT


 Regards,
 Arnout

[Buildroot] [PATCH v4 1/4] busybox: rewrite logging init script

[Arnout Vandecappelle]

On 05/11/18 14:57, Matthew Weber wrote:

> I can see this naming change will cause a number of people to end up
> debugging their targets as a custom S01logging will be copied over and
> then the installs of the new files with different names below.  Is
> there any thoughts on making this a visible or obvious change so users
> don't get caught up?

 The best we can doe is to mention it in the release notes.

[snip]
> What's the impact of someone restarting syslogd without stoping klogd first?

 That shouldn't affect klogd, as far as I understand, because klogd uses
standard syslog().

 Regards,
 Arnout

[Buildroot] [PATCH v4 1/4] busybox: rewrite logging init script

[Carlos Santos]

> From: "Matthew Weber" <matthew.weber@rockwellcollins.com>
> To: "DATACOM" <casantos@datacom.com.br>
> Cc: "buildroot" <buildroot@buildroot.org>, "ratbert90" <aduskett@gmail.com>, "Chris Packham" <judge.packham@gmail.com>
> Sent: Segunda-feira, 5 de novembro de 2018 11:57:03
> Subject: Re: [PATCH v4 1/4] busybox: rewrite logging init script

> Carlos,
[...]
> 
> I can see this naming change will cause a number of people to end up
> debugging their targets as a custom S01logging will be copied over and
> then the installs of the new files with different names below.  Is
> there any thoughts on making this a visible or obvious change so users
> don't get caught up?

I can't imagine a mechanism do this but I'm open to suggestions. It's hard
to foresee all changes that users make on the default build by means of
custom rootfs skeletons, private packages, post-{build,image} scripts and
so on.

Of course the change deserves to be mentioned in the release notes.

[...]

>> +# 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.
> 
> sylogd -> syslogd
> 
> Since this script only handles syslogd now, assuming maybe the text
> should only reference that daemon? (nit)

Yes. Thanks.

> Reviewed-by: Matt Weber <matthew.weber@rockwellcollins.com>

-- 
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

[Buildroot] [PATCH v4 1/4] busybox: rewrite logging init script

[Carlos Santos]

> From: "Arnout Vandecappelle" <arnout@mind.be>
> To: "Matthew Weber" <matthew.weber@rockwellcollins.com>, "DATACOM" <casantos@datacom.com.br>
> Cc: "ratbert90" <aduskett@gmail.com>, "buildroot" <buildroot@buildroot.org>
> Sent: Ter?a-feira, 6 de novembro de 2018 22:14:42
> Subject: Re: [Buildroot] [PATCH v4 1/4] busybox: rewrite logging init script

> On 05/11/18 14:57, Matthew Weber wrote:
> 
>> I can see this naming change will cause a number of people to end up
>> debugging their targets as a custom S01logging will be copied over and
>> then the installs of the new files with different names below.  Is
>> there any thoughts on making this a visible or obvious change so users
>> don't get caught up?
> 
> The best we can doe is to mention it in the release notes.
> 
> [snip]
>> What's the impact of someone restarting syslogd without stoping klogd first?
> 
> That shouldn't affect klogd, as far as I understand, because klogd uses
> standard syslog().

In the case of a restart the klog messages (all syslog() calls, in fact)
will be sent to the system console for a short period.

-- 
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

[Buildroot] [PATCH v4 3/4] sysklogd: rewrite init script

[Matthew Weber]

Arnout,

On Tue, Nov 6, 2018 at 6:08 PM Arnout Vandecappelle <arnout@mind.be> wrote:
>
>
>
> On 05/11/18 15:07, Matthew Weber wrote:
> > So by default the user may take a busybox based target and get the
> > logging daemons from busybox's default config.  Then they choose to go
> > enable one of the other logging options but don't understand they need
> > to make a custom busybox config.  For sysklogd, the install would just
> > copy over the init scripts busybox installed, however do we need to
> > handle this case with rsyslog(S02klogd would be left in the target
> > from busybox) and syslog-ng(both S01/S02 would be left) ?  Maybe a
> > check for existance and exit with warning that the busybox
> > configuration needs to have logging disabled....
>
>  That is handled in busybox.mk:
>
> +# Only install our logging scripts if no other package does it.
> +ifeq ($(BR2_PACKAGE_SYSKLOGD)$(BR2_PACKAGE_RSYSLOG)$(BR2_PACKAGE_SYSLOG_NG),)
>  define BUSYBOX_INSTALL_LOGGING_SCRIPT
>
>

Agree, just more of a "if you don't do a clean build" what questions
will hit the mailing list and how could we prevent that.....

Matt

[Buildroot] [PATCH v4 1/4] busybox: rewrite logging init script

[Carlos Santos]

Superseded-by: https://patchwork.ozlabs.org/patch/994015/

[Buildroot] [PATCH v4 2/4] rsyslog: rewrite init script

[Carlos Santos]

Superseded-by: https://patchwork.ozlabs.org/patch/994013/

[Buildroot] [PATCH v4 3/4] sysklogd: rewrite init script

[Carlos Santos]

Superseded-by: https://patchwork.ozlabs.org/patch/994014/

[Buildroot] [PATCH v4 4/4] syslog-ng: rewrite init script

[Carlos Santos]

Superseded-by: https://patchwork.ozlabs.org/patch/994016/