* [PATCHv2 2/5] modprobe: Update error message when path is missing
2015-09-28 22:39 [PATCHv2 1/5] README: Indicate cython requirements Laura Abbott
@ 2015-09-28 22:39 ` Laura Abbott
2015-09-30 18:35 ` Lucas De Marchi
2015-09-28 22:39 ` [PATCHv2 3/5] Change default log level Laura Abbott
` (3 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Laura Abbott @ 2015-09-28 22:39 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: Laura Abbott, linux-modules
Currently, modprobe fails with no output by default if the
search paths it tries are missing:
$ modprobe -S notakernel dm-crypt
$
$ modprobe -S notakernel lkjjweiojo
$
This is fairly cryptic and not at all obvious there is a problem
unless the error code is checked or verbose flags are used.
Update the error message to indicate a problem and print out the
directory that failed.
---
libkmod/libkmod-internal.h | 2 --
libkmod/libkmod.h | 1 +
tools/modprobe.c | 7 +++----
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h
index 3e9839d..4d9db6b 100644
--- a/libkmod/libkmod-internal.h
+++ b/libkmod/libkmod-internal.h
@@ -85,8 +85,6 @@ struct kmod_list *kmod_list_append_list(struct kmod_list *list1, struct kmod_lis
container_of(list_entry->node.prev, struct kmod_list, node)))
/* libkmod.c */
-const char *kmod_get_dirname(const struct kmod_ctx *ctx) __attribute__((nonnull(1)));
-
int kmod_lookup_alias_from_config(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3)));
int kmod_lookup_alias_from_symbols_file(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3)));
int kmod_lookup_alias_from_aliases_file(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3)));
diff --git a/libkmod/libkmod.h b/libkmod/libkmod.h
index fe53a59..f9e33c6 100644
--- a/libkmod/libkmod.h
+++ b/libkmod/libkmod.h
@@ -51,6 +51,7 @@ void kmod_set_log_priority(struct kmod_ctx *ctx, int priority);
void *kmod_get_userdata(const struct kmod_ctx *ctx);
void kmod_set_userdata(struct kmod_ctx *ctx, const void *userdata);
+const char *kmod_get_dirname(const struct kmod_ctx *ctx);
/*
* Management of libkmod's resources
diff --git a/tools/modprobe.c b/tools/modprobe.c
index 3ba8f52..43605cc 100644
--- a/tools/modprobe.c
+++ b/tools/modprobe.c
@@ -489,11 +489,10 @@ static int insmod(struct kmod_ctx *ctx, const char *alias,
const char *options) = NULL;
err = kmod_module_new_from_lookup(ctx, alias, &list);
- if (err < 0)
- return err;
- if (list == NULL) {
- LOG("Module %s not found.\n", alias);
+ if (list == NULL || err < 0) {
+ LOG("Module %s not found in directory %s\n", alias,
+ ctx ? kmod_get_dirname(ctx) : "(missing)");
return -ENOENT;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCHv2 2/5] modprobe: Update error message when path is missing
2015-09-28 22:39 ` [PATCHv2 2/5] modprobe: Update error message when path is missing Laura Abbott
@ 2015-09-30 18:35 ` Lucas De Marchi
0 siblings, 0 replies; 7+ messages in thread
From: Lucas De Marchi @ 2015-09-30 18:35 UTC (permalink / raw)
To: Laura Abbott; +Cc: Lucas De Marchi, linux-modules
On Mon, Sep 28, 2015 at 7:39 PM, Laura Abbott <labbott@fedoraproject.org> wrote:
>
> Currently, modprobe fails with no output by default if the
> search paths it tries are missing:
>
> $ modprobe -S notakernel dm-crypt
> $
> $ modprobe -S notakernel lkjjweiojo
> $
>
> This is fairly cryptic and not at all obvious there is a problem
> unless the error code is checked or verbose flags are used.
> Update the error message to indicate a problem and print out the
> directory that failed.
> ---
> libkmod/libkmod-internal.h | 2 --
> libkmod/libkmod.h | 1 +
> tools/modprobe.c | 7 +++----
> 3 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h
> index 3e9839d..4d9db6b 100644
> --- a/libkmod/libkmod-internal.h
> +++ b/libkmod/libkmod-internal.h
> @@ -85,8 +85,6 @@ struct kmod_list *kmod_list_append_list(struct kmod_list *list1, struct kmod_lis
> container_of(list_entry->node.prev, struct kmod_list, node)))
>
> /* libkmod.c */
> -const char *kmod_get_dirname(const struct kmod_ctx *ctx) __attribute__((nonnull(1)));
> -
> int kmod_lookup_alias_from_config(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3)));
> int kmod_lookup_alias_from_symbols_file(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3)));
> int kmod_lookup_alias_from_aliases_file(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3)));
> diff --git a/libkmod/libkmod.h b/libkmod/libkmod.h
> index fe53a59..f9e33c6 100644
> --- a/libkmod/libkmod.h
> +++ b/libkmod/libkmod.h
> @@ -51,6 +51,7 @@ void kmod_set_log_priority(struct kmod_ctx *ctx, int priority);
> void *kmod_get_userdata(const struct kmod_ctx *ctx);
> void kmod_set_userdata(struct kmod_ctx *ctx, const void *userdata);
>
> +const char *kmod_get_dirname(const struct kmod_ctx *ctx);
I also added a patch to make sure this function is exported in the
library. Otherwise we limit its usage to the internal tools and it
doesn't fail since we statically link the tools with libkmod:
https://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/?id=f7f28510244e22c3676cea40f55dbed6338005f3
Lucas De Marchi
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCHv2 3/5] Change default log level
2015-09-28 22:39 [PATCHv2 1/5] README: Indicate cython requirements Laura Abbott
2015-09-28 22:39 ` [PATCHv2 2/5] modprobe: Update error message when path is missing Laura Abbott
@ 2015-09-28 22:39 ` Laura Abbott
2015-09-28 22:39 ` [PATCHv2 4/5] depmod: Remove unprinted debug messages Laura Abbott
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Laura Abbott @ 2015-09-28 22:39 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: Laura Abbott, linux-modules
The default log level is currently LOG_ERR. Tools can override this
default but there is a non-trivial amount of setup that needs to
happen before the log level can be changed. Since tools may want to
use the warn level for things such as deprecated flags, change the
default to LOG_WARNING to ensure messages get printed.
---
tools/log.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/log.c b/tools/log.c
index e29a150..3317a35 100644
--- a/tools/log.c
+++ b/tools/log.c
@@ -29,7 +29,7 @@
#define PRIO_MAX_SIZE 32
static bool log_use_syslog;
-static int log_priority = LOG_ERR;
+static int log_priority = LOG_WARNING;
static const char *prio_to_str(char buf[static PRIO_MAX_SIZE], int prio)
{
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCHv2 4/5] depmod: Remove unprinted debug messages
2015-09-28 22:39 [PATCHv2 1/5] README: Indicate cython requirements Laura Abbott
2015-09-28 22:39 ` [PATCHv2 2/5] modprobe: Update error message when path is missing Laura Abbott
2015-09-28 22:39 ` [PATCHv2 3/5] Change default log level Laura Abbott
@ 2015-09-28 22:39 ` Laura Abbott
2015-09-28 22:39 ` [PATCHv2 5/5] depmod: Don't fall back to uname on bad version Laura Abbott
2015-09-30 18:30 ` [PATCHv2 1/5] README: Indicate cython requirements Lucas De Marchi
4 siblings, 0 replies; 7+ messages in thread
From: Laura Abbott @ 2015-09-28 22:39 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: Laura Abbott, linux-modules
In between the start of the program and the call to log_setup_kmod_log,
the only messages that will be printed are the ones at or above the
global default level. Debug messages in this range will never be printed
so remove them.
---
tools/depmod.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tools/depmod.c b/tools/depmod.c
index 2a08b6e..348735f 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -2498,11 +2498,8 @@ static int do_depmod(int argc, char *argv[])
if (out == stdout)
goto done;
/* ignore up-to-date errors (< 0) */
- if (depfile_up_to_date(cfg.dirname) == 1) {
- DBG("%s/modules.dep is up to date!\n", cfg.dirname);
+ if (depfile_up_to_date(cfg.dirname) == 1)
goto done;
- }
- DBG("%s/modules.dep is outdated, do -a\n", cfg.dirname);
all = 1;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCHv2 5/5] depmod: Don't fall back to uname on bad version
2015-09-28 22:39 [PATCHv2 1/5] README: Indicate cython requirements Laura Abbott
` (2 preceding siblings ...)
2015-09-28 22:39 ` [PATCHv2 4/5] depmod: Remove unprinted debug messages Laura Abbott
@ 2015-09-28 22:39 ` Laura Abbott
2015-09-30 18:30 ` [PATCHv2 1/5] README: Indicate cython requirements Lucas De Marchi
4 siblings, 0 replies; 7+ messages in thread
From: Laura Abbott @ 2015-09-28 22:39 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: Laura Abbott, linux-modules
Currently, if a value that doesn't match a kernel version
("%u.%u") is passed in, depmod silently falls back to
using uname. Rather than try and work around the caller passing
bad data, just exit out instead.
---
tools/depmod.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/depmod.c b/tools/depmod.c
index 348735f..a585d47 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -2476,7 +2476,11 @@ static int do_depmod(int argc, char *argv[])
}
}
- if (optind < argc && is_version_number(argv[optind])) {
+ if (optind < argc) {
+ if (!is_version_number(argv[optind])) {
+ ERR("Bad version passed %s\n", argv[optind]);
+ goto cmdline_failed;
+ }
cfg.kversion = argv[optind];
optind++;
} else {
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCHv2 1/5] README: Indicate cython requirements
2015-09-28 22:39 [PATCHv2 1/5] README: Indicate cython requirements Laura Abbott
` (3 preceding siblings ...)
2015-09-28 22:39 ` [PATCHv2 5/5] depmod: Don't fall back to uname on bad version Laura Abbott
@ 2015-09-30 18:30 ` Lucas De Marchi
4 siblings, 0 replies; 7+ messages in thread
From: Lucas De Marchi @ 2015-09-30 18:30 UTC (permalink / raw)
To: Laura Abbott; +Cc: Lucas De Marchi, linux-modules
Hi Laura,
thank you for the patches, they are all applied.
Lucas De Marchi
On Mon, Sep 28, 2015 at 7:39 PM, Laura Abbott <labbott@fedoraproject.org> wrote:
>
> The recommended flags require cython be installed to compile
> successfully. Note this in the documentation.
> ---
> README | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/README b/README
> index b977e1f..4933268 100644
> --- a/README
> +++ b/README
> @@ -63,7 +63,8 @@ Hacking
> =======
>
> Run 'bootstrap' script before configure. If you want to accept the recommended
> -flags, you just need to run 'bootstrap-configure'.
> +flags, you just need to run 'bootstrap-configure'. Note that the recommended
> +flags require cython be installed to compile successfully.
>
> Make sure to read the CODING-STYLE file and the other READMEs: libkmod/README
> and testsuite/README.
> --
> 2.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-modules" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Lucas De Marchi
^ permalink raw reply [flat|nested] 7+ messages in thread