From 232ac0d6810498dc7a3a6b9d1fd4dca983e1535f Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 8 Oct 2017 15:55:24 +0200 Subject: util-lib: introdude _cleanup_ macros for kmod objects --- src/basic/meson.build | 1 + src/basic/module-util.h | 28 ++++++++++++++++++++++++++++ src/core/kmod-setup.c | 10 +++------- src/modules-load/modules-load.c | 13 +++++-------- src/test/test-netlink-manual.c | 21 +++++++++------------ src/udev/udev-builtin-kmod.c | 10 +++++----- 6 files changed, 51 insertions(+), 32 deletions(-) create mode 100644 src/basic/module-util.h diff --git a/src/basic/meson.build b/src/basic/meson.build index 1ddefb7fbb..eda7b1159b 100644 --- a/src/basic/meson.build +++ b/src/basic/meson.build @@ -113,6 +113,7 @@ basic_sources_plain = files(''' mkdir-label.c mkdir.c mkdir.h + module-util.h mount-util.c mount-util.h nss-util.h diff --git a/src/basic/module-util.h b/src/basic/module-util.h new file mode 100644 index 0000000000..20bb2f3bab --- /dev/null +++ b/src/basic/module-util.h @@ -0,0 +1,28 @@ +#pragma once + +/*** + This file is part of systemd. + + Copyright 2017 Zbigniew Jędrzejewski-Szmek + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + +#include + +#include "macro.h" + +DEFINE_TRIVIAL_CLEANUP_FUNC(struct kmod_ctx*, kmod_unref); +DEFINE_TRIVIAL_CLEANUP_FUNC(struct kmod_module*, kmod_module_unref); +DEFINE_TRIVIAL_CLEANUP_FUNC(struct kmod_list*, kmod_module_unref_list); diff --git a/src/core/kmod-setup.c b/src/core/kmod-setup.c index 066b959770..10d3ced93d 100644 --- a/src/core/kmod-setup.c +++ b/src/core/kmod-setup.c @@ -31,6 +31,7 @@ #include "fileio.h" #include "kmod-setup.h" #include "macro.h" +#include "module-util.h" #include "string-util.h" #if HAVE_KMOD @@ -110,7 +111,7 @@ int kmod_setup(void) { /* virtio_rng would be loaded by udev later, but real entropy might be needed very early */ { "virtio_rng", NULL, false, false, has_virtio_rng }, }; - struct kmod_ctx *ctx = NULL; + _cleanup_(kmod_unrefp) struct kmod_ctx *ctx = NULL; unsigned int i; int r; @@ -118,7 +119,7 @@ int kmod_setup(void) { return 0; for (i = 0; i < ELEMENTSOF(kmod_table); i++) { - struct kmod_module *mod; + _cleanup_(kmod_module_unrefp) struct kmod_module *mod = NULL; if (kmod_table[i].path && access(kmod_table[i].path, F_OK) >= 0) continue; @@ -157,13 +158,8 @@ int kmod_setup(void) { log_full_errno(print_warning ? LOG_WARNING : LOG_DEBUG, r, "Failed to insert module '%s': %m", kmod_module_get_name(mod)); } - - kmod_module_unref(mod); } - if (ctx) - kmod_unref(ctx); - #endif return 0; } diff --git a/src/modules-load/modules-load.c b/src/modules-load/modules-load.c index 6efebcd94f..1cb9001c39 100644 --- a/src/modules-load/modules-load.c +++ b/src/modules-load/modules-load.c @@ -29,6 +29,7 @@ #include "fd-util.h" #include "fileio.h" #include "log.h" +#include "module-util.h" #include "proc-cmdline.h" #include "string-util.h" #include "strv.h" @@ -77,7 +78,8 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat static int load_module(struct kmod_ctx *ctx, const char *m) { const int probe_flags = KMOD_PROBE_APPLY_BLACKLIST; - struct kmod_list *itr, *modlist = NULL; + struct kmod_list *itr; + _cleanup_(kmod_module_unref_listp) struct kmod_list *modlist = NULL; int r = 0; log_debug("load: %s", m); @@ -92,7 +94,7 @@ static int load_module(struct kmod_ctx *ctx, const char *m) { } kmod_list_foreach(itr, modlist) { - struct kmod_module *mod; + _cleanup_(kmod_module_unrefp) struct kmod_module *mod = NULL; int state, err; mod = kmod_module_get_module(itr); @@ -120,12 +122,8 @@ static int load_module(struct kmod_ctx *ctx, const char *m) { r = err; } } - - kmod_module_unref(mod); } - kmod_module_unref_list(modlist); - return r; } @@ -218,7 +216,7 @@ static int parse_argv(int argc, char *argv[]) { int main(int argc, char *argv[]) { int r, k; - struct kmod_ctx *ctx; + _cleanup_(kmod_unrefp) struct kmod_ctx *ctx = NULL; r = parse_argv(argc, argv); if (r <= 0) @@ -280,7 +278,6 @@ int main(int argc, char *argv[]) { } finish: - kmod_unref(ctx); strv_free(arg_proc_cmdline_modules); return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; diff --git a/src/test/test-netlink-manual.c b/src/test/test-netlink-manual.c index bc6dd0926c..f0471cf04e 100644 --- a/src/test/test-netlink-manual.c +++ b/src/test/test-netlink-manual.c @@ -26,38 +26,35 @@ #include "sd-netlink.h" #include "macro.h" +#include "module-util.h" #include "util.h" static int load_module(const char *mod_name) { - struct kmod_ctx *ctx; - struct kmod_list *list = NULL, *l; + _cleanup_(kmod_unrefp) struct kmod_ctx *ctx = NULL; + _cleanup_(kmod_module_unref_listp) struct kmod_list *list = NULL; + struct kmod_list *l; int r; ctx = kmod_new(NULL, NULL); - if (!ctx) { - kmod_unref(ctx); - return -ENOMEM; - } + if (!ctx) + return log_oom(); r = kmod_module_new_from_lookup(ctx, mod_name, &list); if (r < 0) return -1; kmod_list_foreach(l, list) { - struct kmod_module *mod = kmod_module_get_module(l); + _cleanup_(kmod_module_unrefp) struct kmod_module *mod = NULL; + + mod = kmod_module_get_module(l); r = kmod_module_probe_insert_module(mod, 0, NULL, NULL, NULL, NULL); if (r >= 0) r = 0; else r = -1; - - kmod_module_unref(mod); } - kmod_module_unref_list(list); - kmod_unref(ctx); - return r; } diff --git a/src/udev/udev-builtin-kmod.c b/src/udev/udev-builtin-kmod.c index 9665f678fd..e0e0fa4b46 100644 --- a/src/udev/udev-builtin-kmod.c +++ b/src/udev/udev-builtin-kmod.c @@ -24,13 +24,14 @@ #include #include +#include "module-util.h" #include "string-util.h" #include "udev.h" static struct kmod_ctx *ctx = NULL; static int load_module(struct udev *udev, const char *alias) { - struct kmod_list *list = NULL; + _cleanup_(kmod_module_unref_listp) struct kmod_list *list = NULL; struct kmod_list *l; int err; @@ -42,7 +43,9 @@ static int load_module(struct udev *udev, const char *alias) { log_debug("No module matches '%s'", alias); kmod_list_foreach(l, list) { - struct kmod_module *mod = kmod_module_get_module(l); + _cleanup_(kmod_module_unrefp) struct kmod_module *mod = NULL; + + mod = kmod_module_get_module(l); err = kmod_module_probe_insert_module(mod, KMOD_PROBE_APPLY_BLACKLIST, NULL, NULL, NULL, NULL); if (err == KMOD_PROBE_APPLY_BLACKLIST) @@ -51,11 +54,8 @@ static int load_module(struct udev *udev, const char *alias) { log_debug("Inserted '%s'", kmod_module_get_name(mod)); else log_debug("Failed to insert '%s'", kmod_module_get_name(mod)); - - kmod_module_unref(mod); } - kmod_module_unref_list(list); return err; } -- cgit v1.2.3 From 2c3f0bb207f89f0347a4a84a8cde57e979f33896 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 8 Oct 2017 16:18:57 +0200 Subject: kmod_module_probe_insert_module returns 0 on success, != 0 on failure More specifically, it should return > 0 only for conditions specified in probe_flags. We only set KMOD_PROBE_APPLY_BLACKLIST in probe_flags, so the code was correct, but add an assert to clarify this. --- src/modules-load/modules-load.c | 5 +++-- src/test/test-netlink-manual.c | 8 +++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/modules-load/modules-load.c b/src/modules-load/modules-load.c index 1cb9001c39..c1a89cf822 100644 --- a/src/modules-load/modules-load.c +++ b/src/modules-load/modules-load.c @@ -118,8 +118,9 @@ static int load_module(struct kmod_ctx *ctx, const char *m) { else if (err == KMOD_PROBE_APPLY_BLACKLIST) log_info("Module '%s' is blacklisted", kmod_module_get_name(mod)); else { - log_error_errno(err, "Failed to insert '%s': %m", kmod_module_get_name(mod)); - r = err; + assert(err < 0); + r = log_error_errno(err, "Failed to insert '%s': %m", + kmod_module_get_name(mod)); } } } diff --git a/src/test/test-netlink-manual.c b/src/test/test-netlink-manual.c index f0471cf04e..ce1c1c0b19 100644 --- a/src/test/test-netlink-manual.c +++ b/src/test/test-netlink-manual.c @@ -41,7 +41,7 @@ static int load_module(const char *mod_name) { r = kmod_module_new_from_lookup(ctx, mod_name, &list); if (r < 0) - return -1; + return r; kmod_list_foreach(l, list) { _cleanup_(kmod_module_unrefp) struct kmod_module *mod = NULL; @@ -49,10 +49,8 @@ static int load_module(const char *mod_name) { mod = kmod_module_get_module(l); r = kmod_module_probe_insert_module(mod, 0, NULL, NULL, NULL, NULL); - if (r >= 0) - r = 0; - else - r = -1; + if (r > 0) + r = -EINVAL; } return r; -- cgit v1.2.3 From 6cbb0af16e41f8ed665b07f75738c24eea4de8a5 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 8 Oct 2017 16:21:06 +0200 Subject: modules-load: downgrade error on ENODEV/ENOENT Some kernel modules may be loaded if the hardware does not exist (usually when the hardware is hot-pluggable), while others fail with ENODEV. Let's make those two cases more similar, and simply log modules which cannot be loaded because of missing hardware without failing systemd-modules-load.service. For modules which don't exist, let's warn, but not fail the whole service. I think a warning is appropriate because it's likely that a typo was made. --- src/modules-load/modules-load.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/modules-load/modules-load.c b/src/modules-load/modules-load.c index c1a89cf822..aaf2927113 100644 --- a/src/modules-load/modules-load.c +++ b/src/modules-load/modules-load.c @@ -119,8 +119,15 @@ static int load_module(struct kmod_ctx *ctx, const char *m) { log_info("Module '%s' is blacklisted", kmod_module_get_name(mod)); else { assert(err < 0); - r = log_error_errno(err, "Failed to insert '%s': %m", - kmod_module_get_name(mod)); + + log_full_errno(err == ENODEV ? LOG_NOTICE : + err == ENOENT ? LOG_WARNING : + LOG_ERR, + err, + "Failed to insert '%s': %m", + kmod_module_get_name(mod)); + if (!IN_SET(err, ENODEV, ENOENT)) + r = err; } } } -- cgit v1.2.3