Re: [Buildroot] [PATCH 2/2] package/busybox: Add ifplugd init script

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Fiona,

On Wed,  5 Jun 2024 13:13:47 +0200
Fiona Klute via buildroot <buildroot@buildroot.org> wrote:

> The init script will be installed only if no other package provides
> ifplugd (currently that could be package/ifplugd). Users can configure
> the interface(s) which ifplugd should watch either using
> /etc/default/ifplugd (for one interface) or using per-interface
> symlinks to the script in /etc/init.d (for any number of
> interfaces). The action script that ifplugd runs when a link change is
> detected must be supplied separately.
> 
> Signed-off-by: Fiona Klute <fiona.klute+wiwa@gmx.de>

I applied your patch, with some changes. See below.

> ---
> make check-package complains about "Incorrect PIDFILE value" in the
> script. To support running multiple instances I need to support
> multiple (per-interface) PID files. Is there a better way to handle
> that, or explicitly silence the check?

Silence the check. See below :-)

> 
>  package/busybox/S41ifplugd | 84 ++++++++++++++++++++++++++++++++++++++
>  package/busybox/busybox.mk | 13 ++++++
>  2 files changed, 97 insertions(+)
>  create mode 100644 package/busybox/S41ifplugd
> 
> diff --git a/package/busybox/S41ifplugd b/package/busybox/S41ifplugd
> new file mode 100644
> index 0000000000..4d3b56ab6d
> --- /dev/null
> +++ b/package/busybox/S41ifplugd
> @@ -0,0 +1,84 @@
> +#!/bin/sh
> +
> +DAEMON="ifplugd"
> +
> +# Each ifplugd instance handles only one interface, so this script is
> +# designed to be symlinked per interface. For each interface create a
> +# symlink with .IFACE appended to the name. E.g. to launch ifplugd for
> +# eth1 create a symlink from /etc/init.d/S41ifplugd.eth1 to this
> +# script. DEFAULT_IFACE sets the interface the non-symlink script will
> +# use, set it to empty in /etc/default/ifplugd to disable the default
> +# instance and use symlinked instances only.
> +DEFAULT_IFACE="eth0"
> +# If your action script is not in the default location
> +# /etc/ifplugd/ifplugd.action, use the "-r" option to set the
> +# location.
> +IFPLUGD_ARGS="-M"
> +
> +# shellcheck source=/dev/null
> +[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
> +
> +NAME_IFACE="$(basename "$0" | cut -s -d. -f2)"
> +if [ -n "$NAME_IFACE" ]; then
> +	IFACE="${NAME_IFACE}"
> +elif [ -n "$DEFAULT_IFACE" ]; then
> +	IFACE="${DEFAULT_IFACE}"
> +else
> +	# no interface configured
> +	exit 0
> +fi
> +
> +PIDFILE="/var/run/${DAEMON}.${IFACE}.pid"

I've added:

+# check-package disable Variables

above this line to ignore the PIDFILE warning.

> +# BusyBox' ifplugd 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 for %s: ' "$DAEMON" "$IFACE"
> +	# shellcheck disable=SC2086 # we need the word splitting
> +	start-stop-daemon -b -m -S -q -p "$PIDFILE" -x "/usr/sbin/$DAEMON" \
> +		-- -n $IFPLUGD_ARGS

And I fixed the script to use the long options.

> +	while true; do
> +		pid="$( cat "${PIDFILE}" 2>/dev/null || true )"
> +		{ [ -n "${pid}" ] && [ -d "/proc/${pid}" ]; } || break
> +		sleep 0.1
> +	done

And the start-stop-daemon loop.

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot