From 39540de8abe24886693ca29a9caeea85c88089aa Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 09:22:22 +0200 Subject: sysctl: do not fail systemd-sysctl.service if /proc/sys is mounted read-only Let's make missing write access to /proc/sys non-fatal to the sysctl service. This is a follow-up to 411e869f497c7c7bd0688f1e3500f9043bc56e48 which altered the condition for running the sysctl service to check for /proc/sys/net being writable, accepting that /proc/sys might be read-only. In order to ensure the boot-up stays clean in containers lower the log level for the EROFS errors generated due to this. --- src/sysctl/sysctl.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'src/sysctl/sysctl.c') diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c index fbc1e0eb1a..1363a93830 100644 --- a/src/sysctl/sysctl.c +++ b/src/sysctl/sysctl.c @@ -51,11 +51,18 @@ static int apply_all(OrderedHashmap *sysctl_options) { k = sysctl_write(property, value); if (k < 0) { - log_full_errno(k == -ENOENT ? LOG_INFO : LOG_WARNING, k, - "Couldn't write '%s' to '%s', ignoring: %m", value, property); - - if (r == 0 && k != -ENOENT) - r = k; + /* If the sysctl is not available in the kernel or we are running with reduced privileges and + * cannot write it, then log about the issue at LOG_NOTICE level, and proceed without + * failing. (EROFS is treated as a permission problem here, since that's how container managers + * usually protected their sysctls.) In all other cases log an error and make the tool fail. */ + + if (IN_SET(k, -EPERM, -EACCES, -EROFS, -ENOENT)) + log_notice_errno(k, "Couldn't write '%s' to '%s', ignoring: %m", value, property); + else { + log_error_errno(k, "Couldn't write '%s' to '%s': %m", value, property); + if (r == 0) + r = k; + } } } -- cgit v1.2.3 From 9c37b41c6166ca511b317255cfad271c906e597a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 09:25:21 +0200 Subject: sysctl: split out condition check into its own function This way, we can get rid of a label/goto. --- src/sysctl/sysctl.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) (limited to 'src/sysctl/sysctl.c') diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c index 1363a93830..a2a7a10f69 100644 --- a/src/sysctl/sysctl.c +++ b/src/sysctl/sysctl.c @@ -69,6 +69,25 @@ static int apply_all(OrderedHashmap *sysctl_options) { return r; } +static bool test_prefix(const char *p) { + char **i; + + if (strv_isempty(arg_prefixes)) + return true; + + STRV_FOREACH(i, arg_prefixes) { + const char *t; + + t = path_startswith(*i, "/proc/sys/"); + if (!t) + t = *i; + if (path_startswith(p, t)) + return true; + } + + return false; +} + static int parse_file(OrderedHashmap *sysctl_options, const char *path, bool ignore_enoent) { _cleanup_fclose_ FILE *f = NULL; int r; @@ -118,20 +137,9 @@ static int parse_file(OrderedHashmap *sysctl_options, const char *path, bool ign p = sysctl_normalize(strstrip(p)); value = strstrip(value); - if (!strv_isempty(arg_prefixes)) { - char **i, *t; - STRV_FOREACH(i, arg_prefixes) { - t = path_startswith(*i, "/proc/sys/"); - if (t == NULL) - t = *i; - if (path_startswith(p, t)) - goto found; - } - /* not found */ + if (!test_prefix(p)) continue; - } -found: existing = ordered_hashmap_get2(sysctl_options, p, &v); if (existing) { if (streq(value, existing)) -- cgit v1.2.3 From 98bf5011fe670ea18b7b35d6c8ca3e84d4efbccf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 09:26:10 +0200 Subject: sysctl: when failing to process a config line, show line nr --- src/sysctl/sysctl.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'src/sysctl/sysctl.c') diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c index a2a7a10f69..7117955568 100644 --- a/src/sysctl/sysctl.c +++ b/src/sysctl/sysctl.c @@ -90,6 +90,7 @@ static bool test_prefix(const char *p) { static int parse_file(OrderedHashmap *sysctl_options, const char *path, bool ignore_enoent) { _cleanup_fclose_ FILE *f = NULL; + unsigned c = 0; int r; assert(path); @@ -115,6 +116,8 @@ static int parse_file(OrderedHashmap *sysctl_options, const char *path, bool ign return log_error_errno(errno, "Failed to read file '%s', ignoring: %m", path); } + c++; + p = strstrip(l); if (!*p) continue; @@ -124,7 +127,7 @@ static int parse_file(OrderedHashmap *sysctl_options, const char *path, bool ign value = strchr(p, '='); if (!value) { - log_error("Line is not an assignment in file '%s': %s", path, value); + log_error("Line is not an assignment at '%s:%u': %s", path, c, value); if (r == 0) r = -EINVAL; @@ -145,7 +148,7 @@ static int parse_file(OrderedHashmap *sysctl_options, const char *path, bool ign if (streq(value, existing)) continue; - log_debug("Overwriting earlier assignment of %s in file '%s'.", p, path); + log_debug("Overwriting earlier assignment of %s at '%s:%u'.", p, path, c); free(ordered_hashmap_remove(sysctl_options, p)); free(v); } -- cgit v1.2.3 From 4f14f2bb6f7402302fd4fcad6754e5ee218f4487 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 09:26:31 +0200 Subject: sysctl: no need to check for eof twice Let's only check for eof once after the fgets(). There's no point in checking EOF before the first read, and twice in each loop. --- src/sysctl/sysctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/sysctl/sysctl.c') diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c index 7117955568..cce91b3d67 100644 --- a/src/sysctl/sysctl.c +++ b/src/sysctl/sysctl.c @@ -104,7 +104,7 @@ static int parse_file(OrderedHashmap *sysctl_options, const char *path, bool ign } log_debug("Parsing %s", path); - while (!feof(f)) { + for (;;) { char l[LINE_MAX], *p, *value, *new_value, *property, *existing; void *v; int k; -- cgit v1.2.3 From e5105081152c3eb4558d9e5d9c8aaa33f9d802ff Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 09:27:39 +0200 Subject: sysctl: minor simplification Let's place only one ternary operator. --- src/sysctl/sysctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/sysctl/sysctl.c') diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c index cce91b3d67..b3587e249d 100644 --- a/src/sysctl/sysctl.c +++ b/src/sysctl/sysctl.c @@ -247,12 +247,12 @@ static int parse_argv(int argc, char *argv[]) { } int main(int argc, char *argv[]) { + OrderedHashmap *sysctl_options = NULL; int r = 0, k; - OrderedHashmap *sysctl_options; r = parse_argv(argc, argv); if (r <= 0) - return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; + goto finish; log_set_target(LOG_TARGET_AUTO); log_parse_environment(); -- cgit v1.2.3