Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

community/lxcfs: Modprobe modules instead of failing when absent #4991

Closed
wants to merge 1 commit into from
Closed

community/lxcfs: Modprobe modules instead of failing when absent #4991

wants to merge 1 commit into from

Conversation

nero
Copy link
Contributor

@nero nero commented Aug 12, 2018

Previously, lxcfs failed when the kernel modules were not listed the lsmod
output. Built-in kernel modules never show up in lsmod, thus the service
could not be properly started on such systems.

Fixes #9214.

@nero nero changed the title community/lxcfs: Warn instead of hard fail when kernel modules not lo… community/lxcfs: Warn instead of hard fail when kernel modules not loaded Aug 12, 2018
@nero
Copy link
Contributor Author

nero commented Aug 12, 2018

Ping @itoffshore

@nero nero changed the title community/lxcfs: Warn instead of hard fail when kernel modules not loaded community/lxcfs: Modprobe modules instead of failing when absent Aug 12, 2018
@andypost andypost added the A-fix Fixes an abuild label Aug 13, 2018
@itoffshore
Copy link
Contributor

itoffshore commented Aug 13, 2018

needs tabs instead of spaces in the patch to fix the alignment of return 1

running git diff before making a commit shows these alignment type errors.

@nero
Copy link
Contributor Author

nero commented Aug 13, 2018

Branch updated.

@ncopa
Copy link
Contributor

ncopa commented Aug 23, 2018

I don' think this will work. modprobe will return an error when the filesystem is compiled in to kernel instead of module: modprobe: FATAL: Module ... not found in directory /lib/modules/4.14.65-0-vanilla

We need to check if the filessystem is available in kernel and try modprobe if it isnt. Something like:

diff --git a/community/lxcfs/lxcfs.initd b/community/lxcfs/lxcfs.initd
index 1a6e9c9759..496d6156fb 100644
--- a/community/lxcfs/lxcfs.initd
+++ b/community/lxcfs/lxcfs.initd
@@ -19,14 +19,11 @@ depend() {
 }
 
 start_pre() {
-       local module=
+       local filesystem=
        checkpath --directory ${VARDIR}
-       for module in fuse autofs4; do
-               if ! $(lsmod | grep -q ^$module); then
-                       eerror "Enable module: $module"
-                       eerror "modprobe $module"
-                       eerror "echo $module >> /etc/modules"
-                       eend 1
+       for filesystem in fuse autofs4; do
+               if ! grep -q -w $filesystem /proc/filesystems; then
+                       modprobe $filesystem || return 1
                fi
        done
 }

modprobe will print error message so I don't think we need print any error message with eerror.

@nero
Copy link
Contributor Author

nero commented Aug 23, 2018

@ncopa There is /lib/modules/$(uname -r)/modules.builtin.bin, which contains a list of built-in modules. Modprobe uses that as ignore list.

See second headline here:

modules.builtin

This file lists all modules that are built into the kernel. This is used
by modprobe to not fail when trying to load something builtin.

You can confirm this behavior by modprobing brd on alpine, which is not present in the modules directory:

nyu:~# modprobe brd; echo $?
0
nyu:~# find /lib/modules/$(uname -r) -name '*brd*'
nyu:~#

Previously, lxcfs failed when the kernel modules were not listed the lsmod
output. Built-in kernel modules never show up in lsmod, thus the service
could not be properly started on such systems.

Fixes #9214.
@ncopa
Copy link
Contributor

ncopa commented Aug 30, 2018

2 files changed, 3 insertions(+), 11 deletions(-)

❤️

@algitbot
Copy link

Merged in c0513a0 by @ncopa. Thanks for your contribution!

(This pull request has been closed automatically by GitHub PR Closer. If you think that it’s not resolved yet, please add a comment.)

@algitbot algitbot closed this Aug 30, 2018
@yarons
Copy link

yarons commented Aug 30, 2018

You guys ares awesome! Thanks @ncopa, I'm a big fan!

algitbot pushed a commit that referenced this pull request Feb 28, 2023
'modprobe fuse autofs4' will pass autofs4 as an argument to fuse
module loading, which is not what was intended here.
Using modprobe with -a properly loads both modules, or skips if
they were built-in, and fails if one of those was neither
built-in nor a module.

Not actually using lxc so not tested, found by manually looking at the
PR looking at autofs or fuse: both actually should be loaded
automatically by the kernel when a pre-created device is opened,
but alpine does not seem to have a way of pre-creating such devices,
so keeping the modprobe here.

Link: #4991
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-fix Fixes an abuild
Projects
None yet
6 participants