[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;

[Michal Marek]

From: Tom Gundersen <teg-***@public.gmane.org>

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];

[Lucas De Marchi]

Hi Michal and Tom.

Post by Michal Marek
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.

[Tom Gundersen]

On Tue, Apr 1, 2014 at 1:18 PM, Lucas De Marchi

Post by Lucas De Marchi
Hi Michal and Tom.

Post by Michal Marek
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

[Lucas De Marchi]

Hi Michal,

Post by 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.
---

Applied. Thanks.