[PATCH 1/8] testsuite: add test for kcmdline params with no value

[Lucas De Marchi]

Use "modprobe -c" to dump the configuration. Since we configure our
rootfs to have only a /proc/cmdline file, this should dump the knowledge
we have from its parsed content.

Test if <module>.option, without any value is correctly parsed, as fixed
in commit 493dc65 ("libkmod: Fix getting param with no value from kcmdline")
---
.../module-param-kcmdline2/correct.txt | 5 +++++
.../module-param-kcmdline2/proc/cmdline | 1 +
testsuite/test-modprobe.c | 25 ++++++++++++++++++++++
3 files changed, 31 insertions(+)
create mode 100644 testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline2/correct.txt
create mode 100644 testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline2/proc/cmdline

diff --git a/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline2/correct.txt b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline2/correct.txt
new file mode 100644
index 0000000..b73a680
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline2/correct.txt
@@ -0,0 +1,5 @@
+options psmouse foo
+options psmouse bar=1
+
+# End of configuration files. Dumping indexes now:
+
diff --git a/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline2/proc/cmdline b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline2/proc/cmdline
new file mode 100644
index 0000000..f048fdd
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline2/proc/cmdline
@@ -0,0 +1 @@
+psmouse.foo psmouse.bar=1 quiet rw
diff --git a/testsuite/test-modprobe.c b/testsuite/test-modprobe.c
index b675f48..492b4d8 100644
--- a/testsuite/test-modprobe.c
+++ b/testsuite/test-modprobe.c
@@ -187,6 +187,30 @@ static DEFINE_TEST(modprobe_param_kcmdline,
.modules_loaded = "",
);

