2023-07-11 15:56:37

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH 0/4] kmod /usr support

Hello,

with these patches it is possible to install kernel modules under an
arbitrary prefix - eg. moving the /lib/modules to /usr/lib/modules with
/usr prefix.

While the modprobe.d and depmod.d search which already includes multiple
paths is expanded to also include $(prefix) the module directory still
supports only one location, only a different one under $(module_prefix).

Having kmod search multiple module locations while only one is supported
now might break some assumption about relative modulke path
corresponding to a specific file, would require more invasive changes to
implement, and is not supportive of the goal of moving the modules away
from /lib.

Both kmod and the kernel need to be patched to make use of this feature.
Patched kernel is backwards compatible with older kmod. Patched kmod
with empty $(module_prefix) is equivalent to unpatched kmod.

Thanks

Michal

Link: https://lore.kernel.org/linux-modules/[email protected]/

Michal Suchanek (4):
man/depmod.d: Fix incorrect /usr/lib search path
libkmod, depmod: Load modprobe.d, depmod.d from $prefix/lib.
kmod: Add config command to show compile time configuration as JSON
libkmod, depmod, modprobe: Search for kernel modules under
${module_prefix}

Makefile.am | 4 +-
configure.ac | 7 ++
libkmod/libkmod.c | 7 +-
man/Makefile.am | 10 ++-
man/depmod.d.xml | 9 ++-
man/depmod.xml | 4 +-
man/kmod.xml | 6 ++
man/modinfo.xml | 2 +-
man/modprobe.d.xml | 1 +
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 | 7 +-
tools/kmod.c | 40 ++++++++++
tools/modinfo.c | 4 +-
tools/modprobe.c | 4 +-
tools/static-nodes.c | 6 +-
20 files changed, 166 insertions(+), 88 deletions(-)

--
2.41.0



2023-07-11 16:00:21

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH 2/4] libkmod, depmod: Load modprobe.d, depmod.d from $prefix/lib.

There is an ongoing effort to limit use of files outside of /usr (or
$prefix on general). Currently all modprobe.d paths are hardcoded to
outside of $prefix. Teach kmod to load modprobe.d from $prefix/lib.

On some distributions /usr/lib and /lib are the same directory because
of a compatibility symlink, and it is possible to craft configuration
files with sideeffects that would behave differently when loaded twice.
However, the override semantic ensures that one 'overrides' the other,
and only one configuration file of the same name is loaded from any of
the seach directories.

Cc: Takashi Iwai <[email protected]>
Signed-off-by: Michal Suchanek <[email protected]>
---
Makefile.am | 1 +
libkmod/libkmod.c | 3 ++-
man/Makefile.am | 9 +++++++--
man/depmod.d.xml | 1 +
man/modprobe.d.xml | 1 +
tools/depmod.c | 1 +
6 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 8ba85c91a0f3..7aa5bfa5638d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -19,6 +19,7 @@ AM_CPPFLAGS = \
-include $(top_builddir)/config.h \
-I$(top_srcdir) \
-DSYSCONFDIR=\""$(sysconfdir)"\" \
+ -DPREFIX=\""$(prefix)"\" \
${zlib_CFLAGS}

AM_CFLAGS = $(OUR_CFLAGS)
diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c
index 2670f9a4611a..13c82b069e84 100644
--- a/libkmod/libkmod.c
+++ b/libkmod/libkmod.c
@@ -65,6 +65,7 @@ static const char *const default_config_paths[] = {
SYSCONFDIR "/modprobe.d",
"/run/modprobe.d",
"/usr/local/lib/modprobe.d",
+ PREFIX "/lib/modprobe.d",
"/lib/modprobe.d",
NULL
};
@@ -237,7 +238,7 @@ static char *get_kernel_release(const char *dirname)
* to load from user-defined configuration parameters such as
* alias, blacklists, commands (install, remove). If NULL
* defaults to /etc/modprobe.d, /run/modprobe.d,
- * /usr/local/lib/modprobe.d and /lib/modprobe.d. Give an empty
+ * /usr/local/lib/modprobe.d and PREFIX/lib/modprobe.d. Give an empty
* vector if configuration should not be read. This array must
* be null terminated.
*
diff --git a/man/Makefile.am b/man/Makefile.am
index 11514d52a190..ad07c30bbd24 100644
--- a/man/Makefile.am
+++ b/man/Makefile.am
@@ -17,9 +17,14 @@ EXTRA_DIST = $(MAN5:%.5=%.xml) $(MAN8:%.8=%.xml)
CLEANFILES = $(dist_man_MANS)

%.5 %.8: %.xml
- $(AM_V_XSLT)$(XSLT) \
+ $(AM_V_XSLT)if [ -n '$(prefix)' ] ; then \
+ sed -e 's|@PREFIX@|$(prefix)|g' $< ; \
+ else \
+ sed -e '/@PREFIX@/d' $< ; \
+ fi | \
+ $(XSLT) \
-o $@ \
--nonet \
--stringparam man.output.quietly 1 \
--param funcsynopsis.style "'ansi'" \
- http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $<
+ http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl -
diff --git a/man/depmod.d.xml b/man/depmod.d.xml
index 8d3d821cddc8..431ebca6654b 100644
--- a/man/depmod.d.xml
+++ b/man/depmod.d.xml
@@ -40,6 +40,7 @@

