[PATCH 1/2] libkmod: Ignore errors from softdeps

[PATCH 1/2] libkmod: Ignore errors from softdeps

[Michal Marek]

Before we had softdeps, the usual idiom was

install foo /sbin/modprobe bar; /sbin/modprobe --ignore-install foo

ignoring errors from the first modprobe invocation. This also matches
the behavior of module-init-tools' implementation of softdep.
---
 libkmod/libkmod-internal.h |  2 ++
 libkmod/libkmod-module.c   | 40 ++++++++++++++++++++++++++++++++++++----
 libkmod/libkmod.c          | 10 ++++++++++
 3 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h
index 0180124..93a00c1 100644
--- a/libkmod/libkmod-internal.h
+++ b/libkmod/libkmod-internal.h
@@ -93,6 +93,7 @@ int kmod_lookup_alias_from_moddep_file(struct kmod_ctx *ctx, const char *name, s
 int kmod_lookup_alias_from_builtin_file(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3)));
 int kmod_lookup_alias_from_commands(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3)));
 void kmod_set_modules_visited(struct kmod_ctx *ctx, bool visited) __attribute__((nonnull((1))));
+void kmod_set_modules_required(struct kmod_ctx *ctx, bool required) __attribute__((nonnull((1))));
 
 char *kmod_search_moddep(struct kmod_ctx *ctx, const char *name) __attribute__((nonnull(1,2)));
 
@@ -142,6 +143,7 @@ void kmod_module_set_install_commands(struct kmod_module *mod, const char *cmd)
 void kmod_module_set_remove_commands(struct kmod_module *mod, const char *cmd) __attribute__((nonnull(1)));
 void kmod_module_set_visited(struct kmod_module *mod, bool visited) __attribute__((nonnull(1)));
 void kmod_module_set_builtin(struct kmod_module *mod, bool builtin) __attribute__((nonnull((1))));
+void kmod_module_set_required(struct kmod_module *mod, bool required) __attribute__((nonnull(1)));
 
 /* libkmod-hash.c */
 
diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
index a6c8a6e..1d90f34 100644
--- a/libkmod/libkmod-module.c
+++ b/libkmod/libkmod-module.c
@@ -87,6 +87,13 @@ struct kmod_module {
 	bool ignorecmd : 1;
 
 	/*
+	 * set by kmod_module_get_probe_list: indicates whether this is the
+	 * module the user asked for or its dependency, or whether this
+	 * is a softdep only
+	 */
+	bool required : 1;
+
+	/*
 	 * if module was created by searching the modules.builtin file, this
 	 * is set. There's nothing much useful one can do with such a
 	 * "module", except knowing it's builtin.
@@ -208,6 +215,11 @@ void kmod_module_set_builtin(struct kmod_module *mod, bool builtin)
 	mod->builtin = builtin;
 }
 
+void kmod_module_set_required(struct kmod_module *mod, bool required)
+{
+	mod->required = required;
+}
+
 /*
  * Memory layout with alias:
  *
@@ -1033,6 +1045,7 @@ static char *module_options_concat(const char *opt, const char *xopt)
 }
 
 static int __kmod_module_get_probe_list(struct kmod_module *mod,
+						bool required,
 						bool ignorecmd,
 						struct kmod_list **list);
 
@@ -1052,7 +1065,7 @@ static int __kmod_module_fill_softdep(struct kmod_module *mod,
 
 	kmod_list_foreach(l, pre) {
 		struct kmod_module *m = l->data;
-		err = __kmod_module_get_probe_list(m, false, list);
+		err = __kmod_module_get_probe_list(m, false, false, list);
 		if (err < 0)
 			goto fail;
 	}
@@ -1068,7 +1081,7 @@ static int __kmod_module_fill_softdep(struct kmod_module *mod,
 
 	kmod_list_foreach(l, post) {
 		struct kmod_module *m = l->data;
-		err = __kmod_module_get_probe_list(m, false, list);
+		err = __kmod_module_get_probe_list(m, false, false, list);
 		if (err < 0)
 			goto fail;
 	}
@@ -1082,6 +1095,7 @@ fail:
 
 /* re-entrant */
 static int __kmod_module_get_probe_list(struct kmod_module *mod,
+						bool required,
 						bool ignorecmd,
 						struct kmod_list **list)
 {
@@ -1096,6 +1110,19 @@ static int __kmod_module_get_probe_list(struct kmod_module *mod,
 	mod->visited = true;
 
 	dep = kmod_module_get_dependencies(mod);
+	if (required) {
+		/*
+		 * Called from kmod_module_probe_insert_module(); set the
+		 * ->required flag on mod and all its dependencies before
+		 * they are possibly visited through some softdeps.
+		 */
+		mod->required = true;
+		kmod_list_foreach(l, dep) {
+			struct kmod_module *m = l->data;
+			m->required = true;
+		}
+	}
+
 	kmod_list_foreach(l, dep) {
 		struct kmod_module *m = l->data;
 		err = __kmod_module_fill_softdep(m, list);
@@ -1133,8 +1160,9 @@ static int kmod_module_get_probe_list(struct kmod_module *mod,
 	 * Make sure we don't get screwed by previous calls to this function
 	 */
 	kmod_set_modules_visited(mod->ctx, false);
+	kmod_set_modules_required(mod->ctx, false);
 
-	err = __kmod_module_get_probe_list(mod, ignorecmd, list);
+	err = __kmod_module_get_probe_list(mod, true, ignorecmd, list);
 	if (err < 0) {
 		kmod_module_unref_list(*list);
 		*list = NULL;
@@ -1295,8 +1323,12 @@ finish_module:
 				(flags & KMOD_PROBE_FAIL_ON_LOADED))
 			break;
 
-		if (err == -EEXIST)
+		/*
+		 * Ignore errors from softdeps
+		 */
+		if (err == -EEXIST || !m->required)
 			err = 0;
+
 		else if (err < 0)
 			break;
 	}
diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c
index ef83e31..fe68134 100644
--- a/libkmod/libkmod.c
+++ b/libkmod/libkmod.c
@@ -720,6 +720,16 @@ void kmod_set_modules_visited(struct kmod_ctx *ctx, bool visited)
 		kmod_module_set_visited((struct kmod_module *)v, visited);
 }
 
+void kmod_set_modules_required(struct kmod_ctx *ctx, bool required)
+{
+	struct hash_iter iter;
+	const void *v;
+
+	hash_iter_init(ctx->modules_by_name, &iter);
+	while (hash_iter_next(&iter, NULL, &v))
+		kmod_module_set_required((struct kmod_module *)v, required);
+}
+
 static bool is_cache_invalid(const char *path, unsigned long long stamp)
 {
 	struct stat st;
-- 
1.8.4.5

[PATCH 2/2] config: also parse softdeps from modules

[Michal Marek]

From: Tom Gundersen <teg@jklm.no>

This information can be found in /lib/modules/`uname -r`/modules.softdep, and
has only recently been exported by the kernel.

Also remove the advice about copying modules.softdep to /lib/modules as it is
not clear how to do this correctly with several kernels installed with
potentially conflicting soft dependencies.
---
 libkmod/libkmod-config.c | 2 ++
 tools/depmod.c           | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libkmod/libkmod-config.c b/libkmod/libkmod-config.c
index 9905d5e..0953924 100644
--- a/libkmod/libkmod-config.c
+++ b/libkmod/libkmod-config.c
@@ -848,6 +848,8 @@ int kmod_config_new(struct kmod_ctx *ctx, struct kmod_config **p_config,
 	struct kmod_list *path_list = NULL;
 	size_t i;
 
+	conf_files_insert_sorted(ctx, &list, kmod_get_dirname(ctx), "modules.softdep");
+
 	for (i = 0; config_paths[i] != NULL; i++) {
 		const char *path = config_paths[i];
 		unsigned long long path_stamp = 0;
diff --git a/tools/depmod.c b/tools/depmod.c
index 37e6afd..1aedaaf 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -1952,8 +1952,6 @@ static int output_softdeps(struct depmod *depmod, FILE *out)
 	size_t i;
 
 	fputs("# Soft dependencies extracted from modules themselves.\n", out);
-	fputs("# Copy, with a .conf extension, to /etc/modprobe.d to use "
-	      "it with modprobe.\n", out);
 
 	for (i = 0; i < depmod->modules.count; i++) {
 		const struct mod *mod = depmod->modules.array[i];
-- 
1.8.4.5

Re: [PATCH 1/2] libkmod: Ignore errors from softdeps

[Lucas De Marchi]

Hi Michal,

On Mon, Mar 31, 2014 at 03:18:50PM +0200, Michal Marek wrote:
> Before we had softdeps, the usual idiom was
> 
> install foo /sbin/modprobe bar; /sbin/modprobe --ignore-install foo
> 
> ignoring errors from the first modprobe invocation. This also matches
> the behavior of module-init-tools' implementation of softdep.
> ---

Applied. Thanks.

-- 
Lucas De Marchi

Re: [PATCH 2/2] config: also parse softdeps from modules

[Lucas De Marchi]

Hi Michal and Tom.


On Mon, Mar 31, 2014 at 03:18:51PM +0200, Michal Marek wrote:
> From: Tom Gundersen <teg@jklm.no>
> 
> This information can be found in /lib/modules/`uname -r`/modules.softdep, and
> has only recently been exported by the kernel.
> 
> Also remove the advice about copying modules.softdep to /lib/modules as it is
> not clear how to do this correctly with several kernels installed with
> potentially conflicting soft dependencies.
> ---

I think in most of the cases using a softdep from the kernel is wrong,
since there are other better ways to do it. But there are some it's
indeed difficult to avoid... crct10dif is one of them.

Applied.

-- 
Lucas De Marchi

Re: [PATCH 2/2] config: also parse softdeps from modules

[Tom Gundersen]

On Tue, Apr 1, 2014 at 1:18 PM, Lucas De Marchi
<lucas.demarchi@intel.com> wrote:
> Hi Michal and Tom.
>
>
> On Mon, Mar 31, 2014 at 03:18:51PM +0200, Michal Marek wrote:
>> From: Tom Gundersen <teg@jklm.no>
>>
>> This information can be found in /lib/modules/`uname -r`/modules.softdep, and
>> has only recently been exported by the kernel.
>>
>> Also remove the advice about copying modules.softdep to /lib/modules as it is
>> not clear how to do this correctly with several kernels installed with
>> potentially conflicting soft dependencies.
>> ---
>
> I think in most of the cases using a softdep from the kernel is wrong,
> since there are other better ways to do it. But there are some it's
> indeed difficult to avoid... crct10dif is one of them.
>
> Applied.

Thanks to both. I had forgotten about this.

Cheers,

Tom