[[PATCH,RFC] 1/6] package/kmod: add modules-load init script

From: Carlos Santos <unixmania@gmail.com>

Use some scripting to mimic the systemd "modules-load" and the OpenRC
"modules" services (load kernel modules based on static configuration).

The configuration files should simply contain a list of kernel module
names to load, separated by newlines. Empty lines and lines whose first
non-whitespace character is # or ; are ignored.

The configuration directory list and file format is the same as the one
described in modules-load.d(5). Files are loaded in the following order:

  /etc/modules-load.d/*.conf
  /run/modules-load.d/*.conf
  /usr/lib/modules-load.d/*.conf

This roughly mimics the logic used by systemd but the files are not
sorted by their filename in lexicographic order as systemd does.

Notice that OpenRC uses /etc/modules-load.d/*.conf, only, and does not
ignore lines beginning with ';'.

The file redirections do the following:

- stdout is redirected to syslog with facility.level "kern.info"
- stderr is redirected to syslog with facility.level "kern.err"
- file dscriptor 4 is used to pass the result to the "start" function.

Signed-off-by: Carlos Santos <unixmania@gmail.com>

foo
---
 package/kmod/S02modules-load | 85 ++++++++++++++++++++++++++++++++++++
 package/kmod/kmod.mk         |  5 +++
 2 files changed, 90 insertions(+)
 create mode 100644 package/kmod/S02modules-load

Carlos, All,

On 2020-03-20 20:35 -0300, unixmania@gmail.com spake thusly:
> From: Carlos Santos <unixmania@gmail.com>
> Use some scripting to mimic the systemd "modules-load" and the OpenRC
> "modules" services (load kernel modules based on static configuration).

So, the goal is laudable, but the implementation is overly complicated
and, imho, flawed.

> The configuration files should simply contain a list of kernel module
> names to load, separated by newlines. Empty lines and lines whose first
> non-whitespace character is # or ; are ignored.

This is avery simple format to parse:

    for d in ${MODULES_DIRS_LIST}; do
        for f in "${d}"/*; do
            # Skip empty or missing directories, and dangling symlinks
            [ -e "${f}" ] || continue

            for m in $(sed -r -e 's/#.+//' "${f}"); do
                printf 'Loading module %s: ' "${m}"
                mpdorobe -s "${m}" && printf 'OK\n' || printf 'FAIL\n'
            done
        done
    done

No need for anything more complex than that.

Also, logger is from busybox, and a system may entirely lack busybox.
Remeber that sysvinit scripts are also used when the init system is,
well, initsysv instead of busybox.

I've marked the entire series (except patch 3 that I already appiled) as
changes requested.

Also, you made that S02. But I believe this should come after S10mdev or
S10udev, which both are going to trigger cold-plug events that may in
turn trigger module loading.

Regards,
Yann E. MORIN.

> The configuration directory list and file format is the same as the one
> described in modules-load.d(5). Files are loaded in the following order:
> 
>   /etc/modules-load.d/*.conf
>   /run/modules-load.d/*.conf
>   /usr/lib/modules-load.d/*.conf
> 
> This roughly mimics the logic used by systemd but the files are not
> sorted by their filename in lexicographic order as systemd does.
> 
> Notice that OpenRC uses /etc/modules-load.d/*.conf, only, and does not
> ignore lines beginning with ';'.
> 
> The file redirections do the following:
> 
> - stdout is redirected to syslog with facility.level "kern.info"
> - stderr is redirected to syslog with facility.level "kern.err"
> - file dscriptor 4 is used to pass the result to the "start" function.
> 
> Signed-off-by: Carlos Santos <unixmania@gmail.com>
> 
> foo
> ---
>  package/kmod/S02modules-load | 85 ++++++++++++++++++++++++++++++++++++
>  package/kmod/kmod.mk         |  5 +++
>  2 files changed, 90 insertions(+)
>  create mode 100644 package/kmod/S02modules-load
> 
> diff --git a/package/kmod/S02modules-load b/package/kmod/S02modules-load
> new file mode 100644
> index 0000000000..099cc63023
> --- /dev/null
> +++ b/package/kmod/S02modules-load
> @@ -0,0 +1,85 @@
> +#!/bin/sh
> +
> +PROGRAM="modules-load"
> +
> +MODULES_LOAD_ARGS=""
> +
> +# shellcheck source=/dev/null
> +[ -r "/etc/default/$PROGRAM" ] && . "/etc/default/$PROGRAM"
> +
> +# Use some scripting to mimic the systemd "modules-load" and the OpenRC
> +# "modules" services (load kernel modules based on static configuration).
> +# 
> +# The configuration files should simply contain a list of kernel module
> +# names to load, separated by newlines. Empty lines and lines whose first
> +# non-whitespace character is # or ; are ignored.
> +# 
> +# The configuration directory list and file format is the same as the one
> +# described in modules-load.d(5). Files are loaded in the following order:
> +# 
> +#   /etc/modules-load.d/*.conf
> +#   /run/modules-load.d/*.conf
> +#   /usr/lib/modules-load.d/*.conf
> +# 
> +# This roughly mimics the logic used by systemd but the files are not sorted
> +# by their filename in lexicographic order as systemd does.
> +# 
> +# Notice that OpenRC uses /etc/modules-load.d/*.conf, only, and does not
> +# ignore lines beginning with ';'.
> +#
> +# The file redirections do the following:
> +#
> +# - stdout is redirected to syslog with facility.level "kern.info"
> +# - stderr is redirected to syslog with facility.level "kern.err"
> +# - file dscriptor 4 is used to pass the result to the "start" function.
> +
> +MODULES_LOAD_D="/etc/modules-load.d/ /run/modules-load.d/ /usr/lib/modules-load.d/"
> +
> +# Use some scripting to mimic the systemd-modules-load utility provided by
> +# systemd, reporting results to syslog. Uers not interested on messages can
> +# put "-q" in MODULES_LOAD_ARGS.
> +
> +run_program() {
> +	# shellcheck disable=SC2086 # we need the word splitting
> +	find $MODULES_LOAD_D -maxdepth 1 -name '*.conf' -print0 2> /dev/null | \
> +	xargs -0 -r -n 1 readlink -f | {
> +		prog_status="OK"
> +		while :; do
> +			read -r file
> +			if [ -z "$file" ]; then
> +				echo "$prog_status" >&4
> +				break
> +			fi
> +			echo "* Applying $file ..."
> +			while :; do
> +				read -r mod || break
> +				case "$mod" in
> +					''|'#'*|';'*) ;;
> +					*) /sbin/modprobe -s "$mod" || prog_status="FAIL"
> +				esac
> +			done < "$file"
> +		done 2>&1 >&3 | /usr/bin/logger -t "$PROGRAM" -p kern.err
> +	} 3>&1 | /usr/bin/logger -t "$PROGRAM" -p kern.info
> +}
> +
> +start() {
> +	printf '%s %s: ' "$1" "$PROGRAM"
> +	status=$(run_program 4>&1)
> +	echo "$status"
> +	if [ "$status" = "OK" ]; then
> +		return 0
> +	fi
> +	return 1
> +}
> +
> +case "$1" in
> +	start)
> +		start "Running";;
> +	restart|reload)
> +		start "Rerunning";;
> +	stop)
> +		:;;
> +	*)
> +		echo "Usage: $0 {start|stop|restart|reload}"
> +		exit 1
> +esac
> diff --git a/package/kmod/kmod.mk b/package/kmod/kmod.mk
> index e2dfea5c7b..31bdac6d81 100644
> --- a/package/kmod/kmod.mk
> +++ b/package/kmod/kmod.mk
> @@ -60,6 +60,11 @@ else
>  KMOD_BIN_PATH = ../usr/bin/kmod
>  endif
>  
> +define KMOD_INSTALL_INIT_SYSV
> +	$(INSTALL) -m 0755 -D package/kmod/S02modules-load \
> +		$(TARGET_DIR)/etc/init.d/S02modules-load
> +endef
> +
>  define KMOD_INSTALL_TOOLS
>  	for i in depmod insmod lsmod modinfo modprobe rmmod; do \
>  		ln -sf $(KMOD_BIN_PATH) $(TARGET_DIR)/sbin/$$i; \
> -- 
> 2.18.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

On Sat, Mar 21, 2020 at 6:03 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> Carlos, All,
>
> On 2020-03-20 20:35 -0300, unixmania@gmail.com spake thusly:
> > From: Carlos Santos <unixmania@gmail.com>
> > Use some scripting to mimic the systemd "modules-load" and the OpenRC
> > "modules" services (load kernel modules based on static configuration).
>
> So, the goal is laudable, but the implementation is overly complicated
> and, imho, flawed.

I agree that it is complicated but it's definitely not flawed. It was
thoroughly tested and matches the construction already used in
S02sysctl (check commits 7ab7c6c6b2 and 398c1af1d5).

> > The configuration files should simply contain a list of kernel module
> > names to load, separated by newlines. Empty lines and lines whose first
> > non-whitespace character is # or ; are ignored.
>
> This is avery simple format to parse:
>
>     for d in ${MODULES_DIRS_LIST}; do
>         for f in "${d}"/*; do
>             # Skip empty or missing directories, and dangling symlinks
>             [ -e "${f}" ] || continue

test -e does not fail on empty files or directories:

$ : > /tmp/empty-file
$ ls -l /tmp/empty-file
-rw-rw-r--. 1 casantos casantos 0 Mar 28 18:09 /tmp/empty-file
$ test -e /tmp/empty-file && echo ok
ok
$ mkdir /tmp/empty-dir
$ test -e /tmp/empty-dir && echo ok
ok

>             for m in $(sed -r -e 's/#.+//' "${f}"); do

This goes against good shell scripting practices, as pointed by
shellcheck: "SC2013: To read lines rather than words, pipe/redirect to
a 'while read' loop."

>                 printf 'Loading module %s: ' "${m}"
>                 mpdorobe -s "${m}" && printf 'OK\n' || printf 'FAIL\n'
>             done
>         done
>     done
>
> No need for anything more complex than that.

We need something more complex than that if we care about the script robustness.

> Also, logger is from busybox, and a system may entirely lack busybox.
> Remeber that sysvinit scripts are also used when the init system is,
> well, initsysv instead of busybox.

That's unfortunate. Not being able to send the errors to syslog is
terrible on embedded systems, since usually nobody watches the console
(if any) at boot time.

S02sysctl already assumes that logger is available, so I suppose that
will need to fix it too.

> I've marked the entire series (except patch 3 that I already appiled) as
> changes requested.
>
> Also, you made that S02. But I believe this should come after S10mdev or
> S10udev, which both are going to trigger cold-plug events that may in
> turn trigger module loading.

Good catch. Thanks.