<refsynopsisdiv>
<para><filename>/lib/depmod.d/*.conf</filename></para>
+ <para><filename>@PREFIX@/lib/depmod.d/*.conf</filename></para>
<para><filename>/usr/local/lib/depmod.d/*.conf</filename></para>
<para><filename>/run/depmod.d/*.conf</filename></para>
<para><filename>/etc/depmod.d/*.conf</filename></para>
diff --git a/man/modprobe.d.xml b/man/modprobe.d.xml
index 0ab3e9110a7e..e8a91d7668af 100644
--- a/man/modprobe.d.xml
+++ b/man/modprobe.d.xml
@@ -41,6 +41,7 @@

<refsynopsisdiv>
<para><filename>/lib/modprobe.d/*.conf</filename></para>
+ <para><filename>@PREFIX@/lib/modprobe.d/*.conf</filename></para>
<para><filename>/usr/local/lib/modprobe.d/*.conf</filename></para>
<para><filename>/run/modprobe.d/*.conf</filename></para>
<para><filename>/etc/modprobe.d/*.conf</filename></para>
diff --git a/tools/depmod.c b/tools/depmod.c
index 1d1d41db860f..7e9339923809 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -54,6 +54,7 @@ static const char *const default_cfg_paths[] = {
SYSCONFDIR "/depmod.d",
"/run/depmod.d",
"/usr/local/lib/depmod.d",
+ PREFIX "/lib/depmod.d",
"/lib/depmod.d",
NULL
};
--
2.41.0


2023-07-11 16:07:10

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH] depmod: Handle installing modules under a prefix

Some distributions aim at not shipping any files in / ustside of usr.

The path under which kernel modules are instaleld is hardcoded to /lib
which conflicts with this goal.

When kmod provides the config command use it to determine the correct
module installation prefix.

On kmod that does not provide the command / is used as before.

Signed-off-by: Michal Suchanek <[email protected]>
---
Makefile | 4 +++-
scripts/depmod.sh | 8 ++++----
2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 47690c28456a..b05d696f06bd 100644
--- a/Makefile
+++ b/Makefile
@@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
# makefile but the argument can be passed to make if needed.
#

-MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
+export KERNEL_MODULE_PREFIX := $(shell kmod config | jq -r .module_prefix)
+
+MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
export MODLIB

PHONY += prepare0
diff --git a/scripts/depmod.sh b/scripts/depmod.sh
index 3643b4f896ed..88ac79056153 100755
--- a/scripts/depmod.sh
+++ b/scripts/depmod.sh
@@ -27,16 +27,16 @@ fi
# numbers, so we cheat with a symlink here
depmod_hack_needed=true
tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
-mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
+mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
- if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
- -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
+ if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
+ -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
depmod_hack_needed=false
fi
fi
rm -rf "$tmp_dir"
if $depmod_hack_needed; then
- symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
+ symlink="$INSTALL_MOD_PATH$KERNEL_MODULE_PREFIX/lib/modules/99.98.$KERNELRELEASE"
ln -s "$KERNELRELEASE" "$symlink"
KERNELRELEASE=99.98.$KERNELRELEASE
fi
--
2.41.0


2023-07-11 16:11:29

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH 3/4] kmod: Add config command to show compile time configuration as JSON

Show prefix (where configuration files are searched/to be installed)
and module compressions supported.

Signed-off-by: Michal Suchanek <[email protected]>
---
man/kmod.xml | 6 ++++++
tools/kmod.c | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 45 insertions(+)

diff --git a/man/kmod.xml b/man/kmod.xml
index 0706ad58c2cc..f992a500f836 100644
--- a/man/kmod.xml
+++ b/man/kmod.xml
@@ -71,6 +71,12 @@
<para>Show the help message.</para>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><command>config</command></term>
+ <listitem>
+ <para>Show compile time options in JSON.</para>
+ </listitem>
+ </varlistentry>
<varlistentry>
<term><command>list</command></term>
<listitem>
diff --git a/tools/kmod.c b/tools/kmod.c
index 55689c075ab1..5a13716955c1 100644
--- a/tools/kmod.c
+++ b/tools/kmod.c
@@ -37,9 +37,11 @@ static const struct option options[] = {
};

static const struct kmod_cmd kmod_cmd_help;
+static const struct kmod_cmd kmod_cmd_config;

static const struct kmod_cmd *kmod_cmds[] = {
&kmod_cmd_help,
+ &kmod_cmd_config,
&kmod_cmd_list,
&kmod_cmd_static_nodes,

@@ -95,6 +97,43 @@ static const struct kmod_cmd kmod_cmd_help = {
.help = "Show help message",
};

+static const char *compressions[] = {
+#ifdef ENABLE_ZSTD
+ "zstd",
+#endif
+#ifdef ENABLE_XZ
+ "xz",
+#endif
+#ifdef ENABLE_ZLIB
+ "gz",
+#endif
+ NULL
+};
+
+static int kmod_config(int argc, char *argv[])
+{
+ unsigned i;
+ printf("{\"prefix\":\"" PREFIX "\""
+ ",\"module_signature\":["
+#ifdef ENABLE_OPENSSL
+ "\"PKCS#7\","
+#endif
+ "\"legacy\"]"
+ ",\"module_compression\":[");
+ for(i = 0; compressions[i]; i++) {
+ printf("%s\"%s\"", i ? "," : "", compressions[i]);
+ }
+ printf("]}\n");
+
+ return EXIT_SUCCESS;
+}
+
+static const struct kmod_cmd kmod_cmd_config = {
+ .name = "config",
+ .cmd = kmod_config,
+ .help = "Show compile time options in JSON",
+};
+
static int handle_kmod_commands(int argc, char *argv[])
{
const char *cmd;
--
2.41.0


2023-07-12 05:57:01

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH] depmod: Handle installing modules under a prefix

On 11. 07. 23, 17:34, Michal Suchanek wrote:
> Some distributions aim at not shipping any files in / ustside of usr.
>
> The path under which kernel modules are instaleld is hardcoded to /lib
> which conflicts with this goal.
>
> When kmod provides the config command use it to determine the correct
> module installation prefix.
>
> On kmod that does not provide the command / is used as before.
>
> Signed-off-by: Michal Suchanek <[email protected]>
> ---
> Makefile | 4 +++-
> scripts/depmod.sh | 8 ++++----
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 47690c28456a..b05d696f06bd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
> # makefile but the argument can be passed to make if needed.
> #
>
> -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> +export KERNEL_MODULE_PREFIX := $(shell kmod config | jq -r .module_prefix)

echo -e 'KERNEL_MODULE_PREFIX := $(shell kmod config | jq -r
.module_prefix)\nall:'|make -f -
invalid command 'config'
parse error: Invalid numeric literal at line 1, column 5

I think you should pipe kmod's 2> /dev/null to support older kmod. Ah,
but you'd need 2> /dev/null for jq too. That would not be good as jq
might not be installed and a user wouldn't see the error. So instead, I
would do:

$(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)

thanks,
--
js
suse labs


2023-07-12 07:22:37

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 2/4] libkmod, depmod: Load modprobe.d, depmod.d from $prefix/lib.

On 11. 07. 23, 17:31, Michal Suchanek wrote:
> There is an ongoing effort to limit use of files outside of /usr (or
> $prefix on general). Currently all modprobe.d paths are hardcoded to
> outside of $prefix. Teach kmod to load modprobe.d from $prefix/lib.
>
> On some distributions /usr/lib and /lib are the same directory because
> of a compatibility symlink, and it is possible to craft configuration
> files with sideeffects that would behave differently when loaded twice.
> However, the override semantic ensures that one 'overrides' the other,
> and only one configuration file of the same name is loaded from any of
> the seach directories.

search

...
> --- a/man/Makefile.am
> +++ b/man/Makefile.am
> @@ -17,9 +17,14 @@ EXTRA_DIST = $(MAN5:%.5=%.xml) $(MAN8:%.8=%.xml)
> CLEANFILES = $(dist_man_MANS)
>
> %.5 %.8: %.xml
> - $(AM_V_XSLT)$(XSLT) \
> + $(AM_V_XSLT)if [ -n '$(prefix)' ] ; then \
> + sed -e 's|@PREFIX@|$(prefix)|g' $< ; \

Hmm, if prefix is empty, this will remove @PREFIX@. So why you need this
'if' at all?

> + else \
> + sed -e '/@PREFIX@/d' $< ; \
> + fi | \


--
js
suse labs


2023-07-12 07:39:59

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH 2/4] libkmod, depmod: Load modprobe.d, depmod.d from $prefix/lib.

Hello,

On Wed, Jul 12, 2023 at 08:47:20AM +0200, Jiri Slaby wrote:
> On 11. 07. 23, 17:31, Michal Suchanek wrote:
> > There is an ongoing effort to limit use of files outside of /usr (or
> > $prefix on general). Currently all modprobe.d paths are hardcoded to
> > outside of $prefix. Teach kmod to load modprobe.d from $prefix/lib.
> >
> > On some distributions /usr/lib and /lib are the same directory because
> > of a compatibility symlink, and it is possible to craft configuration
> > files with sideeffects that would behave differently when loaded twice.
> > However, the override semantic ensures that one 'overrides' the other,
> > and only one configuration file of the same name is loaded from any of
> > the seach directories.
>
> search
>
> ...
> > --- a/man/Makefile.am
> > +++ b/man/Makefile.am
> > @@ -17,9 +17,14 @@ EXTRA_DIST = $(MAN5:%.5=%.xml) $(MAN8:%.8=%.xml)
> > CLEANFILES = $(dist_man_MANS)
> > %.5 %.8: %.xml
> > - $(AM_V_XSLT)$(XSLT) \
> > + $(AM_V_XSLT)if [ -n '$(prefix)' ] ; then \
> > + sed -e 's|@PREFIX@|$(prefix)|g' $< ; \
>
> Hmm, if prefix is empty, this will remove @PREFIX@. So why you need this
> 'if' at all?

It removes the whole duplicate line.

Thanks

Michal

>
> > + else \
> > + sed -e '/@PREFIX@/d' $< ; \
> > + fi | \
>
>
> --
> js
> suse labs
>

2023-07-12 08:13:44

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH] depmod: Handle installing modules under a prefix

On Wed, Jul 12, 2023 at 07:47:13AM +0200, Jiri Slaby wrote:
> On 11. 07. 23, 17:34, Michal Suchanek wrote:
> > Some distributions aim at not shipping any files in / ustside of usr.
> >
> > The path under which kernel modules are instaleld is hardcoded to /lib
> > which conflicts with this goal.
> >
> > When kmod provides the config command use it to determine the correct
> > module installation prefix.
> >
> > On kmod that does not provide the command / is used as before.
> >
> > Signed-off-by: Michal Suchanek <[email protected]>
> > ---
> > Makefile | 4 +++-
> > scripts/depmod.sh | 8 ++++----
> > 2 files changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 47690c28456a..b05d696f06bd 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
> > # makefile but the argument can be passed to make if needed.
> > #
> > -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> > +export KERNEL_MODULE_PREFIX := $(shell kmod config | jq -r .module_prefix)
>
> echo -e 'KERNEL_MODULE_PREFIX := $(shell kmod config | jq -r
> .module_prefix)\nall:'|make -f -
> invalid command 'config'
> parse error: Invalid numeric literal at line 1, column 5
>
> I think you should pipe kmod's 2> /dev/null to support older kmod. Ah, but
> you'd need 2> /dev/null for jq too. That would not be good as jq might not
> be installed and a user wouldn't see the error. So instead, I would do:
>
> $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)

Yes, that sounds reasonable. Also would cover the potential problem of
kmod changing the error output into something that is a valid JSON in
the future.

Thanks

Michal

2023-07-12 13:58:59

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH [email protected]] depmod: Handle installing modules under a prefix

Some distributions aim at not shipping any files in / ustside of usr.

The path under which kernel modules are instaleld is hardcoded to /lib
which conflicts with this goal.

When kmod provides the config command use it to determine the correct
module installation prefix.

On kmod that does not provide the command / is used as before.

Signed-off-by: Michal Suchanek <[email protected]>
---
v2: Avoid error on systems with kmod that does not support config
command
---
Makefile | 4 +++-
scripts/depmod.sh | 8 ++++----
2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 47690c28456a..b1fea135bdec 100644
--- a/Makefile
+++ b/Makefile
@@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
# makefile but the argument can be passed to make if needed.
#

-MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
+export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)
+
+MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
export MODLIB

PHONY += prepare0
diff --git a/scripts/depmod.sh b/scripts/depmod.sh
index 3643b4f896ed..88ac79056153 100755
--- a/scripts/depmod.sh
+++ b/scripts/depmod.sh
@@ -27,16 +27,16 @@ fi
# numbers, so we cheat with a symlink here
depmod_hack_needed=true
tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
-mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
+mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
- if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
- -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
+ if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
+ -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
depmod_hack_needed=false
fi
fi
rm -rf "$tmp_dir"
if $depmod_hack_needed; then
- symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
+ symlink="$INSTALL_MOD_PATH$KERNEL_MODULE_PREFIX/lib/modules/99.98.$KERNELRELEASE"
ln -s "$KERNELRELEASE" "$symlink"
KERNELRELEASE=99.98.$KERNELRELEASE
fi
--
2.41.0


2023-07-12 14:05:35

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH kmod v2 1/4] man/depmod.d: Fix incorrect /usr/lib search path

depmod searches /lib/depmod.d but the man page says /usr/lib/depmod.d is
searched. Align the documentation with the code.

Signed-off-by: Michal Suchanek <[email protected]>
---
v2: Fix commit message typo
---
man/depmod.d.xml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/depmod.d.xml b/man/depmod.d.xml
index 76548e92a312..8d3d821cddc8 100644
--- a/man/depmod.d.xml
+++ b/man/depmod.d.xml
@@ -39,7 +39,7 @@
</refnamediv>

<refsynopsisdiv>
- <para><filename>/usr/lib/depmod.d/*.conf</filename></para>
+ <para><filename>/lib/depmod.d/*.conf</filename></para>
<para><filename>/usr/local/lib/depmod.d/*.conf</filename></para>
<para><filename>/run/depmod.d/*.conf</filename></para>
<para><filename>/etc/depmod.d/*.conf</filename></para>
--
2.41.0


2023-07-12 14:06:01

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH kmod v2 3/4] kmod: Add config command to show compile time configuration as JSON

Show prefix (where configuration files are searched/to be installed),
module compressions, and module signatures supported.

Signed-off-by: Michal Suchanek <[email protected]>
---
v2: mention module signature in commit message
---
man/kmod.xml | 6 ++++++
tools/kmod.c | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 45 insertions(+)

diff --git a/man/kmod.xml b/man/kmod.xml
index 0706ad58c2cc..f992a500f836 100644
--- a/man/kmod.xml
+++ b/man/kmod.xml
@@ -71,6 +71,12 @@
<para>Show the help message.</para>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><command>config</command></term>
+ <listitem>
+ <para>Show compile time options in JSON.</para>
+ </listitem>
+ </varlistentry>
<varlistentry>
<term><command>list</command></term>
<listitem>
diff --git a/tools/kmod.c b/tools/kmod.c
index 55689c075ab1..5a13716955c1 100644
--- a/tools/kmod.c
+++ b/tools/kmod.c
@@ -37,9 +37,11 @@ static const struct option options[] = {
};

static const struct kmod_cmd kmod_cmd_help;
+static const struct kmod_cmd kmod_cmd_config;

static const struct kmod_cmd *kmod_cmds[] = {
&kmod_cmd_help,
+ &kmod_cmd_config,
&kmod_cmd_list,
&kmod_cmd_static_nodes,

@@ -95,6 +97,43 @@ static const struct kmod_cmd kmod_cmd_help = {
.help = "Show help message",
};

+static const char *compressions[] = {
+#ifdef ENABLE_ZSTD
+ "zstd",
+#endif
+#ifdef ENABLE_XZ
+ "xz",
+#endif
+#ifdef ENABLE_ZLIB
+ "gz",
+#endif
+ NULL
+};
+
+static int kmod_config(int argc, char *argv[])
+{
+ unsigned i;
+ printf("{\"prefix\":\"" PREFIX "\""
+ ",\"module_signature\":["
+#ifdef ENABLE_OPENSSL
+ "\"PKCS#7\","
+#endif
+ "\"legacy\"]"
+ ",\"module_compression\":[");
+ for(i = 0; compressions[i]; i++) {
+ printf("%s\"%s\"", i ? "," : "", compressions[i]);
+ }
+ printf("]}\n");
+
+ return EXIT_SUCCESS;
+}
+
+static const struct kmod_cmd kmod_cmd_config = {
+ .name = "config",
+ .cmd = kmod_config,
+ .help = "Show compile time options in JSON",
+};
+
static int handle_kmod_commands(int argc, char *argv[])
{
const char *cmd;
--
2.41.0


2023-07-12 14:06:05

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH kmod v2 2/4] libkmod, depmod: Load modprobe.d, depmod.d from $prefix/lib.

There is an ongoing effort to limit use of files outside of /usr (or
$prefix on general). Currently all modprobe.d paths are hardcoded to
outside of $prefix. Teach kmod to load modprobe.d from $prefix/lib.

On some distributions /usr/lib and /lib are the same directory because
of a compatibility symlink, and it is possible to craft configuration
files with sideeffects that would behave differently when loaded twice.
However, the override semantic ensures that one 'overrides' the other,
and only one configuration file of the same name is loaded from any of
the search directories.

Signed-off-by: Michal Suchanek <[email protected]>
---
v2: Fix commit message typo
---
Makefile.am | 1 +
libkmod/libkmod.c | 3 ++-
man/Makefile.am | 9 +++++++--
man/depmod.d.xml | 1 +
man/modprobe.d.xml | 1 +
tools/depmod.c | 1 +
6 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 8ba85c91a0f3..7aa5bfa5638d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -19,6 +19,7 @@ AM_CPPFLAGS = \
-include $(top_builddir)/config.h \
-I$(top_srcdir) \
-DSYSCONFDIR=\""$(sysconfdir)"\" \
+ -DPREFIX=\""$(prefix)"\" \
${zlib_CFLAGS}

AM_CFLAGS = $(OUR_CFLAGS)
diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c
index 2670f9a4611a..13c82b069e84 100644
--- a/libkmod/libkmod.c
+++ b/libkmod/libkmod.c
@@ -65,6 +65,7 @@ static const char *const default_config_paths[] = {
SYSCONFDIR "/modprobe.d",
"/run/modprobe.d",
"/usr/local/lib/modprobe.d",
+ PREFIX "/lib/modprobe.d",
"/lib/modprobe.d",
NULL
};
@@ -237,7 +238,7 @@ static char *get_kernel_release(const char *dirname)
* to load from user-defined configuration parameters such as
* alias, blacklists, commands (install, remove). If NULL
* defaults to /etc/modprobe.d, /run/modprobe.d,
- * /usr/local/lib/modprobe.d and /lib/modprobe.d. Give an empty
+ * /usr/local/lib/modprobe.d and PREFIX/lib/modprobe.d. Give an empty
* vector if configuration should not be read. This array must
* be null terminated.
*
diff --git a/man/Makefile.am b/man/Makefile.am
index 11514d52a190..ad07c30bbd24 100644
--- a/man/Makefile.am
+++ b/man/Makefile.am
@@ -17,9 +17,14 @@ EXTRA_DIST = $(MAN5:%.5=%.xml) $(MAN8:%.8=%.xml)
CLEANFILES = $(dist_man_MANS)

%.5 %.8: %.xml
- $(AM_V_XSLT)$(XSLT) \
+ $(AM_V_XSLT)if [ -n '$(prefix)' ] ; then \
+ sed -e 's|@PREFIX@|$(prefix)|g' $< ; \
+ else \
+ sed -e '/@PREFIX@/d' $< ; \
+ fi | \
+ $(XSLT) \
-o $@ \
--nonet \
--stringparam man.output.quietly 1 \
--param funcsynopsis.style "'ansi'" \
- http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $<
+ http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl -
diff --git a/man/depmod.d.xml b/man/depmod.d.xml
index 8d3d821cddc8..431ebca6654b 100644
--- a/man/depmod.d.xml
+++ b/man/depmod.d.xml
@@ -40,6 +40,7 @@

<refsynopsisdiv>
<para><filename>/lib/depmod.d/*.conf</filename></para>
+ <para><filename>@PREFIX@/lib/depmod.d/*.conf</filename></para>
<para><filename>/usr/local/lib/depmod.d/*.conf</filename></para>
<para><filename>/run/depmod.d/*.conf</filename></para>
<para><filename>/etc/depmod.d/*.conf</filename></para>
diff --git a/man/modprobe.d.xml b/man/modprobe.d.xml
index 0ab3e9110a7e..e8a91d7668af 100644
--- a/man/modprobe.d.xml
+++ b/man/modprobe.d.xml
@@ -41,6 +41,7 @@

<refsynopsisdiv>
<para><filename>/lib/modprobe.d/*.conf</filename></para>
+ <para><filename>@PREFIX@/lib/modprobe.d/*.conf</filename></para>
<para><filename>/usr/local/lib/modprobe.d/*.conf</filename></para>
<para><filename>/run/modprobe.d/*.conf</filename></para>
<para><filename>/etc/modprobe.d/*.conf</filename></para>
diff --git a/tools/depmod.c b/tools/depmod.c
index 1d1d41db860f..7e9339923809 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -54,6 +54,7 @@ static const char *const default_cfg_paths[] = {
SYSCONFDIR "/depmod.d",
"/run/depmod.d",
"/usr/local/lib/depmod.d",
+ PREFIX "/lib/depmod.d",
"/lib/depmod.d",
NULL
};
--
2.41.0


2023-07-12 14:07:02

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH kmod v2 4/4] libkmod, depmod, modprobe: Search for kernel modules under ${module_prefix}

modprobe.d is now searched under ${prefix}/lib, add ${module_prefix} to
specify the directory where to search for kernel modules.

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 <[email protected]>
---
Makefile.am | 3 +-
configure.ac | 7 ++
libkmod/libkmod.c | 6 +-
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.c | 1 +
tools/modinfo.c | 4 +-
tools/modprobe.c | 4 +-
tools/static-nodes.c | 6 +-
18 files changed, 108 insertions(+), 85 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 7aa5bfa5638d..96ae1edd7366 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -20,6 +20,7 @@ AM_CPPFLAGS = \
-I$(top_srcdir) \
-DSYSCONFDIR=\""$(sysconfdir)"\" \
-DPREFIX=\""$(prefix)"\" \
+ -DMODULE_PREFIX=\""$(module_prefix)"\" \
${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_PREFIX=$(module_prefix) $(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 6064dee77ae6..29d3ff8ae41d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -84,6 +84,12 @@ AC_ARG_WITH([rootlibdir],
[], [with_rootlibdir=$libdir])
AC_SUBST([rootlibdir], [$with_rootlibdir])

+# Ideally this would be $prefix but default to empty for compatibility with earlier versions
+AC_ARG_WITH([module_prefix],
+ AS_HELP_STRING([--with-module-prefix=DIR], [directory in which to look for /lib/modules directory with kernel modules - typically '' or ${prefix}]),
+ [], [with_module_prefix=])
+AC_SUBST([module_prefix], [$with_module_prefix])
+
AC_ARG_WITH([zstd],
AS_HELP_STRING([--with-zstd], [handle Zstandard-compressed modules @<:@default=disabled@:>@]),
[], [with_zstd=no])
@@ -305,6 +311,7 @@ AC_MSG_RESULT([
=======

prefix: ${prefix}
+ module_prefix: ${module_prefix}
sysconfdir: ${sysconfdir}
libdir: ${libdir}
rootlibdir: ${rootlibdir}
diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c
index 13c82b069e84..e41acae6f9fc 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_PREFIX "/lib/modules";

static char *get_kernel_release(const char *dirname)
{
@@ -231,14 +231,14 @@ 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
* to load from user-defined configuration parameters such as
* alias, blacklists, commands (install, remove). If NULL
* defaults to /etc/modprobe.d, /run/modprobe.d,
- * /usr/local/lib/modprobe.d and PREFIX/lib/modprobe.d. Give an empty
+ * /usr/local/lib/modprobe.d and ${module_prefix}/lib/modprobe.d. Give an empty
* vector if configuration should not be read. This array must
* be null terminated.
*
diff --git a/man/Makefile.am b/man/Makefile.am
index ad07c30bbd24..1a9a92f9c224 100644
--- a/man/Makefile.am
+++ b/man/Makefile.am
@@ -22,6 +22,7 @@ CLEANFILES = $(dist_man_MANS)
else \
sed -e '/@PREFIX@/d' $< ; \
fi | \
+ sed -e 's|@MODULE_PREFIX@|$(module_prefix)|g' | \
$(XSLT) \
-o $@ \
--nonet \
diff --git a/man/depmod.d.xml b/man/depmod.d.xml
index 431ebca6654b..2583a8d8a5fc 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_PREFIX@/lib/modules
(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_PREFIX@/lib/modules (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_PREFIX@/lib/modules (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..9d9cf195a355 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_PREFIX@/lib/modules/</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_PREFIX@/lib/modules/</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..d48c64560e9d 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_PREFIX@/lib/modules/</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..6e8bb8c9ce9e 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_PREFIX@/lib/modules/`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..9130ec392089 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_PREFIX@/lib/modules/modules.dep</filename></para>
+ <para><filename>@MODULE_PREFIX@/lib/modules/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_PREFIX@/lib/modules/</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..db41da36fedf 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 [ -n "$MODULE_PREFIX" ] ; then
+ sed -i -e "s|/lib/modules|$MODULE_PREFIX/lib/modules|g" $(find "$ROOTFS" -name \*.txt -o -name \*.conf -o -name \*.dep)
+ sed -i -e "s|$MODULE_PREFIX/lib/modules/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
+ i=$(dirname $i)
+ lib="$(dirname $i)"
+ up="$(dirname $lib)$MODULE_PREFIX"
+ mkdir -p "$up"
+ mv "$lib" "$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_PREFIX/lib/modules/4.4.4/kernel/crypto/"]="mod-simple.ko"
+ ["test-depmod/search-order-simple$MODULE_PREFIX/lib/modules/4.4.4/updates/"]="mod-simple.ko"
+ ["test-depmod/search-order-same-prefix$MODULE_PREFIX/lib/modules/4.4.4/foo/"]="mod-simple.ko"
+ ["test-depmod/search-order-same-prefix$MODULE_PREFIX/lib/modules/4.4.4/foobar/"]="mod-simple.ko"
+ ["test-depmod/detect-loop$MODULE_PREFIX/lib/modules/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko"
+ ["test-depmod/detect-loop$MODULE_PREFIX/lib/modules/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko"
+ ["test-depmod/detect-loop$MODULE_PREFIX/lib/modules/4.4.4/kernel/mod-loop-c.ko"]="mod-loop-c.ko"
+ ["test-depmod/detect-loop$MODULE_PREFIX/lib/modules/4.4.4/kernel/mod-loop-d.ko"]="mod-loop-d.ko"
+ ["test-depmod/detect-loop$MODULE_PREFIX/lib/modules/4.4.4/kernel/mod-loop-e.ko"]="mod-loop-e.ko"
+ ["test-depmod/detect-loop$MODULE_PREFIX/lib/modules/4.4.4/kernel/mod-loop-f.ko"]="mod-loop-f.ko"
+ ["test-depmod/detect-loop$MODULE_PREFIX/lib/modules/4.4.4/kernel/mod-loop-g.ko"]="mod-loop-g.ko"
+ ["test-depmod/detect-loop$MODULE_PREFIX/lib/modules/4.4.4/kernel/mod-loop-h.ko"]="mod-loop-h.ko"
+ ["test-depmod/detect-loop$MODULE_PREFIX/lib/modules/4.4.4/kernel/mod-loop-i.ko"]="mod-loop-i.ko"
+ ["test-depmod/detect-loop$MODULE_PREFIX/lib/modules/4.4.4/kernel/mod-loop-j.ko"]="mod-loop-j.ko"
+ ["test-depmod/detect-loop$MODULE_PREFIX/lib/modules/4.4.4/kernel/mod-loop-k.ko"]="mod-loop-k.ko"
+ ["test-depmod/search-order-external-first$MODULE_PREFIX/lib/modules/4.4.4/foo/"]="mod-simple.ko"
+ ["test-depmod/search-order-external-first$MODULE_PREFIX/lib/modules/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_PREFIX/lib/modules/4.4.4/foo/"]="mod-simple.ko"
+ ["test-depmod/search-order-external-last$MODULE_PREFIX/lib/modules/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_PREFIX/lib/modules/4.4.4/foo/"]="mod-simple.ko"
+ ["test-depmod/search-order-override$MODULE_PREFIX/lib/modules/4.4.4/override/"]="mod-simple.ko"
+ ["test-dependencies$MODULE_PREFIX/lib/modules/4.0.20-kmod/kernel/fs/foo/"]="mod-foo-b.ko"
+ ["test-dependencies$MODULE_PREFIX/lib/modules/4.0.20-kmod/kernel/"]="mod-foo-c.ko"
+ ["test-dependencies$MODULE_PREFIX/lib/modules/4.0.20-kmod/kernel/lib/"]="mod-foo-a.ko"
+ ["test-dependencies$MODULE_PREFIX/lib/modules/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_PREFIX/lib/modules/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko"
+ ["test-modprobe/show-depends$MODULE_PREFIX/lib/modules/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko"
+ ["test-modprobe/show-depends$MODULE_PREFIX/lib/modules/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_PREFIX/lib/modules/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko"
+ ["test-modprobe/softdep-loop$MODULE_PREFIX/lib/modules/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko"
+ ["test-modprobe/install-cmd-loop$MODULE_PREFIX/lib/modules/4.4.4/kernel/mod-loop-a.ko"]="mod-loop-a.ko"
+ ["test-modprobe/install-cmd-loop$MODULE_PREFIX/lib/modules/4.4.4/kernel/mod-loop-b.ko"]="mod-loop-b.ko"
+ ["test-modprobe/force$MODULE_PREFIX/lib/modules/4.4.4/kernel/"]="mod-simple.ko"
+ ["test-modprobe/oldkernel$MODULE_PREFIX/lib/modules/3.3.3/kernel/"]="mod-simple.ko"
+ ["test-modprobe/oldkernel-force$MODULE_PREFIX/lib/modules/3.3.3/kernel/"]="mod-simple.ko"
+ ["test-modprobe/alias-to-none$MODULE_PREFIX/lib/modules/4.4.4/kernel/"]="mod-simple.ko"
+ ["test-modprobe/module-param-kcmdline$MODULE_PREFIX/lib/modules/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_PREFIX/lib/modules/4.4.4/kernel/drivers/block/cciss.ko"]="mod-fake-cciss.ko"
+ ["test-depmod/modules-order-compressed$MODULE_PREFIX/lib/modules/4.4.4/kernel/drivers/scsi/hpsa.ko"]="mod-fake-hpsa.ko"
+ ["test-depmod/modules-order-compressed$MODULE_PREFIX/lib/modules/4.4.4/kernel/drivers/scsi/scsi_mod.ko"]="mod-fake-scsi-mod.ko"
+ ["test-depmod/modules-outdir$MODULE_PREFIX/lib/modules/4.4.4/kernel/drivers/block/cciss.ko"]="mod-fake-cciss.ko"
+ ["test-depmod/modules-outdir$MODULE_PREFIX/lib/modules/4.4.4/kernel/drivers/scsi/hpsa.ko"]="mod-fake-hpsa.ko"
+ ["test-depmod/modules-outdir$MODULE_PREFIX/lib/modules/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_PREFIX/lib/modules/4.4.4/kernel/"]="mod-simple.ko"
+ ["test-tools/remove$MODULE_PREFIX/lib/modules/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_PREFIX/lib/modules/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_PREFIX/lib/modules/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_PREFIX/lib/modules/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..870f2667862b 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_PREFIX "/lib/modules/" 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_PREFIX "/lib/modules/" MODULES_UNAME
+#define MODULES_OUTDIR_LIB_MODULES_INPUT MODULES_OUTDIR_ROOTFS MODULE_PREFIX "/lib/modules/" 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_PREFIX "/lib/modules/" 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_PREFIX "/lib/modules/" 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_PREFIX "/lib/modules/" 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_PREFIX "/lib/modules/" 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_PREFIX "/lib/modules/" 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..903a0102105e 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_PREFIX "/lib/modules/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_PREFIX "/lib/modules/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_PREFIX "/lib/modules/a", F_OK) < 0) {
ERR("access failed: %m\n");
return EXIT_FAILURE;
}

- if (stat("/lib/modules/a", &st) < 0) {
+ if (stat(MODULE_PREFIX "/lib/modules/a", &st) < 0) {
ERR("stat failed: %m\n");
return EXIT_FAILURE;
}
diff --git a/tools/depmod.c b/tools/depmod.c
index 7e9339923809..686525b4adea 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_PREFIX/lib/modules/$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_PREFIX "/lib/modules/%s",
root ?: "", cfg.kversion);

cfg.outdirnamelen = snprintf(cfg.outdirname, PATH_MAX,
- "%s/lib/modules/%s",
+ "%s" MODULE_PREFIX "/lib/modules/%s",
out_root ?: (root ?: ""), cfg.kversion);

if (optind == argc)
diff --git a/tools/kmod.c b/tools/kmod.c
index 5a13716955c1..4384752ab5bc 100644
--- a/tools/kmod.c
+++ b/tools/kmod.c
@@ -114,6 +114,7 @@ static int kmod_config(int argc, char *argv[])
{
unsigned i;
printf("{\"prefix\":\"" PREFIX "\""
+ ",\"module_prefix\":\"" MODULE_PREFIX "\""
",\"module_signature\":["
#ifdef ENABLE_OPENSSL
"\"PKCS#7\","
diff --git a/tools/modinfo.c b/tools/modinfo.c
index d0aab200af4e..c10ce7ec5ef3 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_PREFIX "/lib/modules\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_PREFIX "/lib/modules/%s",
root, kversion);
dirname = dirname_buf;
}
diff --git a/tools/modprobe.c b/tools/modprobe.c
index e891028349a8..f386c57275e0 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_PREFIX "/lib/modules\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_PREFIX "/lib/modules/%s", root,
kversion);
dirname = dirname_buf;
}
diff --git a/tools/static-nodes.c b/tools/static-nodes.c
index 8d2356da73f3..868af3b58ac7 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_PREFIX "/lib/modules/%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_PREFIX "/lib/modules/%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_PREFIX "/lib/modules/%s/modules.devname - %m\n",
kernel.release);
ret = EXIT_FAILURE;
}
--
2.41.0


2023-07-12 14:28:45

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH [email protected]] depmod: Handle installing modules under a prefix

On Wed, Jul 12, 2023 at 10:45 PM Michal Suchanek <[email protected]> wrote:
>
> Some distributions aim at not shipping any files in / ustside of usr.
>
> The path under which kernel modules are instaleld is hardcoded to /lib
> which conflicts with this goal.
>
> When kmod provides the config command use it to determine the correct
> module installation prefix.
>
> On kmod that does not provide the command / is used as before.
>
> Signed-off-by: Michal Suchanek <[email protected]>
> ---
> v2: Avoid error on systems with kmod that does not support config
> command
> ---
> Makefile | 4 +++-
> scripts/depmod.sh | 8 ++++----
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 47690c28456a..b1fea135bdec 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
> # makefile but the argument can be passed to make if needed.
> #
>
> -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> +export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)
> +
> +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)



You can do "make modules_install INSTALL_MOD_PATH=/usr/what/ever/prefix"


This patch is unneeded.












> export MODLIB
>
> PHONY += prepare0
> diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> index 3643b4f896ed..88ac79056153 100755
> --- a/scripts/depmod.sh
> +++ b/scripts/depmod.sh
> @@ -27,16 +27,16 @@ fi
> # numbers, so we cheat with a symlink here
> depmod_hack_needed=true
> tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
> -mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
> +mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
> if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> - if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
> - -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> + if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
> + -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> depmod_hack_needed=false
> fi
> fi
> rm -rf "$tmp_dir"
> if $depmod_hack_needed; then
> - symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
> + symlink="$INSTALL_MOD_PATH$KERNEL_MODULE_PREFIX/lib/modules/99.98.$KERNELRELEASE"
> ln -s "$KERNELRELEASE" "$symlink"
> KERNELRELEASE=99.98.$KERNELRELEASE
> fi
> --
> 2.41.0
>


--
Best Regards
Masahiro Yamada

2023-07-12 16:32:36

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH [email protected]] depmod: Handle installing modules under a prefix

On Wed, Jul 12, 2023 at 11:14:33PM +0900, Masahiro Yamada wrote:
> On Wed, Jul 12, 2023 at 10:45 PM Michal Suchanek <[email protected]> wrote:
> >
> > Some distributions aim at not shipping any files in / ustside of usr.
> >
> > The path under which kernel modules are instaleld is hardcoded to /lib
> > which conflicts with this goal.
> >
> > When kmod provides the config command use it to determine the correct
> > module installation prefix.
> >
> > On kmod that does not provide the command / is used as before.
> >
> > Signed-off-by: Michal Suchanek <[email protected]>
> > ---
> > v2: Avoid error on systems with kmod that does not support config
> > command
> > ---
> > Makefile | 4 +++-
> > scripts/depmod.sh | 8 ++++----
> > 2 files changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 47690c28456a..b1fea135bdec 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
> > # makefile but the argument can be passed to make if needed.
> > #
> >
> > -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> > +export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)
> > +
> > +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
>
> You can do "make modules_install INSTALL_MOD_PATH=/usr/what/ever/prefix"
>
> This patch is unneeded.

It's very much needed.

INSTALL_MOD_PATH is temporary staging location, KERNEL_MODULE_PREFIX is
permanent prefix under which modules are searched.

Note that depmod.sh does not use INSTALL_MOD_PATH but uses
KERNEL_MODULE_PREFIX.

Support for this is added by the corresponding kmod patchset.

Thanks

Michal

> > export MODLIB
> >
> > PHONY += prepare0
> > diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> > index 3643b4f896ed..88ac79056153 100755
> > --- a/scripts/depmod.sh
> > +++ b/scripts/depmod.sh
> > @@ -27,16 +27,16 @@ fi
> > # numbers, so we cheat with a symlink here
> > depmod_hack_needed=true
> > tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
> > -mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
> > +mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
> > if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> > - if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > - -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > + if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > + -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > depmod_hack_needed=false
> > fi
> > fi
> > rm -rf "$tmp_dir"
> > if $depmod_hack_needed; then
> > - symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
> > + symlink="$INSTALL_MOD_PATH$KERNEL_MODULE_PREFIX/lib/modules/99.98.$KERNELRELEASE"
> > ln -s "$KERNELRELEASE" "$symlink"
> > KERNELRELEASE=99.98.$KERNELRELEASE
> > fi
> > --
> > 2.41.0
> >
>
>
> --
> Best Regards
> Masahiro Yamada

2023-07-14 06:33:16

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH v2] depmod: Handle installing modules under a prefix

On 12. 07. 23, 15:45, Michal Suchanek wrote:
> Some distributions aim at not shipping any files in / ustside of usr.

"outside".

> The path under which kernel modules are instaleld is hardcoded to /lib

"installed"

> which conflicts with this goal.
>
> When kmod provides the config command use it to determine the correct
> module installation prefix.
>
> On kmod that does not provide the command / is used as before.

Can you spice it with more commas? While the text is understandable
after a while of staring, it's hard to parse.

Like:
When kmod provides the config command, use it to determine the correct
module installation prefix.

On kmod that does not provide the command, / is used as before.





I would also argue here in the commit log on what Masahiro already
pointed out. I.e. that INSTALL_MOD_PATH is useless in this case and why.

> Signed-off-by: Michal Suchanek <[email protected]>
> ---
> v2: Avoid error on systems with kmod that does not support config
> command
> ---
> Makefile | 4 +++-
> scripts/depmod.sh | 8 ++++----
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 47690c28456a..b1fea135bdec 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
> # makefile but the argument can be passed to make if needed.
> #
>
> -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> +export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)
> +
> +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
> export MODLIB
>
> PHONY += prepare0
> diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> index 3643b4f896ed..88ac79056153 100755
> --- a/scripts/depmod.sh
> +++ b/scripts/depmod.sh
> @@ -27,16 +27,16 @@ fi
> # numbers, so we cheat with a symlink here
> depmod_hack_needed=true
> tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
> -mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
> +mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
> if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> - if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
> - -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> + if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
> + -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> depmod_hack_needed=false
> fi
> fi
> rm -rf "$tmp_dir"
> if $depmod_hack_needed; then
> - symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
> + symlink="$INSTALL_MOD_PATH$KERNEL_MODULE_PREFIX/lib/modules/99.98.$KERNELRELEASE"
> ln -s "$KERNELRELEASE" "$symlink"
> KERNELRELEASE=99.98.$KERNELRELEASE
> fi

--
js
suse labs


2023-07-14 12:46:12

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH v3] depmod: Handle installing modules under a prefix

Some distributions aim at not shipping any files in / outside of usr.

The path under which kernel modules are installed is hardcoded to /lib
which conflicts with this goal.

When kmod provides the config command, use it to determine the correct
module installation prefix.

This is a prefix under which the modules are searched by kmod on the
system, and is separate from the temporary staging location already
supported by INSTALL_MOD_PATH.

With kmod that does not provide the config command empty prefix is used
as before.

Signed-off-by: Michal Suchanek <[email protected]>
---
v2: Avoid error on systems with kmod that does not support config
command
v3: More verbose commit message
---
Makefile | 4 +++-
scripts/depmod.sh | 8 ++++----
2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 47690c28456a..b1fea135bdec 100644
--- a/Makefile
+++ b/Makefile
@@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
# makefile but the argument can be passed to make if needed.
#

-MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
+export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)
+
+MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
export MODLIB

PHONY += prepare0
diff --git a/scripts/depmod.sh b/scripts/depmod.sh
index 3643b4f896ed..88ac79056153 100755
--- a/scripts/depmod.sh
+++ b/scripts/depmod.sh
@@ -27,16 +27,16 @@ fi
# numbers, so we cheat with a symlink here
depmod_hack_needed=true
tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
-mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
+mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
- if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
- -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
+ if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
+ -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
depmod_hack_needed=false
fi
fi
rm -rf "$tmp_dir"
if $depmod_hack_needed; then
- symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
+ symlink="$INSTALL_MOD_PATH$KERNEL_MODULE_PREFIX/lib/modules/99.98.$KERNELRELEASE"
ln -s "$KERNELRELEASE" "$symlink"
KERNELRELEASE=99.98.$KERNELRELEASE
fi
--
2.41.0


2023-07-14 14:02:46

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH v3] depmod: Handle installing modules under a prefix

Hello,

On Fri, Jul 14, 2023 at 03:38:18PM +0200, Jan Engelhardt wrote:
>
> On Friday 2023-07-14 14:21, Michal Suchanek wrote:
>
> >Some distributions aim at not shipping any files in / outside of usr.
> >
> >The path under which kernel modules are installed is hardcoded to /lib
> >which conflicts with this goal.
> >
> >+MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
>
> Ok, so if the problem statement is that hardcoded paths are bad, then why
> continue to hardcode the "/lib/modules" fragment? Just make it so that
> KERNEL_MODULE_PREFIX can be set to the exact string "/usr/lib/modules" and not
> just "/usr".

That's certainly an option.

The feature is modelled after the installation prefix option that can
move the whole filesystem hierarchy of installed files under /usr/local,
/opt, or any other directory of choice. However, in that case the
subdirectories in the hierarchy can be configured as well while in this
case /lib/modules remains hardcoded.

Making it possible to set the whole path is generally more flexible
although there is no need to set the later part for this particular use
case.

Thanks

Michal

2023-07-14 14:02:47

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH v3] depmod: Handle installing modules under a prefix


On Friday 2023-07-14 14:21, Michal Suchanek wrote:

>Some distributions aim at not shipping any files in / outside of usr.
>
>The path under which kernel modules are installed is hardcoded to /lib
>which conflicts with this goal.
>
>+MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)

Ok, so if the problem statement is that hardcoded paths are bad, then why
continue to hardcode the "/lib/modules" fragment? Just make it so that
KERNEL_MODULE_PREFIX can be set to the exact string "/usr/lib/modules" and not
just "/usr".

2023-07-14 14:04:06

by Michal Koutný

[permalink] [raw]
Subject: Re: [PATCH v3] depmod: Handle installing modules under a prefix

On Fri, Jul 14, 2023 at 03:38:18PM +0200, Jan Engelhardt <[email protected]> wrote:
> Ok, so if the problem statement is that hardcoded paths are bad, then why
> continue to hardcode the "/lib/modules" fragment? Just make it so that
> KERNEL_MODULE_PREFIX can be set to the exact string "/usr/lib/modules" and not
> just "/usr".

That sounds cleaner but I'm worried it would be a BC break in setups
that expect the existing layout under INSTALL_MOD_PATH, wouldn't it?

Michal


Attachments:
(No filename) (484.00 B)
signature.asc (235.00 B)
Download all attachments

2023-07-14 14:06:44

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH kmod v2 4/4] libkmod, depmod, modprobe: Search for kernel modules under ${module_prefix}


On Wednesday 2023-07-12 16:00, Michal Suchanek wrote:

>modprobe.d is now searched under ${prefix}/lib, add ${module_prefix} to
>specify the directory where to search for kernel modules.
>
>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.

>+# Ideally this would be $prefix but default to empty for compatibility with earlier versions
>+AC_ARG_WITH([module_prefix],
>+ AS_HELP_STRING([--with-module-prefix=DIR], [directory in which to look for /lib/modules directory with kernel modules - typically '' or ${prefix}]),
>+ [], [with_module_prefix=])
>+AC_SUBST([module_prefix], [$with_module_prefix])

Why stop there, let's make it fully configurable such that

./configure --with-module-prefix=/usr/lib/modules

works. Then one can specify arbitrary paths without fearing of getting a
/lib/modules tacked on anywhere down the line.

2023-07-14 14:09:38

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH kmod v2 3/4] kmod: Add config command to show compile time configuration as JSON


On Wednesday 2023-07-12 16:00, Michal Suchanek wrote:

>Show prefix (where configuration files are searched/to be installed),
>module compressions, and module signatures supported.

What about doing it like systemd and generate a .pc file instead
that can then be queried like so, e.g.:

$ pkg-config kmod --variable=modulesdir
/usr/lib/modules

2023-07-14 14:17:04

by Nicolas Schier

[permalink] [raw]
Subject: Re: [PATCH v3] depmod: Handle installing modules under a prefix

On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> Some distributions aim at not shipping any files in / outside of usr.

For me, preventing negation often makes things easier, e.g.: "... aim at
shipping files only below /usr".

>
> The path under which kernel modules are installed is hardcoded to /lib
> which conflicts with this goal.
>
> When kmod provides the config command, use it to determine the correct
> module installation prefix.
>
> This is a prefix under which the modules are searched by kmod on the
> system, and is separate from the temporary staging location already
> supported by INSTALL_MOD_PATH.
>
> With kmod that does not provide the config command empty prefix is used
> as before.
>
> Signed-off-by: Michal Suchanek <[email protected]>
> ---
> v2: Avoid error on systems with kmod that does not support config
> command
> v3: More verbose commit message
> ---
> Makefile | 4 +++-
> scripts/depmod.sh | 8 ++++----
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 47690c28456a..b1fea135bdec 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
> # makefile but the argument can be passed to make if needed.
> #
>
> -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> +export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)

All other calls of `jq` that I could find are located at tools/; as this here
is evaluated on each invocation, this should probably be documented in
Documentation/process/changes.rst?

(Absence of `jq` will cause error messages, even with CONFIG_MODULES=n.)

> +
> +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
> export MODLIB
>
> PHONY += prepare0
> diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> index 3643b4f896ed..88ac79056153 100755
> --- a/scripts/depmod.sh
> +++ b/scripts/depmod.sh
> @@ -27,16 +27,16 @@ fi
> # numbers, so we cheat with a symlink here
> depmod_hack_needed=true
> tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
> -mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
> +mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
> if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> - if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
> - -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> + if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
> + -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> depmod_hack_needed=false
> fi
> fi

I'd like to come back to the statement from Masahiro: Is the check above,
against some very old versions of depmod [1], the only reason for this patch?

If we could remove that, would

make INSTALL_MOD_PATH="$(kmod config | jq -r .module_prefix)" modules_install

be sufficient?

Kind regards,
Nicolas


[1]: https://lore.kernel.org/linux-kbuild/[email protected]/

> rm -rf "$tmp_dir"
> if $depmod_hack_needed; then
> - symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
> + symlink="$INSTALL_MOD_PATH$KERNEL_MODULE_PREFIX/lib/modules/99.98.$KERNELRELEASE"
> ln -s "$KERNELRELEASE" "$symlink"
> KERNELRELEASE=99.98.$KERNELRELEASE
> fi
> --
> 2.41.0

--
epost|xmpp: [email protected] irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb c82b 7d97 0932 55a0 ce7f
-- frykten for herren er opphav til kunnskap --


Attachments:
(No filename) (3.60 kB)
signature.asc (849.00 B)
Download all attachments

2023-07-14 14:17:15

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH kmod v2 3/4] kmod: Add config command to show compile time configuration as JSON

Hello,

On Fri, Jul 14, 2023 at 03:52:05PM +0200, Jan Engelhardt wrote:
>
> On Wednesday 2023-07-12 16:00, Michal Suchanek wrote:
>
> >Show prefix (where configuration files are searched/to be installed),
> >module compressions, and module signatures supported.
>
> What about doing it like systemd and generate a .pc file instead
> that can then be queried like so, e.g.:
>
> $ pkg-config kmod --variable=modulesdir
> /usr/lib/modules

- AFAICS tools packed with kernel generate but do not consume .pc files
while JSON and jq are commonly used througout the kernel ecosystem
- .pc files would be shipped with libkmod development package, not the
kmod tool in a binary distribution

Other than that JSON and .pc files are roughly quivalent in usability.

Thanks

Michal

2023-07-14 14:27:35

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH kmod v2 3/4] kmod: Add config command to show compile time configuration as JSON


On Friday 2023-07-14 16:02, Michal Suchánek wrote:
>>
>> What about doing it like systemd and generate a .pc file instead
>> that can then be queried like so, e.g.:
>>
>> $ pkg-config kmod --variable=modulesdir
>
> - .pc files would be shipped with libkmod development package, not the
> kmod tool in a binary distribution

On that point: No, they would not.
Again, confer with systemd: systemd.rpm(SUSE) provides systemd.pc,
systemd-devel.rpm provides libsystemd.pc. kmod.rpm would provide
kmod.pc, kmod-devel.rpm would continue to provide libkmod.pc as
it does today.

2023-07-14 14:31:27

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH v3] depmod: Handle installing modules under a prefix

Hello,

On Fri, Jul 14, 2023 at 03:59:17PM +0200, Michal Koutn? wrote:
> On Fri, Jul 14, 2023 at 03:38:18PM +0200, Jan Engelhardt <[email protected]> wrote:
> > Ok, so if the problem statement is that hardcoded paths are bad, then why
> > continue to hardcode the "/lib/modules" fragment? Just make it so that
> > KERNEL_MODULE_PREFIX can be set to the exact string "/usr/lib/modules" and not
> > just "/usr".
>
> That sounds cleaner but I'm worried it would be a BC break in setups
> that expect the existing layout under INSTALL_MOD_PATH, wouldn't it?

It's a break either way, the expected directory righ now is exactly
/lib/modules. /usr/lib/modules works to some extent for some use cases
only when the compatibility symlink lib -> usr/lib is present.

Thanks

Michal

2023-07-14 14:33:54

by Nicolas Schier

[permalink] [raw]
Subject: Re: [PATCH kmod v2 2/4] libkmod, depmod: Load modprobe.d, depmod.d from $prefix/lib.

On Wed, Jul 12, 2023 at 04:00:46PM +0200 Michal Suchanek wrote:
> There is an ongoing effort to limit use of files outside of /usr (or
> $prefix on general). Currently all modprobe.d paths are hardcoded to
> outside of $prefix. Teach kmod to load modprobe.d from $prefix/lib.
>
> On some distributions /usr/lib and /lib are the same directory because
> of a compatibility symlink, and it is possible to craft configuration
> files with sideeffects that would behave differently when loaded twice.
> However, the override semantic ensures that one 'overrides' the other,
> and only one configuration file of the same name is loaded from any of
> the search directories.
>
> Signed-off-by: Michal Suchanek <[email protected]>
> ---
> v2: Fix commit message typo
> ---
> Makefile.am | 1 +
> libkmod/libkmod.c | 3 ++-
> man/Makefile.am | 9 +++++++--
> man/depmod.d.xml | 1 +
> man/modprobe.d.xml | 1 +
> tools/depmod.c | 1 +
> 6 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 8ba85c91a0f3..7aa5bfa5638d 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -19,6 +19,7 @@ AM_CPPFLAGS = \
> -include $(top_builddir)/config.h \
> -I$(top_srcdir) \
> -DSYSCONFDIR=\""$(sysconfdir)"\" \
> + -DPREFIX=\""$(prefix)"\" \
> ${zlib_CFLAGS}
>
> AM_CFLAGS = $(OUR_CFLAGS)
> diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c
> index 2670f9a4611a..13c82b069e84 100644
> --- a/libkmod/libkmod.c
> +++ b/libkmod/libkmod.c
> @@ -65,6 +65,7 @@ static const char *const default_config_paths[] = {
> SYSCONFDIR "/modprobe.d",
> "/run/modprobe.d",
> "/usr/local/lib/modprobe.d",
> + PREFIX "/lib/modprobe.d",
> "/lib/modprobe.d",
> NULL
> };
> @@ -237,7 +238,7 @@ static char *get_kernel_release(const char *dirname)
> * to load from user-defined configuration parameters such as
> * alias, blacklists, commands (install, remove). If NULL
> * defaults to /etc/modprobe.d, /run/modprobe.d,
> - * /usr/local/lib/modprobe.d and /lib/modprobe.d. Give an empty
> + * /usr/local/lib/modprobe.d and PREFIX/lib/modprobe.d. Give an empty

In the chunk above, there still is /lib/modprobe.d included in the
default_config_paths array.

Kind regards,
Nicolas


--
epost|xmpp: [email protected] irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb c82b 7d97 0932 55a0 ce7f
-- frykten for herren er opphav til kunnskap --

2023-07-14 14:54:53

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH v3] depmod: Handle installing modules under a prefix

Hello,

On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > Some distributions aim at not shipping any files in / outside of usr.
>
> For me, preventing negation often makes things easier, e.g.: "... aim at
> shipping files only below /usr".
>
> >
> > The path under which kernel modules are installed is hardcoded to /lib
> > which conflicts with this goal.
> >
> > When kmod provides the config command, use it to determine the correct
> > module installation prefix.
> >
> > This is a prefix under which the modules are searched by kmod on the
> > system, and is separate from the temporary staging location already
> > supported by INSTALL_MOD_PATH.
> >
> > With kmod that does not provide the config command empty prefix is used
> > as before.
> >
> > Signed-off-by: Michal Suchanek <[email protected]>
> > ---
> > v2: Avoid error on systems with kmod that does not support config
> > command
> > v3: More verbose commit message
> > ---
> > Makefile | 4 +++-
> > scripts/depmod.sh | 8 ++++----
> > 2 files changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 47690c28456a..b1fea135bdec 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
> > # makefile but the argument can be passed to make if needed.
> > #
> >
> > -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> > +export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)
>
> All other calls of `jq` that I could find are located at tools/; as this here
> is evaluated on each invocation, this should probably be documented in
> Documentation/process/changes.rst?
>
> (Absence of `jq` will cause error messages, even with CONFIG_MODULES=n.)

That's a good point.

>
> > +
> > +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
> > export MODLIB
> >
> > PHONY += prepare0
> > diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> > index 3643b4f896ed..88ac79056153 100755
> > --- a/scripts/depmod.sh
> > +++ b/scripts/depmod.sh
> > @@ -27,16 +27,16 @@ fi
> > # numbers, so we cheat with a symlink here
> > depmod_hack_needed=true
> > tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
> > -mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
> > +mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
> > if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> > - if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > - -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > + if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > + -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > depmod_hack_needed=false
> > fi
> > fi
>
> I'd like to come back to the statement from Masahiro: Is the check above,
> against some very old versions of depmod [1], the only reason for this patch?
>
> If we could remove that, would
>
> make INSTALL_MOD_PATH="$(kmod config | jq -r .module_prefix)" modules_install
>
> be sufficient?

No, the INSTALL_MOD_PATH is passed as the -b argument to depmod while
the newly added part is not because it's integral part of where the
modules are installed on the system, and not the staging area path.

>
> Kind regards,
> Nicolas
>
>
> [1]: https://lore.kernel.org/linux-kbuild/[email protected]/

Was busybox ever fixed to not require the hack?

Thanks

Michal

2023-07-14 15:05:30

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH v3] depmod: Handle installing modules under a prefix


On Friday 2023-07-14 16:30, Michal Suchánek wrote:
>> >
>> > -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
>> > +export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)
>>
>> All other calls of `jq` that I could find are located at tools/; as this here
>> is evaluated on each invocation, this should probably be documented in
>> Documentation/process/changes.rst?
>>
>> (Absence of `jq` will cause error messages, even with CONFIG_MODULES=n.)
>
>That's a good point.

Speaking of which, "&>" is a bashism and probably should be replaced.

2023-07-14 15:11:46

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH kmod v2 2/4] libkmod, depmod: Load modprobe.d, depmod.d from $prefix/lib.

On Fri, Jul 14, 2023 at 04:16:44PM +0200, Nicolas Schier wrote:
> On Wed, Jul 12, 2023 at 04:00:46PM +0200 Michal Suchanek wrote:
> > There is an ongoing effort to limit use of files outside of /usr (or
> > $prefix on general). Currently all modprobe.d paths are hardcoded to
> > outside of $prefix. Teach kmod to load modprobe.d from $prefix/lib.
> >
> > On some distributions /usr/lib and /lib are the same directory because
> > of a compatibility symlink, and it is possible to craft configuration
> > files with sideeffects that would behave differently when loaded twice.
> > However, the override semantic ensures that one 'overrides' the other,
> > and only one configuration file of the same name is loaded from any of
> > the search directories.
> >
> > Signed-off-by: Michal Suchanek <[email protected]>
> > ---
> > v2: Fix commit message typo
> > ---
> > Makefile.am | 1 +
> > libkmod/libkmod.c | 3 ++-
> > man/Makefile.am | 9 +++++++--
> > man/depmod.d.xml | 1 +
> > man/modprobe.d.xml | 1 +
> > tools/depmod.c | 1 +
> > 6 files changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 8ba85c91a0f3..7aa5bfa5638d 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -19,6 +19,7 @@ AM_CPPFLAGS = \
> > -include $(top_builddir)/config.h \
> > -I$(top_srcdir) \
> > -DSYSCONFDIR=\""$(sysconfdir)"\" \
> > + -DPREFIX=\""$(prefix)"\" \
> > ${zlib_CFLAGS}
> >
> > AM_CFLAGS = $(OUR_CFLAGS)
> > diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c
> > index 2670f9a4611a..13c82b069e84 100644
> > --- a/libkmod/libkmod.c
> > +++ b/libkmod/libkmod.c
> > @@ -65,6 +65,7 @@ static const char *const default_config_paths[] = {
> > SYSCONFDIR "/modprobe.d",
> > "/run/modprobe.d",
> > "/usr/local/lib/modprobe.d",
> > + PREFIX "/lib/modprobe.d",
> > "/lib/modprobe.d",
> > NULL
> > };
> > @@ -237,7 +238,7 @@ static char *get_kernel_release(const char *dirname)
> > * to load from user-defined configuration parameters such as
> > * alias, blacklists, commands (install, remove). If NULL
> > * defaults to /etc/modprobe.d, /run/modprobe.d,
> > - * /usr/local/lib/modprobe.d and /lib/modprobe.d. Give an empty
> > + * /usr/local/lib/modprobe.d and PREFIX/lib/modprobe.d. Give an empty
>
> In the chunk above, there still is /lib/modprobe.d included in the
> default_config_paths array.

Indeed, if this is rendered as documentation somewhere this would be
misleading.

Thanks

Michal

2023-07-14 15:28:36

by Nicolas Schier

[permalink] [raw]
Subject: Re: [PATCH v3] depmod: Handle installing modules under a prefix

On Fri, Jul 14, 2023 at 04:30:02PM +0200, Michal Such�nek wrote:
> Hello,
>
> On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> > On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > > Some distributions aim at not shipping any files in / outside of usr.
> >
> > For me, preventing negation often makes things easier, e.g.: "... aim at
> > shipping files only below /usr".
> >
> > >
> > > The path under which kernel modules are installed is hardcoded to /lib
> > > which conflicts with this goal.
> > >
> > > When kmod provides the config command, use it to determine the correct
> > > module installation prefix.
> > >
> > > This is a prefix under which the modules are searched by kmod on the
> > > system, and is separate from the temporary staging location already
> > > supported by INSTALL_MOD_PATH.
> > >
> > > With kmod that does not provide the config command empty prefix is used
> > > as before.
> > >
> > > Signed-off-by: Michal Suchanek <[email protected]>
> > > ---
> > > v2: Avoid error on systems with kmod that does not support config
> > > command
> > > v3: More verbose commit message
> > > ---
> > > Makefile | 4 +++-
> > > scripts/depmod.sh | 8 ++++----
> > > 2 files changed, 7 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 47690c28456a..b1fea135bdec 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
> > > # makefile but the argument can be passed to make if needed.
> > > #
> > >
> > > -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> > > +export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)
> >
> > All other calls of `jq` that I could find are located at tools/; as this here
> > is evaluated on each invocation, this should probably be documented in
> > Documentation/process/changes.rst?
> >
> > (Absence of `jq` will cause error messages, even with CONFIG_MODULES=n.)
>
> That's a good point.
>
> >
> > > +
> > > +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
> > > export MODLIB
> > >
> > > PHONY += prepare0
> > > diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> > > index 3643b4f896ed..88ac79056153 100755
> > > --- a/scripts/depmod.sh
> > > +++ b/scripts/depmod.sh
> > > @@ -27,16 +27,16 @@ fi
> > > # numbers, so we cheat with a symlink here
> > > depmod_hack_needed=true
> > > tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
> > > -mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
> > > +mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
> > > if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> > > - if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > > - -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > > + if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > > + -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > > depmod_hack_needed=false
> > > fi
> > > fi
> >
> > I'd like to come back to the statement from Masahiro: Is the check above,
> > against some very old versions of depmod [1], the only reason for this patch?
> >
> > If we could remove that, would
> >
> > make INSTALL_MOD_PATH="$(kmod config | jq -r .module_prefix)" modules_install
> >
> > be sufficient?
>
> No, the INSTALL_MOD_PATH is passed as the -b argument to depmod while
> the newly added part is not because it's integral part of where the
> modules are installed on the system, and not the staging area path.

Ah, thanks. So just for my understanding, could this be a (non-gentle)
alternative version of your patch, w/o modifying top-level Makefile?

diff --git a/scripts/depmod.sh b/scripts/depmod.sh
index 3643b4f896ed..72c819de0669 100755
--- a/scripts/depmod.sh
+++ b/scripts/depmod.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
# SPDX-License-Identifier: GPL-2.0
#
# A depmod wrapper used by the toplevel Makefile
@@ -23,6 +23,8 @@ if [ -z $(command -v $DEPMOD) ]; then
exit 0
fi

+kmod_version=$(( $(kmod --version | sed -rne 's/^kmod version ([0-9]+).*$/\1/p') ))
+
# older versions of depmod require the version string to start with three
# numbers, so we cheat with a symlink here
depmod_hack_needed=true
@@ -35,6 +37,13 @@ if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
fi
fi
rm -rf "$tmp_dir"
+
+if [ "${kmod_version}" -gt 32 ]; then
+ kmod_prefix="$(kmod config | jq -r .module_prefix)"
+ INSTALL_MOD_PATH="${INSTALL_MOD_PATH#${kmod_prefix}"
+ depmod_hack_needed=false
+fi
+
if $depmod_hack_needed; then
symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
ln -s "$KERNELRELEASE" "$symlink"

(untested, and assuming that kmod module prefix is in kmod >= 32)

Or are I am still missing something?

> Was busybox ever fixed to not require the hack?

I haven't checked that, yet.

Kind regards,
Nicolas


Attachments:
(No filename) (5.09 kB)
signature.asc (849.00 B)
Download all attachments

2023-07-14 15:36:50

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH v3] depmod: Handle installing modules under a prefix

On Fri, Jul 14, 2023 at 04:54:49PM +0200, Nicolas Schier wrote:
> On Fri, Jul 14, 2023 at 04:30:02PM +0200, Michal Such�nek wrote:
> > Hello,
> >
> > On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> > > On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > > > Some distributions aim at not shipping any files in / outside of usr.
> > >
> > > For me, preventing negation often makes things easier, e.g.: "... aim at
> > > shipping files only below /usr".
> > >
> > > >
> > > > The path under which kernel modules are installed is hardcoded to /lib
> > > > which conflicts with this goal.
> > > >
> > > > When kmod provides the config command, use it to determine the correct
> > > > module installation prefix.
> > > >
> > > > This is a prefix under which the modules are searched by kmod on the
> > > > system, and is separate from the temporary staging location already
> > > > supported by INSTALL_MOD_PATH.
> > > >
> > > > With kmod that does not provide the config command empty prefix is used
> > > > as before.
> > > >
> > > > Signed-off-by: Michal Suchanek <[email protected]>
> > > > ---
> > > > v2: Avoid error on systems with kmod that does not support config
> > > > command
> > > > v3: More verbose commit message
> > > > ---
> > > > Makefile | 4 +++-
> > > > scripts/depmod.sh | 8 ++++----
> > > > 2 files changed, 7 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/Makefile b/Makefile
> > > > index 47690c28456a..b1fea135bdec 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
> > > > # makefile but the argument can be passed to make if needed.
> > > > #
> > > >
> > > > -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> > > > +export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)
> > >
> > > All other calls of `jq` that I could find are located at tools/; as this here
> > > is evaluated on each invocation, this should probably be documented in
> > > Documentation/process/changes.rst?
> > >
> > > (Absence of `jq` will cause error messages, even with CONFIG_MODULES=n.)
> >
> > That's a good point.
> >
> > >
> > > > +
> > > > +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
> > > > export MODLIB
> > > >
> > > > PHONY += prepare0
> > > > diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> > > > index 3643b4f896ed..88ac79056153 100755
> > > > --- a/scripts/depmod.sh
> > > > +++ b/scripts/depmod.sh
> > > > @@ -27,16 +27,16 @@ fi
> > > > # numbers, so we cheat with a symlink here
> > > > depmod_hack_needed=true
> > > > tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
> > > > -mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
> > > > +mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
> > > > if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> > > > - if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > > > - -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > > > + if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > > > + -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > > > depmod_hack_needed=false
> > > > fi
> > > > fi
> > >
> > > I'd like to come back to the statement from Masahiro: Is the check above,
> > > against some very old versions of depmod [1], the only reason for this patch?
> > >
> > > If we could remove that, would
> > >
> > > make INSTALL_MOD_PATH="$(kmod config | jq -r .module_prefix)" modules_install
> > >
> > > be sufficient?
> >
> > No, the INSTALL_MOD_PATH is passed as the -b argument to depmod while
> > the newly added part is not because it's integral part of where the
> > modules are installed on the system, and not the staging area path.
>
> Ah, thanks. So just for my understanding, could this be a (non-gentle)
> alternative version of your patch, w/o modifying top-level Makefile?
>
> diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> index 3643b4f896ed..72c819de0669 100755
> --- a/scripts/depmod.sh
> +++ b/scripts/depmod.sh
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
> # SPDX-License-Identifier: GPL-2.0
> #
> # A depmod wrapper used by the toplevel Makefile
> @@ -23,6 +23,8 @@ if [ -z $(command -v $DEPMOD) ]; then
> exit 0
> fi
>
> +kmod_version=$(( $(kmod --version | sed -rne 's/^kmod version ([0-9]+).*$/\1/p') ))
> +
> # older versions of depmod require the version string to start with three
> # numbers, so we cheat with a symlink here
> depmod_hack_needed=true
> @@ -35,6 +37,13 @@ if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> fi
> fi
> rm -rf "$tmp_dir"
> +
> +if [ "${kmod_version}" -gt 32 ]; then
> + kmod_prefix="$(kmod config | jq -r .module_prefix)"
> + INSTALL_MOD_PATH="${INSTALL_MOD_PATH#${kmod_prefix}"
> + depmod_hack_needed=false
> +fi
> +
> if $depmod_hack_needed; then
> symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
> ln -s "$KERNELRELEASE" "$symlink"
>
> (untested, and assuming that kmod module prefix is in kmod >= 32)

It can be detected by running the 'kmod config' command first and
ignoring the output when it fails which the above patch already did.
The version check does not sound very reliable.

> Or are I am still missing something?

MODLIB still needs to include the extra prefix so that files are
installed in the correct location. And that's defined in the toplevel
Makefile.

Thanks

Michal

2023-07-14 16:05:30

by Nicolas Schier

[permalink] [raw]
Subject: Re: [PATCH kmod v2 3/4] kmod: Add config command to show compile time configuration as JSON

On Wed, Jul 12, 2023 at 04:00:47PM +0200 Michal Suchanek wrote:
> Show prefix (where configuration files are searched/to be installed),
> module compressions, and module signatures supported.
>
> Signed-off-by: Michal Suchanek <[email protected]>
> ---
> v2: mention module signature in commit message
> ---
> man/kmod.xml | 6 ++++++
> tools/kmod.c | 39 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 45 insertions(+)
>
> diff --git a/man/kmod.xml b/man/kmod.xml
> index 0706ad58c2cc..f992a500f836 100644
> --- a/man/kmod.xml
> +++ b/man/kmod.xml
> @@ -71,6 +71,12 @@
> <para>Show the help message.</para>
> </listitem>
> </varlistentry>
> + <varlistentry>
> + <term><command>config</command></term>
> + <listitem>
> + <para>Show compile time options in JSON.</para>
> + </listitem>
> + </varlistentry>
> <varlistentry>
> <term><command>list</command></term>
> <listitem>
> diff --git a/tools/kmod.c b/tools/kmod.c
> index 55689c075ab1..5a13716955c1 100644
> --- a/tools/kmod.c
> +++ b/tools/kmod.c
> @@ -37,9 +37,11 @@ static const struct option options[] = {
> };
>
> static const struct kmod_cmd kmod_cmd_help;
> +static const struct kmod_cmd kmod_cmd_config;
>
> static const struct kmod_cmd *kmod_cmds[] = {
> &kmod_cmd_help,
> + &kmod_cmd_config,
> &kmod_cmd_list,
> &kmod_cmd_static_nodes,
>
> @@ -95,6 +97,43 @@ static const struct kmod_cmd kmod_cmd_help = {
> .help = "Show help message",
> };
>
> +static const char *compressions[] = {
> +#ifdef ENABLE_ZSTD
> + "zstd",
> +#endif
> +#ifdef ENABLE_XZ
> + "xz",
> +#endif
> +#ifdef ENABLE_ZLIB
> + "gz",
> +#endif
> + NULL
> +};
> +
> +static int kmod_config(int argc, char *argv[])
> +{
> + unsigned i;
> + printf("{\"prefix\":\"" PREFIX "\""
> + ",\"module_signature\":["
> +#ifdef ENABLE_OPENSSL
> + "\"PKCS#7\","
> +#endif
> + "\"legacy\"]"
> + ",\"module_compression\":[");
> + for(i = 0; compressions[i]; i++) {
> + printf("%s\"%s\"", i ? "," : "", compressions[i]);
> + }
> + printf("]}\n");
> +
> + return EXIT_SUCCESS;
> +}
> +
> +static const struct kmod_cmd kmod_cmd_config = {
> + .name = "config",
> + .cmd = kmod_config,
> + .help = "Show compile time options in JSON",
> +};
> +
> static int handle_kmod_commands(int argc, char *argv[])
> {
> const char *cmd;
> --
> 2.41.0

If kmod could show selected configs without some (JSON) syntax
around, it could simplify its proposed use in kbuild. E.g.:

kmod config prefix 2>/dev/null

instead of

kmod config &>/dev/null && kmod config | jq -r .prefix
.

--
epost|xmpp: [email protected] irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb c82b 7d97 0932 55a0 ce7f
-- frykten for herren er opphav til kunnskap --


Attachments:
(No filename) (2.85 kB)
signature.asc (849.00 B)
Download all attachments

2023-07-14 17:07:50

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH kmod v2 3/4] kmod: Add config command to show compile time configuration as JSON

On Fri, Jul 14, 2023 at 05:26:43PM +0200, Nicolas Schier wrote:
> On Wed, Jul 12, 2023 at 04:00:47PM +0200 Michal Suchanek wrote:
> > Show prefix (where configuration files are searched/to be installed),
> > module compressions, and module signatures supported.
> >
> > Signed-off-by: Michal Suchanek <[email protected]>
> > ---
> > v2: mention module signature in commit message
> > ---
> > man/kmod.xml | 6 ++++++
> > tools/kmod.c | 39 +++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 45 insertions(+)
> >
> > diff --git a/man/kmod.xml b/man/kmod.xml
> > index 0706ad58c2cc..f992a500f836 100644
> > --- a/man/kmod.xml
> > +++ b/man/kmod.xml
> > @@ -71,6 +71,12 @@
> > <para>Show the help message.</para>
> > </listitem>
> > </varlistentry>
> > + <varlistentry>
> > + <term><command>config</command></term>
> > + <listitem>
> > + <para>Show compile time options in JSON.</para>
> > + </listitem>
> > + </varlistentry>
> > <varlistentry>
> > <term><command>list</command></term>
> > <listitem>
> > diff --git a/tools/kmod.c b/tools/kmod.c
> > index 55689c075ab1..5a13716955c1 100644
> > --- a/tools/kmod.c
> > +++ b/tools/kmod.c
> > @@ -37,9 +37,11 @@ static const struct option options[] = {
> > };
> >
> > static const struct kmod_cmd kmod_cmd_help;
> > +static const struct kmod_cmd kmod_cmd_config;
> >
> > static const struct kmod_cmd *kmod_cmds[] = {
> > &kmod_cmd_help,
> > + &kmod_cmd_config,
> > &kmod_cmd_list,
> > &kmod_cmd_static_nodes,
> >
> > @@ -95,6 +97,43 @@ static const struct kmod_cmd kmod_cmd_help = {
> > .help = "Show help message",
> > };
> >
> > +static const char *compressions[] = {
> > +#ifdef ENABLE_ZSTD
> > + "zstd",
> > +#endif
> > +#ifdef ENABLE_XZ
> > + "xz",
> > +#endif
> > +#ifdef ENABLE_ZLIB
> > + "gz",
> > +#endif
> > + NULL
> > +};
> > +
> > +static int kmod_config(int argc, char *argv[])
> > +{
> > + unsigned i;
> > + printf("{\"prefix\":\"" PREFIX "\""
> > + ",\"module_signature\":["
> > +#ifdef ENABLE_OPENSSL
> > + "\"PKCS#7\","
> > +#endif
> > + "\"legacy\"]"
> > + ",\"module_compression\":[");
> > + for(i = 0; compressions[i]; i++) {
> > + printf("%s\"%s\"", i ? "," : "", compressions[i]);
> > + }
> > + printf("]}\n");
> > +
> > + return EXIT_SUCCESS;
> > +}
> > +
> > +static const struct kmod_cmd kmod_cmd_config = {
> > + .name = "config",
> > + .cmd = kmod_config,
> > + .help = "Show compile time options in JSON",
> > +};
> > +
> > static int handle_kmod_commands(int argc, char *argv[])
> > {
> > const char *cmd;
> > --
> > 2.41.0
>
> If kmod could show selected configs without some (JSON) syntax
> around, it could simplify its proposed use in kbuild. E.g.:
>
> kmod config prefix 2>/dev/null
>
> instead of
>
> kmod config &>/dev/null && kmod config | jq -r .prefix

Which would no longer hold for whole module directory:

kmod config &>/dev/null && kmod config | jq -r .module_directory || echo /lib/modules

vs

kmod config module_directory &>/dev/null && kmod config module_directory || echo /lib/modules

Also JSON has standardized syntax for lists and users that can parse
JSON directly can load the whole configuration at once without several
calls to kmod config or pkg-config.

Thanks

Michal

2023-07-14 19:58:10

by Nicolas Schier

[permalink] [raw]
Subject: Re: [PATCH v3] depmod: Handle installing modules under a prefix

On Fri, Jul 14, 2023 at 05:10:42PM +0200 Michal Suchánek wrote:
> On Fri, Jul 14, 2023 at 04:54:49PM +0200, Nicolas Schier wrote:
> > On Fri, Jul 14, 2023 at 04:30:02PM +0200, Michal Such�nek wrote:
> > > Hello,
> > >
> > > On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> > > > On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > > > > Some distributions aim at not shipping any files in / outside of usr.
> > > >
> > > > For me, preventing negation often makes things easier, e.g.: "... aim at
> > > > shipping files only below /usr".
> > > >
> > > > >
> > > > > The path under which kernel modules are installed is hardcoded to /lib
> > > > > which conflicts with this goal.
> > > > >
> > > > > When kmod provides the config command, use it to determine the correct
> > > > > module installation prefix.
> > > > >
> > > > > This is a prefix under which the modules are searched by kmod on the
> > > > > system, and is separate from the temporary staging location already
> > > > > supported by INSTALL_MOD_PATH.
> > > > >
> > > > > With kmod that does not provide the config command empty prefix is used
> > > > > as before.
> > > > >
> > > > > Signed-off-by: Michal Suchanek <[email protected]>
> > > > > ---
> > > > > v2: Avoid error on systems with kmod that does not support config
> > > > > command
> > > > > v3: More verbose commit message
> > > > > ---
> > > > > Makefile | 4 +++-
> > > > > scripts/depmod.sh | 8 ++++----
> > > > > 2 files changed, 7 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/Makefile b/Makefile
> > > > > index 47690c28456a..b1fea135bdec 100644
> > > > > --- a/Makefile
> > > > > +++ b/Makefile
> > > > > @@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
> > > > > # makefile but the argument can be passed to make if needed.
> > > > > #
> > > > >
> > > > > -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> > > > > +export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)

oh, should this be 'jq -r .prefix' (w/o ".module") to match your other patches?

> > > >
> > > > All other calls of `jq` that I could find are located at tools/; as this here
> > > > is evaluated on each invocation, this should probably be documented in
> > > > Documentation/process/changes.rst?
> > > >
> > > > (Absence of `jq` will cause error messages, even with CONFIG_MODULES=n.)
> > >
> > > That's a good point.
> > >
> > > >
> > > > > +
> > > > > +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
> > > > > export MODLIB
> > > > >
> > > > > PHONY += prepare0
> > > > > diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> > > > > index 3643b4f896ed..88ac79056153 100755
> > > > > --- a/scripts/depmod.sh
> > > > > +++ b/scripts/depmod.sh
> > > > > @@ -27,16 +27,16 @@ fi
> > > > > # numbers, so we cheat with a symlink here
> > > > > depmod_hack_needed=true
> > > > > tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
> > > > > -mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
> > > > > +mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
> > > > > if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> > > > > - if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > > > > - -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > > > > + if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > > > > + -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > > > > depmod_hack_needed=false
> > > > > fi
> > > > > fi
> > > >
> > > > I'd like to come back to the statement from Masahiro: Is the check above,
> > > > against some very old versions of depmod [1], the only reason for this patch?
> > > >
> > > > If we could remove that, would
> > > >
> > > > make INSTALL_MOD_PATH="$(kmod config | jq -r .module_prefix)" modules_install
> > > >
> > > > be sufficient?
> > >
> > > No, the INSTALL_MOD_PATH is passed as the -b argument to depmod while
> > > the newly added part is not because it's integral part of where the
> > > modules are installed on the system, and not the staging area path.
> >
> > Ah, thanks. So just for my understanding, could this be a (non-gentle)
> > alternative version of your patch, w/o modifying top-level Makefile?
> >
> > diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> > index 3643b4f896ed..72c819de0669 100755
> > --- a/scripts/depmod.sh
> > +++ b/scripts/depmod.sh
> > @@ -1,4 +1,4 @@
> > -#!/bin/sh
> > +#!/bin/bash
> > # SPDX-License-Identifier: GPL-2.0
> > #
> > # A depmod wrapper used by the toplevel Makefile
> > @@ -23,6 +23,8 @@ if [ -z $(command -v $DEPMOD) ]; then
> > exit 0
> > fi
> >
> > +kmod_version=$(( $(kmod --version | sed -rne 's/^kmod version ([0-9]+).*$/\1/p') ))
> > +
> > # older versions of depmod require the version string to start with three
> > # numbers, so we cheat with a symlink here
> > depmod_hack_needed=true
> > @@ -35,6 +37,13 @@ if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> > fi
> > fi
> > rm -rf "$tmp_dir"
> > +
> > +if [ "${kmod_version}" -gt 32 ]; then
> > + kmod_prefix="$(kmod config | jq -r .module_prefix)"
> > + INSTALL_MOD_PATH="${INSTALL_MOD_PATH#${kmod_prefix}"
> > + depmod_hack_needed=false
> > +fi
> > +
> > if $depmod_hack_needed; then
> > symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
> > ln -s "$KERNELRELEASE" "$symlink"
> >
> > (untested, and assuming that kmod module prefix is in kmod >= 32)
>
> It can be detected by running the 'kmod config' command first and
> ignoring the output when it fails which the above patch already did.
> The version check does not sound very reliable.
>
> > Or are I am still missing something?
>
> MODLIB still needs to include the extra prefix so that files are
> installed in the correct location. And that's defined in the toplevel
> Makefile.

Well, I think that depends. Technically, you are right; and if we want
to support system with a non-empty kmod prefix fully transparently, then
patching top-level Makefile will probably be necessary.

As for me, I am not convinced yet, that the fully transparent way to support
PREFIX/lib/modules/ is the best way forward. I think it might be better to
first only make script/depmod.sh fit for a kmod prefix and require an adjusted
INSTALL_MOD_PATH for modules_install.

Which concrete distributions did you have in mind while composing the patches?

Kind regards,
Nicolas

--
epost|xmpp: [email protected] irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb c82b 7d97 0932 55a0 ce7f
-- frykten for herren er opphav til kunnskap --


Attachments:
(No filename) (6.81 kB)
signature.asc (849.00 B)
Download all attachments

2023-07-17 10:13:32

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH v3] depmod: Handle installing modules under a prefix

On Fri, Jul 14, 2023 at 09:37:04PM +0200, Nicolas Schier wrote:
> On Fri, Jul 14, 2023 at 05:10:42PM +0200 Michal Suchánek wrote:
> > On Fri, Jul 14, 2023 at 04:54:49PM +0200, Nicolas Schier wrote:
> > > On Fri, Jul 14, 2023 at 04:30:02PM +0200, Michal Such�nek wrote:
> > > > Hello,
> > > >
> > > > On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> > > > > On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > > > > > Some distributions aim at not shipping any files in / outside of usr.
> > > > >
> > > > > For me, preventing negation often makes things easier, e.g.: "... aim at
> > > > > shipping files only below /usr".
> > > > >
> > > > > >
> > > > > > The path under which kernel modules are installed is hardcoded to /lib
> > > > > > which conflicts with this goal.
> > > > > >
> > > > > > When kmod provides the config command, use it to determine the correct
> > > > > > module installation prefix.
> > > > > >
> > > > > > This is a prefix under which the modules are searched by kmod on the
> > > > > > system, and is separate from the temporary staging location already
> > > > > > supported by INSTALL_MOD_PATH.
> > > > > >
> > > > > > With kmod that does not provide the config command empty prefix is used
> > > > > > as before.
> > > > > >
> > > > > > Signed-off-by: Michal Suchanek <[email protected]>
> > > > > > ---
> > > > > > v2: Avoid error on systems with kmod that does not support config
> > > > > > command
> > > > > > v3: More verbose commit message
> > > > > > ---
> > > > > > Makefile | 4 +++-
> > > > > > scripts/depmod.sh | 8 ++++----
> > > > > > 2 files changed, 7 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/Makefile b/Makefile
> > > > > > index 47690c28456a..b1fea135bdec 100644
> > > > > > --- a/Makefile
> > > > > > +++ b/Makefile
> > > > > > @@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
> > > > > > # makefile but the argument can be passed to make if needed.
> > > > > > #
> > > > > >
> > > > > > -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> > > > > > +export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)
>
> oh, should this be 'jq -r .prefix' (w/o ".module") to match your other patches?

No, this aligns perfectly fine, prefix is where kmod is installed.

>
> > > > >
> > > > > All other calls of `jq` that I could find are located at tools/; as this here
> > > > > is evaluated on each invocation, this should probably be documented in
> > > > > Documentation/process/changes.rst?
> > > > >
> > > > > (Absence of `jq` will cause error messages, even with CONFIG_MODULES=n.)
> > > >
> > > > That's a good point.
> > > >
> > > > >
> > > > > > +
> > > > > > +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
> > > > > > export MODLIB
> > > > > >
> > > > > > PHONY += prepare0
> > > > > > diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> > > > > > index 3643b4f896ed..88ac79056153 100755
> > > > > > --- a/scripts/depmod.sh
> > > > > > +++ b/scripts/depmod.sh
> > > > > > @@ -27,16 +27,16 @@ fi
> > > > > > # numbers, so we cheat with a symlink here
> > > > > > depmod_hack_needed=true
> > > > > > tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
> > > > > > -mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
> > > > > > +mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
> > > > > > if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> > > > > > - if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > > > > > - -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > > > > > + if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > > > > > + -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > > > > > depmod_hack_needed=false
> > > > > > fi
> > > > > > fi
> > > > >
> > > > > I'd like to come back to the statement from Masahiro: Is the check above,
> > > > > against some very old versions of depmod [1], the only reason for this patch?
> > > > >
> > > > > If we could remove that, would
> > > > >
> > > > > make INSTALL_MOD_PATH="$(kmod config | jq -r .module_prefix)" modules_install
> > > > >
> > > > > be sufficient?
> > > >
> > > > No, the INSTALL_MOD_PATH is passed as the -b argument to depmod while
> > > > the newly added part is not because it's integral part of where the
> > > > modules are installed on the system, and not the staging area path.
> > >
> > > Ah, thanks. So just for my understanding, could this be a (non-gentle)
> > > alternative version of your patch, w/o modifying top-level Makefile?
> > >
> > > diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> > > index 3643b4f896ed..72c819de0669 100755
> > > --- a/scripts/depmod.sh
> > > +++ b/scripts/depmod.sh
> > > @@ -1,4 +1,4 @@
> > > -#!/bin/sh
> > > +#!/bin/bash
> > > # SPDX-License-Identifier: GPL-2.0
> > > #
> > > # A depmod wrapper used by the toplevel Makefile
> > > @@ -23,6 +23,8 @@ if [ -z $(command -v $DEPMOD) ]; then
> > > exit 0
> > > fi
> > >
> > > +kmod_version=$(( $(kmod --version | sed -rne 's/^kmod version ([0-9]+).*$/\1/p') ))
> > > +
> > > # older versions of depmod require the version string to start with three
> > > # numbers, so we cheat with a symlink here
> > > depmod_hack_needed=true
> > > @@ -35,6 +37,13 @@ if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> > > fi
> > > fi
> > > rm -rf "$tmp_dir"
> > > +
> > > +if [ "${kmod_version}" -gt 32 ]; then
> > > + kmod_prefix="$(kmod config | jq -r .module_prefix)"
> > > + INSTALL_MOD_PATH="${INSTALL_MOD_PATH#${kmod_prefix}"
> > > + depmod_hack_needed=false
> > > +fi
> > > +
> > > if $depmod_hack_needed; then
> > > symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
> > > ln -s "$KERNELRELEASE" "$symlink"
> > >
> > > (untested, and assuming that kmod module prefix is in kmod >= 32)
> >
> > It can be detected by running the 'kmod config' command first and
> > ignoring the output when it fails which the above patch already did.
> > The version check does not sound very reliable.
> >
> > > Or are I am still missing something?
> >
> > MODLIB still needs to include the extra prefix so that files are
> > installed in the correct location. And that's defined in the toplevel
> > Makefile.
>
> Well, I think that depends. Technically, you are right; and if we want
> to support system with a non-empty kmod prefix fully transparently, then
> patching top-level Makefile will probably be necessary.
>
> As for me, I am not convinced yet, that the fully transparent way to support
> PREFIX/lib/modules/ is the best way forward. I think it might be better to
> first only make script/depmod.sh fit for a kmod prefix and require an adjusted
> INSTALL_MOD_PATH for modules_install.

Nevermind, I will update the patch to change the whole path. That will
make it crystal clear that no amount of fiddling with INSTALL_MOD_PATH
will work.

> Which concrete distributions did you have in mind while composing the patches?

If you STFW for usrmerge you can find that a number of distributions is
in different stages of experimenting with packing all shipped files into
/usr.

In the early experiment stages when the distribution is built with /lib
/bin, and /sbin in place and massaged after the fact to remove these
directories it does not matter much where files were installed
initially.

In later stages when the distribution is built without these extra
directories they are missing in the staging area for building packages,
and creating them is an error because files in these directories are not
to be shipped in packages. At this point the kernel insisting that
modules must be in /lib is sticking out like a sore thumb, it's on every
system.

Sure, there is a number of hacks that can be used to work around the
problem. Still it's a problem the needs to be addressed for usrmerge to
fully work.

openSUSE is maybe 75% there - it's nowhere near complete but the
packages that insist on using these directories outside of /usr are
becoming problematic.

Thanks

Michal

2023-07-17 11:08:13

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH kmod v4 0/4] kmod /usr support

Hello,

with these patches it is possible to install kernel modules in an arbitrary
directory - eg. moving the /lib/modules to /usr/lib/modules or /opt/linux.

While the modprobe.d and depmod.d search which already includes multiple
paths is expanded to also include $(prefix) the module directory still
supports only one location, only a different one under $(module_directory).

Having kmod search multiple module locations while only one is supported now
might break some assumption about relative module path corresponding to a
specific file, would require more invasive changes to implement, and is not
supportive of the goal of moving the modules away from /lib.

Both kmod and the kernel need to be patched to make use of this feature.
Patched kernel is backwards compatible with older kmod. Patched kmod
with $(module_directory) set to /lib/modules is equivalent to unpatched kmod.

With this patch the kmod tool can spit out a JSON with the tool build-time
configuration, and jq is used for getting a single value out of that.

I opted for this because crating .pc files requires putting the logic into the
autohell files which is very clumsy. Also pkg-config insists on hiding
the actual .pc file data and only allowing clumsy queries through its
commandline interface.

jq has its downsides, too. So far it is not used by kernel build, only
by other tools shipped with the kernel. This adds another dependency for
kernel builds.

It turns out that the kernel already does use pkg-config. There are two
alternate spellings: pkg-config and pkgconfig. Searching for the latter
gives a few matches in the kernel giving off the impression that it's a
thing but not really used.

Thanks

Michal

Link: https://lore.kernel.org/linux-modules/[email protected]/


Michal Suchanek (4):
man/depmod.d: Fix incorrect /usr/lib search path
libkmod, depmod: Load modprobe.d, depmod.d from $prefix/lib.
kmod: Add config command to show compile time configuration as JSON
libkmod, depmod, modprobe: Make directory for kernel modules
configurable

Makefile.am | 4 +-
configure.ac | 7 ++
libkmod/libkmod.c | 11 +--
man/Makefile.am | 10 ++-
man/depmod.d.xml | 9 ++-
man/depmod.xml | 4 +-
man/kmod.xml | 6 ++
man/modinfo.xml | 2 +-
man/modprobe.d.xml | 1 +
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 | 7 +-
tools/kmod.c | 41 ++++++++++
tools/modinfo.c | 4 +-
tools/modprobe.c | 4 +-
tools/static-nodes.c | 6 +-
20 files changed, 169 insertions(+), 90 deletions(-)

--
2.41.0


2023-07-17 19:29:17

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3] depmod: Handle installing modules under a prefix

On Fri, Jul 14, 2023 at 11:55 PM Nicolas Schier <[email protected]> wrote:
>
> On Fri, Jul 14, 2023 at 04:30:02PM +0200, Michal Such�nek wrote:
> > Hello,
> >
> > On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> > > On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > > > Some distributions aim at not shipping any files in / outside of usr.
> > >
> > > For me, preventing negation often makes things easier, e.g.: "... aim at
> > > shipping files only below /usr".
> > >
> > > >
> > > > The path under which kernel modules are installed is hardcoded to /lib
> > > > which conflicts with this goal.
> > > >
> > > > When kmod provides the config command, use it to determine the correct
> > > > module installation prefix.
> > > >
> > > > This is a prefix under which the modules are searched by kmod on the
> > > > system, and is separate from the temporary staging location already
> > > > supported by INSTALL_MOD_PATH.
> > > >
> > > > With kmod that does not provide the config command empty prefix is used
> > > > as before.
> > > >
> > > > Signed-off-by: Michal Suchanek <[email protected]>
> > > > ---
> > > > v2: Avoid error on systems with kmod that does not support config
> > > > command
> > > > v3: More verbose commit message
> > > > ---
> > > > Makefile | 4 +++-
> > > > scripts/depmod.sh | 8 ++++----
> > > > 2 files changed, 7 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/Makefile b/Makefile
> > > > index 47690c28456a..b1fea135bdec 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
> > > > # makefile but the argument can be passed to make if needed.
> > > > #
> > > >
> > > > -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> > > > +export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)
> > >
> > > All other calls of `jq` that I could find are located at tools/; as this here
> > > is evaluated on each invocation, this should probably be documented in
> > > Documentation/process/changes.rst?
> > >
> > > (Absence of `jq` will cause error messages, even with CONFIG_MODULES=n.)
> >
> > That's a good point.
> >
> > >
> > > > +
> > > > +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
> > > > export MODLIB
> > > >
> > > > PHONY += prepare0
> > > > diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> > > > index 3643b4f896ed..88ac79056153 100755
> > > > --- a/scripts/depmod.sh
> > > > +++ b/scripts/depmod.sh
> > > > @@ -27,16 +27,16 @@ fi
> > > > # numbers, so we cheat with a symlink here
> > > > depmod_hack_needed=true
> > > > tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
> > > > -mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
> > > > +mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
> > > > if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> > > > - if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > > > - -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > > > + if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > > > + -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > > > depmod_hack_needed=false
> > > > fi
> > > > fi
> > >
> > > I'd like to come back to the statement from Masahiro: Is the check above,
> > > against some very old versions of depmod [1], the only reason for this patch?
> > >
> > > If we could remove that, would
> > >
> > > make INSTALL_MOD_PATH="$(kmod config | jq -r .module_prefix)" modules_install
> > >
> > > be sufficient?
> >
> > No, the INSTALL_MOD_PATH is passed as the -b argument to depmod while
> > the newly added part is not because it's integral part of where the
> > modules are installed on the system, and not the staging area path.
>
> Ah, thanks. So just for my understanding, could this be a (non-gentle)
> alternative version of your patch, w/o modifying top-level Makefile?
>
> diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> index 3643b4f896ed..72c819de0669 100755
> --- a/scripts/depmod.sh
> +++ b/scripts/depmod.sh
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
> # SPDX-License-Identifier: GPL-2.0
> #
> # A depmod wrapper used by the toplevel Makefile
> @@ -23,6 +23,8 @@ if [ -z $(command -v $DEPMOD) ]; then
> exit 0
> fi
>
> +kmod_version=$(( $(kmod --version | sed -rne 's/^kmod version ([0-9]+).*$/\1/p') ))
> +
> # older versions of depmod require the version string to start with three
> # numbers, so we cheat with a symlink here
> depmod_hack_needed=true
> @@ -35,6 +37,13 @@ if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> fi
> fi
> rm -rf "$tmp_dir"
> +
> +if [ "${kmod_version}" -gt 32 ]; then
> + kmod_prefix="$(kmod config | jq -r .module_prefix)"
> + INSTALL_MOD_PATH="${INSTALL_MOD_PATH#${kmod_prefix}"
> + depmod_hack_needed=false
> +fi
> +
> if $depmod_hack_needed; then
> symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
> ln -s "$KERNELRELEASE" "$symlink"
>
> (untested, and assuming that kmod module prefix is in kmod >= 32)
>
> Or are I am still missing something?
>
> > Was busybox ever fixed to not require the hack?
>
> I haven't checked that, yet.



I believe we can revert

8fc62e59425389a6d48429b9d146223122743435
bfe5424a8b31624e7a476f959d552999f931e7c7

There is no good reason to keep such old hacky code.


The old module-init-tools project was replaced by kmod.
I do not know whether busybox fixed the issue or not.
Anyway, Linux 3.0 was released 12 years ago.
There was plenty of time to fix the issue if we wanted.
If busybox is still not able to handle the X.Y version form,
it is just that nobody is caring about the busybox's depmod.




--
Best Regards
Masahiro Yamada

2023-07-17 19:55:13

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3] depmod: Handle installing modules under a prefix

On Sat, Jul 15, 2023 at 12:10 AM Michal Suchánek <[email protected]> wrote:
>
> On Fri, Jul 14, 2023 at 04:54:49PM +0200, Nicolas Schier wrote:
> > On Fri, Jul 14, 2023 at 04:30:02PM +0200, Michal Such�nek wrote:
> > > Hello,
> > >
> > > On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> > > > On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > > > > Some distributions aim at not shipping any files in / outside of usr.
> > > >
> > > > For me, preventing negation often makes things easier, e.g.: "... aim at
> > > > shipping files only below /usr".
> > > >
> > > > >
> > > > > The path under which kernel modules are installed is hardcoded to /lib
> > > > > which conflicts with this goal.
> > > > >
> > > > > When kmod provides the config command, use it to determine the correct
> > > > > module installation prefix.
> > > > >
> > > > > This is a prefix under which the modules are searched by kmod on the
> > > > > system, and is separate from the temporary staging location already
> > > > > supported by INSTALL_MOD_PATH.
> > > > >
> > > > > With kmod that does not provide the config command empty prefix is used
> > > > > as before.
> > > > >
> > > > > Signed-off-by: Michal Suchanek <[email protected]>
> > > > > ---
> > > > > v2: Avoid error on systems with kmod that does not support config
> > > > > command
> > > > > v3: More verbose commit message
> > > > > ---
> > > > > Makefile | 4 +++-
> > > > > scripts/depmod.sh | 8 ++++----
> > > > > 2 files changed, 7 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/Makefile b/Makefile
> > > > > index 47690c28456a..b1fea135bdec 100644
> > > > > --- a/Makefile
> > > > > +++ b/Makefile
> > > > > @@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
> > > > > # makefile but the argument can be passed to make if needed.
> > > > > #
> > > > >
> > > > > -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> > > > > +export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)
> > > >
> > > > All other calls of `jq` that I could find are located at tools/; as this here
> > > > is evaluated on each invocation, this should probably be documented in
> > > > Documentation/process/changes.rst?
> > > >
> > > > (Absence of `jq` will cause error messages, even with CONFIG_MODULES=n.)
> > >
> > > That's a good point.
> > >
> > > >
> > > > > +
> > > > > +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
> > > > > export MODLIB
> > > > >
> > > > > PHONY += prepare0
> > > > > diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> > > > > index 3643b4f896ed..88ac79056153 100755
> > > > > --- a/scripts/depmod.sh
> > > > > +++ b/scripts/depmod.sh
> > > > > @@ -27,16 +27,16 @@ fi
> > > > > # numbers, so we cheat with a symlink here
> > > > > depmod_hack_needed=true
> > > > > tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
> > > > > -mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
> > > > > +mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
> > > > > if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> > > > > - if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > > > > - -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > > > > + if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > > > > + -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > > > > depmod_hack_needed=false
> > > > > fi
> > > > > fi
> > > >
> > > > I'd like to come back to the statement from Masahiro: Is the check above,
> > > > against some very old versions of depmod [1], the only reason for this patch?
> > > >
> > > > If we could remove that, would
> > > >
> > > > make INSTALL_MOD_PATH="$(kmod config | jq -r .module_prefix)" modules_install
> > > >
> > > > be sufficient?
> > >
> > > No, the INSTALL_MOD_PATH is passed as the -b argument to depmod while
> > > the newly added part is not because it's integral part of where the
> > > modules are installed on the system, and not the staging area path.
> >
> > Ah, thanks. So just for my understanding, could this be a (non-gentle)
> > alternative version of your patch, w/o modifying top-level Makefile?
> >
> > diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> > index 3643b4f896ed..72c819de0669 100755
> > --- a/scripts/depmod.sh
> > +++ b/scripts/depmod.sh
> > @@ -1,4 +1,4 @@
> > -#!/bin/sh
> > +#!/bin/bash
> > # SPDX-License-Identifier: GPL-2.0
> > #
> > # A depmod wrapper used by the toplevel Makefile
> > @@ -23,6 +23,8 @@ if [ -z $(command -v $DEPMOD) ]; then
> > exit 0
> > fi
> >
> > +kmod_version=$(( $(kmod --version | sed -rne 's/^kmod version ([0-9]+).*$/\1/p') ))
> > +
> > # older versions of depmod require the version string to start with three
> > # numbers, so we cheat with a symlink here
> > depmod_hack_needed=true
> > @@ -35,6 +37,13 @@ if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> > fi
> > fi
> > rm -rf "$tmp_dir"
> > +
> > +if [ "${kmod_version}" -gt 32 ]; then
> > + kmod_prefix="$(kmod config | jq -r .module_prefix)"
> > + INSTALL_MOD_PATH="${INSTALL_MOD_PATH#${kmod_prefix}"
> > + depmod_hack_needed=false
> > +fi
> > +
> > if $depmod_hack_needed; then
> > symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
> > ln -s "$KERNELRELEASE" "$symlink"
> >
> > (untested, and assuming that kmod module prefix is in kmod >= 32)
>
> It can be detected by running the 'kmod config' command first and
> ignoring the output when it fails which the above patch already did.
> The version check does not sound very reliable.
>
> > Or are I am still missing something?
>
> MODLIB still needs to include the extra prefix so that files are
> installed in the correct location. And that's defined in the toplevel
> Makefile.
>
> Thanks
>
> Michal



As Documentation/kbuild/kbuild.rst mentions,
you can override MODLIB.

Kbuild already provides the maximum flexibility.


$ make modules_install \
INSTALL_MOD_PATH=/path/to/whatever/staging/dir \
MODLIB=/path/to/whatever/real/install/destination



--
Best Regards
Masahiro Yamada

2023-07-18 09:15:12

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH v3] depmod: Handle installing modules under a prefix

On Tue, Jul 18, 2023 at 04:14:11AM +0900, Masahiro Yamada wrote:
> On Sat, Jul 15, 2023 at 12:10 AM Michal Suchánek <[email protected]> wrote:
> >
> > On Fri, Jul 14, 2023 at 04:54:49PM +0200, Nicolas Schier wrote:
> > > On Fri, Jul 14, 2023 at 04:30:02PM +0200, Michal Such�nek wrote:
> > > > Hello,
> > > >
> > > > On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> > > > > On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > > > > > Some distributions aim at not shipping any files in / outside of usr.
> > > > >
> > > > > For me, preventing negation often makes things easier, e.g.: "... aim at
> > > > > shipping files only below /usr".
> > > > >
> > > > > >
> > > > > > The path under which kernel modules are installed is hardcoded to /lib
> > > > > > which conflicts with this goal.
> > > > > >
> > > > > > When kmod provides the config command, use it to determine the correct
> > > > > > module installation prefix.
> > > > > >
> > > > > > This is a prefix under which the modules are searched by kmod on the
> > > > > > system, and is separate from the temporary staging location already
> > > > > > supported by INSTALL_MOD_PATH.
> > > > > >
> > > > > > With kmod that does not provide the config command empty prefix is used
> > > > > > as before.
> > > > > >
> > > > > > Signed-off-by: Michal Suchanek <[email protected]>
> > > > > > ---
> > > > > > v2: Avoid error on systems with kmod that does not support config
> > > > > > command
> > > > > > v3: More verbose commit message
> > > > > > ---
> > > > > > Makefile | 4 +++-
> > > > > > scripts/depmod.sh | 8 ++++----
> > > > > > 2 files changed, 7 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/Makefile b/Makefile
> > > > > > index 47690c28456a..b1fea135bdec 100644
> > > > > > --- a/Makefile
> > > > > > +++ b/Makefile
> > > > > > @@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
> > > > > > # makefile but the argument can be passed to make if needed.
> > > > > > #
> > > > > >
> > > > > > -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> > > > > > +export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)
> > > > >
> > > > > All other calls of `jq` that I could find are located at tools/; as this here
> > > > > is evaluated on each invocation, this should probably be documented in
> > > > > Documentation/process/changes.rst?
> > > > >
> > > > > (Absence of `jq` will cause error messages, even with CONFIG_MODULES=n.)
> > > >
> > > > That's a good point.
> > > >
> > > > >
> > > > > > +
> > > > > > +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
> > > > > > export MODLIB
> > > > > >
> > > > > > PHONY += prepare0
> > > > > > diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> > > > > > index 3643b4f896ed..88ac79056153 100755
> > > > > > --- a/scripts/depmod.sh
> > > > > > +++ b/scripts/depmod.sh
> > > > > > @@ -27,16 +27,16 @@ fi
> > > > > > # numbers, so we cheat with a symlink here
> > > > > > depmod_hack_needed=true
> > > > > > tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
> > > > > > -mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
> > > > > > +mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
> > > > > > if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> > > > > > - if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > > > > > - -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > > > > > + if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
> > > > > > + -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
> > > > > > depmod_hack_needed=false
> > > > > > fi
> > > > > > fi
> > > > >
> > > > > I'd like to come back to the statement from Masahiro: Is the check above,
> > > > > against some very old versions of depmod [1], the only reason for this patch?
> > > > >
> > > > > If we could remove that, would
> > > > >
> > > > > make INSTALL_MOD_PATH="$(kmod config | jq -r .module_prefix)" modules_install
> > > > >
> > > > > be sufficient?
> > > >
> > > > No, the INSTALL_MOD_PATH is passed as the -b argument to depmod while
> > > > the newly added part is not because it's integral part of where the
> > > > modules are installed on the system, and not the staging area path.
> > >
> > > Ah, thanks. So just for my understanding, could this be a (non-gentle)
> > > alternative version of your patch, w/o modifying top-level Makefile?
> > >
> > > diff --git a/scripts/depmod.sh b/scripts/depmod.sh
> > > index 3643b4f896ed..72c819de0669 100755
> > > --- a/scripts/depmod.sh
> > > +++ b/scripts/depmod.sh
> > > @@ -1,4 +1,4 @@
> > > -#!/bin/sh
> > > +#!/bin/bash
> > > # SPDX-License-Identifier: GPL-2.0
> > > #
> > > # A depmod wrapper used by the toplevel Makefile
> > > @@ -23,6 +23,8 @@ if [ -z $(command -v $DEPMOD) ]; then
> > > exit 0
> > > fi
> > >
> > > +kmod_version=$(( $(kmod --version | sed -rne 's/^kmod version ([0-9]+).*$/\1/p') ))
> > > +
> > > # older versions of depmod require the version string to start with three
> > > # numbers, so we cheat with a symlink here
> > > depmod_hack_needed=true
> > > @@ -35,6 +37,13 @@ if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
> > > fi
> > > fi
> > > rm -rf "$tmp_dir"
> > > +
> > > +if [ "${kmod_version}" -gt 32 ]; then
> > > + kmod_prefix="$(kmod config | jq -r .module_prefix)"
> > > + INSTALL_MOD_PATH="${INSTALL_MOD_PATH#${kmod_prefix}"
> > > + depmod_hack_needed=false
> > > +fi
> > > +
> > > if $depmod_hack_needed; then
> > > symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
> > > ln -s "$KERNELRELEASE" "$symlink"
> > >
> > > (untested, and assuming that kmod module prefix is in kmod >= 32)
> >
> > It can be detected by running the 'kmod config' command first and
> > ignoring the output when it fails which the above patch already did.
> > The version check does not sound very reliable.
> >
> > > Or are I am still missing something?
> >
> > MODLIB still needs to include the extra prefix so that files are
> > installed in the correct location. And that's defined in the toplevel
> > Makefile.
> >
> > Thanks
> >
> > Michal
>
>
>
> As Documentation/kbuild/kbuild.rst mentions,
> you can override MODLIB.
>
> Kbuild already provides the maximum flexibility.
>
>
> $ make modules_install \
> INSTALL_MOD_PATH=/path/to/whatever/staging/dir \
> MODLIB=/path/to/whatever/real/install/destination

That's great that we have another workaround for the case when the
kernel build system and kmod do not agree on the location of the kernel
modules.

However, they should by default agree, and if this feature is merged
into kmod the kernel should also adapt to make sure the modules are
installed where kmod expects them.

Thanks

Michal

2023-07-18 12:26:15

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH kmod v5 0/5] kmod /usr support

Hello,

with these patches it is possible to install kernel modules in an arbitrary
directory - eg. moving the /lib/modules to /usr/lib/modules or /opt/linux.

While the modprobe.d and depmod.d search which already includes multiple
paths is expanded to also include $(prefix) the module directory still
supports only one location, only a different one under $(module_directory).

Having kmod search multiple module locations while only one is supported now
might break some assumption about relative module path corresponding to a
specific file, would require more invasive changes to implement, and is not
supportive of the goal of moving the modules away from /lib.

Both kmod and the kernel need to be patched to make use of this feature.
Patched kernel is backwards compatible with older kmod. Patched kmod
with $(module_directory) set to /lib/modules is equivalent to unpatched kmod.

Thanks

Michal

Link: https://lore.kernel.org/linux-modules/[email protected]/

v4: set whole path to module directory instead of adding prefix
v5: use pkg-config instead of jq, fix build on openssl without sm3 support


Michal Suchanek (5):
configure: Detect openssl sm3 support
man/depmod.d: Fix incorrect /usr/lib search path
libkmod, depmod: Load modprobe.d, depmod.d from ${prefix}/lib.
kmod: Add pkgconfig file with kmod compile time configuration
libkmod, depmod, modprobe: Make directory for kernel modules
configurable

Makefile.am | 6 +-
configure.ac | 30 ++++++++
libkmod/libkmod.c | 11 +--
man/Makefile.am | 10 ++-
man/depmod.d.xml | 9 ++-
man/depmod.xml | 4 +-
man/modinfo.xml | 2 +-
man/modprobe.d.xml | 1 +
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 | 7 +-
tools/kmod.pc.in | 10 +++
tools/modinfo.c | 4 +-
tools/modprobe.c | 4 +-
tools/static-nodes.c | 6 +-
19 files changed, 156 insertions(+), 91 deletions(-)
create mode 100644 tools/kmod.pc.in

--
2.41.0


2023-07-18 12:27:10

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH kmod v5 4/5] kmod: Add pkgconfig file with kmod compile time configuration

Show distconfdir (where system configuration files are searched/to be
installed), sysconfdir (where user configuration files are searched),
module compressions, and module signatures supported.

Signed-off-by: Michal Suchanek <[email protected]>
---
v2: mention module signature in commit message
v3: add sysconfdir
v5: add distconfdir, switch to pkgconfig
---
Makefile.am | 2 +-
configure.ac | 11 +++++++++++
tools/kmod.pc.in | 9 +++++++++
3 files changed, 21 insertions(+), 1 deletion(-)
create mode 100644 tools/kmod.pc.in

diff --git a/Makefile.am b/Makefile.am
index 14eb07e63cea..6d0b2decfef3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -96,7 +96,7 @@ libkmod_libkmod_internal_la_DEPENDENCIES = $(libkmod_libkmod_la_DEPENDENCIES)
libkmod_libkmod_internal_la_LIBADD = $(libkmod_libkmod_la_LIBADD)

pkgconfigdir = $(libdir)/pkgconfig
-pkgconfig_DATA = libkmod/libkmod.pc
+pkgconfig_DATA = libkmod/libkmod.pc tools/kmod.pc

bashcompletiondir=@bashcompletiondir@
dist_bashcompletion_DATA = \
diff --git a/configure.ac b/configure.ac
index 728f88a56704..b4584d6cdc67 100644
--- a/configure.ac
+++ b/configure.ac
@@ -21,6 +21,9 @@ LT_INIT([disable-static pic-only])
AS_IF([test "x$enable_static" = "xyes"], [AC_MSG_ERROR([--enable-static is not supported by kmod])])
AS_IF([test "x$enable_largefile" = "xno"], [AC_MSG_ERROR([--disable-largefile is not supported by kmod])])

+module_compressions=""
+module_signatures="legacy"
+
#####################################################################
# Program checks and configurations
#####################################################################
@@ -94,6 +97,7 @@ AC_ARG_WITH([zstd],
AS_IF([test "x$with_zstd" != "xno"], [
PKG_CHECK_MODULES([libzstd], [libzstd >= 1.4.4], [LIBS="$LIBS $libzstd_LIBS"])
AC_DEFINE([ENABLE_ZSTD], [1], [Enable Zstandard for modules.])
+ module_compressions="zstd $module_compressions"
], [
AC_MSG_NOTICE([Zstandard support not requested])
])
@@ -105,6 +109,7 @@ AC_ARG_WITH([xz],
AS_IF([test "x$with_xz" != "xno"], [
PKG_CHECK_MODULES([liblzma], [liblzma >= 4.99], [LIBS="$LIBS $liblzma_LIBS"])
AC_DEFINE([ENABLE_XZ], [1], [Enable Xz for modules.])
+ module_compressions="xz $module_compressions"
], [
AC_MSG_NOTICE([Xz support not requested])
])
@@ -116,6 +121,7 @@ AC_ARG_WITH([zlib],
AS_IF([test "x$with_zlib" != "xno"], [
PKG_CHECK_MODULES([zlib], [zlib], [LIBS="$LIBS $zlib_LIBS"])
AC_DEFINE([ENABLE_ZLIB], [1], [Enable zlib for modules.])
+ module_compressions="gzip $module_compressions"
], [
AC_MSG_NOTICE([zlib support not requested])
])
@@ -134,6 +140,7 @@ AS_IF([test "x$with_openssl" != "xno"], [
AC_MSG_NOTICE([openssl sm3 support not detected])
CPPFLAGS="$CPPFLAGS -DOPENSSL_NO_SM3"
])
+ module_signatures="PKCS7 $module_signatures"
], [
AC_MSG_NOTICE([openssl support not requested])
])
@@ -298,6 +305,9 @@ AC_DEFINE_UNQUOTED(KMOD_FEATURES, ["$with_features"], [Features in this build])
# Generate files from *.in
#####################################################################

+AC_SUBST([module_compressions], $module_compressions)
+AC_SUBST([module_signatures], $module_signatures)
+
AC_CONFIG_FILES([
Makefile
man/Makefile
@@ -305,6 +315,7 @@ AC_CONFIG_FILES([
libkmod/docs/version.xml
libkmod/libkmod.pc
libkmod/python/kmod/version.py
+ tools/kmod.pc
])


diff --git a/tools/kmod.pc.in b/tools/kmod.pc.in
new file mode 100644
index 000000000000..2595980a6b35
--- /dev/null
+++ b/tools/kmod.pc.in
@@ -0,0 +1,9 @@
+prefix=@prefix@
+sysconfdir=@sysconfdir@
+distconfdir=@distconfdir@
+module_compressions=@module_compressions@
+module_signatures=@module_signatures@
+
+Name: kmod
+Description: Tools to deal with kernel modules
+Version: @VERSION@
--
2.41.0


2023-07-18 12:27:29

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH kmod v5 3/5] libkmod, depmod: Load modprobe.d, depmod.d from ${prefix}/lib.

There is an ongoing effort to limit use of files outside of /usr (or
${prefix} on general). Currently all modprobe.d paths are hardcoded to
outside of $prefix. Teach kmod to load modprobe.d from ${prefix}/lib.

On some distributions /usr/lib and /lib are the same directory because
of a compatibility symlink, and it is possible to craft configuration
files with sideeffects that would behave differently when loaded twice.
However, the override semantic ensures that one 'overrides' the other,
and only one configuration file of the same name is loaded from any of
the search directories.

Signed-off-by: Michal Suchanek <[email protected]>
---
v2: Fix commit message typo
v3: Fix modprobe.d path list in code comment
v5: Add distconfdir
---
Makefile.am | 1 +
configure.ac | 5 +++++
libkmod/libkmod.c | 7 ++++---
man/Makefile.am | 9 +++++++--
man/depmod.d.xml | 1 +
man/modprobe.d.xml | 1 +
tools/depmod.c | 1 +
7 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 8ba85c91a0f3..14eb07e63cea 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -19,6 +19,7 @@ AM_CPPFLAGS = \
-include $(top_builddir)/config.h \
-I$(top_srcdir) \
-DSYSCONFDIR=\""$(sysconfdir)"\" \
+ -DDISTCONFDIR=\""$(distconfdir)"\" \
${zlib_CFLAGS}

AM_CFLAGS = $(OUR_CFLAGS)
diff --git a/configure.ac b/configure.ac
index 331cc8a1ffd5..728f88a56704 100644
--- a/configure.ac
+++ b/configure.ac
@@ -79,6 +79,10 @@ AC_COMPILE_IFELSE(
# --with-
#####################################################################

+AC_ARG_WITH([distconfdir], AS_HELP_STRING([--with-distconfdir=DIR], [directory to search for distribution configuration files]),
+ [], [with_distconfdir='${prefix}/lib'])
+AC_SUBST([distconfdir], [$with_distconfdir])
+
AC_ARG_WITH([rootlibdir],
AS_HELP_STRING([--with-rootlibdir=DIR], [rootfs directory to install shared libraries]),
[], [with_rootlibdir=$libdir])
@@ -313,6 +317,7 @@ AC_MSG_RESULT([

prefix: ${prefix}
sysconfdir: ${sysconfdir}
+ distconfdir: ${distconfdir}
libdir: ${libdir}
rootlibdir: ${rootlibdir}
includedir: ${includedir}
diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c
index 2670f9a4611a..09e6041461b0 100644
--- a/libkmod/libkmod.c
+++ b/libkmod/libkmod.c
@@ -65,6 +65,7 @@ static const char *const default_config_paths[] = {
SYSCONFDIR "/modprobe.d",
"/run/modprobe.d",
"/usr/local/lib/modprobe.d",
+ DISTCONFDIR "/modprobe.d",
"/lib/modprobe.d",
NULL
};
@@ -237,9 +238,9 @@ static char *get_kernel_release(const char *dirname)
* to load from user-defined configuration parameters such as
* alias, blacklists, commands (install, remove). If NULL
* defaults to /etc/modprobe.d, /run/modprobe.d,
- * /usr/local/lib/modprobe.d and /lib/modprobe.d. Give an empty
- * vector if configuration should not be read. This array must
- * be null terminated.
+ * /usr/local/lib/modprobe.d, DISTCONFDIR/modprobe.d, and
+ * /lib/modprobe.d. Give an empty vector if configuration should
+ * not be read. This array must be null terminated.
*
* Create kmod library context. This reads the kmod configuration
* and fills in the default values.
diff --git a/man/Makefile.am b/man/Makefile.am
index 11514d52a190..2fea8e46bf2f 100644
--- a/man/Makefile.am
+++ b/man/Makefile.am
@@ -17,9 +17,14 @@ EXTRA_DIST = $(MAN5:%.5=%.xml) $(MAN8:%.8=%.xml)
CLEANFILES = $(dist_man_MANS)

%.5 %.8: %.xml
- $(AM_V_XSLT)$(XSLT) \
+ $(AM_V_XSLT)if [ '$(distconfdir)' != '/lib' ] ; then \
+ sed -e 's|@DISTCONFDIR@|$(distconfdir)|g' $< ; \
+ else \
+ sed -e '/@DISTCONFDIR@/d' $< ; \
+ fi | \
+ $(XSLT) \
-o $@ \
--nonet \
--stringparam man.output.quietly 1 \
--param funcsynopsis.style "'ansi'" \
- http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $<
+ http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl -
diff --git a/man/depmod.d.xml b/man/depmod.d.xml
index 8d3d821cddc8..f282a39cc840 100644
--- a/man/depmod.d.xml
+++ b/man/depmod.d.xml
@@ -40,6 +40,7 @@

<refsynopsisdiv>
<para><filename>/lib/depmod.d/*.conf</filename></para>
+ <para><filename>@DISTCONFDIR@/depmod.d/*.conf</filename></para>
<para><filename>/usr/local/lib/depmod.d/*.conf</filename></para>
<para><filename>/run/depmod.d/*.conf</filename></para>
<para><filename>/etc/depmod.d/*.conf</filename></para>
diff --git a/man/modprobe.d.xml b/man/modprobe.d.xml
index 0ab3e9110a7e..2bf6537f07e6 100644
--- a/man/modprobe.d.xml
+++ b/man/modprobe.d.xml
@@ -41,6 +41,7 @@

<refsynopsisdiv>
<para><filename>/lib/modprobe.d/*.conf</filename></para>
+ <para><filename>@DISTCONFDIR@/modprobe.d/*.conf</filename></para>
<para><filename>/usr/local/lib/modprobe.d/*.conf</filename></para>
<para><filename>/run/modprobe.d/*.conf</filename></para>
<para><filename>/etc/modprobe.d/*.conf</filename></para>
diff --git a/tools/depmod.c b/tools/depmod.c
index 1d1d41db860f..630fef9c8fb0 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -54,6 +54,7 @@ static const char *const default_cfg_paths[] = {
SYSCONFDIR "/depmod.d",
"/run/depmod.d",
"/usr/local/lib/depmod.d",
+ DISTCONFDIR "/depmod.d",
"/lib/depmod.d",
NULL
};
--
2.41.0


2023-10-17 15:46:25

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH kmod v5 0/5] kmod /usr support

Hello,

it has been a few months since these kmod patches have been posted, and
a new kmod versio has been released since.

Is there any interest in adding this to kmod?

Thanks

Michal

On Tue, Jul 18, 2023 at 02:01:51PM +0200, Michal Suchanek wrote:
> Hello,
>
> with these patches it is possible to install kernel modules in an arbitrary
> directory - eg. moving the /lib/modules to /usr/lib/modules or /opt/linux.
>
> While the modprobe.d and depmod.d search which already includes multiple
> paths is expanded to also include $(prefix) the module directory still
> supports only one location, only a different one under $(module_directory).
>
> Having kmod search multiple module locations while only one is supported now
> might break some assumption about relative module path corresponding to a
> specific file, would require more invasive changes to implement, and is not
> supportive of the goal of moving the modules away from /lib.
>
> Both kmod and the kernel need to be patched to make use of this feature.
> Patched kernel is backwards compatible with older kmod. Patched kmod
> with $(module_directory) set to /lib/modules is equivalent to unpatched kmod.
>
> Thanks
>
> Michal
>
> Link: https://lore.kernel.org/linux-modules/[email protected]/
>
> v4: set whole path to module directory instead of adding prefix
> v5: use pkg-config instead of jq, fix build on openssl without sm3 support
>
>
> Michal Suchanek (5):
> configure: Detect openssl sm3 support
> man/depmod.d: Fix incorrect /usr/lib search path
> libkmod, depmod: Load modprobe.d, depmod.d from ${prefix}/lib.
> kmod: Add pkgconfig file with kmod compile time configuration
> libkmod, depmod, modprobe: Make directory for kernel modules
> configurable
>
> Makefile.am | 6 +-
> configure.ac | 30 ++++++++
> libkmod/libkmod.c | 11 +--
> man/Makefile.am | 10 ++-
> man/depmod.d.xml | 9 ++-
> man/depmod.xml | 4 +-
> man/modinfo.xml | 2 +-
> man/modprobe.d.xml | 1 +
> 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 | 7 +-
> tools/kmod.pc.in | 10 +++
> tools/modinfo.c | 4 +-
> tools/modprobe.c | 4 +-
> tools/static-nodes.c | 6 +-
> 19 files changed, 156 insertions(+), 91 deletions(-)
> create mode 100644 tools/kmod.pc.in
>
> --
> 2.41.0
>

2023-10-17 16:18:45

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH kmod v5 0/5] kmod /usr support

On Tue, Oct 17, 2023 at 05:45:39PM +0200, Michal Such?nek wrote:
>Hello,
>
>it has been a few months since these kmod patches have been posted, and
>a new kmod versio has been released since.
>
>Is there any interest in adding this to kmod?

yes, but I think the main drag is deciding with the kernel build system
maintainers what they are willing to accept as an interface. There isn't
much point in exporting a json config if from the kernel side they would
rather use something else. Or to use pkg-config.

I confess I lost track of that discussion. Did that settle with
pkg-config being the preferred solution?

Lucas De Marchi

2023-10-17 16:41:08

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH kmod v5 0/5] kmod /usr support

On Tue, Oct 17, 2023 at 11:18:23AM -0500, Lucas De Marchi wrote:
> On Tue, Oct 17, 2023 at 05:45:39PM +0200, Michal Such?nek wrote:
> > Hello,
> >
> > it has been a few months since these kmod patches have been posted, and
> > a new kmod versio has been released since.
> >
> > Is there any interest in adding this to kmod?
>
> yes, but I think the main drag is deciding with the kernel build system
> maintainers what they are willing to accept as an interface. There isn't
> much point in exporting a json config if from the kernel side they would
> rather use something else. Or to use pkg-config.
>
> I confess I lost track of that discussion. Did that settle with
> pkg-config being the preferred solution?

The current discussion about the kernel side can be found here:

https://lore.kernel.org/linux-kbuild/[email protected]/T/#t

My impression is that pkg-config is accepted as an interface on the
basis that it's already required for building the kernel while jq is
currently required only for some additional optional tools.

Thanks

Michal