| Message ID | b878a01f09e250bb24dbaede71cc776217a8f862.1689681454.git.msuchanek@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kmod,v5,1/5] configure: Detect openssl sm3 support | expand |
On Tue, Jul 18, 2023 at 02:01:56PM +0200, Michal Suchanek wrote: >Now that modprobe.d is searched under ${prefix}/lib, allow a complete >transition to files only under ${prefix} by adding a ${module_directory} >configuration. This specifies the directory where to search for kernel >modules and should match the location where the kernel/distro installs >them. > >With this distributions that do not want to ship files in /lib can also >move kernel modules to /usr while others can keep them in /lib. > >Signed-off-by: Michal Suchanek <msuchanek@suse.de> >--- >v4: Make the whole path configurable >v5: More verbose commit message >--- > Makefile.am | 3 +- > configure.ac | 7 ++ > libkmod/libkmod.c | 4 +- > man/Makefile.am | 1 + > man/depmod.d.xml | 6 +- > man/depmod.xml | 4 +- > man/modinfo.xml | 2 +- > man/modprobe.xml | 2 +- > man/modules.dep.xml | 6 +- > testsuite/module-playground/Makefile | 2 +- > testsuite/setup-rootfs.sh | 109 +++++++++++++++------------ > testsuite/test-depmod.c | 16 ++-- > testsuite/test-testsuite.c | 8 +- > tools/depmod.c | 6 +- > tools/kmod.pc.in | 1 + > tools/modinfo.c | 4 +- > tools/modprobe.c | 4 +- > tools/static-nodes.c | 6 +- > 18 files changed, 107 insertions(+), 84 deletions(-) > >diff --git a/Makefile.am b/Makefile.am >index 6d0b2decfef3..019aa749fdf1 100644 >--- a/Makefile.am >+++ b/Makefile.am >@@ -20,6 +20,7 @@ AM_CPPFLAGS = \ > -I$(top_srcdir) \ > -DSYSCONFDIR=\""$(sysconfdir)"\" \ > -DDISTCONFDIR=\""$(distconfdir)"\" \ >+ -DMODULE_DIRECTORY=\""$(module_directory)"\" \ > ${zlib_CFLAGS} > > AM_CFLAGS = $(OUR_CFLAGS) >@@ -220,7 +221,7 @@ EXTRA_DIST += testsuite/setup-rootfs.sh > MODULE_PLAYGROUND = testsuite/module-playground > ROOTFS = testsuite/rootfs > ROOTFS_PRISTINE = $(top_srcdir)/testsuite/rootfs-pristine >-CREATE_ROOTFS = $(AM_V_GEN) $(top_srcdir)/testsuite/setup-rootfs.sh $(ROOTFS_PRISTINE) $(ROOTFS) $(MODULE_PLAYGROUND) $(top_builddir)/config.h $(sysconfdir) >+CREATE_ROOTFS = $(AM_V_GEN) MODULE_DIRECTORY=$(module_directory) $(top_srcdir)/testsuite/setup-rootfs.sh $(ROOTFS_PRISTINE) $(ROOTFS) $(MODULE_PLAYGROUND) $(top_builddir)/config.h $(sysconfdir) > > build-module-playground: > $(AM_V_GEN)if test "$(top_srcdir)" != "$(top_builddir)"; then \ >diff --git a/configure.ac b/configure.ac >index b4584d6cdc67..4051dc9249e2 100644 >--- a/configure.ac >+++ b/configure.ac >@@ -91,6 +91,12 @@ AC_ARG_WITH([rootlibdir], > [], [with_rootlibdir=$libdir]) > AC_SUBST([rootlibdir], [$with_rootlibdir]) > >+# Ideally this would be $prefix/lib/modules but default to /lib/modules for compatibility with earlier versions >+AC_ARG_WITH([module_directory], >+ AS_HELP_STRING([--with-module-directory=DIR], [directory in which to look for kernel modules - typically '/lib/modules' or '${prefix}/lib/modules']), >+ [], [with_module_directory=/lib/modules]) >+AC_SUBST([module_directory], [$with_module_directory]) we will probably have "fun" results if we accept a relative path here. >+ > AC_ARG_WITH([zstd], > AS_HELP_STRING([--with-zstd], [handle Zstandard-compressed modules @<:@default=disabled@:>@]), > [], [with_zstd=no]) >@@ -326,6 +332,7 @@ AC_MSG_RESULT([ > $PACKAGE $VERSION > ======= > >+ module_directory: ${module_directory} > prefix: ${prefix} > sysconfdir: ${sysconfdir} > distconfdir: ${distconfdir} >diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c >index 09e6041461b0..63719e886de8 100644 >--- a/libkmod/libkmod.c >+++ b/libkmod/libkmod.c >@@ -209,7 +209,7 @@ static int log_priority(const char *priority) > return 0; > } > >-static const char *dirname_default_prefix = "/lib/modules"; >+static const char *dirname_default_prefix = MODULE_DIRECTORY; > > static char *get_kernel_release(const char *dirname) > { >@@ -231,7 +231,7 @@ static char *get_kernel_release(const char *dirname) > /** > * kmod_new: > * @dirname: what to consider as linux module's directory, if NULL >- * defaults to /lib/modules/`uname -r`. If it's relative, >+ * defaults to ${module_prefix}/lib/modules/`uname -r`. If it's relative, module_prefix? did you mean to use $MODULE_DIRECTORY/`uname -r`? > * it's treated as relative to the current working directory. > * Otherwise, give an absolute dirname. > * @config_paths: ordered array of paths (directories or files) where >diff --git a/man/Makefile.am b/man/Makefile.am >index 2fea8e46bf2f..f550091a216a 100644 >--- a/man/Makefile.am >+++ b/man/Makefile.am >@@ -22,6 +22,7 @@ CLEANFILES = $(dist_man_MANS) > else \ > sed -e '/@DISTCONFDIR@/d' $< ; \ > fi | \ >+ sed -e 's|@MODULE_DIRECTORY@|$(module_directory)|g' | \ > $(XSLT) \ > -o $@ \ > --nonet \ >diff --git a/man/depmod.d.xml b/man/depmod.d.xml >index f282a39cc840..b07e6a2bd4fe 100644 >--- a/man/depmod.d.xml >+++ b/man/depmod.d.xml >@@ -70,7 +70,7 @@ > </term> > <listitem> > <para> >- This allows you to specify the order in which /lib/modules >+ This allows you to specify the order in which @MODULE_DIRECTORY@ > (or other configured module location) subdirectories will > be processed by <command>depmod</command>. Directories are > listed in order, with the highest priority given to the >@@ -101,7 +101,7 @@ > <command>depmod</command> command. It is possible to > specify one kernel or all kernels using the * wildcard. > <replaceable>modulesubdirectory</replaceable> is the >- name of the subdirectory under /lib/modules (or other >+ name of the subdirectory under @MODULE_DIRECTORY@ (or other > module location) where the target module is installed. > </para> > <para> >@@ -110,7 +110,7 @@ > specifying the following command: "override kmod * extra". > This will ensure that any matching module name installed > under the <command>extra</command> subdirectory within >- /lib/modules (or other module location) will take priority >+ @MODULE_DIRECTORY@ (or other module location) will take priority > over any likenamed module already provided by the kernel. > </para> > </listitem> >diff --git a/man/depmod.xml b/man/depmod.xml >index 3b0097184fd7..fce2a4a67a89 100644 >--- a/man/depmod.xml >+++ b/man/depmod.xml >@@ -80,7 +80,7 @@ > </para> > <para> <command>depmod</command> creates a list of module dependencies by > reading each module under >- <filename>/lib/modules/</filename><replaceable>version</replaceable> and >+ <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable> and > determining what symbols it exports and what symbols it needs. By > default, this list is written to <filename>modules.dep</filename>, and a > binary hashed version named <filename>modules.dep.bin</filename>, in the >@@ -141,7 +141,7 @@ > <listitem> > <para> > If your modules are not currently in the (normal) directory >- <filename>/lib/modules/</filename><replaceable>version</replaceable>, >+ <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable>, > but in a staging area, you can specify a > <replaceable>basedir</replaceable> which is prepended to the > directory name. This <replaceable>basedir</replaceable> is >diff --git a/man/modinfo.xml b/man/modinfo.xml >index 9fe0324a2527..b6c4d6045829 100644 >--- a/man/modinfo.xml >+++ b/man/modinfo.xml >@@ -54,7 +54,7 @@ > <command>modinfo</command> extracts information from the Linux Kernel > modules given on the command line. If the module name is not a filename, > then the >- <filename>/lib/modules/</filename><replaceable>version</replaceable> >+ <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable> > directory is searched, as is also done by > <citerefentry><refentrytitle>modprobe</refentrytitle><manvolnum>8</manvolnum></citerefentry> > when loading kernel modules. >diff --git a/man/modprobe.xml b/man/modprobe.xml >index 91f9e27997cd..4d1fd59c000b 100644 >--- a/man/modprobe.xml >+++ b/man/modprobe.xml >@@ -78,7 +78,7 @@ > is no difference between _ and - in module names (automatic > underscore conversion is performed). > <command>modprobe</command> looks in the module directory >- <filename>/lib/modules/`uname -r`</filename> for all >+ <filename>@MODULE_DIRECTORY@/`uname -r`</filename> for all > the modules and other files, except for the optional > configuration files in the > <filename>/etc/modprobe.d</filename> directory >diff --git a/man/modules.dep.xml b/man/modules.dep.xml >index ed633694ec9e..8ef6d8b3536e 100644 >--- a/man/modules.dep.xml >+++ b/man/modules.dep.xml >@@ -34,8 +34,8 @@ > </refnamediv> > > <refsynopsisdiv> >- <para><filename>/lib/modules/modules.dep</filename></para> >- <para><filename>/lib/modules/modules.dep.bin</filename></para> >+ <para><filename>@MODULE_DIRECTORY@/modules.dep</filename></para> >+ <para><filename>@MODULE_DIRECTORY@/modules.dep.bin</filename></para> > </refsynopsisdiv> > > <refsect1><title>DESCRIPTION</title> >@@ -43,7 +43,7 @@ > <filename>modules.dep.bin</filename> is a binary file generated by > <command>depmod</command> listing the dependencies for > every module in the directories under >- <filename>/lib/modules/</filename><replaceable>version</replaceable>. >+ <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable>. > It is used by kmod tools such as <command>modprobe</command> and > libkmod. > </para> >diff --git a/testsuite/module-playground/Makefile b/testsuite/module-playground/Makefile >index e6045b0dd932..a7ab09bea2bf 100644 >--- a/testsuite/module-playground/Makefile >+++ b/testsuite/module-playground/Makefile >@@ -47,7 +47,7 @@ endif > > else > # normal makefile >-KDIR ?= /lib/modules/`uname -r`/build >+KDIR ?= $(module_prefix)/lib/modules/`uname -r`/build > KVER ?= `uname -r` > ifeq ($(FAKE_BUILD),) > FAKE_BUILD=0 >diff --git a/testsuite/setup-rootfs.sh b/testsuite/setup-rootfs.sh >index 4440ddcd6b4d..a780f9381b3c 100755 >--- a/testsuite/setup-rootfs.sh >+++ b/testsuite/setup-rootfs.sh >@@ -16,6 +16,19 @@ create_rootfs() { > cp -r "$ROOTFS_PRISTINE" "$ROOTFS" > find "$ROOTFS" -type d -exec chmod +w {} \; > find "$ROOTFS" -type f -name .gitignore -exec rm -f {} \; >+ if [ "$MODULE_DIRECTORY" != "/lib/modules" ] ; then >+ sed -i -e "s|/lib/modules|$MODULE_DIRECTORY|g" $(find "$ROOTFS" -name \*.txt -o -name \*.conf -o -name \*.dep) >+ sed -i -e "s|$MODULE_DIRECTORY/external|/lib/modules/external|g" $(find "$ROOTFS" -name \*.txt -o -name \*.conf -o -name \*.dep) >+ for i in "$ROOTFS"/*/lib/modules/* "$ROOTFS"/*/*/lib/modules/* ; do >+ version="$(basename $i)" >+ [ $version != 'external' ] || continue >+ mod="$(dirname $i)" >+ lib="$(dirname $mod)" >+ up="$(dirname $lib)$MODULE_DIRECTORY" >+ mkdir -p "$up" >+ mv "$i" "$up" >+ done >+ fi > > if [ "$SYSCONFDIR" != "/etc" ]; then > find "$ROOTFS" -type d -name etc -printf "%h\n" | while read -r e; do >@@ -32,57 +45,57 @@ feature_enabled() { > > declare -A map > map=( >- ["test-depmod/search-order-simple/lib/modules/4.4.4/kernel/crypto/"]="mod-simple.ko" >- ["test-depmod/search-order-simple/lib/modules/4.4.4/updates/"]="mod-simple.ko" >- ["test-depmod/search-order-same-prefix/lib/modules/4.4.4/foo/"]="mod-simple.ko" >- ["test-depmod/search-order-same-prefix/lib/modules/4.4.4/foobar/"]="mod-simple.ko" >- ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko" >- ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko" >- ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-c.ko"]="mod-loop-c.ko" >- ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-d.ko"]="mod-loop-d.ko" >- ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-e.ko"]="mod-loop-e.ko" >- ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-f.ko"]="mod-loop-f.ko" >- ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-g.ko"]="mod-loop-g.ko" >- ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-h.ko"]="mod-loop-h.ko" >- ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-i.ko"]="mod-loop-i.ko" >- ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-j.ko"]="mod-loop-j.ko" >- ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-k.ko"]="mod-loop-k.ko" >- ["test-depmod/search-order-external-first/lib/modules/4.4.4/foo/"]="mod-simple.ko" >- ["test-depmod/search-order-external-first/lib/modules/4.4.4/foobar/"]="mod-simple.ko" >+ ["test-depmod/search-order-simple$MODULE_DIRECTORY/4.4.4/kernel/crypto/"]="mod-simple.ko" >+ ["test-depmod/search-order-simple$MODULE_DIRECTORY/4.4.4/updates/"]="mod-simple.ko" >+ ["test-depmod/search-order-same-prefix$MODULE_DIRECTORY/4.4.4/foo/"]="mod-simple.ko" >+ ["test-depmod/search-order-same-prefix$MODULE_DIRECTORY/4.4.4/foobar/"]="mod-simple.ko" >+ ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko" >+ ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko" >+ ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-c.ko"]="mod-loop-c.ko" >+ ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-d.ko"]="mod-loop-d.ko" >+ ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-e.ko"]="mod-loop-e.ko" >+ ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-f.ko"]="mod-loop-f.ko" >+ ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-g.ko"]="mod-loop-g.ko" >+ ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-h.ko"]="mod-loop-h.ko" >+ ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-i.ko"]="mod-loop-i.ko" >+ ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-j.ko"]="mod-loop-j.ko" >+ ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-k.ko"]="mod-loop-k.ko" >+ ["test-depmod/search-order-external-first$MODULE_DIRECTORY/4.4.4/foo/"]="mod-simple.ko" >+ ["test-depmod/search-order-external-first$MODULE_DIRECTORY/4.4.4/foobar/"]="mod-simple.ko" > ["test-depmod/search-order-external-first/lib/modules/external/"]="mod-simple.ko" why didn't you change it here? >- ["test-depmod/search-order-external-last/lib/modules/4.4.4/foo/"]="mod-simple.ko" >- ["test-depmod/search-order-external-last/lib/modules/4.4.4/foobar/"]="mod-simple.ko" >+ ["test-depmod/search-order-external-last$MODULE_DIRECTORY/4.4.4/foo/"]="mod-simple.ko" >+ ["test-depmod/search-order-external-last$MODULE_DIRECTORY/4.4.4/foobar/"]="mod-simple.ko" > ["test-depmod/search-order-external-last/lib/modules/external/"]="mod-simple.ko" and here... Otherwise looks good to me. Pushed the first 4 patches. thanks Lucas De Marchi
On Tuesday 2023-10-17 19:50, Lucas De Marchi wrote: >> +AC_ARG_WITH([module_directory], >> + AS_HELP_STRING([--with-module-directory=DIR], [directory in which to >> look for kernel modules - typically '/lib/modules' or >> '${prefix}/lib/modules']), >> + [], [with_module_directory=/lib/modules]) >> +AC_SUBST([module_directory], [$with_module_directory]) > > we will probably have "fun" results if we accept a relative path here. $ ./configure --prefix=/usr --bindir=../bin configure: error: expected an absolute directory name for --bindir: ../bin While such check does not exist for --with-module-directory, everyone has likely been well-trained not to use relative paths. Even if, say, cmake/meson *could* do it, it just would have never occurred to me to actually *utilize* it, because it is just too ambiguous and potentially dangerous. Just think of all the fun you could have with LD_LIBRARY_PATH=".."
On Wed, Oct 18, 2023 at 03:25:19AM +0200, Jan Engelhardt wrote: > On Tuesday 2023-10-17 19:50, Lucas De Marchi wrote: > >> +AC_ARG_WITH([module_directory], > >> + AS_HELP_STRING([--with-module-directory=DIR], [directory in which to > >> look for kernel modules - typically '/lib/modules' or > >> '${prefix}/lib/modules']), > >> + [], [with_module_directory=/lib/modules]) > >> +AC_SUBST([module_directory], [$with_module_directory]) > > > > we will probably have "fun" results if we accept a relative path here. > > $ ./configure --prefix=/usr --bindir=../bin > configure: error: expected an absolute directory name for --bindir: ../bin > > While such check does not exist for --with-module-directory, everyone > has likely been well-trained not to use relative paths. Even if, say, > cmake/meson *could* do it, it just would have never occurred to me to > actually *utilize* it, because it is just too ambiguous and > potentially dangerous. > > Just think of all the fun you could have with LD_LIBRARY_PATH=".." Is there an example of how this guarding against relative paths is done in other options that accept directories? There may be something for the built-in options but the existing custom rootlibdir option does not seem to have anything either. Thanks Michal
On Tue, Oct 17, 2023 at 12:50:15PM -0500, Lucas De Marchi wrote: > On Tue, Jul 18, 2023 at 02:01:56PM +0200, Michal Suchanek wrote: > > Now that modprobe.d is searched under ${prefix}/lib, allow a complete > > transition to files only under ${prefix} by adding a ${module_directory} > > configuration. This specifies the directory where to search for kernel > > modules and should match the location where the kernel/distro installs > > them. > > > > With this distributions that do not want to ship files in /lib can also > > move kernel modules to /usr while others can keep them in /lib. > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > > --- > > v4: Make the whole path configurable > > v5: More verbose commit message > > --- > > Makefile.am | 3 +- > > configure.ac | 7 ++ > > libkmod/libkmod.c | 4 +- > > man/Makefile.am | 1 + > > man/depmod.d.xml | 6 +- > > man/depmod.xml | 4 +- > > man/modinfo.xml | 2 +- > > man/modprobe.xml | 2 +- > > man/modules.dep.xml | 6 +- > > testsuite/module-playground/Makefile | 2 +- > > testsuite/setup-rootfs.sh | 109 +++++++++++++++------------ > > testsuite/test-depmod.c | 16 ++-- > > testsuite/test-testsuite.c | 8 +- > > tools/depmod.c | 6 +- > > tools/kmod.pc.in | 1 + > > tools/modinfo.c | 4 +- > > tools/modprobe.c | 4 +- > > tools/static-nodes.c | 6 +- > > 18 files changed, 107 insertions(+), 84 deletions(-) > > > > diff --git a/Makefile.am b/Makefile.am > > index 6d0b2decfef3..019aa749fdf1 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -20,6 +20,7 @@ AM_CPPFLAGS = \ > > -I$(top_srcdir) \ > > -DSYSCONFDIR=\""$(sysconfdir)"\" \ > > -DDISTCONFDIR=\""$(distconfdir)"\" \ > > + -DMODULE_DIRECTORY=\""$(module_directory)"\" \ > > ${zlib_CFLAGS} > > > > AM_CFLAGS = $(OUR_CFLAGS) > > @@ -220,7 +221,7 @@ EXTRA_DIST += testsuite/setup-rootfs.sh > > MODULE_PLAYGROUND = testsuite/module-playground > > ROOTFS = testsuite/rootfs > > ROOTFS_PRISTINE = $(top_srcdir)/testsuite/rootfs-pristine > > -CREATE_ROOTFS = $(AM_V_GEN) $(top_srcdir)/testsuite/setup-rootfs.sh $(ROOTFS_PRISTINE) $(ROOTFS) $(MODULE_PLAYGROUND) $(top_builddir)/config.h $(sysconfdir) > > +CREATE_ROOTFS = $(AM_V_GEN) MODULE_DIRECTORY=$(module_directory) $(top_srcdir)/testsuite/setup-rootfs.sh $(ROOTFS_PRISTINE) $(ROOTFS) $(MODULE_PLAYGROUND) $(top_builddir)/config.h $(sysconfdir) > > > > build-module-playground: > > $(AM_V_GEN)if test "$(top_srcdir)" != "$(top_builddir)"; then \ > > diff --git a/configure.ac b/configure.ac > > index b4584d6cdc67..4051dc9249e2 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -91,6 +91,12 @@ AC_ARG_WITH([rootlibdir], > > [], [with_rootlibdir=$libdir]) > > AC_SUBST([rootlibdir], [$with_rootlibdir]) > > > > +# Ideally this would be $prefix/lib/modules but default to /lib/modules for compatibility with earlier versions > > +AC_ARG_WITH([module_directory], > > + AS_HELP_STRING([--with-module-directory=DIR], [directory in which to look for kernel modules - typically '/lib/modules' or '${prefix}/lib/modules']), > > + [], [with_module_directory=/lib/modules]) > > +AC_SUBST([module_directory], [$with_module_directory]) > > we will probably have "fun" results if we accept a relative path here. > > > + > > AC_ARG_WITH([zstd], > > AS_HELP_STRING([--with-zstd], [handle Zstandard-compressed modules @<:@default=disabled@:>@]), > > [], [with_zstd=no]) > > @@ -326,6 +332,7 @@ AC_MSG_RESULT([ > > $PACKAGE $VERSION > > ======= > > > > + module_directory: ${module_directory} > > prefix: ${prefix} > > sysconfdir: ${sysconfdir} > > distconfdir: ${distconfdir} > > diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c > > index 09e6041461b0..63719e886de8 100644 > > --- a/libkmod/libkmod.c > > +++ b/libkmod/libkmod.c > > @@ -209,7 +209,7 @@ static int log_priority(const char *priority) > > return 0; > > } > > > > -static const char *dirname_default_prefix = "/lib/modules"; > > +static const char *dirname_default_prefix = MODULE_DIRECTORY; > > > > static char *get_kernel_release(const char *dirname) > > { > > @@ -231,7 +231,7 @@ static char *get_kernel_release(const char *dirname) > > /** > > * kmod_new: > > * @dirname: what to consider as linux module's directory, if NULL > > - * defaults to /lib/modules/`uname -r`. If it's relative, > > + * defaults to ${module_prefix}/lib/modules/`uname -r`. If it's relative, > > module_prefix? did you mean to use $MODULE_DIRECTORY/`uname -r`? Yes, that's leftover from previous revision, will fix up. > > > * it's treated as relative to the current working directory. > > * Otherwise, give an absolute dirname. > > * @config_paths: ordered array of paths (directories or files) where > > diff --git a/man/Makefile.am b/man/Makefile.am > > index 2fea8e46bf2f..f550091a216a 100644 > > --- a/man/Makefile.am > > +++ b/man/Makefile.am > > @@ -22,6 +22,7 @@ CLEANFILES = $(dist_man_MANS) > > else \ > > sed -e '/@DISTCONFDIR@/d' $< ; \ > > fi | \ > > + sed -e 's|@MODULE_DIRECTORY@|$(module_directory)|g' | \ > > $(XSLT) \ > > -o $@ \ > > --nonet \ > > diff --git a/man/depmod.d.xml b/man/depmod.d.xml > > index f282a39cc840..b07e6a2bd4fe 100644 > > --- a/man/depmod.d.xml > > +++ b/man/depmod.d.xml > > @@ -70,7 +70,7 @@ > > </term> > > <listitem> > > <para> > > - This allows you to specify the order in which /lib/modules > > + This allows you to specify the order in which @MODULE_DIRECTORY@ > > (or other configured module location) subdirectories will > > be processed by <command>depmod</command>. Directories are > > listed in order, with the highest priority given to the > > @@ -101,7 +101,7 @@ > > <command>depmod</command> command. It is possible to > > specify one kernel or all kernels using the * wildcard. > > <replaceable>modulesubdirectory</replaceable> is the > > - name of the subdirectory under /lib/modules (or other > > + name of the subdirectory under @MODULE_DIRECTORY@ (or other > > module location) where the target module is installed. > > </para> > > <para> > > @@ -110,7 +110,7 @@ > > specifying the following command: "override kmod * extra". > > This will ensure that any matching module name installed > > under the <command>extra</command> subdirectory within > > - /lib/modules (or other module location) will take priority > > + @MODULE_DIRECTORY@ (or other module location) will take priority > > over any likenamed module already provided by the kernel. > > </para> > > </listitem> > > diff --git a/man/depmod.xml b/man/depmod.xml > > index 3b0097184fd7..fce2a4a67a89 100644 > > --- a/man/depmod.xml > > +++ b/man/depmod.xml > > @@ -80,7 +80,7 @@ > > </para> > > <para> <command>depmod</command> creates a list of module dependencies by > > reading each module under > > - <filename>/lib/modules/</filename><replaceable>version</replaceable> and > > + <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable> and > > determining what symbols it exports and what symbols it needs. By > > default, this list is written to <filename>modules.dep</filename>, and a > > binary hashed version named <filename>modules.dep.bin</filename>, in the > > @@ -141,7 +141,7 @@ > > <listitem> > > <para> > > If your modules are not currently in the (normal) directory > > - <filename>/lib/modules/</filename><replaceable>version</replaceable>, > > + <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable>, > > but in a staging area, you can specify a > > <replaceable>basedir</replaceable> which is prepended to the > > directory name. This <replaceable>basedir</replaceable> is > > diff --git a/man/modinfo.xml b/man/modinfo.xml > > index 9fe0324a2527..b6c4d6045829 100644 > > --- a/man/modinfo.xml > > +++ b/man/modinfo.xml > > @@ -54,7 +54,7 @@ > > <command>modinfo</command> extracts information from the Linux Kernel > > modules given on the command line. If the module name is not a filename, > > then the > > - <filename>/lib/modules/</filename><replaceable>version</replaceable> > > + <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable> > > directory is searched, as is also done by > > <citerefentry><refentrytitle>modprobe</refentrytitle><manvolnum>8</manvolnum></citerefentry> > > when loading kernel modules. > > diff --git a/man/modprobe.xml b/man/modprobe.xml > > index 91f9e27997cd..4d1fd59c000b 100644 > > --- a/man/modprobe.xml > > +++ b/man/modprobe.xml > > @@ -78,7 +78,7 @@ > > is no difference between _ and - in module names (automatic > > underscore conversion is performed). > > <command>modprobe</command> looks in the module directory > > - <filename>/lib/modules/`uname -r`</filename> for all > > + <filename>@MODULE_DIRECTORY@/`uname -r`</filename> for all > > the modules and other files, except for the optional > > configuration files in the > > <filename>/etc/modprobe.d</filename> directory > > diff --git a/man/modules.dep.xml b/man/modules.dep.xml > > index ed633694ec9e..8ef6d8b3536e 100644 > > --- a/man/modules.dep.xml > > +++ b/man/modules.dep.xml > > @@ -34,8 +34,8 @@ > > </refnamediv> > > > > <refsynopsisdiv> > > - <para><filename>/lib/modules/modules.dep</filename></para> > > - <para><filename>/lib/modules/modules.dep.bin</filename></para> > > + <para><filename>@MODULE_DIRECTORY@/modules.dep</filename></para> > > + <para><filename>@MODULE_DIRECTORY@/modules.dep.bin</filename></para> > > </refsynopsisdiv> > > > > <refsect1><title>DESCRIPTION</title> > > @@ -43,7 +43,7 @@ > > <filename>modules.dep.bin</filename> is a binary file generated by > > <command>depmod</command> listing the dependencies for > > every module in the directories under > > - <filename>/lib/modules/</filename><replaceable>version</replaceable>. > > + <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable>. > > It is used by kmod tools such as <command>modprobe</command> and > > libkmod. > > </para> > > diff --git a/testsuite/module-playground/Makefile b/testsuite/module-playground/Makefile > > index e6045b0dd932..a7ab09bea2bf 100644 > > --- a/testsuite/module-playground/Makefile > > +++ b/testsuite/module-playground/Makefile > > @@ -47,7 +47,7 @@ endif > > > > else > > # normal makefile > > -KDIR ?= /lib/modules/`uname -r`/build > > +KDIR ?= $(module_prefix)/lib/modules/`uname -r`/build > > KVER ?= `uname -r` > > ifeq ($(FAKE_BUILD),) > > FAKE_BUILD=0 > > diff --git a/testsuite/setup-rootfs.sh b/testsuite/setup-rootfs.sh > > index 4440ddcd6b4d..a780f9381b3c 100755 > > --- a/testsuite/setup-rootfs.sh > > +++ b/testsuite/setup-rootfs.sh > > @@ -16,6 +16,19 @@ create_rootfs() { > > cp -r "$ROOTFS_PRISTINE" "$ROOTFS" > > find "$ROOTFS" -type d -exec chmod +w {} \; > > find "$ROOTFS" -type f -name .gitignore -exec rm -f {} \; > > + if [ "$MODULE_DIRECTORY" != "/lib/modules" ] ; then > > + sed -i -e "s|/lib/modules|$MODULE_DIRECTORY|g" $(find "$ROOTFS" -name \*.txt -o -name \*.conf -o -name \*.dep) > > + sed -i -e "s|$MODULE_DIRECTORY/external|/lib/modules/external|g" $(find "$ROOTFS" -name \*.txt -o -name \*.conf -o -name \*.dep) > > + for i in "$ROOTFS"/*/lib/modules/* "$ROOTFS"/*/*/lib/modules/* ; do > > + version="$(basename $i)" > > + [ $version != 'external' ] || continue > > + mod="$(dirname $i)" > > + lib="$(dirname $mod)" > > + up="$(dirname $lib)$MODULE_DIRECTORY" > > + mkdir -p "$up" > > + mv "$i" "$up" > > + done > > + fi > > > > if [ "$SYSCONFDIR" != "/etc" ]; then > > find "$ROOTFS" -type d -name etc -printf "%h\n" | while read -r e; do > > @@ -32,57 +45,57 @@ feature_enabled() { > > > > declare -A map > > map=( > > - ["test-depmod/search-order-simple/lib/modules/4.4.4/kernel/crypto/"]="mod-simple.ko" > > - ["test-depmod/search-order-simple/lib/modules/4.4.4/updates/"]="mod-simple.ko" > > - ["test-depmod/search-order-same-prefix/lib/modules/4.4.4/foo/"]="mod-simple.ko" > > - ["test-depmod/search-order-same-prefix/lib/modules/4.4.4/foobar/"]="mod-simple.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-c.ko"]="mod-loop-c.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-d.ko"]="mod-loop-d.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-e.ko"]="mod-loop-e.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-f.ko"]="mod-loop-f.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-g.ko"]="mod-loop-g.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-h.ko"]="mod-loop-h.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-i.ko"]="mod-loop-i.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-j.ko"]="mod-loop-j.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-k.ko"]="mod-loop-k.ko" > > - ["test-depmod/search-order-external-first/lib/modules/4.4.4/foo/"]="mod-simple.ko" > > - ["test-depmod/search-order-external-first/lib/modules/4.4.4/foobar/"]="mod-simple.ko" > > + ["test-depmod/search-order-simple$MODULE_DIRECTORY/4.4.4/kernel/crypto/"]="mod-simple.ko" > > + ["test-depmod/search-order-simple$MODULE_DIRECTORY/4.4.4/updates/"]="mod-simple.ko" > > + ["test-depmod/search-order-same-prefix$MODULE_DIRECTORY/4.4.4/foo/"]="mod-simple.ko" > > + ["test-depmod/search-order-same-prefix$MODULE_DIRECTORY/4.4.4/foobar/"]="mod-simple.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-c.ko"]="mod-loop-c.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-d.ko"]="mod-loop-d.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-e.ko"]="mod-loop-e.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-f.ko"]="mod-loop-f.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-g.ko"]="mod-loop-g.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-h.ko"]="mod-loop-h.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-i.ko"]="mod-loop-i.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-j.ko"]="mod-loop-j.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-k.ko"]="mod-loop-k.ko" > > + ["test-depmod/search-order-external-first$MODULE_DIRECTORY/4.4.4/foo/"]="mod-simple.ko" > > + ["test-depmod/search-order-external-first$MODULE_DIRECTORY/4.4.4/foobar/"]="mod-simple.ko" > > ["test-depmod/search-order-external-first/lib/modules/external/"]="mod-simple.ko" > > why didn't you change it here? That's a module that's not installed into the directory of the current kernel -> does not relally matter where it is so long as it is not in that one directory corresponding to the tested kernel version AFAICT. > > > - ["test-depmod/search-order-external-last/lib/modules/4.4.4/foo/"]="mod-simple.ko" > > - ["test-depmod/search-order-external-last/lib/modules/4.4.4/foobar/"]="mod-simple.ko" > > + ["test-depmod/search-order-external-last$MODULE_DIRECTORY/4.4.4/foo/"]="mod-simple.ko" > > + ["test-depmod/search-order-external-last$MODULE_DIRECTORY/4.4.4/foobar/"]="mod-simple.ko" > > ["test-depmod/search-order-external-last/lib/modules/external/"]="mod-simple.ko" > > and here... Same. Thanks Michal
Hello,
This is resend of the last patch in the series that adds prefix support
to kernel module location together with additional patch for validating
the user supplied input to options that are interpreted as directories.
Thanks
Michal
Michal Suchanek (2):
libkmod, depmod, modprobe: Make directory for kernel modules
configurable
configure: Check that provided paths are absolute
Makefile.am | 3 +-
configure.ac | 24 ++++++
libkmod/libkmod.c | 4 +-
man/Makefile.am | 1 +
man/depmod.d.xml | 6 +-
man/depmod.xml | 4 +-
man/modinfo.xml | 2 +-
man/modprobe.xml | 2 +-
man/modules.dep.xml | 6 +-
testsuite/module-playground/Makefile | 2 +-
testsuite/setup-rootfs.sh | 109 +++++++++++++++------------
testsuite/test-depmod.c | 16 ++--
testsuite/test-testsuite.c | 8 +-
tools/depmod.c | 6 +-
tools/kmod.pc.in | 1 +
tools/modinfo.c | 4 +-
tools/modprobe.c | 4 +-
tools/static-nodes.c | 6 +-
18 files changed, 124 insertions(+), 84 deletions(-)
On Tue, Oct 17, 2023 at 12:50:15PM -0500, Lucas De Marchi wrote: > On Tue, Jul 18, 2023 at 02:01:56PM +0200, Michal Suchanek wrote: > > Now that modprobe.d is searched under ${prefix}/lib, allow a complete > > transition to files only under ${prefix} by adding a ${module_directory} > > configuration. This specifies the directory where to search for kernel > > modules and should match the location where the kernel/distro installs > > them. > > > > With this distributions that do not want to ship files in /lib can also > > move kernel modules to /usr while others can keep them in /lib. > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > > --- > > v4: Make the whole path configurable > > v5: More verbose commit message > > --- > > Makefile.am | 3 +- > > configure.ac | 7 ++ > > libkmod/libkmod.c | 4 +- > > man/Makefile.am | 1 + > > man/depmod.d.xml | 6 +- > > man/depmod.xml | 4 +- > > man/modinfo.xml | 2 +- > > man/modprobe.xml | 2 +- > > man/modules.dep.xml | 6 +- > > testsuite/module-playground/Makefile | 2 +- > > testsuite/setup-rootfs.sh | 109 +++++++++++++++------------ > > testsuite/test-depmod.c | 16 ++-- > > testsuite/test-testsuite.c | 8 +- > > tools/depmod.c | 6 +- > > tools/kmod.pc.in | 1 + > > tools/modinfo.c | 4 +- > > tools/modprobe.c | 4 +- > > tools/static-nodes.c | 6 +- > > 18 files changed, 107 insertions(+), 84 deletions(-) > > > > diff --git a/Makefile.am b/Makefile.am > > index 6d0b2decfef3..019aa749fdf1 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -20,6 +20,7 @@ AM_CPPFLAGS = \ > > -I$(top_srcdir) \ > > -DSYSCONFDIR=\""$(sysconfdir)"\" \ > > -DDISTCONFDIR=\""$(distconfdir)"\" \ > > + -DMODULE_DIRECTORY=\""$(module_directory)"\" \ > > ${zlib_CFLAGS} > > > > AM_CFLAGS = $(OUR_CFLAGS) > > @@ -220,7 +221,7 @@ EXTRA_DIST += testsuite/setup-rootfs.sh > > MODULE_PLAYGROUND = testsuite/module-playground > > ROOTFS = testsuite/rootfs > > ROOTFS_PRISTINE = $(top_srcdir)/testsuite/rootfs-pristine > > -CREATE_ROOTFS = $(AM_V_GEN) $(top_srcdir)/testsuite/setup-rootfs.sh $(ROOTFS_PRISTINE) $(ROOTFS) $(MODULE_PLAYGROUND) $(top_builddir)/config.h $(sysconfdir) > > +CREATE_ROOTFS = $(AM_V_GEN) MODULE_DIRECTORY=$(module_directory) $(top_srcdir)/testsuite/setup-rootfs.sh $(ROOTFS_PRISTINE) $(ROOTFS) $(MODULE_PLAYGROUND) $(top_builddir)/config.h $(sysconfdir) > > > > build-module-playground: > > $(AM_V_GEN)if test "$(top_srcdir)" != "$(top_builddir)"; then \ > > diff --git a/configure.ac b/configure.ac > > index b4584d6cdc67..4051dc9249e2 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -91,6 +91,12 @@ AC_ARG_WITH([rootlibdir], > > [], [with_rootlibdir=$libdir]) > > AC_SUBST([rootlibdir], [$with_rootlibdir]) > > > > +# Ideally this would be $prefix/lib/modules but default to /lib/modules for compatibility with earlier versions > > +AC_ARG_WITH([module_directory], > > + AS_HELP_STRING([--with-module-directory=DIR], [directory in which to look for kernel modules - typically '/lib/modules' or '${prefix}/lib/modules']), > > + [], [with_module_directory=/lib/modules]) > > +AC_SUBST([module_directory], [$with_module_directory]) > > we will probably have "fun" results if we accept a relative path here. > > > + > > AC_ARG_WITH([zstd], > > AS_HELP_STRING([--with-zstd], [handle Zstandard-compressed modules @<:@default=disabled@:>@]), > > [], [with_zstd=no]) > > @@ -326,6 +332,7 @@ AC_MSG_RESULT([ > > $PACKAGE $VERSION > > ======= > > > > + module_directory: ${module_directory} > > prefix: ${prefix} > > sysconfdir: ${sysconfdir} > > distconfdir: ${distconfdir} > > diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c > > index 09e6041461b0..63719e886de8 100644 > > --- a/libkmod/libkmod.c > > +++ b/libkmod/libkmod.c > > @@ -209,7 +209,7 @@ static int log_priority(const char *priority) > > return 0; > > } > > > > -static const char *dirname_default_prefix = "/lib/modules"; > > +static const char *dirname_default_prefix = MODULE_DIRECTORY; > > > > static char *get_kernel_release(const char *dirname) > > { > > @@ -231,7 +231,7 @@ static char *get_kernel_release(const char *dirname) > > /** > > * kmod_new: > > * @dirname: what to consider as linux module's directory, if NULL > > - * defaults to /lib/modules/`uname -r`. If it's relative, > > + * defaults to ${module_prefix}/lib/modules/`uname -r`. If it's relative, > > module_prefix? did you mean to use $MODULE_DIRECTORY/`uname -r`? > > > * it's treated as relative to the current working directory. > > * Otherwise, give an absolute dirname. > > * @config_paths: ordered array of paths (directories or files) where > > diff --git a/man/Makefile.am b/man/Makefile.am > > index 2fea8e46bf2f..f550091a216a 100644 > > --- a/man/Makefile.am > > +++ b/man/Makefile.am > > @@ -22,6 +22,7 @@ CLEANFILES = $(dist_man_MANS) > > else \ > > sed -e '/@DISTCONFDIR@/d' $< ; \ > > fi | \ > > + sed -e 's|@MODULE_DIRECTORY@|$(module_directory)|g' | \ > > $(XSLT) \ > > -o $@ \ > > --nonet \ > > diff --git a/man/depmod.d.xml b/man/depmod.d.xml > > index f282a39cc840..b07e6a2bd4fe 100644 > > --- a/man/depmod.d.xml > > +++ b/man/depmod.d.xml > > @@ -70,7 +70,7 @@ > > </term> > > <listitem> > > <para> > > - This allows you to specify the order in which /lib/modules > > + This allows you to specify the order in which @MODULE_DIRECTORY@ > > (or other configured module location) subdirectories will > > be processed by <command>depmod</command>. Directories are > > listed in order, with the highest priority given to the > > @@ -101,7 +101,7 @@ > > <command>depmod</command> command. It is possible to > > specify one kernel or all kernels using the * wildcard. > > <replaceable>modulesubdirectory</replaceable> is the > > - name of the subdirectory under /lib/modules (or other > > + name of the subdirectory under @MODULE_DIRECTORY@ (or other > > module location) where the target module is installed. > > </para> > > <para> > > @@ -110,7 +110,7 @@ > > specifying the following command: "override kmod * extra". > > This will ensure that any matching module name installed > > under the <command>extra</command> subdirectory within > > - /lib/modules (or other module location) will take priority > > + @MODULE_DIRECTORY@ (or other module location) will take priority > > over any likenamed module already provided by the kernel. > > </para> > > </listitem> > > diff --git a/man/depmod.xml b/man/depmod.xml > > index 3b0097184fd7..fce2a4a67a89 100644 > > --- a/man/depmod.xml > > +++ b/man/depmod.xml > > @@ -80,7 +80,7 @@ > > </para> > > <para> <command>depmod</command> creates a list of module dependencies by > > reading each module under > > - <filename>/lib/modules/</filename><replaceable>version</replaceable> and > > + <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable> and > > determining what symbols it exports and what symbols it needs. By > > default, this list is written to <filename>modules.dep</filename>, and a > > binary hashed version named <filename>modules.dep.bin</filename>, in the > > @@ -141,7 +141,7 @@ > > <listitem> > > <para> > > If your modules are not currently in the (normal) directory > > - <filename>/lib/modules/</filename><replaceable>version</replaceable>, > > + <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable>, > > but in a staging area, you can specify a > > <replaceable>basedir</replaceable> which is prepended to the > > directory name. This <replaceable>basedir</replaceable> is > > diff --git a/man/modinfo.xml b/man/modinfo.xml > > index 9fe0324a2527..b6c4d6045829 100644 > > --- a/man/modinfo.xml > > +++ b/man/modinfo.xml > > @@ -54,7 +54,7 @@ > > <command>modinfo</command> extracts information from the Linux Kernel > > modules given on the command line. If the module name is not a filename, > > then the > > - <filename>/lib/modules/</filename><replaceable>version</replaceable> > > + <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable> > > directory is searched, as is also done by > > <citerefentry><refentrytitle>modprobe</refentrytitle><manvolnum>8</manvolnum></citerefentry> > > when loading kernel modules. > > diff --git a/man/modprobe.xml b/man/modprobe.xml > > index 91f9e27997cd..4d1fd59c000b 100644 > > --- a/man/modprobe.xml > > +++ b/man/modprobe.xml > > @@ -78,7 +78,7 @@ > > is no difference between _ and - in module names (automatic > > underscore conversion is performed). > > <command>modprobe</command> looks in the module directory > > - <filename>/lib/modules/`uname -r`</filename> for all > > + <filename>@MODULE_DIRECTORY@/`uname -r`</filename> for all > > the modules and other files, except for the optional > > configuration files in the > > <filename>/etc/modprobe.d</filename> directory > > diff --git a/man/modules.dep.xml b/man/modules.dep.xml > > index ed633694ec9e..8ef6d8b3536e 100644 > > --- a/man/modules.dep.xml > > +++ b/man/modules.dep.xml > > @@ -34,8 +34,8 @@ > > </refnamediv> > > > > <refsynopsisdiv> > > - <para><filename>/lib/modules/modules.dep</filename></para> > > - <para><filename>/lib/modules/modules.dep.bin</filename></para> > > + <para><filename>@MODULE_DIRECTORY@/modules.dep</filename></para> > > + <para><filename>@MODULE_DIRECTORY@/modules.dep.bin</filename></para> > > </refsynopsisdiv> > > > > <refsect1><title>DESCRIPTION</title> > > @@ -43,7 +43,7 @@ > > <filename>modules.dep.bin</filename> is a binary file generated by > > <command>depmod</command> listing the dependencies for > > every module in the directories under > > - <filename>/lib/modules/</filename><replaceable>version</replaceable>. > > + <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable>. > > It is used by kmod tools such as <command>modprobe</command> and > > libkmod. > > </para> > > diff --git a/testsuite/module-playground/Makefile b/testsuite/module-playground/Makefile > > index e6045b0dd932..a7ab09bea2bf 100644 > > --- a/testsuite/module-playground/Makefile > > +++ b/testsuite/module-playground/Makefile > > @@ -47,7 +47,7 @@ endif > > > > else > > # normal makefile > > -KDIR ?= /lib/modules/`uname -r`/build > > +KDIR ?= $(module_prefix)/lib/modules/`uname -r`/build > > KVER ?= `uname -r` > > ifeq ($(FAKE_BUILD),) > > FAKE_BUILD=0 > > diff --git a/testsuite/setup-rootfs.sh b/testsuite/setup-rootfs.sh > > index 4440ddcd6b4d..a780f9381b3c 100755 > > --- a/testsuite/setup-rootfs.sh > > +++ b/testsuite/setup-rootfs.sh > > @@ -16,6 +16,19 @@ create_rootfs() { > > cp -r "$ROOTFS_PRISTINE" "$ROOTFS" > > find "$ROOTFS" -type d -exec chmod +w {} \; > > find "$ROOTFS" -type f -name .gitignore -exec rm -f {} \; > > + if [ "$MODULE_DIRECTORY" != "/lib/modules" ] ; then > > + sed -i -e "s|/lib/modules|$MODULE_DIRECTORY|g" $(find "$ROOTFS" -name \*.txt -o -name \*.conf -o -name \*.dep) > > + sed -i -e "s|$MODULE_DIRECTORY/external|/lib/modules/external|g" $(find "$ROOTFS" -name \*.txt -o -name \*.conf -o -name \*.dep) > > + for i in "$ROOTFS"/*/lib/modules/* "$ROOTFS"/*/*/lib/modules/* ; do > > + version="$(basename $i)" > > + [ $version != 'external' ] || continue > > + mod="$(dirname $i)" > > + lib="$(dirname $mod)" > > + up="$(dirname $lib)$MODULE_DIRECTORY" > > + mkdir -p "$up" > > + mv "$i" "$up" > > + done > > + fi > > > > if [ "$SYSCONFDIR" != "/etc" ]; then > > find "$ROOTFS" -type d -name etc -printf "%h\n" | while read -r e; do > > @@ -32,57 +45,57 @@ feature_enabled() { > > > > declare -A map > > map=( > > - ["test-depmod/search-order-simple/lib/modules/4.4.4/kernel/crypto/"]="mod-simple.ko" > > - ["test-depmod/search-order-simple/lib/modules/4.4.4/updates/"]="mod-simple.ko" > > - ["test-depmod/search-order-same-prefix/lib/modules/4.4.4/foo/"]="mod-simple.ko" > > - ["test-depmod/search-order-same-prefix/lib/modules/4.4.4/foobar/"]="mod-simple.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-c.ko"]="mod-loop-c.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-d.ko"]="mod-loop-d.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-e.ko"]="mod-loop-e.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-f.ko"]="mod-loop-f.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-g.ko"]="mod-loop-g.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-h.ko"]="mod-loop-h.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-i.ko"]="mod-loop-i.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-j.ko"]="mod-loop-j.ko" > > - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-k.ko"]="mod-loop-k.ko" > > - ["test-depmod/search-order-external-first/lib/modules/4.4.4/foo/"]="mod-simple.ko" > > - ["test-depmod/search-order-external-first/lib/modules/4.4.4/foobar/"]="mod-simple.ko" > > + ["test-depmod/search-order-simple$MODULE_DIRECTORY/4.4.4/kernel/crypto/"]="mod-simple.ko" > > + ["test-depmod/search-order-simple$MODULE_DIRECTORY/4.4.4/updates/"]="mod-simple.ko" > > + ["test-depmod/search-order-same-prefix$MODULE_DIRECTORY/4.4.4/foo/"]="mod-simple.ko" > > + ["test-depmod/search-order-same-prefix$MODULE_DIRECTORY/4.4.4/foobar/"]="mod-simple.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-c.ko"]="mod-loop-c.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-d.ko"]="mod-loop-d.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-e.ko"]="mod-loop-e.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-f.ko"]="mod-loop-f.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-g.ko"]="mod-loop-g.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-h.ko"]="mod-loop-h.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-i.ko"]="mod-loop-i.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-j.ko"]="mod-loop-j.ko" > > + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-k.ko"]="mod-loop-k.ko" > > + ["test-depmod/search-order-external-first$MODULE_DIRECTORY/4.4.4/foo/"]="mod-simple.ko" > > + ["test-depmod/search-order-external-first$MODULE_DIRECTORY/4.4.4/foobar/"]="mod-simple.ko" > > ["test-depmod/search-order-external-first/lib/modules/external/"]="mod-simple.ko" > > why didn't you change it here? > > > - ["test-depmod/search-order-external-last/lib/modules/4.4.4/foo/"]="mod-simple.ko" > > - ["test-depmod/search-order-external-last/lib/modules/4.4.4/foobar/"]="mod-simple.ko" > > + ["test-depmod/search-order-external-last$MODULE_DIRECTORY/4.4.4/foo/"]="mod-simple.ko" > > + ["test-depmod/search-order-external-last$MODULE_DIRECTORY/4.4.4/foobar/"]="mod-simple.ko" > > ["test-depmod/search-order-external-last/lib/modules/external/"]="mod-simple.ko" > > and here... The path is embedded in binary files: t grep '/lib/modules' | grep Binary Binary file testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.dep.bin matches Binary file testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.dep.bin matches Binary file testsuite/rootfs-pristine/test-modprobe/module-from-abspath/lib/modules/4.4.4/modules.dep.bin matches Binary file testsuite/rootfs-pristine/test-modprobe/module-from-relpath/lib/modules/4.4.4/modules.dep.bin matches The reason is that path to 'external' modules that are not in $MODULE_DIRECTORY/$(KERNELRELEASE) is recorded as absolute path. The way these tests are designed the binary files cannot be changed. To get the same file the non-'external' modules have to be moved to match the new location of $MODULE_DIRECTORY which results in the same path relative to $MODULE_DIRECTORY while the 'external' ones are not moved getting the same absolute path regardless of $MODULE_DIRECTORY. Thanks Michal
On Fri, Nov 10, 2023 at 01:13:53PM +0100, Michal Suchanek wrote: >Hello, > >This is resend of the last patch in the series that adds prefix support >to kernel module location together with additional patch for validating >the user supplied input to options that are interpreted as directories. > >Thanks applied, thanks Lucas De Marchi
On Thu, Dec 7, 2023 at 3:37 AM Lucas De Marchi <lucas.demarchi@intel.com> wrote: > > On Fri, Nov 10, 2023 at 01:13:53PM +0100, Michal Suchanek wrote: > >Hello, > > > >This is resend of the last patch in the series that adds prefix support > >to kernel module location together with additional patch for validating > >the user supplied input to options that are interpreted as directories. > > > >Thanks > > applied, thanks > > Lucas De Marchi If I understood this correctly, MODULE_DIRECTORY is determined by "configure --with-module-directory=...", and there is no way to change it after that. If so, how to work with cross-building? Cross-building is typical when building embedded Linux systems. Consider this scenario: - Your build machine adopts MODULE_DIRECTORY=/usr/lib/modules - The target embedded system adopts MODULE_DIRECTORY=/lib/modules (or vice a versa) depmod is used also for cross-building because it is executed as a part of "make module_install". The counterpart patch set for Kbuild provides KERNEL_MODULE_DIRECTORY, which only changes the destination directory to which *.ko are copied. You cannot change the directory where the depmod searches for modules, as it is fixed at the compile-time of kmod. In this case, what we can do is to build another instance of kmod configured for the target system, and use it for modules_install: 1. In the kmod source directory ./configure --with=module-directory=/lib/modules make 2. make modules_install INSTALL_MOD_PATH=<staging-dir> KERNEL_MODULE_DIRECTORY=/lib/modules DEPMOD=<new-depmod-you-has-just-built> If you use OpenEmbedded etc., this is what you do because host tools are built from sources. But, should it be required all the time? Even when the target embedded system uses busybox-based modprobe instead of kmod? depmod provides --basedir option, which changes the prefix part, but there is no way to override the stem part, MODULE_DIRECTRY. In the review of the counter patch set, I am suggesting an option to override MODULE_DIRECTRY (let's say --moduledir) at least for depmod. (Perhaps modinfo too, as it also supports --basedir) Then, we can change scripts/depmod.sh so that Kbuild can propagate KERNEL_MODULE_DIRECTORY to depmod. if <depmod supports --moduledir>; then set -- "$@" --moduledir "${KERNEL_MODULE_DIRECTORY}" fi Does it make sense?
On Tue, Dec 19, 2023 at 05:37:31PM +0900, Masahiro Yamada wrote: > On Thu, Dec 7, 2023 at 3:37 AM Lucas De Marchi <lucas.demarchi@intel.com> wrote: > > > > On Fri, Nov 10, 2023 at 01:13:53PM +0100, Michal Suchanek wrote: > > >Hello, > > > > > >This is resend of the last patch in the series that adds prefix support > > >to kernel module location together with additional patch for validating > > >the user supplied input to options that are interpreted as directories. > > > > > >Thanks > > > > applied, thanks > > > > Lucas De Marchi > > > > If I understood this correctly, MODULE_DIRECTORY is determined > by "configure --with-module-directory=...", and there is no > way to change it after that. > > > If so, how to work with cross-building? > > Cross-building is typical when building embedded Linux systems. > > > Consider this scenario: > > - Your build machine adopts > MODULE_DIRECTORY=/usr/lib/modules > - The target embedded system adopts > MODULE_DIRECTORY=/lib/modules > > (or vice a versa) > > > > > depmod is used also for cross-building because > it is executed as a part of "make module_install". > > > The counterpart patch set for Kbuild provides > KERNEL_MODULE_DIRECTORY, which only changes > the destination directory to which *.ko are copied. > > You cannot change the directory where the > depmod searches for modules, as it is fixed > at the compile-time of kmod. > > > > > In this case, what we can do is to build another > instance of kmod configured for the target system, > and use it for modules_install: > > 1. In the kmod source directory > ./configure --with=module-directory=/lib/modules > make > > 2. make modules_install INSTALL_MOD_PATH=<staging-dir> > KERNEL_MODULE_DIRECTORY=/lib/modules > DEPMOD=<new-depmod-you-has-just-built> > > > > If you use OpenEmbedded etc., this is what you do > because host tools are built from sources. > > But, should it be required all the time? > Even when the target embedded system uses > busybox-based modprobe instead of kmod? > > > > depmod provides --basedir option, which changes > the prefix part, but there is no way to override > the stem part, MODULE_DIRECTRY. > > In the review of the counter patch set, > I am suggesting an option to override MODULE_DIRECTRY > (let's say --moduledir) at least for depmod. > > (Perhaps modinfo too, as it also supports --basedir) > > > > Then, we can change scripts/depmod.sh so that > Kbuild can propagate KERNEL_MODULE_DIRECTORY > to depmod. > > > if <depmod supports --moduledir>; then > set -- "$@" --moduledir "${KERNEL_MODULE_DIRECTORY}" > fi > > > > Does it make sense? Did this conversation go anywhere? After the upgrade to kmod 33 in Arch Linux, which includes building with the configuration option '--with-module-directory' set to '/usr/lib/modules' [1], building a tarzst-pkg breaks for me, seemingly for the reason noted above. $ make -skj"$(nproc)" ARCH=x86_64 CROSS_COMPILE=x86_64-linux- O=$HOME/tmp/build/linux defconfig tarzst-pkg depmod: ERROR: could not open directory /home/nathan/tmp/build/linux/tar-install/usr/lib/modules/6.11.0-rc4-00019-gb311c1b497e5: No such file or directory depmod: FATAL: could not search modules: No such file or directory $ ls $HOME/tmp/build/linux/tar-install boot lib I don't see how to get around this without an option to override MODULE_DIRECTORY. I guess I'll ask Arch Linux to revert this option for the time being, as it mentions they do not really need it at the moment. [1]: https://gitlab.archlinux.org/archlinux/packaging/packages/kmod/-/commit/0efd732cb78bc0b7851a8367f4dc8e6933f5b99d Cheers, Nathan
On Wed, Aug 21, 2024 at 10:58:43AM GMT, Nathan Chancellor wrote:
>Did this conversation go anywhere? After the upgrade to kmod 33 in Arch
no, it stalled, thanks for reminding me. I will follow up on it.
Lucas De Marchi
On Tue, Dec 19, 2023 at 05:37:31PM GMT, Masahiro Yamada wrote: >On Thu, Dec 7, 2023 at 3:37 AM Lucas De Marchi <lucas.demarchi@intel.com> wrote: >> >> On Fri, Nov 10, 2023 at 01:13:53PM +0100, Michal Suchanek wrote: >> >Hello, >> > >> >This is resend of the last patch in the series that adds prefix support >> >to kernel module location together with additional patch for validating >> >the user supplied input to options that are interpreted as directories. >> > >> >Thanks >> >> applied, thanks >> >> Lucas De Marchi > > > >If I understood this correctly, MODULE_DIRECTORY is determined >by "configure --with-module-directory=...", and there is no >way to change it after that. > > >If so, how to work with cross-building? > >Cross-building is typical when building embedded Linux systems. I was thinking the `pkg-config --variable=module_directory` from the target would be sufficient, but... > > >Consider this scenario: > >- Your build machine adopts > MODULE_DIRECTORY=/usr/lib/modules >- The target embedded system adopts > MODULE_DIRECTORY=/lib/modules > >(or vice a versa) >depmod is used also for cross-building because >it is executed as a part of "make module_install". > > >The counterpart patch set for Kbuild provides >KERNEL_MODULE_DIRECTORY, which only changes >the destination directory to which *.ko are copied. > >You cannot change the directory where the >depmod searches for modules, as it is fixed >at the compile-time of kmod. > > > > >In this case, what we can do is to build another >instance of kmod configured for the target system, the target system may not even have depmod actually, so using just the host one seems more appropriate. But target should have the kmod.pc for the pkg-config call to work. >and use it for modules_install: > >1. In the kmod source directory > ./configure --with=module-directory=/lib/modules > make > >2. make modules_install INSTALL_MOD_PATH=<staging-dir> > KERNEL_MODULE_DIRECTORY=/lib/modules > DEPMOD=<new-depmod-you-has-just-built> > > > >If you use OpenEmbedded etc., this is what you do >because host tools are built from sources. > >But, should it be required all the time? >Even when the target embedded system uses >busybox-based modprobe instead of kmod? no, I don't think we can rely on depmod from the target. > > > >depmod provides --basedir option, which changes >the prefix part, but there is no way to override >the stem part, MODULE_DIRECTRY. > >In the review of the counter patch set, >I am suggesting an option to override MODULE_DIRECTRY >(let's say --moduledir) at least for depmod. ok > >(Perhaps modinfo too, as it also supports --basedir) > > > >Then, we can change scripts/depmod.sh so that >Kbuild can propagate KERNEL_MODULE_DIRECTORY >to depmod. > > >if <depmod supports --moduledir>; then > set -- "$@" --moduledir "${KERNEL_MODULE_DIRECTORY}" >fi > > > >Does it make sense? yep thanks and sorry for missing this reply Lucas De Marchi > > > > >-- >Best Regards >Masahiro Yamada
Hello, On Thu, Aug 22, 2024 at 01:05:11AM -0500, Lucas De Marchi wrote: > On Tue, Dec 19, 2023 at 05:37:31PM GMT, Masahiro Yamada wrote: > > On Thu, Dec 7, 2023 at 3:37 AM Lucas De Marchi <lucas.demarchi@intel.com> wrote: > > > > > > On Fri, Nov 10, 2023 at 01:13:53PM +0100, Michal Suchanek wrote: > > > >Hello, > > > > > > > >This is resend of the last patch in the series that adds prefix support > > > >to kernel module location together with additional patch for validating > > > >the user supplied input to options that are interpreted as directories. > > > > > > > >Thanks > > > > > > applied, thanks > > > > > > Lucas De Marchi > > > > > > > > If I understood this correctly, MODULE_DIRECTORY is determined > > by "configure --with-module-directory=...", and there is no > > way to change it after that. > > > > > > If so, how to work with cross-building? > > > > Cross-building is typical when building embedded Linux systems. > > I was thinking the `pkg-config --variable=module_directory` > from the target would be sufficient, but... > > > > > > > Consider this scenario: > > > > - Your build machine adopts > > MODULE_DIRECTORY=/usr/lib/modules > > - The target embedded system adopts > > MODULE_DIRECTORY=/lib/modules > > > > (or vice a versa) > > depmod is used also for cross-building because > > it is executed as a part of "make module_install". > > > > > > The counterpart patch set for Kbuild provides > > KERNEL_MODULE_DIRECTORY, which only changes > > the destination directory to which *.ko are copied. > > > > You cannot change the directory where the > > depmod searches for modules, as it is fixed > > at the compile-time of kmod. > > > > > > > > > > In this case, what we can do is to build another > > instance of kmod configured for the target system, > > the target system may not even have depmod actually, so using just the > host one seems more appropriate. But target should have the kmod.pc for > the pkg-config call to work. > > > and use it for modules_install: > > > > 1. In the kmod source directory > > ./configure --with=module-directory=/lib/modules > > make > > > > 2. make modules_install INSTALL_MOD_PATH=<staging-dir> > > KERNEL_MODULE_DIRECTORY=/lib/modules > > DEPMOD=<new-depmod-you-has-just-built> > > > > > > > > If you use OpenEmbedded etc., this is what you do > > because host tools are built from sources. > > > > But, should it be required all the time? > > Even when the target embedded system uses > > busybox-based modprobe instead of kmod? > > no, I don't think we can rely on depmod from the target. > > > > > > > > > depmod provides --basedir option, which changes > > the prefix part, but there is no way to override > > the stem part, MODULE_DIRECTRY. > > > > In the review of the counter patch set, > > I am suggesting an option to override MODULE_DIRECTRY > > (let's say --moduledir) at least for depmod. > > ok > > > > > (Perhaps modinfo too, as it also supports --basedir) > > > > > > > > Then, we can change scripts/depmod.sh so that > > Kbuild can propagate KERNEL_MODULE_DIRECTORY > > to depmod. > > > > > > if <depmod supports --moduledir>; then > > set -- "$@" --moduledir "${KERNEL_MODULE_DIRECTORY}" > > fi > > > > > > > > Does it make sense? It does not make sense for the common case: building kernel for the host system. Then overriding the directory is wrong, and using what kmod was compiled with is needed to get correct module directory layout. Or it would make sense if both was done: Default KERNEL_MODULE_DIRECTORY to what kmod was compiled with, and then pass the actual value to depmod so that depmod uses the compiled-in value by default, and the user-provided value when KERNEL_MODULE_DIRECTORY was overridden by the user. Thanks Michal
On 2024/08/21, Nathan Chancellor wrote: > On Tue, Dec 19, 2023 at 05:37:31PM +0900, Masahiro Yamada wrote: > > On Thu, Dec 7, 2023 at 3:37 AM Lucas De Marchi <lucas.demarchi@intel.com> wrote: > > > > > > On Fri, Nov 10, 2023 at 01:13:53PM +0100, Michal Suchanek wrote: > > > >Hello, > > > > > > > >This is resend of the last patch in the series that adds prefix support > > > >to kernel module location together with additional patch for validating > > > >the user supplied input to options that are interpreted as directories. > > > > > > > >Thanks > > > > > > applied, thanks > > > > > > Lucas De Marchi > > > > > > > > If I understood this correctly, MODULE_DIRECTORY is determined > > by "configure --with-module-directory=...", and there is no > > way to change it after that. > > > > > > If so, how to work with cross-building? > > > > Cross-building is typical when building embedded Linux systems. > > > > > > Consider this scenario: > > > > - Your build machine adopts > > MODULE_DIRECTORY=/usr/lib/modules > > - The target embedded system adopts > > MODULE_DIRECTORY=/lib/modules > > > > (or vice a versa) > > > > > > > > > > depmod is used also for cross-building because > > it is executed as a part of "make module_install". > > > > > > The counterpart patch set for Kbuild provides > > KERNEL_MODULE_DIRECTORY, which only changes > > the destination directory to which *.ko are copied. > > > > You cannot change the directory where the > > depmod searches for modules, as it is fixed > > at the compile-time of kmod. > > > > > > > > > > In this case, what we can do is to build another > > instance of kmod configured for the target system, > > and use it for modules_install: > > > > 1. In the kmod source directory > > ./configure --with=module-directory=/lib/modules > > make > > > > 2. make modules_install INSTALL_MOD_PATH=<staging-dir> > > KERNEL_MODULE_DIRECTORY=/lib/modules > > DEPMOD=<new-depmod-you-has-just-built> > > > > > > > > If you use OpenEmbedded etc., this is what you do > > because host tools are built from sources. > > > > But, should it be required all the time? > > Even when the target embedded system uses > > busybox-based modprobe instead of kmod? > > > > > > > > depmod provides --basedir option, which changes > > the prefix part, but there is no way to override > > the stem part, MODULE_DIRECTRY. > > > > In the review of the counter patch set, > > I am suggesting an option to override MODULE_DIRECTRY > > (let's say --moduledir) at least for depmod. > > > > (Perhaps modinfo too, as it also supports --basedir) > > > > > > > > Then, we can change scripts/depmod.sh so that > > Kbuild can propagate KERNEL_MODULE_DIRECTORY > > to depmod. > > > > > > if <depmod supports --moduledir>; then > > set -- "$@" --moduledir "${KERNEL_MODULE_DIRECTORY}" > > fi > > > > > > > > Does it make sense? > > Did this conversation go anywhere? After the upgrade to kmod 33 in Arch > Linux, which includes building with the configuration option > '--with-module-directory' set to '/usr/lib/modules' [1], building a > tarzst-pkg breaks for me, seemingly for the reason noted above. > > $ make -skj"$(nproc)" ARCH=x86_64 CROSS_COMPILE=x86_64-linux- O=$HOME/tmp/build/linux defconfig tarzst-pkg > depmod: ERROR: could not open directory /home/nathan/tmp/build/linux/tar-install/usr/lib/modules/6.11.0-rc4-00019-gb311c1b497e5: No such file or directory > depmod: FATAL: could not search modules: No such file or directory > > $ ls $HOME/tmp/build/linux/tar-install > boot lib > > I don't see how to get around this without an option to override > MODULE_DIRECTORY. > > I guess I'll ask Arch Linux to revert this option for the time being, as > it mentions they do not really need it at the moment. > > [1]: https://gitlab.archlinux.org/archlinux/packaging/packages/kmod/-/commit/0efd732cb78bc0b7851a8367f4dc8e6933f5b99d > Since we've officially (?) switched to Github for issues, I opened one [1]. I suspect it would be a little harder to get lost in the noise, although Github and all that. Everyone feel free to subscribe, comment ... or even send some patches. Ideally we'll get Michal to help this although I'm not sure how busy he is. HTH Emil [1] https://github.com/kmod-project/kmod/issues/85
On Thu, Aug 22, 2024 at 5:36 PM Michal Suchánek <msuchanek@suse.de> wrote: > > Hello, > > On Thu, Aug 22, 2024 at 01:05:11AM -0500, Lucas De Marchi wrote: > > On Tue, Dec 19, 2023 at 05:37:31PM GMT, Masahiro Yamada wrote: > > > On Thu, Dec 7, 2023 at 3:37 AM Lucas De Marchi <lucas.demarchi@intel.com> wrote: > > > > > > > > On Fri, Nov 10, 2023 at 01:13:53PM +0100, Michal Suchanek wrote: > > > > >Hello, > > > > > > > > > >This is resend of the last patch in the series that adds prefix support > > > > >to kernel module location together with additional patch for validating > > > > >the user supplied input to options that are interpreted as directories. > > > > > > > > > >Thanks > > > > > > > > applied, thanks > > > > > > > > Lucas De Marchi > > > > > > > > > > > > If I understood this correctly, MODULE_DIRECTORY is determined > > > by "configure --with-module-directory=...", and there is no > > > way to change it after that. > > > > > > > > > If so, how to work with cross-building? > > > > > > Cross-building is typical when building embedded Linux systems. > > > > I was thinking the `pkg-config --variable=module_directory` > > from the target would be sufficient, but... > > > > > > > > > > > Consider this scenario: > > > > > > - Your build machine adopts > > > MODULE_DIRECTORY=/usr/lib/modules > > > - The target embedded system adopts > > > MODULE_DIRECTORY=/lib/modules > > > > > > (or vice a versa) > > > depmod is used also for cross-building because > > > it is executed as a part of "make module_install". > > > > > > > > > The counterpart patch set for Kbuild provides > > > KERNEL_MODULE_DIRECTORY, which only changes > > > the destination directory to which *.ko are copied. > > > > > > You cannot change the directory where the > > > depmod searches for modules, as it is fixed > > > at the compile-time of kmod. > > > > > > > > > > > > > > > In this case, what we can do is to build another > > > instance of kmod configured for the target system, > > > > the target system may not even have depmod actually, so using just the > > host one seems more appropriate. But target should have the kmod.pc for > > the pkg-config call to work. > > > > > and use it for modules_install: > > > > > > 1. In the kmod source directory > > > ./configure --with=module-directory=/lib/modules > > > make > > > > > > 2. make modules_install INSTALL_MOD_PATH=<staging-dir> > > > KERNEL_MODULE_DIRECTORY=/lib/modules > > > DEPMOD=<new-depmod-you-has-just-built> > > > > > > > > > > > > If you use OpenEmbedded etc., this is what you do > > > because host tools are built from sources. > > > > > > But, should it be required all the time? > > > Even when the target embedded system uses > > > busybox-based modprobe instead of kmod? > > > > no, I don't think we can rely on depmod from the target. > > > > > > > > > > > > > > depmod provides --basedir option, which changes > > > the prefix part, but there is no way to override > > > the stem part, MODULE_DIRECTRY. > > > > > > In the review of the counter patch set, > > > I am suggesting an option to override MODULE_DIRECTRY > > > (let's say --moduledir) at least for depmod. > > > > ok > > > > > > > > (Perhaps modinfo too, as it also supports --basedir) > > > > > > > > > > > > Then, we can change scripts/depmod.sh so that > > > Kbuild can propagate KERNEL_MODULE_DIRECTORY > > > to depmod. > > > > > > > > > if <depmod supports --moduledir>; then > > > set -- "$@" --moduledir "${KERNEL_MODULE_DIRECTORY}" > > > fi > > > > > > > > > > > > Does it make sense? > > It does not make sense for the common case: building kernel for the host > system. > > Then overriding the directory is wrong, and using what kmod was compiled > with is needed to get correct module directory layout. NACK. scripts/Makefile.modinst and depmod must agree about the install destination. Both must refer to the same ${KERNEL_MODULE_DIRECTORY}. > > Or it would make sense if both was done: > > Default KERNEL_MODULE_DIRECTORY to what kmod was compiled with, and > then pass the actual value to depmod so that depmod uses the compiled-in > value by default, and the user-provided value when > KERNEL_MODULE_DIRECTORY was overridden by the user. > > Thanks > > Michal
On Fri, Aug 23, 2024 at 10:03:05PM +0900, Masahiro Yamada wrote: > On Thu, Aug 22, 2024 at 5:36 PM Michal Suchánek <msuchanek@suse.de> wrote: > > > > Hello, > > > > On Thu, Aug 22, 2024 at 01:05:11AM -0500, Lucas De Marchi wrote: > > > On Tue, Dec 19, 2023 at 05:37:31PM GMT, Masahiro Yamada wrote: > > > > On Thu, Dec 7, 2023 at 3:37 AM Lucas De Marchi <lucas.demarchi@intel.com> wrote: > > > > > > > > > > On Fri, Nov 10, 2023 at 01:13:53PM +0100, Michal Suchanek wrote: > > > > > >Hello, > > > > > > > > > > > >This is resend of the last patch in the series that adds prefix support > > > > > >to kernel module location together with additional patch for validating > > > > > >the user supplied input to options that are interpreted as directories. > > > > > > > > > > > >Thanks > > > > > > > > > > applied, thanks > > > > > > > > > > Lucas De Marchi > > > > > > > > > > > > > > > > If I understood this correctly, MODULE_DIRECTORY is determined > > > > by "configure --with-module-directory=...", and there is no > > > > way to change it after that. > > > > > > > > > > > > If so, how to work with cross-building? > > > > > > > > Cross-building is typical when building embedded Linux systems. > > > > > > I was thinking the `pkg-config --variable=module_directory` > > > from the target would be sufficient, but... > > > > > > > > > > > > > > > Consider this scenario: > > > > > > > > - Your build machine adopts > > > > MODULE_DIRECTORY=/usr/lib/modules > > > > - The target embedded system adopts > > > > MODULE_DIRECTORY=/lib/modules > > > > > > > > (or vice a versa) > > > > depmod is used also for cross-building because > > > > it is executed as a part of "make module_install". > > > > > > > > > > > > The counterpart patch set for Kbuild provides > > > > KERNEL_MODULE_DIRECTORY, which only changes > > > > the destination directory to which *.ko are copied. > > > > > > > > You cannot change the directory where the > > > > depmod searches for modules, as it is fixed > > > > at the compile-time of kmod. > > > > > > > > > > > > > > > > > > > > In this case, what we can do is to build another > > > > instance of kmod configured for the target system, > > > > > > the target system may not even have depmod actually, so using just the > > > host one seems more appropriate. But target should have the kmod.pc for > > > the pkg-config call to work. > > > > > > > and use it for modules_install: > > > > > > > > 1. In the kmod source directory > > > > ./configure --with=module-directory=/lib/modules > > > > make > > > > > > > > 2. make modules_install INSTALL_MOD_PATH=<staging-dir> > > > > KERNEL_MODULE_DIRECTORY=/lib/modules > > > > DEPMOD=<new-depmod-you-has-just-built> > > > > > > > > > > > > > > > > If you use OpenEmbedded etc., this is what you do > > > > because host tools are built from sources. > > > > > > > > But, should it be required all the time? > > > > Even when the target embedded system uses > > > > busybox-based modprobe instead of kmod? > > > > > > no, I don't think we can rely on depmod from the target. > > > > > > > > > > > > > > > > > > > depmod provides --basedir option, which changes > > > > the prefix part, but there is no way to override > > > > the stem part, MODULE_DIRECTRY. > > > > > > > > In the review of the counter patch set, > > > > I am suggesting an option to override MODULE_DIRECTRY > > > > (let's say --moduledir) at least for depmod. > > > > > > ok > > > > > > > > > > > (Perhaps modinfo too, as it also supports --basedir) > > > > > > > > > > > > > > > > Then, we can change scripts/depmod.sh so that > > > > Kbuild can propagate KERNEL_MODULE_DIRECTORY > > > > to depmod. > > > > > > > > > > > > if <depmod supports --moduledir>; then > > > > set -- "$@" --moduledir "${KERNEL_MODULE_DIRECTORY}" > > > > fi > > > > > > > > > > > > > > > > Does it make sense? > > > > It does not make sense for the common case: building kernel for the host > > system. > > > > Then overriding the directory is wrong, and using what kmod was compiled > > with is needed to get correct module directory layout. > > > NACK. > > scripts/Makefile.modinst and depmod must agree about > the install destination. > > Both must refer to the same ${KERNEL_MODULE_DIRECTORY}. Nack to what, exactly? And what needs to agree here, exactly? If the kmod was compiled with a non-default kernel module directory then for depmod and scripts/Makefile.modinst to agree the kernel makefile should extract the directory it was compile with from kmod kmod, and that is the change that was not merged. Overriding the directory with an option only for the kernel build will make modules_install install the modules in the wrong directory. Of course, the user is free to move them elsewhere afterwards but I would say they should not need to do that. Then there is the possibility that the build is for a different target system, and the host kmod and scripts/Makefile.modinst agreeing is not enough. Then either a 'cross' kmod can be built, and this will again work so long as both path and pkgconfig path point to this modified kmod. Or kmod can grow an option to set the kernel module directory dynamically. However, setting it to the current hardcoded value will again break the case when building for the current system with non-default kernel module directory location. Unless both is done - that is the default is read from kmod, user is provided with an option to override the default, and whatever ends up being used by scripts/Makefile.modinst is then passed back to depmod. Or do you envision some other solution? Thanks Michal > > Or it would make sense if both was done: > > > > Default KERNEL_MODULE_DIRECTORY to what kmod was compiled with, and > > then pass the actual value to depmod so that depmod uses the compiled-in > > value by default, and the user-provided value when > > KERNEL_MODULE_DIRECTORY was overridden by the user. > > > > Thanks > > > > Michal > > > > -- > Best Regards > Masahiro Yamada
diff --git a/Makefile.am b/Makefile.am index 6d0b2decfef3..019aa749fdf1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -20,6 +20,7 @@ AM_CPPFLAGS = \ -I$(top_srcdir) \ -DSYSCONFDIR=\""$(sysconfdir)"\" \ -DDISTCONFDIR=\""$(distconfdir)"\" \ + -DMODULE_DIRECTORY=\""$(module_directory)"\" \ ${zlib_CFLAGS} AM_CFLAGS = $(OUR_CFLAGS) @@ -220,7 +221,7 @@ EXTRA_DIST += testsuite/setup-rootfs.sh MODULE_PLAYGROUND = testsuite/module-playground ROOTFS = testsuite/rootfs ROOTFS_PRISTINE = $(top_srcdir)/testsuite/rootfs-pristine -CREATE_ROOTFS = $(AM_V_GEN) $(top_srcdir)/testsuite/setup-rootfs.sh $(ROOTFS_PRISTINE) $(ROOTFS) $(MODULE_PLAYGROUND) $(top_builddir)/config.h $(sysconfdir) +CREATE_ROOTFS = $(AM_V_GEN) MODULE_DIRECTORY=$(module_directory) $(top_srcdir)/testsuite/setup-rootfs.sh $(ROOTFS_PRISTINE) $(ROOTFS) $(MODULE_PLAYGROUND) $(top_builddir)/config.h $(sysconfdir) build-module-playground: $(AM_V_GEN)if test "$(top_srcdir)" != "$(top_builddir)"; then \ diff --git a/configure.ac b/configure.ac index b4584d6cdc67..4051dc9249e2 100644 --- a/configure.ac +++ b/configure.ac @@ -91,6 +91,12 @@ AC_ARG_WITH([rootlibdir], [], [with_rootlibdir=$libdir]) AC_SUBST([rootlibdir], [$with_rootlibdir]) +# Ideally this would be $prefix/lib/modules but default to /lib/modules for compatibility with earlier versions +AC_ARG_WITH([module_directory], + AS_HELP_STRING([--with-module-directory=DIR], [directory in which to look for kernel modules - typically '/lib/modules' or '${prefix}/lib/modules']), + [], [with_module_directory=/lib/modules]) +AC_SUBST([module_directory], [$with_module_directory]) + AC_ARG_WITH([zstd], AS_HELP_STRING([--with-zstd], [handle Zstandard-compressed modules @<:@default=disabled@:>@]), [], [with_zstd=no]) @@ -326,6 +332,7 @@ AC_MSG_RESULT([ $PACKAGE $VERSION ======= + module_directory: ${module_directory} prefix: ${prefix} sysconfdir: ${sysconfdir} distconfdir: ${distconfdir} diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c index 09e6041461b0..63719e886de8 100644 --- a/libkmod/libkmod.c +++ b/libkmod/libkmod.c @@ -209,7 +209,7 @@ static int log_priority(const char *priority) return 0; } -static const char *dirname_default_prefix = "/lib/modules"; +static const char *dirname_default_prefix = MODULE_DIRECTORY; static char *get_kernel_release(const char *dirname) { @@ -231,7 +231,7 @@ static char *get_kernel_release(const char *dirname) /** * kmod_new: * @dirname: what to consider as linux module's directory, if NULL - * defaults to /lib/modules/`uname -r`. If it's relative, + * defaults to ${module_prefix}/lib/modules/`uname -r`. If it's relative, * it's treated as relative to the current working directory. * Otherwise, give an absolute dirname. * @config_paths: ordered array of paths (directories or files) where diff --git a/man/Makefile.am b/man/Makefile.am index 2fea8e46bf2f..f550091a216a 100644 --- a/man/Makefile.am +++ b/man/Makefile.am @@ -22,6 +22,7 @@ CLEANFILES = $(dist_man_MANS) else \ sed -e '/@DISTCONFDIR@/d' $< ; \ fi | \ + sed -e 's|@MODULE_DIRECTORY@|$(module_directory)|g' | \ $(XSLT) \ -o $@ \ --nonet \ diff --git a/man/depmod.d.xml b/man/depmod.d.xml index f282a39cc840..b07e6a2bd4fe 100644 --- a/man/depmod.d.xml +++ b/man/depmod.d.xml @@ -70,7 +70,7 @@ </term> <listitem> <para> - This allows you to specify the order in which /lib/modules + This allows you to specify the order in which @MODULE_DIRECTORY@ (or other configured module location) subdirectories will be processed by <command>depmod</command>. Directories are listed in order, with the highest priority given to the @@ -101,7 +101,7 @@ <command>depmod</command> command. It is possible to specify one kernel or all kernels using the * wildcard. <replaceable>modulesubdirectory</replaceable> is the - name of the subdirectory under /lib/modules (or other + name of the subdirectory under @MODULE_DIRECTORY@ (or other module location) where the target module is installed. </para> <para> @@ -110,7 +110,7 @@ specifying the following command: "override kmod * extra". This will ensure that any matching module name installed under the <command>extra</command> subdirectory within - /lib/modules (or other module location) will take priority + @MODULE_DIRECTORY@ (or other module location) will take priority over any likenamed module already provided by the kernel. </para> </listitem> diff --git a/man/depmod.xml b/man/depmod.xml index 3b0097184fd7..fce2a4a67a89 100644 --- a/man/depmod.xml +++ b/man/depmod.xml @@ -80,7 +80,7 @@ </para> <para> <command>depmod</command> creates a list of module dependencies by reading each module under - <filename>/lib/modules/</filename><replaceable>version</replaceable> and + <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable> and determining what symbols it exports and what symbols it needs. By default, this list is written to <filename>modules.dep</filename>, and a binary hashed version named <filename>modules.dep.bin</filename>, in the @@ -141,7 +141,7 @@ <listitem> <para> If your modules are not currently in the (normal) directory - <filename>/lib/modules/</filename><replaceable>version</replaceable>, + <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable>, but in a staging area, you can specify a <replaceable>basedir</replaceable> which is prepended to the directory name. This <replaceable>basedir</replaceable> is diff --git a/man/modinfo.xml b/man/modinfo.xml index 9fe0324a2527..b6c4d6045829 100644 --- a/man/modinfo.xml +++ b/man/modinfo.xml @@ -54,7 +54,7 @@ <command>modinfo</command> extracts information from the Linux Kernel modules given on the command line. If the module name is not a filename, then the - <filename>/lib/modules/</filename><replaceable>version</replaceable> + <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable> directory is searched, as is also done by <citerefentry><refentrytitle>modprobe</refentrytitle><manvolnum>8</manvolnum></citerefentry> when loading kernel modules. diff --git a/man/modprobe.xml b/man/modprobe.xml index 91f9e27997cd..4d1fd59c000b 100644 --- a/man/modprobe.xml +++ b/man/modprobe.xml @@ -78,7 +78,7 @@ is no difference between _ and - in module names (automatic underscore conversion is performed). <command>modprobe</command> looks in the module directory - <filename>/lib/modules/`uname -r`</filename> for all + <filename>@MODULE_DIRECTORY@/`uname -r`</filename> for all the modules and other files, except for the optional configuration files in the <filename>/etc/modprobe.d</filename> directory diff --git a/man/modules.dep.xml b/man/modules.dep.xml index ed633694ec9e..8ef6d8b3536e 100644 --- a/man/modules.dep.xml +++ b/man/modules.dep.xml @@ -34,8 +34,8 @@ </refnamediv> <refsynopsisdiv> - <para><filename>/lib/modules/modules.dep</filename></para> - <para><filename>/lib/modules/modules.dep.bin</filename></para> + <para><filename>@MODULE_DIRECTORY@/modules.dep</filename></para> + <para><filename>@MODULE_DIRECTORY@/modules.dep.bin</filename></para> </refsynopsisdiv> <refsect1><title>DESCRIPTION</title> @@ -43,7 +43,7 @@ <filename>modules.dep.bin</filename> is a binary file generated by <command>depmod</command> listing the dependencies for every module in the directories under - <filename>/lib/modules/</filename><replaceable>version</replaceable>. + <filename>@MODULE_DIRECTORY@/</filename><replaceable>version</replaceable>. It is used by kmod tools such as <command>modprobe</command> and libkmod. </para> diff --git a/testsuite/module-playground/Makefile b/testsuite/module-playground/Makefile index e6045b0dd932..a7ab09bea2bf 100644 --- a/testsuite/module-playground/Makefile +++ b/testsuite/module-playground/Makefile @@ -47,7 +47,7 @@ endif else # normal makefile -KDIR ?= /lib/modules/`uname -r`/build +KDIR ?= $(module_prefix)/lib/modules/`uname -r`/build KVER ?= `uname -r` ifeq ($(FAKE_BUILD),) FAKE_BUILD=0 diff --git a/testsuite/setup-rootfs.sh b/testsuite/setup-rootfs.sh index 4440ddcd6b4d..a780f9381b3c 100755 --- a/testsuite/setup-rootfs.sh +++ b/testsuite/setup-rootfs.sh @@ -16,6 +16,19 @@ create_rootfs() { cp -r "$ROOTFS_PRISTINE" "$ROOTFS" find "$ROOTFS" -type d -exec chmod +w {} \; find "$ROOTFS" -type f -name .gitignore -exec rm -f {} \; + if [ "$MODULE_DIRECTORY" != "/lib/modules" ] ; then + sed -i -e "s|/lib/modules|$MODULE_DIRECTORY|g" $(find "$ROOTFS" -name \*.txt -o -name \*.conf -o -name \*.dep) + sed -i -e "s|$MODULE_DIRECTORY/external|/lib/modules/external|g" $(find "$ROOTFS" -name \*.txt -o -name \*.conf -o -name \*.dep) + for i in "$ROOTFS"/*/lib/modules/* "$ROOTFS"/*/*/lib/modules/* ; do + version="$(basename $i)" + [ $version != 'external' ] || continue + mod="$(dirname $i)" + lib="$(dirname $mod)" + up="$(dirname $lib)$MODULE_DIRECTORY" + mkdir -p "$up" + mv "$i" "$up" + done + fi if [ "$SYSCONFDIR" != "/etc" ]; then find "$ROOTFS" -type d -name etc -printf "%h\n" | while read -r e; do @@ -32,57 +45,57 @@ feature_enabled() { declare -A map map=( - ["test-depmod/search-order-simple/lib/modules/4.4.4/kernel/crypto/"]="mod-simple.ko" - ["test-depmod/search-order-simple/lib/modules/4.4.4/updates/"]="mod-simple.ko" - ["test-depmod/search-order-same-prefix/lib/modules/4.4.4/foo/"]="mod-simple.ko" - ["test-depmod/search-order-same-prefix/lib/modules/4.4.4/foobar/"]="mod-simple.ko" - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko" - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko" - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-c.ko"]="mod-loop-c.ko" - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-d.ko"]="mod-loop-d.ko" - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-e.ko"]="mod-loop-e.ko" - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-f.ko"]="mod-loop-f.ko" - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-g.ko"]="mod-loop-g.ko" - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-h.ko"]="mod-loop-h.ko" - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-i.ko"]="mod-loop-i.ko" - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-j.ko"]="mod-loop-j.ko" - ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-k.ko"]="mod-loop-k.ko" - ["test-depmod/search-order-external-first/lib/modules/4.4.4/foo/"]="mod-simple.ko" - ["test-depmod/search-order-external-first/lib/modules/4.4.4/foobar/"]="mod-simple.ko" + ["test-depmod/search-order-simple$MODULE_DIRECTORY/4.4.4/kernel/crypto/"]="mod-simple.ko" + ["test-depmod/search-order-simple$MODULE_DIRECTORY/4.4.4/updates/"]="mod-simple.ko" + ["test-depmod/search-order-same-prefix$MODULE_DIRECTORY/4.4.4/foo/"]="mod-simple.ko" + ["test-depmod/search-order-same-prefix$MODULE_DIRECTORY/4.4.4/foobar/"]="mod-simple.ko" + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko" + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko" + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-c.ko"]="mod-loop-c.ko" + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-d.ko"]="mod-loop-d.ko" + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-e.ko"]="mod-loop-e.ko" + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-f.ko"]="mod-loop-f.ko" + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-g.ko"]="mod-loop-g.ko" + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-h.ko"]="mod-loop-h.ko" + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-i.ko"]="mod-loop-i.ko" + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-j.ko"]="mod-loop-j.ko" + ["test-depmod/detect-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-k.ko"]="mod-loop-k.ko" + ["test-depmod/search-order-external-first$MODULE_DIRECTORY/4.4.4/foo/"]="mod-simple.ko" + ["test-depmod/search-order-external-first$MODULE_DIRECTORY/4.4.4/foobar/"]="mod-simple.ko" ["test-depmod/search-order-external-first/lib/modules/external/"]="mod-simple.ko" - ["test-depmod/search-order-external-last/lib/modules/4.4.4/foo/"]="mod-simple.ko" - ["test-depmod/search-order-external-last/lib/modules/4.4.4/foobar/"]="mod-simple.ko" + ["test-depmod/search-order-external-last$MODULE_DIRECTORY/4.4.4/foo/"]="mod-simple.ko" + ["test-depmod/search-order-external-last$MODULE_DIRECTORY/4.4.4/foobar/"]="mod-simple.ko" ["test-depmod/search-order-external-last/lib/modules/external/"]="mod-simple.ko" - ["test-depmod/search-order-override/lib/modules/4.4.4/foo/"]="mod-simple.ko" - ["test-depmod/search-order-override/lib/modules/4.4.4/override/"]="mod-simple.ko" - ["test-dependencies/lib/modules/4.0.20-kmod/kernel/fs/foo/"]="mod-foo-b.ko" - ["test-dependencies/lib/modules/4.0.20-kmod/kernel/"]="mod-foo-c.ko" - ["test-dependencies/lib/modules/4.0.20-kmod/kernel/lib/"]="mod-foo-a.ko" - ["test-dependencies/lib/modules/4.0.20-kmod/kernel/fs/"]="mod-foo.ko" + ["test-depmod/search-order-override$MODULE_DIRECTORY/4.4.4/foo/"]="mod-simple.ko" + ["test-depmod/search-order-override$MODULE_DIRECTORY/4.4.4/override/"]="mod-simple.ko" + ["test-dependencies$MODULE_DIRECTORY/4.0.20-kmod/kernel/fs/foo/"]="mod-foo-b.ko" + ["test-dependencies$MODULE_DIRECTORY/4.0.20-kmod/kernel/"]="mod-foo-c.ko" + ["test-dependencies$MODULE_DIRECTORY/4.0.20-kmod/kernel/lib/"]="mod-foo-a.ko" + ["test-dependencies$MODULE_DIRECTORY/4.0.20-kmod/kernel/fs/"]="mod-foo.ko" ["test-init/"]="mod-simple.ko" ["test-remove/"]="mod-simple.ko" - ["test-modprobe/show-depends/lib/modules/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko" - ["test-modprobe/show-depends/lib/modules/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko" - ["test-modprobe/show-depends/lib/modules/4.4.4/kernel/mod-simple.ko"]="mod-simple.ko" + ["test-modprobe/show-depends$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko" + ["test-modprobe/show-depends$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko" + ["test-modprobe/show-depends$MODULE_DIRECTORY/4.4.4/kernel/mod-simple.ko"]="mod-simple.ko" ["test-modprobe/show-exports/mod-loop-a.ko"]="mod-loop-a.ko" - ["test-modprobe/softdep-loop/lib/modules/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko" - ["test-modprobe/softdep-loop/lib/modules/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko" - ["test-modprobe/install-cmd-loop/lib/modules/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko" - ["test-modprobe/install-cmd-loop/lib/modules/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko" - ["test-modprobe/force/lib/modules/4.4.4/kernel/"]="mod-simple.ko" - ["test-modprobe/oldkernel/lib/modules/3.3.3/kernel/"]="mod-simple.ko" - ["test-modprobe/oldkernel-force/lib/modules/3.3.3/kernel/"]="mod-simple.ko" - ["test-modprobe/alias-to-none/lib/modules/4.4.4/kernel/"]="mod-simple.ko" - ["test-modprobe/module-param-kcmdline/lib/modules/4.4.4/kernel/"]="mod-simple.ko" + ["test-modprobe/softdep-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko" + ["test-modprobe/softdep-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko" + ["test-modprobe/install-cmd-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko" + ["test-modprobe/install-cmd-loop$MODULE_DIRECTORY/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko" + ["test-modprobe/force$MODULE_DIRECTORY/4.4.4/kernel/"]="mod-simple.ko" + ["test-modprobe/oldkernel$MODULE_DIRECTORY/3.3.3/kernel/"]="mod-simple.ko" + ["test-modprobe/oldkernel-force$MODULE_DIRECTORY/3.3.3/kernel/"]="mod-simple.ko" + ["test-modprobe/alias-to-none$MODULE_DIRECTORY/4.4.4/kernel/"]="mod-simple.ko" + ["test-modprobe/module-param-kcmdline$MODULE_DIRECTORY/4.4.4/kernel/"]="mod-simple.ko" ["test-modprobe/external/lib/modules/external/"]="mod-simple.ko" ["test-modprobe/module-from-abspath/home/foo/"]="mod-simple.ko" ["test-modprobe/module-from-relpath/home/foo/"]="mod-simple.ko" - ["test-depmod/modules-order-compressed/lib/modules/4.4.4/kernel/drivers/block/cciss.ko"]="mod-fake-cciss.ko" - ["test-depmod/modules-order-compressed/lib/modules/4.4.4/kernel/drivers/scsi/hpsa.ko"]="mod-fake-hpsa.ko" - ["test-depmod/modules-order-compressed/lib/modules/4.4.4/kernel/drivers/scsi/scsi_mod.ko"]="mod-fake-scsi-mod.ko" - ["test-depmod/modules-outdir/lib/modules/4.4.4/kernel/drivers/block/cciss.ko"]="mod-fake-cciss.ko" - ["test-depmod/modules-outdir/lib/modules/4.4.4/kernel/drivers/scsi/hpsa.ko"]="mod-fake-hpsa.ko" - ["test-depmod/modules-outdir/lib/modules/4.4.4/kernel/drivers/scsi/scsi_mod.ko"]="mod-fake-scsi-mod.ko" + ["test-depmod/modules-order-compressed$MODULE_DIRECTORY/4.4.4/kernel/drivers/block/cciss.ko"]="mod-fake-cciss.ko" + ["test-depmod/modules-order-compressed$MODULE_DIRECTORY/4.4.4/kernel/drivers/scsi/hpsa.ko"]="mod-fake-hpsa.ko" + ["test-depmod/modules-order-compressed$MODULE_DIRECTORY/4.4.4/kernel/drivers/scsi/scsi_mod.ko"]="mod-fake-scsi-mod.ko" + ["test-depmod/modules-outdir$MODULE_DIRECTORY/4.4.4/kernel/drivers/block/cciss.ko"]="mod-fake-cciss.ko" + ["test-depmod/modules-outdir$MODULE_DIRECTORY/4.4.4/kernel/drivers/scsi/hpsa.ko"]="mod-fake-hpsa.ko" + ["test-depmod/modules-outdir$MODULE_DIRECTORY/4.4.4/kernel/drivers/scsi/scsi_mod.ko"]="mod-fake-scsi-mod.ko" ["test-modinfo/mod-simple-i386.ko"]="mod-simple-i386.ko" ["test-modinfo/mod-simple-x86_64.ko"]="mod-simple-x86_64.ko" ["test-modinfo/mod-simple-sparc64.ko"]="mod-simple-sparc64.ko" @@ -90,20 +103,20 @@ map=( ["test-modinfo/mod-simple-sha256.ko"]="mod-simple.ko" ["test-modinfo/mod-simple-pkcs7.ko"]="mod-simple.ko" ["test-modinfo/external/lib/modules/external/mod-simple.ko"]="mod-simple.ko" - ["test-tools/insert/lib/modules/4.4.4/kernel/"]="mod-simple.ko" - ["test-tools/remove/lib/modules/4.4.4/kernel/"]="mod-simple.ko" + ["test-tools/insert$MODULE_DIRECTORY/4.4.4/kernel/"]="mod-simple.ko" + ["test-tools/remove$MODULE_DIRECTORY/4.4.4/kernel/"]="mod-simple.ko" ) gzip_array=( - "test-depmod/modules-order-compressed/lib/modules/4.4.4/kernel/drivers/block/cciss.ko" + "test-depmod/modules-order-compressed$MODULE_DIRECTORY/4.4.4/kernel/drivers/block/cciss.ko" ) xz_array=( - "test-depmod/modules-order-compressed/lib/modules/4.4.4/kernel/drivers/scsi/scsi_mod.ko" + "test-depmod/modules-order-compressed$MODULE_DIRECTORY/4.4.4/kernel/drivers/scsi/scsi_mod.ko" ) zstd_array=( - "test-depmod/modules-order-compressed/lib/modules/4.4.4/kernel/drivers/scsi/hpsa.ko" + "test-depmod/modules-order-compressed$MODULE_DIRECTORY/4.4.4/kernel/drivers/scsi/hpsa.ko" ) attach_sha256_array=( diff --git a/testsuite/test-depmod.c b/testsuite/test-depmod.c index 93606947f18a..c96dbf0a62be 100644 --- a/testsuite/test-depmod.c +++ b/testsuite/test-depmod.c @@ -27,7 +27,7 @@ #define MODULES_UNAME "4.4.4" #define MODULES_ORDER_ROOTFS TESTSUITE_ROOTFS "test-depmod/modules-order-compressed" -#define MODULES_ORDER_LIB_MODULES MODULES_ORDER_ROOTFS "/lib/modules/" MODULES_UNAME +#define MODULES_ORDER_LIB_MODULES MODULES_ORDER_ROOTFS MODULE_DIRECTORY "/" MODULES_UNAME static noreturn int depmod_modules_order_for_compressed(const struct test *t) { const char *progname = ABS_TOP_BUILDDIR "/tools/depmod"; @@ -55,8 +55,8 @@ DEFINE_TEST(depmod_modules_order_for_compressed, }); #define MODULES_OUTDIR_ROOTFS TESTSUITE_ROOTFS "test-depmod/modules-outdir" -#define MODULES_OUTDIR_LIB_MODULES_OUTPUT MODULES_OUTDIR_ROOTFS "/outdir/lib/modules/" MODULES_UNAME -#define MODULES_OUTDIR_LIB_MODULES_INPUT MODULES_OUTDIR_ROOTFS "/lib/modules/" MODULES_UNAME +#define MODULES_OUTDIR_LIB_MODULES_OUTPUT MODULES_OUTDIR_ROOTFS "/outdir" MODULE_DIRECTORY "/" MODULES_UNAME +#define MODULES_OUTDIR_LIB_MODULES_INPUT MODULES_OUTDIR_ROOTFS MODULE_DIRECTORY "/" MODULES_UNAME static noreturn int depmod_modules_outdir(const struct test *t) { const char *progname = ABS_TOP_BUILDDIR "/tools/depmod"; @@ -87,7 +87,7 @@ DEFINE_TEST(depmod_modules_outdir, }); #define SEARCH_ORDER_SIMPLE_ROOTFS TESTSUITE_ROOTFS "test-depmod/search-order-simple" -#define SEARCH_ORDER_SIMPLE_LIB_MODULES SEARCH_ORDER_SIMPLE_ROOTFS "/lib/modules/" MODULES_UNAME +#define SEARCH_ORDER_SIMPLE_LIB_MODULES SEARCH_ORDER_SIMPLE_ROOTFS MODULE_DIRECTORY "/" MODULES_UNAME static noreturn int depmod_search_order_simple(const struct test *t) { const char *progname = ABS_TOP_BUILDDIR "/tools/depmod"; @@ -114,7 +114,7 @@ DEFINE_TEST(depmod_search_order_simple, }); #define SEARCH_ORDER_SAME_PREFIX_ROOTFS TESTSUITE_ROOTFS "test-depmod/search-order-same-prefix" -#define SEARCH_ORDER_SAME_PREFIX_LIB_MODULES SEARCH_ORDER_SAME_PREFIX_ROOTFS "/lib/modules/" MODULES_UNAME +#define SEARCH_ORDER_SAME_PREFIX_LIB_MODULES SEARCH_ORDER_SAME_PREFIX_ROOTFS MODULE_DIRECTORY "/" MODULES_UNAME static noreturn int depmod_search_order_same_prefix(const struct test *t) { const char *progname = ABS_TOP_BUILDDIR "/tools/depmod"; @@ -164,7 +164,7 @@ DEFINE_TEST(depmod_detect_loop, }); #define SEARCH_ORDER_EXTERNAL_FIRST_ROOTFS TESTSUITE_ROOTFS "test-depmod/search-order-external-first" -#define SEARCH_ORDER_EXTERNAL_FIRST_LIB_MODULES SEARCH_ORDER_EXTERNAL_FIRST_ROOTFS "/lib/modules/" MODULES_UNAME +#define SEARCH_ORDER_EXTERNAL_FIRST_LIB_MODULES SEARCH_ORDER_EXTERNAL_FIRST_ROOTFS MODULE_DIRECTORY "/" MODULES_UNAME static noreturn int depmod_search_order_external_first(const struct test *t) { const char *progname = ABS_TOP_BUILDDIR "/tools/depmod"; @@ -191,7 +191,7 @@ DEFINE_TEST(depmod_search_order_external_first, }); #define SEARCH_ORDER_EXTERNAL_LAST_ROOTFS TESTSUITE_ROOTFS "test-depmod/search-order-external-last" -#define SEARCH_ORDER_EXTERNAL_LAST_LIB_MODULES SEARCH_ORDER_EXTERNAL_LAST_ROOTFS "/lib/modules/" MODULES_UNAME +#define SEARCH_ORDER_EXTERNAL_LAST_LIB_MODULES SEARCH_ORDER_EXTERNAL_LAST_ROOTFS MODULE_DIRECTORY "/" MODULES_UNAME static noreturn int depmod_search_order_external_last(const struct test *t) { const char *progname = ABS_TOP_BUILDDIR "/tools/depmod"; @@ -218,7 +218,7 @@ DEFINE_TEST(depmod_search_order_external_last, }); #define SEARCH_ORDER_OVERRIDE_ROOTFS TESTSUITE_ROOTFS "test-depmod/search-order-override" -#define SEARCH_ORDER_OVERRIDE_LIB_MODULES SEARCH_ORDER_OVERRIDE_ROOTFS "/lib/modules/" MODULES_UNAME +#define SEARCH_ORDER_OVERRIDE_LIB_MODULES SEARCH_ORDER_OVERRIDE_ROOTFS MODULE_DIRECTORY "/" MODULES_UNAME static noreturn int depmod_search_order_override(const struct test *t) { const char *progname = ABS_TOP_BUILDDIR "/tools/depmod"; diff --git a/testsuite/test-testsuite.c b/testsuite/test-testsuite.c index 56e73609f204..c77c4bbc04eb 100644 --- a/testsuite/test-testsuite.c +++ b/testsuite/test-testsuite.c @@ -64,7 +64,7 @@ static int testsuite_rootfs_fopen(const struct test *t) char s[100]; int n; - fp = fopen("/lib/modules/a", "r"); + fp = fopen(MODULE_DIRECTORY "/a", "r"); if (fp == NULL) return EXIT_FAILURE;; @@ -89,7 +89,7 @@ static int testsuite_rootfs_open(const struct test *t) char buf[100]; int fd, done; - fd = open("/lib/modules/a", O_RDONLY); + fd = open(MODULE_DIRECTORY "/a", O_RDONLY); if (fd < 0) return EXIT_FAILURE; @@ -121,12 +121,12 @@ static int testsuite_rootfs_stat_access(const struct test *t) { struct stat st; - if (access("/lib/modules/a", F_OK) < 0) { + if (access(MODULE_DIRECTORY "/a", F_OK) < 0) { ERR("access failed: %m\n"); return EXIT_FAILURE; } - if (stat("/lib/modules/a", &st) < 0) { + if (stat(MODULE_DIRECTORY "/a", &st) < 0) { ERR("stat failed: %m\n"); return EXIT_FAILURE; } diff --git a/tools/depmod.c b/tools/depmod.c index 630fef9c8fb0..7f7c4b74d216 100644 --- a/tools/depmod.c +++ b/tools/depmod.c @@ -911,7 +911,7 @@ struct vertex; struct mod { struct kmod_module *kmod; char *path; - const char *relpath; /* path relative to '$ROOT/lib/modules/$VER/' */ + const char *relpath; /* path relative to '$ROOT$MODULE_DIRECTORY/$VER/' */ char *uncrelpath; /* same as relpath but ending in .ko */ struct kmod_list *info_list; struct kmod_list *dep_sym_list; @@ -3024,11 +3024,11 @@ static int do_depmod(int argc, char *argv[]) } cfg.dirnamelen = snprintf(cfg.dirname, PATH_MAX, - "%s/lib/modules/%s", + "%s" MODULE_DIRECTORY "/%s", root ?: "", cfg.kversion); cfg.outdirnamelen = snprintf(cfg.outdirname, PATH_MAX, - "%s/lib/modules/%s", + "%s" MODULE_DIRECTORY "/%s", out_root ?: (root ?: ""), cfg.kversion); if (optind == argc) diff --git a/tools/kmod.pc.in b/tools/kmod.pc.in index 2595980a6b35..97215c8ed48f 100644 --- a/tools/kmod.pc.in +++ b/tools/kmod.pc.in @@ -1,6 +1,7 @@ prefix=@prefix@ sysconfdir=@sysconfdir@ distconfdir=@distconfdir@ +module_directory=@module_directory@ module_compressions=@module_compressions@ module_signatures=@module_signatures@ diff --git a/tools/modinfo.c b/tools/modinfo.c index d0aab200af4e..cacc32dc4c40 100644 --- a/tools/modinfo.c +++ b/tools/modinfo.c @@ -367,7 +367,7 @@ static void help(void) "\t-m, --modname Handle argument as module name instead of alias or filename\n" "\t-F, --field=FIELD Print only provided FIELD\n" "\t-k, --set-version=VERSION Use VERSION instead of `uname -r`\n" - "\t-b, --basedir=DIR Use DIR as filesystem root for /lib/modules\n" + "\t-b, --basedir=DIR Use DIR as filesystem root for " MODULE_DIRECTORY "\n" "\t-V, --version Show version\n" "\t-h, --help Show this help\n", program_invocation_short_name); @@ -462,7 +462,7 @@ static int do_modinfo(int argc, char *argv[]) } kversion = u.release; } - snprintf(dirname_buf, sizeof(dirname_buf), "%s/lib/modules/%s", + snprintf(dirname_buf, sizeof(dirname_buf), "%s" MODULE_DIRECTORY "/%s", root, kversion); dirname = dirname_buf; } diff --git a/tools/modprobe.c b/tools/modprobe.c index e891028349a8..5306bef250da 100644 --- a/tools/modprobe.c +++ b/tools/modprobe.c @@ -142,7 +142,7 @@ static void help(void) "\t-n, --show Same as --dry-run\n" "\t-C, --config=FILE Use FILE instead of default search paths\n" - "\t-d, --dirname=DIR Use DIR as filesystem root for /lib/modules\n" + "\t-d, --dirname=DIR Use DIR as filesystem root for " MODULE_DIRECTORY "\n" "\t-S, --set-version=VERSION Use VERSION instead of `uname -r`\n" "\t-s, --syslog print to syslog, not stderr\n" @@ -999,7 +999,7 @@ static int do_modprobe(int argc, char **orig_argv) kversion = u.release; } snprintf(dirname_buf, sizeof(dirname_buf), - "%s/lib/modules/%s", root, + "%s" MODULE_DIRECTORY "/%s", root, kversion); dirname = dirname_buf; } diff --git a/tools/static-nodes.c b/tools/static-nodes.c index 8d2356da73f3..5ef3743e967b 100644 --- a/tools/static-nodes.c +++ b/tools/static-nodes.c @@ -212,15 +212,15 @@ static int do_static_nodes(int argc, char *argv[]) goto finish; } - snprintf(modules, sizeof(modules), "/lib/modules/%s/modules.devname", kernel.release); + snprintf(modules, sizeof(modules), MODULE_DIRECTORY "/%s/modules.devname", kernel.release); in = fopen(modules, "re"); if (in == NULL) { if (errno == ENOENT) { - fprintf(stderr, "Warning: /lib/modules/%s/modules.devname not found - ignoring\n", + fprintf(stderr, "Warning: " MODULE_DIRECTORY "/%s/modules.devname not found - ignoring\n", kernel.release); ret = EXIT_SUCCESS; } else { - fprintf(stderr, "Error: could not open /lib/modules/%s/modules.devname - %m\n", + fprintf(stderr, "Error: could not open " MODULE_DIRECTORY "/%s/modules.devname - %m\n", kernel.release); ret = EXIT_FAILURE; }
Now that modprobe.d is searched under ${prefix}/lib, allow a complete transition to files only under ${prefix} by adding a ${module_directory} configuration. This specifies the directory where to search for kernel modules and should match the location where the kernel/distro installs them. With this distributions that do not want to ship files in /lib can also move kernel modules to /usr while others can keep them in /lib. Signed-off-by: Michal Suchanek <msuchanek@suse.de> --- v4: Make the whole path configurable v5: More verbose commit message --- Makefile.am | 3 +- configure.ac | 7 ++ libkmod/libkmod.c | 4 +- man/Makefile.am | 1 + man/depmod.d.xml | 6 +- man/depmod.xml | 4 +- man/modinfo.xml | 2 +- man/modprobe.xml | 2 +- man/modules.dep.xml | 6 +- testsuite/module-playground/Makefile | 2 +- testsuite/setup-rootfs.sh | 109 +++++++++++++++------------ testsuite/test-depmod.c | 16 ++-- testsuite/test-testsuite.c | 8 +- tools/depmod.c | 6 +- tools/kmod.pc.in | 1 + tools/modinfo.c | 4 +- tools/modprobe.c | 4 +- tools/static-nodes.c | 6 +- 18 files changed, 107 insertions(+), 84 deletions(-)