From f5869324e303a136d7ca802769e8966e8eb26d56 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 24 Oct 2016 21:41:54 +0200 Subject: [PATCH 01/14] core: rework the "no_gc" unit flag to become a more generic "perpetual" flag So far "no_gc" was set on -.slice and init.scope, to units that are always running, cannot be stopped and never exist in an "inactive" state. Since these units are the only users of this flag, let's remodel it and rename it "perpetual" and let's derive more funcitonality off it. Specifically, refuse enqueing stop jobs for these units, and report that they are "unstoppable" in the CanStop bus property. --- src/core/dbus-unit.c | 6 ++---- src/core/scope.c | 6 ++---- src/core/slice.c | 10 ++++------ src/core/unit.c | 27 ++++++++++++++++++++++++--- src/core/unit.h | 6 ++++-- 5 files changed, 36 insertions(+), 19 deletions(-) diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 8f34fa1a52e..69e249c844f 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -263,10 +263,7 @@ static int property_get_can_stop( assert(reply); assert(u); - /* On the lower levels we assume that every unit we can start - * we can also stop */ - - return sd_bus_message_append(reply, "b", unit_can_start(u) && !u->refuse_manual_stop); + return sd_bus_message_append(reply, "b", unit_can_stop(u) && !u->refuse_manual_stop); } static int property_get_can_reload( @@ -760,6 +757,7 @@ const sd_bus_vtable bus_unit_vtable[] = { SD_BUS_PROPERTY("Asserts", "a(sbbsi)", property_get_conditions, offsetof(Unit, asserts), 0), SD_BUS_PROPERTY("LoadError", "(ss)", property_get_load_error, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Transient", "b", bus_property_get_bool, offsetof(Unit, transient), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("Perpetual", "b", bus_property_get_bool, offsetof(Unit, perpetual), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("StartLimitIntervalSec", "t", bus_property_get_usec, offsetof(Unit, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("StartLimitBurst", "u", bus_property_get_unsigned, offsetof(Unit, start_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("StartLimitAction", "s", property_get_emergency_action, offsetof(Unit, start_limit_action), SD_BUS_VTABLE_PROPERTY_CONST), diff --git a/src/core/scope.c b/src/core/scope.c index af0c43c7da7..77f3fdc2aa2 100644 --- a/src/core/scope.c +++ b/src/core/scope.c @@ -154,15 +154,13 @@ static int scope_load_init_scope(Unit *u) { return 0; u->transient = true; - u->no_gc = true; + u->perpetual = true; /* init.scope is a bit special, as it has to stick around forever. Because of its special semantics we * synthesize it here, instead of relying on the unit file on disk. */ u->default_dependencies = false; u->ignore_on_isolate = true; - u->refuse_manual_start = true; - u->refuse_manual_stop = true; SCOPE(u)->kill_context.kill_signal = SIGRTMIN+14; @@ -580,7 +578,7 @@ static void scope_enumerate(Manager *m) { } u->transient = true; - u->no_gc = true; + u->perpetual = true; SCOPE(u)->deserialized_state = SCOPE_RUNNING; unit_add_to_load_queue(u); diff --git a/src/core/slice.c b/src/core/slice.c index 0fef29661fa..c505fa1916e 100644 --- a/src/core/slice.c +++ b/src/core/slice.c @@ -136,15 +136,13 @@ static int slice_load_root_slice(Unit *u) { if (!unit_has_name(u, SPECIAL_ROOT_SLICE)) return 0; - u->no_gc = true; + u->perpetual = true; /* The root slice is a bit special. For example it is always running and cannot be terminated. Because of its * special semantics we synthesize it here, instead of relying on the unit file on disk. */ u->default_dependencies = false; u->ignore_on_isolate = true; - u->refuse_manual_start = true; - u->refuse_manual_stop = true; if (!u->description) u->description = strdup("Root Slice"); @@ -302,7 +300,7 @@ static void slice_enumerate(Manager *m) { u = manager_get_unit(m, SPECIAL_ROOT_SLICE); if (!u) { u = unit_new(m, sizeof(Slice)); - if (!u) { + if (!u) { log_oom(); return; } @@ -310,12 +308,12 @@ static void slice_enumerate(Manager *m) { r = unit_add_name(u, SPECIAL_ROOT_SLICE); if (r < 0) { unit_free(u); - log_error_errno(r, "Failed to add the "SPECIAL_ROOT_SLICE " name: %m"); + log_error_errno(r, "Failed to add the " SPECIAL_ROOT_SLICE " name: %m"); return; } } - u->no_gc = true; + u->perpetual = true; SLICE(u)->deserialized_state = SLICE_ACTIVE; unit_add_to_load_queue(u); diff --git a/src/core/unit.c b/src/core/unit.c index cabb1050a8f..8d74b44f737 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -323,7 +323,7 @@ bool unit_check_gc(Unit *u) { if (state != UNIT_INACTIVE) return true; - if (u->no_gc) + if (u->perpetual) return true; if (u->refs) @@ -924,6 +924,7 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { "%s\tGC Check Good: %s\n" "%s\tNeed Daemon Reload: %s\n" "%s\tTransient: %s\n" + "%s\tPerpetual: %s\n" "%s\tSlice: %s\n" "%s\tCGroup: %s\n" "%s\tCGroup realized: %s\n" @@ -942,6 +943,7 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { prefix, yes_no(unit_check_gc(u)), prefix, yes_no(unit_need_daemon_reload(u)), prefix, yes_no(u->transient), + prefix, yes_no(u->perpetual), prefix, strna(unit_slice_name(u)), prefix, strna(u->cgroup_path), prefix, yes_no(u->cgroup_realized), @@ -1616,6 +1618,18 @@ int unit_stop(Unit *u) { return UNIT_VTABLE(u)->stop(u); } +bool unit_can_stop(Unit *u) { + assert(u); + + if (!unit_supported(u)) + return false; + + if (u->perpetual) + return false; + + return !!UNIT_VTABLE(u)->stop; +} + /* Errors: * -EBADR: This unit type does not support reloading. * -ENOEXEC: Unit is not started. @@ -2150,13 +2164,20 @@ bool unit_job_is_applicable(Unit *u, JobType j) { case JOB_VERIFY_ACTIVE: case JOB_START: - case JOB_STOP: case JOB_NOP: + /* Note that we don't check unit_can_start() here. That's because .device units and suchlike are not + * startable by us but may appear due to external events, and it thus makes sense to permit enqueing + * jobs for it. */ return true; + case JOB_STOP: + /* Similar as above. However, perpetual units can never be stopped (neither explicitly nor due to + * external events), hence it makes no sense to permit enqueing such a request either. */ + return !u->perpetual; + case JOB_RESTART: case JOB_TRY_RESTART: - return unit_can_start(u); + return unit_can_stop(u) && unit_can_start(u); case JOB_RELOAD: case JOB_TRY_RELOAD: diff --git a/src/core/unit.h b/src/core/unit.h index adcdee6db65..e524553ed32 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -236,6 +236,9 @@ struct Unit { /* Is this a transient unit? */ bool transient; + /* Is this a unit that is always running and cannot be stopped? */ + bool perpetual; + bool in_load_queue:1; bool in_dbus_queue:1; bool in_cleanup_queue:1; @@ -244,8 +247,6 @@ struct Unit { bool sent_dbus_new_signal:1; - bool no_gc:1; - bool in_audit:1; bool cgroup_realized:1; @@ -524,6 +525,7 @@ void unit_dump(Unit *u, FILE *f, const char *prefix); bool unit_can_reload(Unit *u) _pure_; bool unit_can_start(Unit *u) _pure_; +bool unit_can_stop(Unit *u) _pure_; bool unit_can_isolate(Unit *u) _pure_; int unit_start(Unit *u); From 11222d0fe0b5abb0cef65359b979e0c7f50129f3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 00:04:55 +0200 Subject: [PATCH 02/14] core: make the root mount perpetual too Now that have a proper concept of "perpetual" units, let's make the root mount one too, since it also cannot go away. --- src/basic/special.h | 3 ++ src/core/mount.c | 81 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 69 insertions(+), 15 deletions(-) diff --git a/src/basic/special.h b/src/basic/special.h index 084d3dfa23c..5276bcf598f 100644 --- a/src/basic/special.h +++ b/src/basic/special.h @@ -117,3 +117,6 @@ /* The scope unit systemd itself lives in. */ #define SPECIAL_INIT_SCOPE "init.scope" + +/* The root directory. */ +#define SPECIAL_ROOT_MOUNT "-.mount" diff --git a/src/core/mount.c b/src/core/mount.c index da480001e11..03e5ea1376a 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -159,17 +159,6 @@ static void mount_init(Unit *u) { m->timeout_usec = u->manager->default_timeout_start_usec; m->directory_mode = 0755; - if (unit_has_name(u, "-.mount")) { - /* Don't allow start/stop for root directory */ - u->refuse_manual_start = true; - u->refuse_manual_stop = true; - } else { - /* The stdio/kmsg bridge socket is on /, in order to avoid a - * dep loop, don't use kmsg logging for -.mount */ - m->exec_context.std_output = u->manager->default_std_output; - m->exec_context.std_error = u->manager->default_std_error; - } - /* We need to make sure that /usr/bin/mount is always called * in the same process group as us, so that the autofs kernel * side doesn't send us another mount request while we are @@ -577,6 +566,25 @@ static int mount_add_extras(Mount *m) { return 0; } +static int mount_load_root_mount(Unit *u) { + assert(u); + + if (!unit_has_name(u, SPECIAL_ROOT_MOUNT)) + return 0; + + u->perpetual = true; + u->default_dependencies = false; + + /* The stdio/kmsg bridge socket is on /, in order to avoid a dep loop, don't use kmsg logging for -.mount */ + MOUNT(u)->exec_context.std_output = EXEC_OUTPUT_NULL; + MOUNT(u)->exec_context.std_input = EXEC_INPUT_NULL; + + if (!u->description) + u->description = strdup("Root Mount"); + + return 1; +} + static int mount_load(Unit *u) { Mount *m = MOUNT(u); int r; @@ -584,11 +592,14 @@ static int mount_load(Unit *u) { assert(u); assert(u->load_state == UNIT_STUB); - if (m->from_proc_self_mountinfo) + r = mount_load_root_mount(u); + if (r < 0) + return r; + + if (m->from_proc_self_mountinfo || u->perpetual) r = unit_load_fragment_and_dropin_optional(u); else r = unit_load_fragment_and_dropin(u); - if (r < 0) return r; @@ -1592,11 +1603,51 @@ static int mount_get_timeout(Unit *u, usec_t *timeout) { return 1; } +static void synthesize_root_mount(Manager *m) { + Unit *u; + int r; + + assert(m); + + /* Whatever happens, we know for sure that the root directory is around, and cannot go away. Let's + * unconditionally synthesize it here and mark it as perpetual. */ + + u = manager_get_unit(m, SPECIAL_ROOT_MOUNT); + if (!u) { + u = unit_new(m, sizeof(Mount)); + if (!u) { + log_oom(); + return; + } + + r = unit_add_name(u, SPECIAL_ROOT_MOUNT); + if (r < 0) { + unit_free(u); + log_error_errno(r, "Failed to add the " SPECIAL_ROOT_MOUNT " name: %m"); + return; + } + } + + u->perpetual = true; + MOUNT(u)->deserialized_state = MOUNT_MOUNTED; + + unit_add_to_load_queue(u); + unit_add_to_dbus_queue(u); +} + +static bool mount_is_mounted(Mount *m) { + assert(m); + + return UNIT(m)->perpetual || m->is_mounted; +} + static void mount_enumerate(Manager *m) { int r; assert(m); + synthesize_root_mount(m); + mnt_init_debug(0); if (!m->mount_monitor) { @@ -1703,7 +1754,7 @@ static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents, LIST_FOREACH(units_by_type, u, m->units_by_type[UNIT_MOUNT]) { Mount *mount = MOUNT(u); - if (!mount->is_mounted) { + if (!mount_is_mounted(mount)) { /* A mount point is not around right now. It * might be gone, or might never have @@ -1764,7 +1815,7 @@ static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents, } } - if (mount->is_mounted && + if (mount_is_mounted(mount) && mount->from_proc_self_mountinfo && mount->parameters_proc_self_mountinfo.what) { From a581e45ae8f9bb5c6693c23c78bc070aa15d0c8a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 00:29:05 +0200 Subject: [PATCH 03/14] unit: unify some code with new unit_new_for_name() call --- src/core/device.c | 6 +----- src/core/mount.c | 17 +++-------------- src/core/scope.c | 11 ++--------- src/core/slice.c | 11 ++--------- src/core/swap.c | 6 +----- src/core/unit.c | 18 ++++++++++++++++++ src/core/unit.h | 1 + 7 files changed, 28 insertions(+), 42 deletions(-) diff --git a/src/core/device.c b/src/core/device.c index 8a3e888e5eb..bd87a447cda 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -331,11 +331,7 @@ static int device_setup_unit(Manager *m, struct udev_device *dev, const char *pa if (!u) { delete = true; - u = unit_new(m, sizeof(Device)); - if (!u) - return log_oom(); - - r = unit_add_name(u, e); + r = unit_new_for_name(m, sizeof(Device), e, &u); if (r < 0) goto fail; diff --git a/src/core/mount.c b/src/core/mount.c index 03e5ea1376a..0641621d8fe 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1404,11 +1404,7 @@ static int mount_setup_unit( if (!u) { delete = true; - u = unit_new(m, sizeof(Mount)); - if (!u) - return log_oom(); - - r = unit_add_name(u, e); + r = unit_new_for_name(m, sizeof(Mount), e, &u); if (r < 0) goto fail; @@ -1614,16 +1610,9 @@ static void synthesize_root_mount(Manager *m) { u = manager_get_unit(m, SPECIAL_ROOT_MOUNT); if (!u) { - u = unit_new(m, sizeof(Mount)); - if (!u) { - log_oom(); - return; - } - - r = unit_add_name(u, SPECIAL_ROOT_MOUNT); + r = unit_new_for_name(m, sizeof(Mount), SPECIAL_ROOT_MOUNT, &u); if (r < 0) { - unit_free(u); - log_error_errno(r, "Failed to add the " SPECIAL_ROOT_MOUNT " name: %m"); + log_error_errno(r, "Failed to allocate the special " SPECIAL_ROOT_MOUNT " unit: %m"); return; } } diff --git a/src/core/scope.c b/src/core/scope.c index 77f3fdc2aa2..d6e1f8e392c 100644 --- a/src/core/scope.c +++ b/src/core/scope.c @@ -563,16 +563,9 @@ static void scope_enumerate(Manager *m) { u = manager_get_unit(m, SPECIAL_INIT_SCOPE); if (!u) { - u = unit_new(m, sizeof(Scope)); - if (!u) { - log_oom(); - return; - } - - r = unit_add_name(u, SPECIAL_INIT_SCOPE); + r = unit_new_for_name(m, sizeof(Scope), SPECIAL_INIT_SCOPE, &u); if (r < 0) { - unit_free(u); - log_error_errno(r, "Failed to add the " SPECIAL_INIT_SCOPE " name: %m"); + log_error_errno(r, "Failed to allocate the special " SPECIAL_INIT_SCOPE " unit: %m"); return; } } diff --git a/src/core/slice.c b/src/core/slice.c index c505fa1916e..ed5d3fd701a 100644 --- a/src/core/slice.c +++ b/src/core/slice.c @@ -299,16 +299,9 @@ static void slice_enumerate(Manager *m) { u = manager_get_unit(m, SPECIAL_ROOT_SLICE); if (!u) { - u = unit_new(m, sizeof(Slice)); - if (!u) { - log_oom(); - return; - } - - r = unit_add_name(u, SPECIAL_ROOT_SLICE); + r = unit_new_for_name(m, sizeof(Slice), SPECIAL_ROOT_SLICE, &u); if (r < 0) { - unit_free(u); - log_error_errno(r, "Failed to add the " SPECIAL_ROOT_SLICE " name: %m"); + log_error_errno(r, "Failed to allocate the special " SPECIAL_ROOT_SLICE " unit: %m"); return; } } diff --git a/src/core/swap.c b/src/core/swap.c index b592abb9fbc..2228a254bbc 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -381,11 +381,7 @@ static int swap_setup_unit( if (!u) { delete = true; - u = unit_new(m, sizeof(Swap)); - if (!u) - return log_oom(); - - r = unit_add_name(u, e); + r = unit_new_for_name(m, sizeof(Swap), e, &u); if (r < 0) goto fail; diff --git a/src/core/unit.c b/src/core/unit.c index 8d74b44f737..068d319206c 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -109,6 +109,24 @@ Unit *unit_new(Manager *m, size_t size) { return u; } +int unit_new_for_name(Manager *m, size_t size, const char *name, Unit **ret) { + Unit *u; + int r; + + u = unit_new(m, size); + if (!u) + return -ENOMEM; + + r = unit_add_name(u, name); + if (r < 0) { + unit_free(u); + return r; + } + + *ret = u; + return r; +} + bool unit_has_name(Unit *u, const char *name) { assert(u); assert(name); diff --git a/src/core/unit.h b/src/core/unit.h index e524553ed32..899cd62c920 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -481,6 +481,7 @@ DEFINE_CAST(SCOPE, Scope); Unit *unit_new(Manager *m, size_t size); void unit_free(Unit *u); +int unit_new_for_name(Manager *m, size_t size, const char *name, Unit **ret); int unit_add_name(Unit *u, const char *name); int unit_add_dependency(Unit *u, UnitDependency d, Unit *other, bool add_reference); From 39540de8abe24886693ca29a9caeea85c88089aa Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 09:22:22 +0200 Subject: [PATCH 04/14] 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 | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c index fbc1e0eb1ad..1363a93830b 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 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 (r == 0 && k != -ENOENT) - r = k; + 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; + } } } From 9c37b41c6166ca511b317255cfad271c906e597a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 09:25:21 +0200 Subject: [PATCH 05/14] 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(-) diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c index 1363a93830b..a2a7a10f69f 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)) From 98bf5011fe670ea18b7b35d6c8ca3e84d4efbccf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 09:26:10 +0200 Subject: [PATCH 06/14] sysctl: when failing to process a config line, show line nr --- src/sysctl/sysctl.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c index a2a7a10f69f..71179555680 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); } From 4f14f2bb6f7402302fd4fcad6754e5ee218f4487 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 09:26:31 +0200 Subject: [PATCH 07/14] 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(-) diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c index 71179555680..cce91b3d679 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; From e5105081152c3eb4558d9e5d9c8aaa33f9d802ff Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 09:27:39 +0200 Subject: [PATCH 08/14] sysctl: minor simplification Let's place only one ternary operator. --- src/sysctl/sysctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c index cce91b3d679..b3587e249dc 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(); From 835a19e02fa96a5f13700d0ce9f4b1813d0c1ad6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 10:00:06 +0200 Subject: [PATCH 09/14] systemctl: properly turn off color after active column If we turn on red color for the active column and it is not combined with underlining, then we need to turn it off explicitly afterwards. Do that. --- src/systemctl/systemctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index d311bbec1a0..558186bd8af 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -513,13 +513,13 @@ static int output_units_list(const UnitInfo *unit_infos, unsigned c) { off_circle = ansi_normal(); circle = true; on_loaded = underline ? ansi_highlight_red_underline() : ansi_highlight_red(); - off_loaded = on_underline; + off_loaded = underline ? on_underline : ansi_normal(); } else if (streq(u->active_state, "failed") && !arg_plain) { on_circle = ansi_highlight_red(); off_circle = ansi_normal(); circle = true; on_active = underline ? ansi_highlight_red_underline() : ansi_highlight_red(); - off_active = on_underline; + off_active = underline ? on_underline : ansi_normal(); } if (u->machine) { From b5d7f1bbfa09f2713d72f76f89d2f830e382382b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 11:06:47 +0200 Subject: [PATCH 10/14] systemctl: tweak the "systemctl list-units" output a bit Make the underlining between the header and the body and between the units of different types span the whole width of the table. Let's never make the table wider than necessary (which is relevant due the above). When space is limited and we can't show the full ID or description string prefer showing the full ID over the full description. The ID is after all something people might want to copy/paste, while the description is mostly just helpful decoration. --- src/systemctl/systemctl.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 558186bd8af..fb5a539bc61 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -410,23 +410,24 @@ static bool output_show_unit(const UnitInfo *u, char **patterns) { } static int output_units_list(const UnitInfo *unit_infos, unsigned c) { - unsigned circle_len = 0, id_len, max_id_len, load_len, active_len, sub_len, job_len; + unsigned circle_len = 0, id_len, max_id_len, load_len, active_len, sub_len, job_len, desc_len, max_desc_len; const UnitInfo *u; unsigned n_shown = 0; - int job_count = 0, desc_len; + int job_count = 0; max_id_len = strlen("UNIT"); load_len = strlen("LOAD"); active_len = strlen("ACTIVE"); sub_len = strlen("SUB"); job_len = strlen("JOB"); - desc_len = 0; + max_desc_len = strlen("DESCRIPTION"); for (u = unit_infos; u < unit_infos + c; u++) { max_id_len = MAX(max_id_len, strlen(u->id) + (u->machine ? strlen(u->machine)+1 : 0)); load_len = MAX(load_len, strlen(u->load_state)); active_len = MAX(active_len, strlen(u->active_state)); sub_len = MAX(sub_len, strlen(u->sub_state)); + max_desc_len = MAX(max_desc_len, strlen(u->description)); if (u->job_id != 0) { job_len = MAX(job_len, strlen(u->job_type)); @@ -442,7 +443,7 @@ static int output_units_list(const UnitInfo *unit_infos, unsigned c) { if (!arg_full && original_stdout_is_tty) { unsigned basic_len; - id_len = MIN(max_id_len, 25u); + id_len = MIN(max_id_len, 25u); /* as much as it needs, but at most 25 for now */ basic_len = circle_len + 5 + id_len + 5 + active_len + sub_len; if (job_count) @@ -455,19 +456,21 @@ static int output_units_list(const UnitInfo *unit_infos, unsigned c) { /* Either UNIT already got 25, or is fully satisfied. * Grant up to 25 to DESC now. */ incr = MIN(extra_len, 25u); - desc_len += incr; + desc_len = incr; extra_len -= incr; - /* split the remaining space between UNIT and DESC, - * but do not give UNIT more than it needs. */ + /* Of the remainder give as much as the ID needs to the ID, and give the rest to the + * description but not more than it needs. */ if (extra_len > 0) { - incr = MIN(extra_len / 2, max_id_len - id_len); + incr = MIN(max_id_len - id_len, extra_len); id_len += incr; - desc_len += extra_len - incr; + desc_len += MIN(extra_len - incr, max_desc_len - desc_len); } } - } else + } else { id_len = max_id_len; + desc_len = max_desc_len; + } for (u = unit_infos; u < unit_infos + c; u++) { _cleanup_free_ char *e = NULL, *j = NULL; @@ -493,8 +496,9 @@ static int output_units_list(const UnitInfo *unit_infos, unsigned c) { if (job_count) printf("%-*s ", job_len, "JOB"); - printf("%.*s%s\n", - !arg_full && arg_no_pager ? desc_len : -1, + printf("%-*.*s%s\n", + desc_len, + !arg_full && arg_no_pager ? (int) desc_len : -1, "DESCRIPTION", ansi_normal()); } @@ -550,8 +554,9 @@ static int output_units_list(const UnitInfo *unit_infos, unsigned c) { sub_len, u->sub_state, off_active, job_count ? job_len + 1 : 0, u->job_id ? u->job_type : ""); - printf("%.*s%s\n", - desc_len > 0 ? desc_len : -1, + printf("%-*.*s%s\n", + desc_len, + !arg_full && arg_no_pager ? (int) desc_len : -1, u->description, off_underline); } From 2bce2acce869686d4417fa679eb794b5dc873110 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 11:39:09 +0200 Subject: [PATCH 11/14] nspawn: if we set up a loopback device, try to mount it with "discard" Let's make sure that our loopback files remain sparse, hence let's set "discard" as mount option on file systems that support it if the backing device is a loopback. --- src/nspawn/nspawn.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 295293858e7..c56af6e6f45 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2260,7 +2260,7 @@ static int dissect_image( static int mount_device(const char *what, const char *where, const char *directory, bool rw) { #ifdef HAVE_BLKID _cleanup_blkid_free_probe_ blkid_probe b = NULL; - const char *fstype, *p; + const char *fstype, *p, *options; int r; assert(what); @@ -2309,7 +2309,17 @@ static int mount_device(const char *what, const char *where, const char *directo return -EOPNOTSUPP; } - return mount_verbose(LOG_ERR, what, p, fstype, MS_NODEV|(rw ? 0 : MS_RDONLY), NULL); + /* If this is a loopback device then let's mount the image with discard, so that the underlying file remains + * sparse when possible. */ + if (STR_IN_SET(fstype, "btrfs", "ext4", "vfat", "xfs")) { + const char *l; + + l = path_startswith(what, "/dev"); + if (l && startswith(l, "loop")) + options = "discard"; + } + + return mount_verbose(LOG_ERR, what, p, fstype, MS_NODEV|(rw ? 0 : MS_RDONLY), options); #else log_error("--image= is not supported, compiled without blkid support."); return -EOPNOTSUPP; From 02affb4e6a16fbec9d6d28265c220ca95479bbd0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 12:08:24 +0200 Subject: [PATCH 12/14] netword: minor memory leak fix --- src/network/networkd-ndisc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index b282634e4bc..6ce6bfa787d 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -680,13 +680,13 @@ void ndisc_vacuum(Link *link) { SET_FOREACH(r, link->ndisc_rdnss, i) if (r->valid_until < time_now) { - (void) set_remove(link->ndisc_rdnss, r); + free(set_remove(link->ndisc_rdnss, r)); link_dirty(link); } SET_FOREACH(d, link->ndisc_dnssl, i) if (d->valid_until < time_now) { - (void) set_remove(link->ndisc_dnssl, d); + free(set_remove(link->ndisc_dnssl, d)); link_dirty(link); } } From c69305ff4fe5a182cd58b66077f3db7bc7c222e1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Oct 2016 12:08:43 +0200 Subject: [PATCH 13/14] networkd: flush DNSSL/RDNSS lists when we lose carrier Fixes: #3870 --- src/network/networkd-link.c | 7 ++++--- src/network/networkd-ndisc.c | 9 +++++++++ src/network/networkd-ndisc.h | 1 + 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index d9e060b6cf7..aefe7335b98 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -514,13 +514,12 @@ static void link_free(Link *link) { sd_lldp_unref(link->lldp); free(link->lldp_file); + ndisc_flush(link); + sd_ipv4ll_unref(link->ipv4ll); sd_dhcp6_client_unref(link->dhcp6_client); sd_ndisc_unref(link->ndisc); - set_free_free(link->ndisc_rdnss); - set_free_free(link->ndisc_dnssl); - if (link->manager) hashmap_remove(link->manager->links, INT_TO_PTR(link->ifindex)); @@ -2427,6 +2426,8 @@ static int link_drop_config(Link *link) { return r; } + ndisc_flush(link); + return 0; } diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 6ce6bfa787d..4853791aa56 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -690,3 +690,12 @@ void ndisc_vacuum(Link *link) { link_dirty(link); } } + +void ndisc_flush(Link *link) { + assert(link); + + /* Removes all RDNSS and DNSSL entries, without exception */ + + link->ndisc_rdnss = set_free_free(link->ndisc_rdnss); + link->ndisc_dnssl = set_free_free(link->ndisc_dnssl); +} diff --git a/src/network/networkd-ndisc.h b/src/network/networkd-ndisc.h index 2002f551070..127126190ed 100644 --- a/src/network/networkd-ndisc.h +++ b/src/network/networkd-ndisc.h @@ -37,3 +37,4 @@ static inline char* NDISC_DNSSL_DOMAIN(const NDiscDNSSL *n) { int ndisc_configure(Link *link); void ndisc_vacuum(Link *link); +void ndisc_flush(Link *link); From 1201cae704c6674fde2c23fdd7feab8493a20159 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 2 Nov 2016 11:38:12 -0600 Subject: [PATCH 14/14] core: change mount_synthesize_root() return to int Let's propagate the error here, instead of eating it up early. In a later change we should probably also change mount_enumerate() to propagate errors up, but that would mean we'd have to change the unit vtable, and thus change all unit types, hence is quite an invasive change. --- src/core/mount.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 0641621d8fe..d749e49df5b 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1599,7 +1599,7 @@ static int mount_get_timeout(Unit *u, usec_t *timeout) { return 1; } -static void synthesize_root_mount(Manager *m) { +static int synthesize_root_mount(Manager *m) { Unit *u; int r; @@ -1611,10 +1611,8 @@ static void synthesize_root_mount(Manager *m) { u = manager_get_unit(m, SPECIAL_ROOT_MOUNT); if (!u) { r = unit_new_for_name(m, sizeof(Mount), SPECIAL_ROOT_MOUNT, &u); - if (r < 0) { - log_error_errno(r, "Failed to allocate the special " SPECIAL_ROOT_MOUNT " unit: %m"); - return; - } + if (r < 0) + return log_error_errno(r, "Failed to allocate the special " SPECIAL_ROOT_MOUNT " unit: %m"); } u->perpetual = true; @@ -1622,6 +1620,8 @@ static void synthesize_root_mount(Manager *m) { unit_add_to_load_queue(u); unit_add_to_dbus_queue(u); + + return 0; } static bool mount_is_mounted(Mount *m) { @@ -1635,7 +1635,9 @@ static void mount_enumerate(Manager *m) { assert(m); - synthesize_root_mount(m); + r = synthesize_root_mount(m); + if (r < 0) + goto fail; mnt_init_debug(0);