Discussion:
[PATCH v5 0/4] init scripts: rewrite S01logging
(too old to reply)
Carlos Santos
2018-11-07 00:49:08 UTC
Permalink
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 | 55 ++++++++++++++++++++++++++++
package/busybox/S02klogd | 55 ++++++++++++++++++++++++++++
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, 371 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
Carlos Santos
2018-11-07 00:49:10 UTC
Permalink
- 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 <***@datacom.com.br>
Reviewed-by: Matt Weber <***@rockwellcollins.com>
---
Supersedes: https://patchwork.ozlabs.org/patch/992672/
---
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 :-)
Changes v4->v5
- None, just series update
---
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
Carlos Santos
2018-11-07 00:49:11 UTC
Permalink
- 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 <***@datacom.com.br>
Reviewed-by: Matt Weber <***@rockwellcollins.com>
---
Supersedes: https://patchwork.ozlabs.org/patch/992676/
---
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 :-)
Changes v4->v5
- None, just series update
---
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
Carlos Santos
2018-11-07 00:49:09 UTC
Permalink
- 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 <***@datacom.com.br>
Reviewed-by: Matt Weber <***@rockwellcollins.com>
---
Supersedes: https://patchwork.ozlabs.org/patch/992673/
---
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 :-)
Changes v4->v5
- Fix typo and simplify comment in the init scripts.
---
package/busybox/S01logging | 40 ---------------------------
package/busybox/S01syslogd | 55 ++++++++++++++++++++++++++++++++++++++
package/busybox/S02klogd | 55 ++++++++++++++++++++++++++++++++++++++
package/busybox/busybox.mk | 19 +++++++------
4 files changed, 121 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..6e642a678a
--- /dev/null
+++ b/package/busybox/S01syslogd
@@ -0,0 +1,55 @@
+#!/bin/sh
+
+DAEMON="syslogd"
+PIDFILE="/var/run/$DAEMON.pid"
+
+SYSLOGD_ARGS=""
+
+# shellcheck source=/dev/null
+[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
+
+# BusyBox' syslogd does not create a pidfile, so pass "-n" in the command line
+# and use "-m" to instruct start-stop-daemon to create one.
+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..a4200cfb34
--- /dev/null
+++ b/package/busybox/S02klogd
@@ -0,0 +1,55 @@
+#!/bin/sh
+
+DAEMON="klogd"
+PIDFILE="/var/run/$DAEMON.pid"
+
+KLOGD_ARGS=""
+
+# shellcheck source=/dev/null
+[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
+
+# BusyBox' klogd does not create a pidfile, so pass "-n" in the command line
+# and use "-m" to instruct start-stop-daemon to create one.
+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
Arnout Vandecappelle
2018-12-10 22:50:40 UTC
Permalink
Post by 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.
I've finally applied the series to master since there wer no further remarks.

For this specific patch, I've added the following to the commit message:

The dependency of busybox on rsyslog and syslog-ng was only needed
because those packages also installed S01logging. Since now they no
longer install the same file, these dependencies are no longer needed.
The dependency on sysklogd is still needed since that one installs the
syslogd and klogd executables with the same name as busybox.

The -n option of syslogd/klogd is obligatory because start-stop-daemon
starts it in the background. Therefore, move it out of the
SYSLOGD_ARGS resp. KLOGD_ARGS variable so the user can no longer remove
it.

And I've made the modification mentioned below.

I do have a few more comments on the formatting of the start scripts. However,
these are more of the bikeshedding nature, and the current state is definitely
progress, that's why I've applied.

[snip]
Post by Carlos Santos
diff --git a/package/busybox/S01syslogd b/package/busybox/S01syslogd
new file mode 100644
index 0000000000..6e642a678a
--- /dev/null
+++ b/package/busybox/S01syslogd
@@ -0,0 +1,55 @@
+#!/bin/sh
+
+DAEMON="syslogd"
+PIDFILE="/var/run/$DAEMON.pid"
+
+SYSLOGD_ARGS=""> +
+# shellcheck source=/dev/null
+[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
+
+# BusyBox' syslogd does not create a pidfile, so pass "-n" in the command line
+# and use "-m" to instruct start-stop-daemon to create one.
+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" \
I think we can do better in the ordering of the options: first -S/-K, then the
identification options (-p, -x), then the other options that are common between
start and stop (-q), and finally the start-specific options (-b -m).

Note that I'm *really* bikeshedding here :-)
Post by Carlos Santos
+ -- -n $SYSLOGD_ARGS
+ status=$?
+ if [ "$status" -eq 0 ]; then
+ echo "OK"
+ else
+ echo "FAIL"
+ fi
+ return "$status"
I find this handling of $status overly verbose, but unfortunately there is no
better way to do it.
Post by Carlos Santos
+}
+
+stop() {
+ printf 'Stopping %s: ' "$DAEMON"
+ start-stop-daemon -K -q -p "$PIDFILE"
I think it would be better to add the -x option here as well. Particularly when
doing 'restart', it's possible that the daemon already had stopped (e.g. had
crashed) and that the PID is reused by a different process. -x certainly doesn't
hurt.
Post by Carlos Santos
+ status=$?
+ if [ "$status" -eq 0 ]; then
+ rm -f "$PIDFILE"
+ echo "OK"
+ else
+ echo "FAIL"
+ fi
+ return "$status"
+}
Since the start and stop are so similar, I was thinking about factoring them in
a single function (which, BTW, would be boilerplate that would be shared between
all init scripts, so could possibly be moved to a separate file).

start_stop() {
start-stop-daemon -q -p "$PIDFILE" -x "$EXE" "$@"
status=$?
if [ "$status" -eq 0 ]; then
echo "OK"
else
echo "FAIL"
fi
return "$status"
}

However, I'm not so sure if it's worth doing that change.


So, the only somewhat important remark is that I think the stop() (and reload()
if it uses start-stop-daemon) should use -x as well.
Post by Carlos Santos
+
+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
[snip]
Post by Carlos Santos
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) \
So as mentioned in the commit log, this one I didn't remove because it's still
needed for the overlapping /sbin/syslog and /sbin/klog binaries.

Regards,
Arnout
Post by Carlos Santos
- $(if $(BR2_PACKAGE_SYSLOG_NG),syslog-ng) \
$(if $(BR2_PACKAGE_SYSTEMD),systemd) \
$(if $(BR2_PACKAGE_SYSVINIT),sysvinit) \
$(if $(BR2_PACKAGE_TAR),tar) \
[snip]

Carlos Santos
2018-11-07 00:49:12 UTC
Permalink
- 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 <***@datacom.com.br>
Reviewed-by: Matt Weber <***@rockwellcollins.com>
---
Supersedes: https://patchwork.ozlabs.org/patch/992674/
---
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 :-)
Changes v4->v5
- None, just series update
---
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
Loading...