[v2,3/5] package/dmraid: use modules-load to load the kernel module

From: Carlos Santos <unixmania@gmail.com>

This is compatible with BusyBox/sysvinit (via S02modules-load), OpenRC
and systemd. It also prevents trying to load the module each time the
init script is executed.

Signed-off-by: Carlos Santos <unixmania@gmail.com>
---
Tested on QEMU with a kernel config with MD=y and DM_<FOO>=m. Only the
module loading was tested, since there is no RAID device.
---
 package/dmraid/S20dmraid |  3 ---
 package/dmraid/dmraid.mk | 12 ++++++++++++
 2 files changed, 12 insertions(+), 3 deletions(-)

CArlos, All,

On 2020-04-18 19:14 -0300, unixmania@gmail.com spake thusly:
> From: Carlos Santos <unixmania@gmail.com>
> 
> This is compatible with BusyBox/sysvinit (via S02modules-load), OpenRC
> and systemd. It also prevents trying to load the module each time the
> init script is executed.
> 
> Signed-off-by: Carlos Santos <unixmania@gmail.com>
> ---
> Tested on QEMU with a kernel config with MD=y and DM_<FOO>=m. Only the
> module loading was tested, since there is no RAID device.
> ---
>  package/dmraid/S20dmraid |  3 ---
>  package/dmraid/dmraid.mk | 12 ++++++++++++
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/package/dmraid/S20dmraid b/package/dmraid/S20dmraid
> index b3bfdcc84b..ff325154c6 100644
> --- a/package/dmraid/S20dmraid
> +++ b/package/dmraid/S20dmraid
> @@ -2,9 +2,6 @@
>  
>  set -e
>  
> -# try to load module in case that hasn't been done yet
> -modprobe dm-mod >/dev/null 2>&1
> -
>  case "$1" in
>  	start|"")
>  		echo "Setting up DMRAID devices..."
> diff --git a/package/dmraid/dmraid.mk b/package/dmraid/dmraid.mk
> index 0382cd4d99..c8facbdabd 100644
> --- a/package/dmraid/dmraid.mk
> +++ b/package/dmraid/dmraid.mk
> @@ -21,4 +21,16 @@ define DMRAID_INSTALL_INIT_SYSV
>  		$(TARGET_DIR)/etc/init.d/S20dmraid
>  endef
>  
> +ifeq ($(BR2_LINUX_KERNEL),y)
> +define DMRAID_GEN_MODULES_CONF
> +	$(INSTALL) -d -m 755 $(TARGET_DIR)/etc/modules-load.d
> +	{ \
> +		find $(TARGET_DIR)/lib/modules/$(LINUX_VERSION_PROBED)/kernel/drivers/md -name 'dm[_-]mod.ko*'; \
> +	} 2> /dev/null | { \
> +		sed 's:^.*/::; s:\.ko[^ ]*::'; \
> +	} > $(TARGET_DIR)/etc/modules-load.d/20-dmraid.conf
> +endef
> +DMRAID_TARGET_FINALIZE_HOOKS += DMRAID_GEN_MODULES_CONF
> +endif

Why do we need this complex code? Furthermore, we'll have to repeat that
complexity in all packages that want to provide modules; that's not
nice... :-(

Can't we simply have a modules table, like we have the device amd user
tables? Something like:

    define DMRAID_MODULES
        dm-stuff
        dm-blurb
        dm-blabla
    endef

And then have the package and rootfs infrastructures handle that like
the other tables?

Regards,
Yann E. MORIN.

>  $(eval $(autotools-package))
> -- 
> 2.18.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

On Sun, 19 Apr 2020 12:42:39 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> Can't we simply have a modules table, like we have the device amd user
> tables? Something like:
> 
>     define DMRAID_MODULES
>         dm-stuff
>         dm-blurb
>         dm-blabla
>     endef
> 
> And then have the package and rootfs infrastructures handle that like
> the other tables?

If we're going to do that, then we don't need an infra at all,
package/dmraid/ can simply contain a file with the list of modules,
which gets installed to /usr/lib/modules-load.d/.

I think the idea here was to avoid maintaining the list of dmraid
modules, and ensure it is generated automatically from the list of
installed modules. But even that is not guaranteed to work 100%, for
example if one day dmraid adds a module that doesn't match the
dm[_-]mod.ko* expression.

Thomas

Thomas, Carlos, All,

On 2020-04-19 14:49 +0200, Thomas Petazzoni spake thusly:
> On Sun, 19 Apr 2020 12:42:39 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> 
> > Can't we simply have a modules table, like we have the device amd user
> > tables? Something like:
> > 
> >     define DMRAID_MODULES
> >         dm-stuff
> >         dm-blurb
> >         dm-blabla
> >     endef
> > 
> > And then have the package and rootfs infrastructures handle that like
> > the other tables?
> 
> If we're going to do that, then we don't need an infra at all,
> package/dmraid/ can simply contain a file with the list of modules,
> which gets installed to /usr/lib/modules-load.d/.

Right, adding an infra just to copy a file would over-engineering.

> I think the idea here was to avoid maintaining the list of dmraid
> modules, and ensure it is generated automatically from the list of
> installed modules. But even that is not guaranteed to work 100%, for
> example if one day dmraid adds a module that doesn't match the
> dm[_-]mod.ko* expression.

But suddenly, it strikes me as odd that the dmraid package has to list
the modules built by the kernel.

Those modules should be auto-loaded on-demand, no?

For dmraid, we may load just the main module, dm-mod, as was done so
far, and leave to the user the responsibility to provide a module file
to load whatever exact module they need?

I would be OK for a long list, but this complex code repeated over and
over again for each package that needs to load modules, is just ugly to
me...

Also, this created files during the target-finalize step, and so those
files are not accounted to the package that installed them.

In the end, I guess such a functionality is mostly useful for those
out-of-tree modules that do not behave properly, and need manual
loading. In which case, those packages know what modules to list, and
they can do so during the installation step (directly or as part of a
post-install hook). Using target-finalize is not so nice...

Regards,
Yann E. MORIN.

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