#909 PR merged: always load modules in /etc/modules (issue905 and others)

Labels: bug, fixed / solved / done

jsmeix opened issue at 2016-07-11 12:38:

Tries to fix https://github.com/rear/rear/commit/0ec871676b7cdc52461574200c8a54867d0cfb37
by changing 40-start-udev-or-load-modules.sh
so taht the code that always loads modules
in /etc/modules is always run.

jsmeix commented at 2016-07-11 12:52:

Now it works well for me.

In particulat I verified with the additional "debug" kernel
command line parameter for the rear recovery system that
now 40-start-udev-or-load-modules.sh runs the code
that loads all modules in /etc/moidules.

I just merge it because I think it is now at least
somewhat better than before (even if it is perhaps
not yet fully perfect).

jsmeix commented at 2016-07-12 15:01:

There is a small thing left where I don't know
how it is actually meant to work:

At the end of 40-start-udev-or-load-modules.sh
I left the "modprobe -q dm-mod" call as is
which means this modprobe call is now also always
done because it does no longer simply "return" in
the "systemd-udevd case" but now it runs through
to the end of the whole script:

RESCUE f121:~ # lsmod | grep dm
dm_mod                110780  0 

I only assume this modprobe call was also meant
to be always done but I don't know.

jsmeix commented at 2016-07-13 10:08:

git forensic via

git log -p usr/share/rear/skel/default/etc/scripts/system-setup.d/40-start-udev-or-load-modules.sh

shows that the unconditioned "modprobe -q dm-mod" call is there
at the very end since 40-start-udev-or-load-modules.sh
was created (i.e. before the "systemd-udevd case" was
added on top of it) which indicates that the "modprobe -q dm-mod"
call is really meant to be done in any case (I assume when
the "systemd-udevd case" was added on top of it it was
forgotten to deal with the "modprobe -q dm-mod" call).

FYI:

git blame -w usr/share/rear/skel/default/etc/scripts/system-setup.d/40-start-udev-or-load-modules.sh

shows it was this git commit: 844d50b75ac4b7722f4fee7a5ee3350b93f3adb7

jsmeix commented at 2017-06-20 09:12:

@schlomo
many thanks for your question!

I think now I understand the reason for my confusion in
https://github.com/rear/rear/issues/626#issuecomment-125125263
(excerpt)

In gereral I noticed at that time that in the rear recovery system
I had more than 100 kernel modules loaded while in my original
system I had something about 40 kernel modules loaded.
From my point of view this indicates that there is something
wrong in general in the rear recovery system that it has much
more kernel modules loaded than in the original system...

I don't know a mandatory reason why the explicitly specified modules
must be loaded after the udev automatism.
My change was only meant to get the explicitly specified modules
loaded at all.
I did not intend to change the odering of what happens when in
skel/default/etc/scripts/system-setup.d/40-start-udev-or-load-modules.sh
so that I simply added loading modules from /etc/modules
below the already existing udev loading code.

But I could imagine a reason why the explicitly specified modules
may sometimes have to be loaded after the udev automatism:

If the explicitly specified modules were loaded before the
automated udev loading, the explicitly specified modules list
(i.e. the MODULES_LOAD array) may have to be
"somewhat complete" which means it may need to specify
all needed modules from the very beginning in the right ordering.

In contrast when the udev automatism runs first
the basic stuff should be automatically loaded so that
the MODULES_LOAD array could contain only
additionally needed modules.

I think in theory the module dependencies should
always automatically load all needed modules from the
very beginning in the right ordering
but perhaps in practice some module dependencies
are implemented only via udev rules nowadays?

Currently usr/share/rear/conf/default.conf
is very terse about how MODULES_LOAD is meant:

# Enforce to load these modules in the given order in the rescue/recovery system:
MODULES_LOAD=()

Currently it only means that MODULES_LOAD=( foo bar )
will load first 'foo' and then 'bar' but it does not tell
if that also means that 'foo' will be the very first module
that gets loaded in the rescue/recovery system.

I think to make MODULES_LOAD really powerful
its meaning should be that the modules that are
specified therein will be the very first modules
that get loaded in the rescue/recovery system.

Furthermore I would like to have a switch so that the
user can - if needed - skip the automated udev module
loading for example via a special module name like

MODULES_LOAD=( 'foo' 'bar' 'skip_automated_udev_module_loading' )

but this makes implementation a bit awkward because I
would have to check everywhere for that special value.
Perhaps I prefer a separated config variable to
skip the automated udev module loading.

Because I think I know now the root cause why the loaded
modules in the recovery system do not match the ones
in the original system, I will fix it and clean it up and
enhance it so that - if needed - the user has the final
power to specify exactly what modules will be loaded
in what ordering in the recovery system.

schlomo commented at 2017-06-20 09:34:

@jsmeix please wait with your effort till I am done with my changes in https://github.com/rear/rear/tree/schlomo-2017-06 (and feel free to look there and comment). Maybe you don't need to do anything at all.

I already took out one sort which should fix the ordering.

In my experience modules auto-load their dependencies without problems. In this example I had mptspi in the MODULES_LOAD array and it auto-loaded all the other SCSI and MPT modules.

I would really prefer to keep things simple here. That means that we use MODULES_LOAD to really mean load these modules before anything else in this order which is my current implementation. This also most closely reflects what the distros do with the modules inside their initrd. They are also loaded before the main system comes up and loads udev or systemd-udev. Therefore I don't see any need to make MODULES_LOAD contain some magic before and after. And I for sure wouldn't build that without a specific use case where our current code does not work.

I am actually much more worried about us supporting systemd-modules-load.service properly. Do we start that? Does systemd start it when we copy it into the rescue system? Unfortunately a quick look did not enlighten me on these questions, our boot process is already too complex.

FYI, on my Ubuntu 17.04 /etc/modules is also loaded by systemd-modules-load via a symlink:

$ ll /etc/modules-load.d/
total 8
drwxr-xr-x 1 root root   58 Jun 15 17:48 ./
drwxr-xr-x 1 root root 4558 Jun 20 10:25 ../
-rw-r--r-- 1 root root  119 Mär 21 19:36 cups-filters.conf
lrwxrwxrwx 1 root root   10 Apr 13 19:10 modules.conf -> ../modules

jsmeix commented at 2017-06-20 09:49:

I am afraid I know basically nothing at all about
systemd related things in the recovery system.

I fully agree to load the MODULES_LOAD modules
before anything else in this order.

I watch your changes in
https://github.com/rear/rear/compare/schlomo-2017-06
and I am in particular happy how loading the specified
modules first in any case makes the code simpler
(no longer duplicated code) and better!

jsmeix commented at 2017-06-20 09:58:

@schlomo
I think in your
usr/share/rear/build/GNU/Linux/400_copy_modules.sh

if ! grep -q $module_to_be_loaded $recovery_system_etc_modules ; then

should be more specific to avoid that substrings match,
e.g. think about module names like 'parport' and 'parport_pc'.
When 'parport_pc' is already in $recovery_system_etc_modules
no 'parport' would be added because the substring 'parport'
matches 'parport_pc'.

schlomo commented at 2017-06-20 10:05:

excellent catch - thanks a lot! (please ignore the last commit there, it was wrong and I pulled it back).


[Export of Github issue for rear/rear.]