+static noreturn int modprobe_param_kcmdline2(const struct test *t)
+{
+ const char *progname = ABS_TOP_BUILDDIR "/tools/modprobe";
+ const char *const args[] = {
+ progname,
+ "-c",
+ NULL,
+ };
+
+ test_spawn_prog(progname, args);
+ exit(EXIT_FAILURE);
+}
+static DEFINE_TEST(modprobe_param_kcmdline2,
+ .description = "check if params with no value are parsed correctly from kcmdline",
+ .config = {
+ [TC_UNAME_R] = "4.4.4",
+ [TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline2",
+ },
+ .output = {
+ .out = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline2/correct.txt",
+ },
+ .modules_loaded = "",
+ );
+
static noreturn int modprobe_force(const struct test *t)
{
const char *progname = ABS_TOP_BUILDDIR "/tools/modprobe";
@@ -262,6 +286,7 @@ static const struct test *tests[] = {
&smodprobe_softdep_loop,
&smodprobe_install_cmd_loop,
&smodprobe_param_kcmdline,
+ &smodprobe_param_kcmdline2,
&smodprobe_force,
&smodprobe_oldkernel,
&smodprobe_oldkernel_force,

[Lucas De Marchi]

We are not only checking if those options are correctly parsed from
kcmdline, but if in fact they are being passed to the final
(f)init_module call. This is why we use 'modprobe --show-depends'
instead of the simpler 'modprobe -c'.
---
testsuite/test-modprobe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testsuite/test-modprobe.c b/testsuite/test-modprobe.c
index 492b4d8..b5eaba3 100644
--- a/testsuite/test-modprobe.c
+++ b/testsuite/test-modprobe.c
@@ -176,7 +176,7 @@ static noreturn int modprobe_param_kcmdline(const struct test *t)
exit(EXIT_FAILURE);
}
static DEFINE_TEST(modprobe_param_kcmdline,
- .description = "check if params are parsed correctly from kcmdline",
+ .description = "check if params from kcmdline are passed in fact passed to (f)init_module call",
.config = {
[TC_UNAME_R] = "4.4.4",
[TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline",

[Lucas De Marchi]

These redirecting makefiles simplifies compiling from some editors and
when CWD is not the root of the source tree. This is similar to what was
introduced in systemd in 340d89e ("build-sys: add small redirecting
Makefiles to simplify compilation from within emacs")
---
.gitignore | 4 ++--
libkmod/Makefile | 13 +++++++++++++
libkmod/docs/.gitignore | 2 ++
man/.gitignore | 2 ++
testsuite/Makefile | 15 +++++++++++++++
tools/Makefile | 13 +++++++++++++
6 files changed, 47 insertions(+), 2 deletions(-)
create mode 100644 libkmod/Makefile
create mode 100644 testsuite/Makefile
create mode 100644 tools/Makefile

diff --git a/.gitignore b/.gitignore
index 5bfab9a..88c8149 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,8 +3,8 @@
/*.md5sum
.deps/
.libs/
-Makefile
-Makefile.in
+/Makefile
+/Makefile.in

/aclocal.m4
/autom4te.cache/
diff --git a/libkmod/Makefile b/libkmod/Makefile
new file mode 100644
index 0000000..223bec2
--- /dev/null
+++ b/libkmod/Makefile
@@ -0,0 +1,13 @@
+# Copyright 2010 Lennart Poettering
+#
+# This file has been copied from systemd. It is a dirty trick to simplify
+# compilation when CWD is not the root of the source tree. This file is not
+# intended to be distributed. So, don't touch it, even better ignore it!
+
+all:
+ $(MAKE) -C ..
+
+clean:
+ $(MAKE) -C .. clean
+
+.PHONY: all clean
diff --git a/libkmod/docs/.gitignore b/libkmod/docs/.gitignore
index 21e2279..7514b08 100644
--- a/libkmod/docs/.gitignore
+++ b/libkmod/docs/.gitignore
@@ -10,3 +10,5 @@ version.xml
xml
html
gtk-doc.make
+Makefile
+Makefile.in
diff --git a/man/.gitignore b/man/.gitignore
index bc0cce8..a229b2f 100644
--- a/man/.gitignore
+++ b/man/.gitignore
@@ -1,2 +1,4 @@
*.5
*.8
+Makefile
+Makefile.in
diff --git a/testsuite/Makefile b/testsuite/Makefile
new file mode 100644
index 0000000..38ba552
--- /dev/null
+++ b/testsuite/Makefile
@@ -0,0 +1,15 @@
+# Copyright 2010 Lennart Poettering
+#
+# This file has been copied from systemd. It is a dirty trick to simplify
+# compilation when CWD is not the root of the source tree. This file is not
+# intended to be distributed. So, don't touch it, even better ignore it!
+
+all:
+ $(MAKE) -C .. check
+check:
+ $(MAKE) -C .. check
+
+clean:
+ $(MAKE) -C .. clean
+
+.PHONY: all clean check
diff --git a/tools/Makefile b/tools/Makefile
new file mode 100644
index 0000000..223bec2
--- /dev/null
+++ b/tools/Makefile
@@ -0,0 +1,13 @@
+# Copyright 2010 Lennart Poettering
+#
+# This file has been copied from systemd. It is a dirty trick to simplify
+# compilation when CWD is not the root of the source tree. This file is not
+# intended to be distributed. So, don't touch it, even better ignore it!
+
+all:
+ $(MAKE) -C ..
+
+clean:
+ $(MAKE) -C .. clean
+
+.PHONY: all clean

[Lucas De Marchi]

In kcmdline it's possible to have a dot in the param's value. The
support for this was added in 66f3228 ("libkmod: Add support for '.' in
module parameter on kcmdline") and is needed to correctly support some
modules that depend on it.

This test was added in order to make sure the commit aa87854
("libkmod-config: Only match dot before '=' in /proc/cmdline") didn't
break it. Although that commit message says it's allowing to match a
dot before '=' it's actually enforcing the first part of the string to
be always in the format "<module-name>.param". Dots after '=' are still
correctly allowed.
---
.../module-param-kcmdline4/correct.txt | 4 ++++
.../module-param-kcmdline4/proc/cmdline | 1 +
testsuite/test-modprobe.c | 25 ++++++++++++++++++++++
3 files changed, 30 insertions(+)
create mode 100644 testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline4/correct.txt
create mode 100644 testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline4/proc/cmdline

diff --git a/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline4/correct.txt b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline4/correct.txt
new file mode 100644
index 0000000..60fa483
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline4/correct.txt
@@ -0,0 +1,4 @@
+options testmodule testparam=1.5G
+
+# End of configuration files. Dumping indexes now:
+
diff --git a/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline4/proc/cmdline b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline4/proc/cmdline
new file mode 100644
index 0000000..c460c5e
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline4/proc/cmdline
@@ -0,0 +1 @@
+testmodule.testparam=1.5G
diff --git a/testsuite/test-modprobe.c b/testsuite/test-modprobe.c
index e5bd925..d7663e6 100644
--- a/testsuite/test-modprobe.c
+++ b/testsuite/test-modprobe.c
@@ -235,6 +235,30 @@ static DEFINE_TEST(modprobe_param_kcmdline3,
.modules_loaded = "",
);

+static noreturn int modprobe_param_kcmdline4(const struct test *t)
+{
+ const char *progname = ABS_TOP_BUILDDIR "/tools/modprobe";
+ const char *const args[] = {
+ progname,
+ "-c",
+ NULL,
+ };
+
+ test_spawn_prog(progname, args);
+ exit(EXIT_FAILURE);
+}
+static DEFINE_TEST(modprobe_param_kcmdline4,
+ .description = "check if unrelated strings in kcmdline are correctly ignored",
+ .config = {
+ [TC_UNAME_R] = "4.4.4",
+ [TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline4",
+ },
+ .output = {
+ .out = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline4/correct.txt",
+ },
+ .modules_loaded = "",
+ );
+
static noreturn int modprobe_force(const struct test *t)
{
const char *progname = ABS_TOP_BUILDDIR "/tools/modprobe";
@@ -312,6 +336,7 @@ static const struct test *tests[] = {
&smodprobe_param_kcmdline,
&smodprobe_param_kcmdline2,
&smodprobe_param_kcmdline3,
+ &smodprobe_param_kcmdline4,
&smodprobe_force,
&smodprobe_oldkernel,
&smodprobe_oldkernel_force,

[Michal Marek]

Post by Lucas De Marchi
In kcmdline it's possible to have a dot in the param's value. The
support for this was added in 66f3228 ("libkmod: Add support for '.' in
module parameter on kcmdline") and is needed to correctly support some
modules that depend on it.

The tests look OK. I tried to revert my fix and the test failed.

Post by Lucas De Marchi
This test was added in order to make sure the commit aa87854
("libkmod-config: Only match dot before '=' in /proc/cmdline") didn't
break it. Although that commit message says it's allowing to match a
dot before '=' it's actually enforcing the first part of the string to
be always in the format "<module-name>.param". Dots after '=' are still
correctly allowed.

Yeah, the wording was not really clear.

Thanks,
Michal

[Lucas De Marchi]

Strings unrelated to modules and modprobe should be ignored and not
appear in the output of "modprobe -c".

This adds a test for the fix provided in aa87854 ("libkmod-config: Only
match dot before '=' in /proc/cmdline").
---
.../module-param-kcmdline3/correct.txt | 5 +++++
.../module-param-kcmdline3/proc/cmdline | 1 +
testsuite/test-modprobe.c | 25 ++++++++++++++++++++++
3 files changed, 31 insertions(+)
create mode 100644 testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline3/correct.txt
create mode 100644 testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline3/proc/cmdline

diff --git a/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline3/correct.txt b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline3/correct.txt
new file mode 100644
index 0000000..b73a680
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline3/correct.txt
@@ -0,0 +1,5 @@
+options psmouse foo
+options psmouse bar=1
+
+# End of configuration files. Dumping indexes now:
+
diff --git a/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline3/proc/cmdline b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline3/proc/cmdline
new file mode 100644
index 0000000..3575bb1
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline3/proc/cmdline
@@ -0,0 +1 @@
+BOOT_IMAGE=/boot/vmlinuz-3.12.12-57.g5f654cf-default initrd=\initramfs-linux.img psmouse.foo psmouse.bar=1 root=/dev/sda2 rootfstype=ext4 add_efi_memmap quiet rw
diff --git a/testsuite/test-modprobe.c b/testsuite/test-modprobe.c
index b5eaba3..e5bd925 100644
--- a/testsuite/test-modprobe.c
+++ b/testsuite/test-modprobe.c
@@ -211,6 +211,30 @@ static DEFINE_TEST(modprobe_param_kcmdline2,
.modules_loaded = "",
);

+static noreturn int modprobe_param_kcmdline3(const struct test *t)
+{
+ const char *progname = ABS_TOP_BUILDDIR "/tools/modprobe";
+ const char *const args[] = {
+ progname,
+ "-c",
+ NULL,
+ };
+
+ test_spawn_prog(progname, args);
+ exit(EXIT_FAILURE);
+}
+static DEFINE_TEST(modprobe_param_kcmdline3,
+ .description = "check if unrelated strings in kcmdline are correctly ignored",
+ .config = {
+ [TC_UNAME_R] = "4.4.4",
+ [TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline3",
+ },
+ .output = {
+ .out = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline3/correct.txt",
+ },
+ .modules_loaded = "",
+ );
+
static noreturn int modprobe_force(const struct test *t)
{
const char *progname = ABS_TOP_BUILDDIR "/tools/modprobe";
@@ -287,6 +311,7 @@ static const struct test *tests[] = {
&smodprobe_install_cmd_loop,
&smodprobe_param_kcmdline,
&smodprobe_param_kcmdline2,
+ &smodprobe_param_kcmdline3,
&smodprobe_force,
&smodprobe_oldkernel,
&smodprobe_oldkernel_force,

[Lucas De Marchi]

Same reason as found in this commit to systemd:

commit 4ca39b280fce3c60d2fdecbd478fd9bf7f9d3e64
Author: Mike Gilbert <floppym-aBrp7R+bbdUdnm+***@public.gmane.org>
Date: Sun Feb 23 11:21:13 2014 -0500

configure: Do not require xsltproc for installation of man pages

The release tarballs ship with pre-generated man pages, so we do not
need xsltproc for a typical end-user build.

Developers will probably have xsltproc anyway, but if not they will now
encounter a build-time failure instead of an error in configure.
---
configure.ac | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index 4d9496e..54a52ea 100644
--- a/configure.ac
+++ b/configure.ac
@@ -36,6 +36,7 @@ AC_C_BIGENDIAN
AC_PROG_SED
AC_PROG_MKDIR_P
PKG_PROG_PKG_CONFIG
+AC_PATH_PROG([XSLTPROC], [xsltproc])


#####################################################################
@@ -111,10 +112,7 @@ AC_ARG_ENABLE([manpages],
AS_HELP_STRING([--disable-manpages], [disable manpages @<:@default=enabled@:>@]),
[], enable_manpages=yes)
AM_CONDITIONAL([BUILD_MANPAGES], [test "x$enable_manpages" = "xyes"])
-AC_PATH_PROG([XSLTPROC], [xsltproc], [no])
-AS_IF([test "x$XSLTPROC" = "xno" && test "x$enable_manpages" = "xyes"], [
- AC_MSG_ERROR([xsltproc command not found, try ./configure --disable-manpages])
-])
+AS_IF(["x$enable_manpages" != "xno"], ["have_manpages=yes"])

AC_ARG_ENABLE([logging],
AS_HELP_STRING([--disable-logging], [disable system logging @<:@default=enabled@:>@]),

[Michal Marek]

Post by Lucas De Marchi
commit 4ca39b280fce3c60d2fdecbd478fd9bf7f9d3e64
Date: Sun Feb 23 11:21:13 2014 -0500
configure: Do not require xsltproc for installation of man pages

Great, it will be possible to build a package without requiring xsltproc.

Thanks,
Michal

[Lucas De Marchi]

Nowadays udev doesn't create nodes in /dev anymore. This role is rather
taken by systemd-tmpfiles on early boot so reference it generically as
systemd.
---
man/depmod.xml | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/man/depmod.xml b/man/depmod.xml
index a9b61d9..380ee7d 100644
--- a/man/depmod.xml
+++ b/man/depmod.xml
@@ -93,9 +93,11 @@
<command>depmod</command> will output a file named
<filename>modules.devname</filename> if modules supply special device
names (devname) that should be populated in /dev on boot (by a utility
- such as udev). </para> <para> If a <replaceable>version</replaceable> is
- provided, then that kernel version's module directory is used rather than
- the current kernel version (as returned by <command>uname -r</command>).
+ such as systemd-tmpfiles).
+ </para>
+ <para> If a <replaceable>version</replaceable> is provided, then that kernel
+ version's module directory is used rather than the current kernel version
+ (as returned by <command>uname -r</command>).
</para>
</refsect1>
<refsect1><title>OPTIONS</title>

[Michal Marek]

Post by Lucas De Marchi
Nowadays udev doesn't create nodes in /dev anymore. This role is rather

^static

As of writing this, udev still takes care of "normal" device nodes.

Michal

[Lucas De Marchi]

Post by Michal Marek

Post by Lucas De Marchi
Nowadays udev doesn't create nodes in /dev anymore. This role is rather

^static
As of writing this, udev still takes care of "normal" device nodes.

nops... it doesn't even have the capability to create nodes anymore:
http://cgit.freedesktop.org/systemd/systemd/commit/units/systemd-udevd.service.in?id=edeb68c53f1cdc452016b4c8512586a70b1262e3

I don' t know what you mean by normal device node... but udev now
relies on 2 sources for creating nodes:

1) devtmpfs in the kernel
2) systemd-tmpfiles, that runs on early boot taking the output of
'kmod static-nodes'

It's this second case I'm referring to in the man page, since it
relies on the devname alias that people put in each module.

Lucas De Marchi

[Lucas De Marchi]

On Thu, Mar 6, 2014 at 6:47 PM, Lucas De Marchi

Post by Lucas De Marchi

Post by Michal Marek

Post by Lucas De Marchi
Nowadays udev doesn't create nodes in /dev anymore. This role is rather

^static
As of writing this, udev still takes care of "normal" device nodes.

http://cgit.freedesktop.org/systemd/systemd/commit/units/systemd-udevd.service.in?id=edeb68c53f1cdc452016b4c8512586a70b1262e3
I don' t know what you mean by normal device node... but udev now
1) devtmpfs in the kernel
2) systemd-tmpfiles, that runs on early boot taking the output of
'kmod static-nodes'
It's this second case I'm referring to in the man page, since it
relies on the devname alias that people put in each module.
Lucas De Marchi

All patches are now applied.

[Michal Marek]

Post by Lucas De Marchi

Post by Michal Marek

Post by Lucas De Marchi
Nowadays udev doesn't create nodes in /dev anymore. This role is rather

^static
As of writing this, udev still takes care of "normal" device nodes.

http://cgit.freedesktop.org/systemd/systemd/commit/units/systemd-udevd.service.in?id=edeb68c53f1cdc452016b4c8512586a70b1262e3
I don' t know what you mean by normal device node... but udev now
1) devtmpfs in the kernel
2) systemd-tmpfiles, that runs on early boot taking the output of
'kmod static-nodes'

Ah, I didn't know that devtmpfs is mandatory now.

Michal

[Lucas De Marchi]

---
man/modprobe.xml | 3 +--
man/modules.dep.xml | 19 ++++++++++---------
2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/man/modprobe.xml b/man/modprobe.xml
index 4c6c832..9cb4476 100644
--- a/man/modprobe.xml
+++ b/man/modprobe.xml
@@ -101,8 +101,7 @@
</para>
<para>
<command>modprobe</command> expects an up-to-date
- <filename>modules.dep.bin</filename> file (or fallback human
- readable <filename>modules.dep</filename> file), as generated
+ <filename>modules.dep.bin</filename> file as generated
by the corresponding <command>depmod</command> utility shipped
along with <command>modprobe</command> (see
<citerefentry><refentrytitle>depmod</refentrytitle><manvolnum>8</manvolnum>
diff --git a/man/modules.dep.xml b/man/modules.dep.xml
index 034c2b4..e53293a 100644
--- a/man/modules.dep.xml
+++ b/man/modules.dep.xml
@@ -40,16 +40,17 @@

<refsect1><title>DESCRIPTION</title>
<para>
- The <filename>modules.dep.bin</filename> as generated by
- module-init-tools <command>depmod</command>, lists the dependencies for
+ <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>,
- where <filename>modules.dep.bin</filename> (or the human readable version
- <filename>modules.dep</filename>) is also located. It is used by
- utilities such as <command>modprobe</command>. The binary version will be
- used by default, if it was generated by a compatible version of
- <command>depmod</command>, with fallback to the generic
- <filename>modules.dep</filename>.
+ <filename>/lib/modules/</filename><replaceable>version</replaceable>.
+ It is used by kmod tools such as <command>modprobe</command> and
+ libkmod.
+ </para>
+ <para>
+ Its text counterpar is located in the same directory with the name
+ <filename>modules.dep</filename>. The text version is maintained only
+ for easy of reading by humans and is in no way used by any kmod tool.
</para>
<para>
These files are not intended for editing or use by any additional