From acc0269cad31d1aaef2034a055b34c07c88a353d Mon Sep 17 00:00:00 2001 From: Christian Hesse Date: Thu, 26 May 2016 15:57:37 +0200 Subject: {machine,system}ctl: always pass &changes and &n_changes (#3350) We have to pass addresses of changes and n_changes to bus_deserialize_and_dump_unit_file_changes(). Otherwise we are hit by missing information (subsequent calls to unit_file_changes_add() to not add anything). Also prevent null pointer dereference in bus_deserialize_and_dump_unit_file_changes() by asserting. Fixes #3339 --- src/systemctl/systemctl.c | 64 +++++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 27 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index b943c68e1b..0500593d06 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2058,6 +2058,8 @@ static int get_default(int argc, char *argv[], void *userdata) { static int set_default(int argc, char *argv[], void *userdata) { _cleanup_free_ char *unit = NULL; + UnitFileChange *changes = NULL; + unsigned n_changes = 0; int r; assert(argc >= 2); @@ -2068,13 +2070,8 @@ static int set_default(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to mangle unit name: %m"); if (install_client_side()) { - UnitFileChange *changes = NULL; - unsigned n_changes = 0; - r = unit_file_set_default(arg_scope, arg_root, unit, true, &changes, &n_changes); unit_file_dump_changes(r, "set default", changes, n_changes, arg_quiet); - unit_file_changes_free(changes, n_changes); - return r; } else { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; @@ -2098,9 +2095,9 @@ static int set_default(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to set default target: %s", bus_error_message(&error, r)); - r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet, NULL, NULL); + r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet, &changes, &n_changes); if (r < 0) - return r; + goto finish; /* Try to reload if enabled */ if (!arg_no_reload) @@ -2109,6 +2106,9 @@ static int set_default(int argc, char *argv[], void *userdata) { r = 0; } +finish: + unit_file_changes_free(changes, n_changes); + return r; } @@ -5650,6 +5650,8 @@ static int add_dependency(int argc, char *argv[], void *userdata) { _cleanup_strv_free_ char **names = NULL; _cleanup_free_ char *target = NULL; const char *verb = argv[0]; + UnitFileChange *changes = NULL; + unsigned n_changes = 0; UnitDependency dep; int r = 0; @@ -5672,13 +5674,8 @@ static int add_dependency(int argc, char *argv[], void *userdata) { assert_not_reached("Unknown verb"); if (install_client_side()) { - UnitFileChange *changes = NULL; - unsigned n_changes = 0; - r = unit_file_add_dependency(arg_scope, arg_runtime, arg_root, names, target, dep, arg_force, &changes, &n_changes); unit_file_dump_changes(r, "add dependency on", changes, n_changes, arg_quiet); - unit_file_changes_free(changes, n_changes); - return r; } else { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL, *m = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; @@ -5712,27 +5709,32 @@ static int add_dependency(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to add dependency: %s", bus_error_message(&error, r)); - r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet, NULL, NULL); + r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet, &changes, &n_changes); if (r < 0) - return r; + goto finish; - if (arg_no_reload) - return 0; - return daemon_reload(argc, argv, userdata); + if (arg_no_reload) { + r = 0; + goto finish; + } + + r = daemon_reload(argc, argv, userdata); } + +finish: + unit_file_changes_free(changes, n_changes); + + return r; } static int preset_all(int argc, char *argv[], void *userdata) { + UnitFileChange *changes = NULL; + unsigned n_changes = 0; int r; if (install_client_side()) { - UnitFileChange *changes = NULL; - unsigned n_changes = 0; - r = unit_file_preset_all(arg_scope, arg_runtime, arg_root, arg_preset_mode, arg_force, &changes, &n_changes); unit_file_dump_changes(r, "preset", changes, n_changes, arg_quiet); - unit_file_changes_free(changes, n_changes); - return r; } else { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; @@ -5759,14 +5761,22 @@ static int preset_all(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to preset all units: %s", bus_error_message(&error, r)); - r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet, NULL, NULL); + r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet, &changes, &n_changes); if (r < 0) - return r; + goto finish; - if (arg_no_reload) - return 0; - return daemon_reload(argc, argv, userdata); + if (arg_no_reload) { + r = 0; + goto finish; + } + + r = daemon_reload(argc, argv, userdata); } + +finish: + unit_file_changes_free(changes, n_changes); + + return r; } static int unit_is_enabled(int argc, char *argv[], void *userdata) { -- cgit v1.2.3-54-g00ecf From 7f30799adee958603ca52ed1ad6a4e298e7ca652 Mon Sep 17 00:00:00 2001 From: Christian Hesse Date: Fri, 27 May 2016 09:21:02 +0200 Subject: systemctl: remove extra comma --- src/systemctl/systemctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 0500593d06..94c590130a 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -7466,7 +7466,7 @@ static int systemctl_main(int argc, char *argv[]) { { "switch-root", 2, VERB_ANY, VERB_NOCHROOT, switch_root }, { "list-dependencies", VERB_ANY, 2, VERB_NOCHROOT, list_dependencies }, { "set-default", 2, 2, 0, set_default }, - { "get-default", VERB_ANY, 1, 0, get_default, }, + { "get-default", VERB_ANY, 1, 0, get_default }, { "set-property", 3, VERB_ANY, VERB_NOCHROOT, set_property }, { "is-system-running", VERB_ANY, 1, 0, is_system_running }, { "add-wants", 3, VERB_ANY, 0, add_dependency }, -- cgit v1.2.3-54-g00ecf From 5f056378b0ceffb6e6fba3513f7eae72e2d09dc8 Mon Sep 17 00:00:00 2001 From: Christian Hesse Date: Fri, 27 May 2016 09:32:41 +0200 Subject: systemctl: fix return values on success --- src/systemctl/systemctl.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 94c590130a..f4cdfa956e 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1439,6 +1439,8 @@ static int list_unit_files(int argc, char *argv[], void *userdata) { assert(c <= n_units); hashmap_free(h); + + r = 0; } else { _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; @@ -2025,6 +2027,7 @@ static int get_default(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to get default target: %m"); path = _path; + r = 0; } else { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; sd_bus *bus; @@ -2072,6 +2075,9 @@ static int set_default(int argc, char *argv[], void *userdata) { if (install_client_side()) { r = unit_file_set_default(arg_scope, arg_root, unit, true, &changes, &n_changes); unit_file_dump_changes(r, "set default", changes, n_changes, arg_quiet); + + if (r > 0) + r = 0; } else { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; @@ -5676,6 +5682,9 @@ static int add_dependency(int argc, char *argv[], void *userdata) { if (install_client_side()) { r = unit_file_add_dependency(arg_scope, arg_runtime, arg_root, names, target, dep, arg_force, &changes, &n_changes); unit_file_dump_changes(r, "add dependency on", changes, n_changes, arg_quiet); + + if (r > 0) + r = 0; } else { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL, *m = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; @@ -5735,6 +5744,9 @@ static int preset_all(int argc, char *argv[], void *userdata) { if (install_client_side()) { r = unit_file_preset_all(arg_scope, arg_runtime, arg_root, arg_preset_mode, arg_force, &changes, &n_changes); unit_file_dump_changes(r, "preset", changes, n_changes, arg_quiet); + + if (r > 0) + r = 0; } else { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; @@ -5817,6 +5829,7 @@ static int unit_is_enabled(int argc, char *argv[], void *userdata) { puts(unit_file_state_to_string(state)); } + r = 0; } else { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; sd_bus *bus; -- cgit v1.2.3-54-g00ecf From 85b78539c994984a6cbcc1095f126a1302db7808 Mon Sep 17 00:00:00 2001 From: Christian Hesse Date: Fri, 27 May 2016 09:33:27 +0200 Subject: systemctl: fix code path (and memory leak) on error --- src/systemctl/systemctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index f4cdfa956e..86feefcb65 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -5514,7 +5514,7 @@ static int enable_unit(int argc, char *argv[], void *userdata) { unit_file_dump_changes(r, verb, changes, n_changes, arg_quiet); if (r < 0) - return r; + goto finish; r = 0; } else { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL, *m = NULL; @@ -5606,7 +5606,7 @@ static int enable_unit(int argc, char *argv[], void *userdata) { r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet, &changes, &n_changes); if (r < 0) - return r; + goto finish; /* Try to reload if enabled */ if (!arg_no_reload) -- cgit v1.2.3-54-g00ecf From da4d897e75e574911cb73ac91fdeef7d4fce8fbe Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Fri, 27 May 2016 09:10:18 -0700 Subject: core: add cgroup memory controller support on the unified hierarchy (#3315) On the unified hierarchy, memory controller implements three control knobs - low, high and max which enables more useable and versatile control over memory usage. This patch implements support for the three control knobs. * MemoryLow, MemoryHigh and MemoryMax are added for memory.low, memory.high and memory.max, respectively. * As all absolute limits on the unified hierarchy use "max" for no limit, make memory limit parse functions accept "max" in addition to "infinity" and document "max" for the new knobs. * Implement compatibility translation between MemoryMax and MemoryLimit. v2: - Fixed missing else's in config_parse_memory_limit(). - Fixed missing newline when writing out drop-ins. - Coding style updates to use "val > 0" instead of "val". - Minor updates to documentation. --- man/systemd.resource-control.xml | 71 +++++++++++++++++++++++++++++++++++ src/core/cgroup.c | 63 +++++++++++++++++++++++-------- src/core/cgroup.h | 4 ++ src/core/dbus-cgroup.c | 28 ++++++++++++++ src/core/load-fragment-gperf.gperf.m4 | 3 ++ src/core/load-fragment.c | 25 +++++++----- src/shared/bus-unit-util.c | 6 +-- src/systemctl/systemctl.c | 39 +++++++++++++++++-- 8 files changed, 206 insertions(+), 33 deletions(-) (limited to 'src/systemctl') diff --git a/man/systemd.resource-control.xml b/man/systemd.resource-control.xml index 066f2cc19b..570619a743 100644 --- a/man/systemd.resource-control.xml +++ b/man/systemd.resource-control.xml @@ -114,6 +114,13 @@ prefixed ones. On unified hierarchy, IO resource control also applies to buffered writes. + + + + MemoryMax replaces MemoryLimit. MemoryLow + and MemoryHigh are effective only on unified hierarchy. + + @@ -212,6 +219,67 @@ + + MemoryLow=bytes + + + Specify the best-effort memory usage protection of the executed processes in this unit. If the memory + usages of this unit and all its ancestors are below their low boundaries, this unit's memory won't be + reclaimed as long as memory can be reclaimed from unprotected units. + + Takes a memory size in bytes. If the value is suffixed with K, M, G or T, the specified memory size is + parsed as Kilobytes, Megabytes, Gigabytes, or Terabytes (with the base 1024), respectively. This controls the + memory.low control group attribute. For details about this control group attribute, see + cgroup-v2.txt. + + Implies MemoryAccounting=true. + + This setting is supported only if the unified control group hierarchy is used. + + + + + MemoryHigh=bytes + + + Specify the high limit on memory usage of the executed processes in this unit. Memory usage may go + above the limit if unavoidable, but the processes are heavily slowed down and memory is taken away + aggressively in such cases. This is the main mechanism to control memory usage of a unit. + + Takes a memory size in bytes. If the value is suffixed with K, M, G or T, the specified memory size is + parsed as Kilobytes, Megabytes, Gigabytes, or Terabytes (with the base 1024), respectively. If assigned the + special value max, no memory limit is applied. This controls the + memory.high control group attribute. For details about this control group attribute, see + cgroup-v2.txt. + + Implies MemoryAccounting=true. + + This setting is supported only if the unified control group hierarchy is used. + + + + + MemoryMax=bytes + + + Specify the absolute limit on memory usage of the executed processes in this unit. If memory usage + cannot be contained under the limit, out-of-memory killer is invoked inside the unit. It is recommended to + use MemoryHigh= as the main control mechanism and use MemoryMax= as the + last line of defense. + + Takes a memory size in bytes. If the value is suffixed with K, M, G or T, the specified memory size is + parsed as Kilobytes, Megabytes, Gigabytes, or Terabytes (with the base 1024), respectively. If assigned the + special value max, no memory limit is applied. This controls the + memory.max control group attribute. For details about this control group attribute, see + cgroup-v2.txt. + + Implies MemoryAccounting=true. + + This setting is supported only if the unified control group hierarchy is used. Use + MemoryLimit= on systems using the legacy control group hierarchy. + + + MemoryLimit=bytes @@ -230,6 +298,9 @@ url="https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt">memory.txt. Implies MemoryAccounting=true. + + This setting is supported only if the legacy control group hierarchy is used. Use + MemoryMax= on systems using the unified control group hierarchy. diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 0fb63b1bd1..fbe69df4e9 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -46,7 +46,10 @@ void cgroup_context_init(CGroupContext *c) { c->startup_cpu_shares = CGROUP_CPU_SHARES_INVALID; c->cpu_quota_per_sec_usec = USEC_INFINITY; - c->memory_limit = (uint64_t) -1; + c->memory_high = CGROUP_LIMIT_MAX; + c->memory_max = CGROUP_LIMIT_MAX; + + c->memory_limit = CGROUP_LIMIT_MAX; c->io_weight = CGROUP_WEIGHT_INVALID; c->startup_io_weight = CGROUP_WEIGHT_INVALID; @@ -147,6 +150,9 @@ void cgroup_context_dump(CGroupContext *c, FILE* f, const char *prefix) { "%sStartupIOWeight=%" PRIu64 "\n" "%sBlockIOWeight=%" PRIu64 "\n" "%sStartupBlockIOWeight=%" PRIu64 "\n" + "%sMemoryLow=%" PRIu64 "\n" + "%sMemoryHigh=%" PRIu64 "\n" + "%sMemoryMax=%" PRIu64 "\n" "%sMemoryLimit=%" PRIu64 "\n" "%sTasksMax=%" PRIu64 "\n" "%sDevicePolicy=%s\n" @@ -163,6 +169,9 @@ void cgroup_context_dump(CGroupContext *c, FILE* f, const char *prefix) { prefix, c->startup_io_weight, prefix, c->blockio_weight, prefix, c->startup_blockio_weight, + prefix, c->memory_low, + prefix, c->memory_high, + prefix, c->memory_max, prefix, c->memory_limit, prefix, c->tasks_max, prefix, cgroup_device_policy_to_string(c->device_policy), @@ -496,6 +505,23 @@ static unsigned cgroup_apply_blkio_device_limit(const char *path, const char *de return n; } +static bool cgroup_context_has_unified_memory_config(CGroupContext *c) { + return c->memory_low > 0 || c->memory_high != CGROUP_LIMIT_MAX || c->memory_max != CGROUP_LIMIT_MAX; +} + +static void cgroup_apply_unified_memory_limit(const char *path, const char *file, uint64_t v) { + char buf[DECIMAL_STR_MAX(uint64_t) + 1] = "max"; + int r; + + if (v != CGROUP_LIMIT_MAX) + xsprintf(buf, "%" PRIu64 "\n", v); + + r = cg_set_attribute("memory", path, file, buf); + if (r < 0) + log_full_errno(IN_SET(r, -ENOENT, -EROFS, -EACCES) ? LOG_DEBUG : LOG_WARNING, r, + "Failed to set %s on %s: %m", file, path); +} + void cgroup_context_apply(CGroupContext *c, CGroupMask mask, const char *path, ManagerState state) { bool is_root; int r; @@ -662,26 +688,30 @@ void cgroup_context_apply(CGroupContext *c, CGroupMask mask, const char *path, M } if ((mask & CGROUP_MASK_MEMORY) && !is_root) { - if (c->memory_limit != (uint64_t) -1) { - char buf[DECIMAL_STR_MAX(uint64_t) + 1]; - - sprintf(buf, "%" PRIu64 "\n", c->memory_limit); + if (cg_unified() > 0) { + uint64_t max = c->memory_max; - if (cg_unified() <= 0) - r = cg_set_attribute("memory", path, "memory.limit_in_bytes", buf); + if (cgroup_context_has_unified_memory_config(c)) + max = c->memory_max; else - r = cg_set_attribute("memory", path, "memory.max", buf); + max = c->memory_limit; + cgroup_apply_unified_memory_limit(path, "memory.low", c->memory_low); + cgroup_apply_unified_memory_limit(path, "memory.high", c->memory_high); + cgroup_apply_unified_memory_limit(path, "memory.max", max); } else { - if (cg_unified() <= 0) - r = cg_set_attribute("memory", path, "memory.limit_in_bytes", "-1"); + char buf[DECIMAL_STR_MAX(uint64_t) + 1]; + + if (c->memory_limit != CGROUP_LIMIT_MAX) + xsprintf(buf, "%" PRIu64 "\n", c->memory_limit); else - r = cg_set_attribute("memory", path, "memory.max", "max"); - } + xsprintf(buf, "%" PRIu64 "\n", c->memory_max); - if (r < 0) - log_full_errno(IN_SET(r, -ENOENT, -EROFS, -EACCES) ? LOG_DEBUG : LOG_WARNING, r, - "Failed to set memory.limit_in_bytes/memory.max on %s: %m", path); + r = cg_set_attribute("memory", path, "memory.limit_in_bytes", buf); + if (r < 0) + log_full_errno(IN_SET(r, -ENOENT, -EROFS, -EACCES) ? LOG_DEBUG : LOG_WARNING, r, + "Failed to set memory.limit_in_bytes on %s: %m", path); + } } if ((mask & CGROUP_MASK_DEVICES) && !is_root) { @@ -778,7 +808,8 @@ CGroupMask cgroup_context_get_mask(CGroupContext *c) { mask |= CGROUP_MASK_IO | CGROUP_MASK_BLKIO; if (c->memory_accounting || - c->memory_limit != (uint64_t) -1) + c->memory_limit != CGROUP_LIMIT_MAX || + cgroup_context_has_unified_memory_config(c)) mask |= CGROUP_MASK_MEMORY; if (c->device_allow || diff --git a/src/core/cgroup.h b/src/core/cgroup.h index 2b1edbafc4..ff87adfba1 100644 --- a/src/core/cgroup.h +++ b/src/core/cgroup.h @@ -94,6 +94,10 @@ struct CGroupContext { LIST_HEAD(CGroupIODeviceWeight, io_device_weights); LIST_HEAD(CGroupIODeviceLimit, io_device_limits); + uint64_t memory_low; + uint64_t memory_high; + uint64_t memory_max; + /* For legacy hierarchies */ uint64_t cpu_shares; uint64_t startup_cpu_shares; diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index d6053581f8..27050b4507 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -228,6 +228,9 @@ const sd_bus_vtable bus_cgroup_vtable[] = { SD_BUS_PROPERTY("BlockIOReadBandwidth", "a(st)", property_get_blockio_device_bandwidths, 0, 0), SD_BUS_PROPERTY("BlockIOWriteBandwidth", "a(st)", property_get_blockio_device_bandwidths, 0, 0), SD_BUS_PROPERTY("MemoryAccounting", "b", bus_property_get_bool, offsetof(CGroupContext, memory_accounting), 0), + SD_BUS_PROPERTY("MemoryLow", "t", NULL, offsetof(CGroupContext, memory_low), 0), + SD_BUS_PROPERTY("MemoryHigh", "t", NULL, offsetof(CGroupContext, memory_high), 0), + SD_BUS_PROPERTY("MemoryMax", "t", NULL, offsetof(CGroupContext, memory_max), 0), SD_BUS_PROPERTY("MemoryLimit", "t", NULL, offsetof(CGroupContext, memory_limit), 0), SD_BUS_PROPERTY("DevicePolicy", "s", property_get_cgroup_device_policy, offsetof(CGroupContext, device_policy), 0), SD_BUS_PROPERTY("DeviceAllow", "a(ss)", property_get_device_allow, 0, 0), @@ -826,6 +829,31 @@ int bus_cgroup_set_property( return 1; + } else if (STR_IN_SET(name, "MemoryLow", "MemoryHigh", "MemoryMax")) { + uint64_t v; + + r = sd_bus_message_read(message, "t", &v); + if (r < 0) + return r; + + if (mode != UNIT_CHECK) { + if (streq(name, "MemoryLow")) + c->memory_low = v; + else if (streq(name, "MemoryHigh")) + c->memory_high = v; + else + c->memory_max = v; + + unit_invalidate_cgroup(u, CGROUP_MASK_MEMORY); + + if (v == CGROUP_LIMIT_MAX) + unit_write_drop_in_private_format(u, mode, name, "%s=max\n", name); + else + unit_write_drop_in_private_format(u, mode, name, "%s=%" PRIu64 "\n", name, v); + } + + return 1; + } else if (streq(name, "MemoryLimit")) { uint64_t limit; diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index 8193418980..00bdc238ce 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -117,6 +117,9 @@ $1.CPUShares, config_parse_cpu_shares, 0, $1.StartupCPUShares, config_parse_cpu_shares, 0, offsetof($1, cgroup_context.startup_cpu_shares) $1.CPUQuota, config_parse_cpu_quota, 0, offsetof($1, cgroup_context) $1.MemoryAccounting, config_parse_bool, 0, offsetof($1, cgroup_context.memory_accounting) +$1.MemoryLow, config_parse_memory_limit, 0, offsetof($1, cgroup_context) +$1.MemoryHigh, config_parse_memory_limit, 0, offsetof($1, cgroup_context) +$1.MemoryMax, config_parse_memory_limit, 0, offsetof($1, cgroup_context) $1.MemoryLimit, config_parse_memory_limit, 0, offsetof($1, cgroup_context) $1.DeviceAllow, config_parse_device_allow, 0, offsetof($1, cgroup_context) $1.DevicePolicy, config_parse_device_policy, 0, offsetof($1, cgroup_context.device_policy) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 86b4fb071b..09d3f65c77 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -2793,21 +2793,26 @@ int config_parse_memory_limit( void *userdata) { CGroupContext *c = data; - uint64_t bytes; + uint64_t bytes = CGROUP_LIMIT_MAX; int r; - if (isempty(rvalue) || streq(rvalue, "infinity")) { - c->memory_limit = (uint64_t) -1; - return 0; + if (!isempty(rvalue) && !streq(rvalue, "infinity") && !streq(rvalue, "max")) { + r = parse_size(rvalue, 1024, &bytes); + if (r < 0 || bytes < 1) { + log_syntax(unit, LOG_ERR, filename, line, r, "Memory limit '%s' invalid. Ignoring.", rvalue); + return 0; + } } - r = parse_size(rvalue, 1024, &bytes); - if (r < 0 || bytes < 1) { - log_syntax(unit, LOG_ERR, filename, line, r, "Memory limit '%s' invalid. Ignoring.", rvalue); - return 0; - } + if (streq(lvalue, "MemoryLow")) + c->memory_low = bytes; + else if (streq(lvalue, "MemoryHigh")) + c->memory_high = bytes; + else if (streq(lvalue, "MemoryMax")) + c->memory_max = bytes; + else + c->memory_limit = bytes; - c->memory_limit = bytes; return 0; } diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index f68c4a41ac..502e98d9dc 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -166,11 +166,11 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen r = sd_bus_message_append(m, "v", "b", r); - } else if (streq(field, "MemoryLimit")) { + } else if (STR_IN_SET(field, "MemoryLow", "MemoryHigh", "MemoryMax", "MemoryLimit")) { uint64_t bytes; - if (isempty(eq) || streq(eq, "infinity")) - bytes = (uint64_t) -1; + if (isempty(eq) || streq(eq, "max") || streq(eq, "infinity")) + bytes = CGROUP_LIMIT_MAX; else { r = parse_size(eq, 1024, &bytes); if (r < 0) { diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 0500593d06..b2ee00fab3 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3493,6 +3493,9 @@ typedef struct UnitStatusInfo { /* CGroup */ uint64_t memory_current; + uint64_t memory_low; + uint64_t memory_high; + uint64_t memory_max; uint64_t memory_limit; uint64_t cpu_usage_nsec; uint64_t tasks_current; @@ -3775,10 +3778,30 @@ static void print_status_info( printf(" Memory: %s", format_bytes(buf, sizeof(buf), i->memory_current)); - if (i->memory_limit != (uint64_t) -1) - printf(" (limit: %s)\n", format_bytes(buf, sizeof(buf), i->memory_limit)); - else - printf("\n"); + if (i->memory_low > 0 || i->memory_high != CGROUP_LIMIT_MAX || i->memory_max != CGROUP_LIMIT_MAX || + i->memory_limit != CGROUP_LIMIT_MAX) { + const char *prefix = ""; + + printf(" ("); + if (i->memory_low > 0) { + printf("%slow: %s", prefix, format_bytes(buf, sizeof(buf), i->memory_low)); + prefix = " "; + } + if (i->memory_high != CGROUP_LIMIT_MAX) { + printf("%shigh: %s", prefix, format_bytes(buf, sizeof(buf), i->memory_high)); + prefix = " "; + } + if (i->memory_max != CGROUP_LIMIT_MAX) { + printf("%smax: %s", prefix, format_bytes(buf, sizeof(buf), i->memory_max)); + prefix = " "; + } + if (i->memory_limit != CGROUP_LIMIT_MAX) { + printf("%slimit: %s", prefix, format_bytes(buf, sizeof(buf), i->memory_limit)); + prefix = " "; + } + printf(")"); + } + printf("\n"); } if (i->cpu_usage_nsec != (uint64_t) -1) { @@ -4007,6 +4030,12 @@ static int status_property(const char *name, sd_bus_message *m, UnitStatusInfo * i->assert_timestamp = (usec_t) u; else if (streq(name, "MemoryCurrent")) i->memory_current = u; + else if (streq(name, "MemoryLow")) + i->memory_low = u; + else if (streq(name, "MemoryHigh")) + i->memory_high = u; + else if (streq(name, "MemoryMax")) + i->memory_max = u; else if (streq(name, "MemoryLimit")) i->memory_limit = u; else if (streq(name, "TasksCurrent")) @@ -4500,6 +4529,8 @@ static int show_one( _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; UnitStatusInfo info = { .memory_current = (uint64_t) -1, + .memory_high = CGROUP_LIMIT_MAX, + .memory_max = CGROUP_LIMIT_MAX, .memory_limit = (uint64_t) -1, .cpu_usage_nsec = (uint64_t) -1, .tasks_current = (uint64_t) -1, -- cgit v1.2.3-54-g00ecf From be8386a3e51183738a61cf46dea698ef9d499bae Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sat, 28 May 2016 10:50:36 -0400 Subject: systemctl: remove empty line between comment and action It's harder to miss the comment without the newline ;) See https://github.com/systemd/systemd/pull/3336#issuecomment-221749423 for context. --- src/systemctl/systemctl.c | 1 - 1 file changed, 1 deletion(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index c301c6a64f..9c8bfffc9b 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -7841,6 +7841,5 @@ finish: release_busses(); /* Note that we return r here, not EXIT_SUCCESS, so that we can implement the LSB-like return codes */ - return r < 0 ? EXIT_FAILURE : r; } -- cgit v1.2.3-54-g00ecf From ca473d572f0d2d8f547ff787ae67afd489a3f15e Mon Sep 17 00:00:00 2001 From: Susant Sahani Date: Mon, 30 May 2016 20:23:15 +0530 Subject: systemctl: return diffrent error code if service exist or not (#3385) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before: [sus@maximus bz-1256858]$ systemctl status rsyslog.service;echo $? ● rsyslog.service - System Logging Service Loaded: loaded (/usr/lib/systemd/system/rsyslog.service; enabled; vendor preset: enabled) Drop-In: /etc/systemd/system/rsyslog.service.d └─50-CPUShares.conf Active: inactive (dead) since Mon 2016-05-30 11:54:25 IST; 2h 26min ago Docs: man:rsyslogd(8) http://www.rsyslog.com/doc/ Process: 1159 ExecStart=/usr/sbin/rsyslogd -n $SYSLOGD_OPTIONS (code=exited, status=0/SUCCESS) Main PID: 1159 (code=exited, status=0/SUCCESS) May 30 11:07:50 maximus systemd[1]: Starting System Logging Service... May 30 11:07:50 maximus systemd[1]: Started System Logging Service. May 30 11:54:25 maximus systemd[1]: Stopping System Logging Service... May 30 11:54:25 maximus systemd[1]: Stopped System Logging Service. 3 [sus@maximus bz-1256858]$ systemctl status hello.service;echo $? ● hello.service Loaded: not-found (Reason: No such file or directory) Active: inactive (dead) 3 After: $ ./systemctl status hello.service;echo $? Failed to dump process list, ignoring: Access denied ● hello.service Loaded: not-found (Reason: No such file or directory) Active: inactive (dead) 4 [sus@maximus bz-1256858]$ ./systemctl status rsyslog.service;echo $? Failed to dump process list, ignoring: Access denied ● rsyslog.service - System Logging Service Loaded: loaded (/usr/lib/systemd/system/rsyslog.service; enabled; vendor preset: enabled) Drop-In: /etc/systemd/system/rsyslog.service.d └─50-CPUShares.conf Active: inactive (dead) since Mon 2016-05-30 11:54:25 IST; 2h 24min ago Docs: man:rsyslogd(8) http://www.rsyslog.com/doc/ Process: 1159 ExecStart=/usr/sbin/rsyslogd -n $SYSLOGD_OPTIONS (code=exited, status=0/SUCCESS) Main PID: 1159 (code=exited, status=0/SUCCESS) May 30 11:07:50 maximus systemd[1]: Starting System Logging Service... May 30 11:07:50 maximus systemd[1]: Started System Logging Service. May 30 11:54:25 maximus systemd[1]: Stopping System Logging Service... May 30 11:54:25 maximus systemd[1]: Stopped System Logging Service. 3 Fixes: 1092 --- src/systemctl/systemctl.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 9c8bfffc9b..58255ae453 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -4633,6 +4633,8 @@ static int show_one( */ if (info.pid_file && access(info.pid_file, F_OK) == 0) r = 1; + else if (streq_ptr(info.load_state, "not-found") && streq_ptr(info.active_state, "inactive")) + r = 4; else r = 3; } -- cgit v1.2.3-54-g00ecf From 3c6f7c340237262b560586cf7cf06be957d4352f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 30 May 2016 17:59:43 +0200 Subject: util-lib: make localed's nonempty() generic, rename it to empty_to_null() and make use of it everywhere --- src/basic/string-util.h | 4 ++++ src/hostname/hostnamed.c | 14 ++++++-------- src/locale/localed.c | 29 +++++++---------------------- src/machine/machine-dbus.c | 3 +-- src/resolve/resolve-tool.c | 12 ++++-------- src/systemctl/systemctl.c | 4 +--- src/sysv-generator/sysv-generator.c | 12 +++--------- 7 files changed, 26 insertions(+), 52 deletions(-) (limited to 'src/systemctl') diff --git a/src/basic/string-util.h b/src/basic/string-util.h index 139cc8c91b..1209e1e2e1 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -66,6 +66,10 @@ static inline bool isempty(const char *p) { return !p || !p[0]; } +static inline const char *empty_to_null(const char *p) { + return isempty(p) ? NULL : p; +} + static inline char *startswith(const char *s, const char *prefix) { size_t l; diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index d11756e615..fe8bb62752 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -479,8 +479,7 @@ static int method_set_static_hostname(sd_bus_message *m, void *userdata, sd_bus_ if (r < 0) return r; - if (isempty(name)) - name = NULL; + name = empty_to_null(name); if (streq_ptr(name, c->data[PROP_STATIC_HOSTNAME])) return sd_bus_reply_method_return(m, NULL); @@ -499,9 +498,9 @@ static int method_set_static_hostname(sd_bus_message *m, void *userdata, sd_bus_ if (r == 0) return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ - if (isempty(name)) { + if (isempty(name)) c->data[PROP_STATIC_HOSTNAME] = mfree(c->data[PROP_STATIC_HOSTNAME]); - } else { + else { char *h; if (!hostname_is_valid(name, false)) @@ -546,8 +545,7 @@ static int set_machine_info(Context *c, sd_bus_message *m, int prop, sd_bus_mess if (r < 0) return r; - if (isempty(name)) - name = NULL; + name = empty_to_null(name); if (streq_ptr(name, c->data[prop])) return sd_bus_reply_method_return(m, NULL); @@ -570,9 +568,9 @@ static int set_machine_info(Context *c, sd_bus_message *m, int prop, sd_bus_mess if (r == 0) return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ - if (isempty(name)) { + if (isempty(name)) c->data[prop] = mfree(c->data[prop]); - } else { + else { char *h; /* The icon name might ultimately be used as file diff --git a/src/locale/localed.c b/src/locale/localed.c index 3b22a582ac..6af59fc830 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -97,10 +97,6 @@ typedef struct Context { Hashmap *polkit_registry; } Context; -static const char* nonempty(const char *s) { - return isempty(s) ? NULL : s; -} - static bool startswith_comma(const char *s, const char *prefix) { const char *t; @@ -171,8 +167,7 @@ static int locale_read_data(Context *c) { for (p = 0; p < _LOCALE_MAX; p++) { assert(names[p]); - r = free_and_strdup(&c->locale[p], - nonempty(getenv(names[p]))); + r = free_and_strdup(&c->locale[p], empty_to_null(getenv(names[p]))); if (r < 0) return r; } @@ -1041,11 +1036,8 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro if (r < 0) return r; - if (isempty(keymap)) - keymap = NULL; - - if (isempty(keymap_toggle)) - keymap_toggle = NULL; + keymap = empty_to_null(keymap); + keymap_toggle = empty_to_null(keymap_toggle); if (!streq_ptr(keymap, c->vc_keymap) || !streq_ptr(keymap_toggle, c->vc_keymap_toggle)) { @@ -1214,17 +1206,10 @@ static int method_set_x11_keyboard(sd_bus_message *m, void *userdata, sd_bus_err if (r < 0) return r; - if (isempty(layout)) - layout = NULL; - - if (isempty(model)) - model = NULL; - - if (isempty(variant)) - variant = NULL; - - if (isempty(options)) - options = NULL; + layout = empty_to_null(layout); + model = empty_to_null(model); + variant = empty_to_null(variant); + options = empty_to_null(options); if (!streq_ptr(layout, c->x11_layout) || !streq_ptr(model, c->x11_model) || diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c index 7b9aa66d63..de5d98f23e 100644 --- a/src/machine/machine-dbus.c +++ b/src/machine/machine-dbus.c @@ -655,8 +655,7 @@ int bus_machine_method_open_shell(sd_bus_message *message, void *userdata, sd_bu r = sd_bus_message_read(message, "ss", &user, &path); if (r < 0) return r; - if (isempty(user)) - user = NULL; + user = empty_to_null(user); if (isempty(path)) path = "/bin/sh"; if (!path_is_absolute(path)) diff --git a/src/resolve/resolve-tool.c b/src/resolve/resolve-tool.c index 14ee01c49d..7e145c64c4 100644 --- a/src/resolve/resolve-tool.c +++ b/src/resolve/resolve-tool.c @@ -658,10 +658,8 @@ static int resolve_service(sd_bus *bus, const char *name, const char *type, cons assert(bus); assert(domain); - if (isempty(name)) - name = NULL; - if (isempty(type)) - type = NULL; + name = empty_to_null(name); + type = empty_to_null(type); if (arg_ifindex > 0 && !if_indextoname(arg_ifindex, ifname)) return log_error_errno(errno, "Failed to resolve interface name for index %i: %m", arg_ifindex); @@ -820,10 +818,8 @@ static int resolve_service(sd_bus *bus, const char *name, const char *type, cons if (r < 0) return bus_log_parse_error(r); - if (isempty(canonical_name)) - canonical_name = NULL; - if (isempty(canonical_type)) - canonical_type = NULL; + canonical_name = empty_to_null(canonical_name); + canonical_type = empty_to_null(canonical_type); if (!streq_ptr(name, canonical_name) || !streq_ptr(type, canonical_type) || diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 58255ae453..e0fde8aa73 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -5177,9 +5177,7 @@ static int switch_root(int argc, char *argv[], void *userdata) { init = cmdline_init; } - if (isempty(init)) - init = NULL; - + init = empty_to_null(init); if (init) { const char *root_systemd_path = NULL, *root_init_path = NULL; diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c index fe4bbeeb75..6a90ca562b 100644 --- a/src/sysv-generator/sysv-generator.c +++ b/src/sysv-generator/sysv-generator.c @@ -527,9 +527,7 @@ static int load_sysv(SysvStub *s) { t[k-1] = 0; } - j = strstrip(t+12); - if (isempty(j)) - j = NULL; + j = empty_to_null(strstrip(t+12)); r = free_and_strdup(&chkconfig_description, j); if (r < 0) @@ -605,9 +603,7 @@ static int load_sysv(SysvStub *s) { state = LSB_DESCRIPTION; - j = strstrip(t+12); - if (isempty(j)) - j = NULL; + j = empty_to_null(strstrip(t+12)); r = free_and_strdup(&long_description, j); if (r < 0) @@ -618,9 +614,7 @@ static int load_sysv(SysvStub *s) { state = LSB; - j = strstrip(t+18); - if (isempty(j)) - j = NULL; + j = empty_to_null(strstrip(t+18)); r = free_and_strdup(&short_description, j); if (r < 0) -- cgit v1.2.3-54-g00ecf From b613907ea988e2994c68be686b5e92cdc7d3fb68 Mon Sep 17 00:00:00 2001 From: Susant Sahani Date: Tue, 31 May 2016 19:06:58 +0530 Subject: systemctl: Replace init script error codes with enum (#3400) Now we just using constants for the init script exit status codes. Replace those error codes with enum so that it's more meaningful and readable. --- src/systemctl/systemctl.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 58255ae453..e4b4e07ee7 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -85,6 +85,25 @@ #include "verbs.h" #include "virt.h" +/* The init script exit status codes + 0 program is running or service is OK + 1 program is dead and /var/run pid file exists + 2 program is dead and /var/lock lock file exists + 3 program is not running + 4 program or service status is unknown + 5-99 reserved for future LSB use + 100-149 reserved for distribution use + 150-199 reserved for application use + 200-254 reserved +*/ +enum { + EXIT_PROGRAM_RUNNING_OR_SERVICE_OK = 0, + EXIT_PROGRAM_DEAD_AND_PID_EXISTS = 1, + EXIT_PROGRAM_DEAD_AND_LOCK_FILE_EXISTS = 2, + EXIT_PROGRAM_NOT_RUNNING = 3, + EXIT_PROGRAM_OR_SERVICES_STATUS_UNKNOWN = 4, +}; + static char **arg_types = NULL; static char **arg_states = NULL; static char **arg_properties = NULL; @@ -3291,12 +3310,12 @@ static int check_unit_generic(int code, const UnitActiveState good_states[], int static int check_unit_active(int argc, char *argv[], void *userdata) { const UnitActiveState states[] = { UNIT_ACTIVE, UNIT_RELOADING }; /* According to LSB: 3, "program is not running" */ - return check_unit_generic(3, states, ELEMENTSOF(states), strv_skip(argv, 1)); + return check_unit_generic(EXIT_PROGRAM_NOT_RUNNING, states, ELEMENTSOF(states), strv_skip(argv, 1)); } static int check_unit_failed(int argc, char *argv[], void *userdata) { const UnitActiveState states[] = { UNIT_FAILED }; - return check_unit_generic(1, states, ELEMENTSOF(states), strv_skip(argv, 1)); + return check_unit_generic(EXIT_PROGRAM_DEAD_AND_PID_EXISTS, states, ELEMENTSOF(states), strv_skip(argv, 1)); } static int kill_unit(int argc, char *argv[], void *userdata) { @@ -4632,11 +4651,11 @@ static int show_one( * 4: program or service status is unknown */ if (info.pid_file && access(info.pid_file, F_OK) == 0) - r = 1; + r = EXIT_PROGRAM_DEAD_AND_PID_EXISTS; else if (streq_ptr(info.load_state, "not-found") && streq_ptr(info.active_state, "inactive")) - r = 4; + r = EXIT_PROGRAM_OR_SERVICES_STATUS_UNKNOWN; else - r = 3; + r = EXIT_PROGRAM_NOT_RUNNING; } while ((p = info.exec)) { -- cgit v1.2.3-54-g00ecf From e33a06a1eb7406ece8e35f6346ba0ea208c11cf1 Mon Sep 17 00:00:00 2001 From: Susant Sahani Date: Tue, 31 May 2016 21:50:25 +0530 Subject: systemctl: systemctl show --property' needs verification of property (#3364) systemctl --property doesn't validate if a requested property is valid or not, and always returns with an exit code of 0, regardless of whether the requested property exists or not. How reproducible: This works fine: Id=multi-user.target But put in a non-existing property: Id=default.targets.service Id=default.targets.service 0 [root@shou18lkvm8 ~]# systemctl show --property Id this.is.rubbish; echo $? Id=this.is.rubbish.service 0 After: sus@maximus bz-95593]$ ./systemctl show --property Id this.is.rubbish; echo $? Can't display property this.is.rubbish. Unit this.is.rubbish.service does not exist. 4 fixes #2295 --- src/systemctl/systemctl.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index e4b4e07ee7..e8f487e9f4 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -4561,6 +4561,14 @@ static int show_one( .tasks_current = (uint64_t) -1, .tasks_max = (uint64_t) -1, }; + struct property_info { + const char *load_state, *active_state; + } property_info = {}; + static const struct bus_properties_map property_map[] = { + { "LoadState", "s", NULL, offsetof(struct property_info, load_state) }, + { "ActiveState", "s", NULL, offsetof(struct property_info, active_state) }, + {} + }; ExecStatusInfo *p; int r; @@ -4581,6 +4589,17 @@ static int show_one( if (r < 0) return log_error_errno(r, "Failed to get properties: %s", bus_error_message(&error, r)); + r = bus_message_map_all_properties(reply, property_map, &property_info); + if (r < 0) + return log_error_errno(r, "Failed to map properties: %s", bus_error_message(&error, r)); + + if (streq_ptr(property_info.load_state, "not-found") && streq_ptr(property_info.active_state, "inactive")) + return EXIT_PROGRAM_OR_SERVICES_STATUS_UNKNOWN; + + r = sd_bus_message_rewind(reply, true); + if (r < 0) + return log_error_errno(r, "Failed to rewind: %s", bus_error_message(&error, r)); + r = sd_bus_message_enter_container(reply, SD_BUS_TYPE_ARRAY, "{sv}"); if (r < 0) return bus_log_parse_error(r); @@ -4889,6 +4908,9 @@ static int show(int argc, char *argv[], void *userdata) { return r; else if (r > 0 && ret == 0) ret = r; + + if (r == EXIT_PROGRAM_OR_SERVICES_STATUS_UNKNOWN) + log_error("Can't display property %s. Unit %s does not exist.", *patterns, *name); } } } -- cgit v1.2.3-54-g00ecf From 592705f2f708b313ef3e07677463fe676cdd774a Mon Sep 17 00:00:00 2001 From: Evgeny Vereshchagin Date: Sun, 5 Jun 2016 02:24:20 +0300 Subject: systemctl: install sigbus handler (#3435) This makes systemctl robust regarding journal truncation. This is a follow-up for 2cf4172a71860c6e44 Fixes: Core was generated by `./systemctl status systemd-journald'. Program terminated with signal SIGBUS, Bus error. PID 8569 - core TID 8569: #0 0x00007f246cc89ed6 __memcmp_sse4_1 #1 0x0000557ebbc6f42c journal_file_init_header #2 0x0000557ebbc77262 journal_file_open #3 0x0000557ebbc42999 file_type_wanted #4 0x0000557ebbc42e08 add_any_file #5 0x0000557ebbc43832 add_directory #6 0x0000557ebbc4401c add_root_directory #7 0x0000557ebbc442e9 add_root_directory #8 0x0000557ebbc446fc add_search_paths #9 0x0000557ebbbacb5e show_journal_by_unit #10 0x0000557ebbb8376d print_status_info #11 0x0000557ebbb86a0b show_one #12 0x0000557ebbb87954 show #13 0x0000557ebbc20b1f dispatch_verb #14 0x0000557ebbb90615 systemctl_main #15 0x0000557ebbb9159f main #16 0x00007f246cb3e731 __libc_start_main #17 0x0000557ebbb75ae9 _start --- src/systemctl/systemctl.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index e6ff299dd4..df41182529 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -70,6 +70,7 @@ #include "process-util.h" #include "rlimit-util.h" #include "set.h" +#include "sigbus.h" #include "signal-util.h" #include "socket-util.h" #include "spawn-ask-password-agent.h" @@ -7807,6 +7808,7 @@ int main(int argc, char*argv[]) { setlocale(LC_ALL, ""); log_parse_environment(); log_open(); + sigbus_install(); /* Explicitly not on_tty() to avoid setting cached value. * This becomes relevant for piping output which might be -- cgit v1.2.3-54-g00ecf From 033f0ab85d9cc5e8ae66e07d4740482abb5617ef Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Jun 2016 21:45:12 +0200 Subject: systemctl: don't suppress error code when handling legacy commands For legacy commands such as /sbin/halt or /sbin/poweroff we support legacy fallbacks that talk via traditional SysV way with PID 1 to issue the desired operation. We do this on any kind of error if the primary method of operation fails. When this is the case we suppress any error message that is normally generated, in order to not confuse the user. When suppressing this log message, don't suppress the original error code, because there's really no reason to. --- src/systemctl/systemctl.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index df41182529..9ff460d158 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2693,10 +2693,9 @@ static int start_unit_one( if (r < 0) { const char *verb; - if (r == -ENOENT && arg_action != ACTION_SYSTEMCTL) - /* There's always a fallback possible for - * legacy actions. */ - return -EADDRNOTAVAIL; + /* There's always a fallback possible for legacy actions. */ + if (arg_action != ACTION_SYSTEMCTL) + return r; verb = method_to_verb(method); -- cgit v1.2.3-54-g00ecf From 2853b60ab553d0692d699897f38d2c175721ba54 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Jun 2016 21:48:08 +0200 Subject: systemctl: prolong timeout of "systemctl daemon-reload" Reloading or reexecuting PID 1 means the unit generators are rerun, which are timed out at 90s. Make sure the method call asking for the reload is timed out at twice that, so that the generators have 90s and the reload operation has 90s too. This reworks the daemon_reload() call in systemctl, and makes it exclusively about reloading/reexecing. Previously it was used for other trivial method calls too, which didn't really help readability. As the code paths are now sufficiently different, split out the old code into a new function trivial_method(). This call also does a similar change as c8ad4efb277c3235d58789170af11bb3c847d655 but for the reload/reexec operation. Fixes: #3353 --- src/systemctl/systemctl.c | 110 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 78 insertions(+), 32 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 9ff460d158..914dba36dc 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -174,6 +174,7 @@ static bool arg_firmware_setup = false; static bool arg_now = false; static int daemon_reload(int argc, char *argv[], void* userdata); +static int trivial_method(int argc, char *argv[], void *userdata); static int halt_now(enum action a); static int get_state_one_unit(sd_bus *bus, const char *name, UnitActiveState *active_state); @@ -2283,7 +2284,7 @@ static int cancel_job(int argc, char *argv[], void *userdata) { int r = 0; if (argc <= 1) - return daemon_reload(argc, argv, userdata); + return trivial_method(argc, argv, userdata); polkit_agent_open_if_enabled(); @@ -3251,7 +3252,7 @@ static int start_special(int argc, char *argv[], void *userdata) { ACTION_REBOOT, ACTION_KEXEC, ACTION_EXIT)) - return daemon_reload(argc, argv, userdata); + return trivial_method(argc, argv, userdata); /* First try logind, to allow authentication with polkit */ if (IN_SET(a, @@ -5052,6 +5053,7 @@ static int set_property(int argc, char *argv[], void *userdata) { static int daemon_reload(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; const char *method; sd_bus *bus; int r; @@ -5062,26 +5064,76 @@ static int daemon_reload(int argc, char *argv[], void *userdata) { if (r < 0) return r; - if (arg_action == ACTION_RELOAD) + switch (arg_action) { + + case ACTION_RELOAD: method = "Reload"; - else if (arg_action == ACTION_REEXEC) + break; + + case ACTION_REEXEC: method = "Reexecute"; - else { - assert(arg_action == ACTION_SYSTEMCTL); + break; + + case ACTION_SYSTEMCTL: + method = streq(argv[0], "daemon-reexec") ? "Reexecute" : + /* "daemon-reload" */ "Reload"; + break; - method = - streq(argv[0], "clear-jobs") || - streq(argv[0], "cancel") ? "ClearJobs" : - streq(argv[0], "daemon-reexec") ? "Reexecute" : - streq(argv[0], "reset-failed") ? "ResetFailed" : - streq(argv[0], "halt") ? "Halt" : - streq(argv[0], "poweroff") ? "PowerOff" : - streq(argv[0], "reboot") ? "Reboot" : - streq(argv[0], "kexec") ? "KExec" : - streq(argv[0], "exit") ? "Exit" : - /* "daemon-reload" */ "Reload"; + default: + assert_not_reached("Unexpected action"); } + r = sd_bus_message_new_method_call( + bus, + &m, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + method); + if (r < 0) + return bus_log_create_error(r); + + /* Note we use an extra-long timeout here. This is because a reload or reexec means generators are rerun which + * are timed out after DEFAULT_TIMEOUT_USEC. Let's use twice that time here, so that the generators can have + * their timeout, and for everything else there's the same time budget in place. */ + + r = sd_bus_call(bus, m, DEFAULT_TIMEOUT_USEC * 2, &error, NULL); + + /* On reexecution, we expect a disconnect, not a reply */ + if (IN_SET(r, -ETIMEDOUT, -ECONNRESET) && streq(method, "Reexecute")) + r = 0; + + if (r < 0 && arg_action == ACTION_SYSTEMCTL) + return log_error_errno(r, "Failed to reload daemon: %s", bus_error_message(&error, r)); + + /* Note that for the legacy commands (i.e. those with action != ACTION_SYSTEMCTL) we support fallbacks to the + * old ways of doing things, hence don't log any error in that case here. */ + + return r < 0 ? r : 0; +} + +static int trivial_method(int argc, char *argv[], void *userdata) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + const char *method; + sd_bus *bus; + int r; + + polkit_agent_open_if_enabled(); + + r = acquire_bus(BUS_MANAGER, &bus); + if (r < 0) + return r; + + method = + streq(argv[0], "clear-jobs") || + streq(argv[0], "cancel") ? "ClearJobs" : + streq(argv[0], "reset-failed") ? "ResetFailed" : + streq(argv[0], "halt") ? "Halt" : + streq(argv[0], "reboot") ? "Reboot" : + streq(argv[0], "kexec") ? "KExec" : + streq(argv[0], "exit") ? "Exit" : + /* poweroff */ "PowerOff"; + r = sd_bus_call_method( bus, "org.freedesktop.systemd1", @@ -5091,16 +5143,11 @@ static int daemon_reload(int argc, char *argv[], void *userdata) { &error, NULL, NULL); - if (r == -ENOENT && arg_action != ACTION_SYSTEMCTL) - /* There's always a fallback possible for - * legacy actions. */ - r = -EADDRNOTAVAIL; - else if ((r == -ETIMEDOUT || r == -ECONNRESET) && streq(method, "Reexecute")) - /* On reexecution, we expect a disconnect, not a - * reply */ - r = 0; - else if (r < 0) - return log_error_errno(r, "Failed to reload daemon: %s", bus_error_message(&error, r)); + if (r < 0 && arg_action == ACTION_SYSTEMCTL) + return log_error_errno(r, "Failed to execute operation: %s", bus_error_message(&error, r)); + + /* Note that for the legacy commands (i.e. those with action != ACTION_SYSTEMCTL) we support fallbacks to the + * old ways of doing things, hence don't log any error in that case here. */ return r < 0 ? r : 0; } @@ -5112,7 +5159,7 @@ static int reset_failed(int argc, char *argv[], void *userdata) { int r, q; if (argc <= 1) - return daemon_reload(argc, argv, userdata); + return trivial_method(argc, argv, userdata); polkit_agent_open_if_enabled(); @@ -7497,7 +7544,7 @@ static int systemctl_main(int argc, char *argv[]) { { "list-timers", VERB_ANY, VERB_ANY, VERB_NOCHROOT, list_timers }, { "list-jobs", VERB_ANY, VERB_ANY, VERB_NOCHROOT, list_jobs }, { "list-machines", VERB_ANY, VERB_ANY, VERB_NOCHROOT, list_machines }, - { "clear-jobs", VERB_ANY, 1, VERB_NOCHROOT, daemon_reload }, + { "clear-jobs", VERB_ANY, 1, VERB_NOCHROOT, trivial_method }, { "cancel", VERB_ANY, VERB_ANY, VERB_NOCHROOT, cancel_job }, { "start", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, { "stop", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, @@ -7570,7 +7617,7 @@ static int reload_with_fallback(void) { return 0; /* Nothing else worked, so let's try signals */ - assert(arg_action == ACTION_RELOAD || arg_action == ACTION_REEXEC); + assert(IN_SET(arg_action, ACTION_RELOAD, ACTION_REEXEC)); if (kill(1, arg_action == ACTION_RELOAD ? SIGHUP : SIGTERM) < 0) return log_error_errno(errno, "kill() failed: %m"); @@ -7584,8 +7631,7 @@ static int start_with_fallback(void) { if (start_unit(0, NULL, NULL) >= 0) return 0; - /* Nothing else worked, so let's try - * /dev/initctl */ + /* Nothing else worked, so let's try /dev/initctl */ if (talk_initctl() > 0) return 0; -- cgit v1.2.3-54-g00ecf From 988b3b1765153051d48f1b47f42e78d02d2f7a9a Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Mon, 13 Jun 2016 00:57:28 -0400 Subject: systemctl: disallow systemctl --user reboot (#3519) ... as well as halt/poweroff/kexec/suspend/hibernate/hybrid-sleep. Running those commands will fail in user mode, but we try to set the wall message first, which might even succeed for privileged users. Best to nip the whole sequence in the bud. https://github.com/systemd/systemd/pull/3453#issuecomment-225455156 --- src/systemctl/systemctl.c | 142 +++++++++++++++++++++++++--------------------- 1 file changed, 77 insertions(+), 65 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 914dba36dc..784c1cd7b5 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3274,6 +3274,18 @@ static int start_special(int argc, char *argv[], void *userdata) { return start_unit(argc, argv, userdata); } +static int start_system_special(int argc, char *argv[], void *userdata) { + /* Like start_special above, but raises an error when running in user mode */ + + if (arg_scope != UNIT_FILE_SYSTEM) { + log_error("Bad action for %s mode.", + arg_scope == UNIT_FILE_GLOBAL ? "--global" : "--user"); + return -EINVAL; + } + + return start_special(argc, argv, userdata); +} + static int check_unit_generic(int code, const UnitActiveState good_states[], int nb_states, char **args) { _cleanup_strv_free_ char **names = NULL; UnitActiveState active_state; @@ -7539,71 +7551,71 @@ static int systemctl_main(int argc, char *argv[]) { static const Verb verbs[] = { { "list-units", VERB_ANY, VERB_ANY, VERB_DEFAULT|VERB_NOCHROOT, list_units }, - { "list-unit-files", VERB_ANY, VERB_ANY, 0, list_unit_files }, - { "list-sockets", VERB_ANY, VERB_ANY, VERB_NOCHROOT, list_sockets }, - { "list-timers", VERB_ANY, VERB_ANY, VERB_NOCHROOT, list_timers }, - { "list-jobs", VERB_ANY, VERB_ANY, VERB_NOCHROOT, list_jobs }, - { "list-machines", VERB_ANY, VERB_ANY, VERB_NOCHROOT, list_machines }, - { "clear-jobs", VERB_ANY, 1, VERB_NOCHROOT, trivial_method }, - { "cancel", VERB_ANY, VERB_ANY, VERB_NOCHROOT, cancel_job }, - { "start", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, - { "stop", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, - { "condstop", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, /* For compatibility with ALTLinux */ - { "reload", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, - { "restart", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, - { "try-restart", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, - { "reload-or-restart", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, - { "reload-or-try-restart", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, /* For compatbility with old systemctl <= 228 */ - { "try-reload-or-restart", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, - { "force-reload", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, /* For compatibility with SysV */ - { "condreload", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, /* For compatibility with ALTLinux */ - { "condrestart", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, /* For compatibility with RH */ - { "isolate", 2, 2, VERB_NOCHROOT, start_unit }, - { "kill", 2, VERB_ANY, VERB_NOCHROOT, kill_unit }, - { "is-active", 2, VERB_ANY, VERB_NOCHROOT, check_unit_active }, - { "check", 2, VERB_ANY, VERB_NOCHROOT, check_unit_active }, - { "is-failed", 2, VERB_ANY, VERB_NOCHROOT, check_unit_failed }, - { "show", VERB_ANY, VERB_ANY, VERB_NOCHROOT, show }, - { "cat", 2, VERB_ANY, VERB_NOCHROOT, cat }, - { "status", VERB_ANY, VERB_ANY, VERB_NOCHROOT, show }, - { "help", VERB_ANY, VERB_ANY, VERB_NOCHROOT, show }, - { "daemon-reload", VERB_ANY, 1, VERB_NOCHROOT, daemon_reload }, - { "daemon-reexec", VERB_ANY, 1, VERB_NOCHROOT, daemon_reload }, - { "show-environment", VERB_ANY, 1, VERB_NOCHROOT, show_environment }, - { "set-environment", 2, VERB_ANY, VERB_NOCHROOT, set_environment }, - { "unset-environment", 2, VERB_ANY, VERB_NOCHROOT, set_environment }, - { "import-environment", VERB_ANY, VERB_ANY, VERB_NOCHROOT, import_environment}, - { "halt", VERB_ANY, 1, VERB_NOCHROOT, start_special }, - { "poweroff", VERB_ANY, 1, VERB_NOCHROOT, start_special }, - { "reboot", VERB_ANY, 2, VERB_NOCHROOT, start_special }, - { "kexec", VERB_ANY, 1, VERB_NOCHROOT, start_special }, - { "suspend", VERB_ANY, 1, VERB_NOCHROOT, start_special }, - { "hibernate", VERB_ANY, 1, VERB_NOCHROOT, start_special }, - { "hybrid-sleep", VERB_ANY, 1, VERB_NOCHROOT, start_special }, - { "default", VERB_ANY, 1, VERB_NOCHROOT, start_special }, - { "rescue", VERB_ANY, 1, VERB_NOCHROOT, start_special }, - { "emergency", VERB_ANY, 1, VERB_NOCHROOT, start_special }, - { "exit", VERB_ANY, 2, VERB_NOCHROOT, start_special }, - { "reset-failed", VERB_ANY, VERB_ANY, VERB_NOCHROOT, reset_failed }, - { "enable", 2, VERB_ANY, 0, enable_unit }, - { "disable", 2, VERB_ANY, 0, enable_unit }, - { "is-enabled", 2, VERB_ANY, 0, unit_is_enabled }, - { "reenable", 2, VERB_ANY, 0, enable_unit }, - { "preset", 2, VERB_ANY, 0, enable_unit }, - { "preset-all", VERB_ANY, 1, 0, preset_all }, - { "mask", 2, VERB_ANY, 0, enable_unit }, - { "unmask", 2, VERB_ANY, 0, enable_unit }, - { "link", 2, VERB_ANY, 0, enable_unit }, - { "revert", 2, VERB_ANY, 0, enable_unit }, - { "switch-root", 2, VERB_ANY, VERB_NOCHROOT, switch_root }, - { "list-dependencies", VERB_ANY, 2, VERB_NOCHROOT, list_dependencies }, - { "set-default", 2, 2, 0, set_default }, - { "get-default", VERB_ANY, 1, 0, get_default }, - { "set-property", 3, VERB_ANY, VERB_NOCHROOT, set_property }, - { "is-system-running", VERB_ANY, 1, 0, is_system_running }, - { "add-wants", 3, VERB_ANY, 0, add_dependency }, - { "add-requires", 3, VERB_ANY, 0, add_dependency }, - { "edit", 2, VERB_ANY, VERB_NOCHROOT, edit }, + { "list-unit-files", VERB_ANY, VERB_ANY, 0, list_unit_files }, + { "list-sockets", VERB_ANY, VERB_ANY, VERB_NOCHROOT, list_sockets }, + { "list-timers", VERB_ANY, VERB_ANY, VERB_NOCHROOT, list_timers }, + { "list-jobs", VERB_ANY, VERB_ANY, VERB_NOCHROOT, list_jobs }, + { "list-machines", VERB_ANY, VERB_ANY, VERB_NOCHROOT, list_machines }, + { "clear-jobs", VERB_ANY, 1, VERB_NOCHROOT, trivial_method }, + { "cancel", VERB_ANY, VERB_ANY, VERB_NOCHROOT, cancel_job }, + { "start", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, + { "stop", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, + { "condstop", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, /* For compatibility with ALTLinux */ + { "reload", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, + { "restart", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, + { "try-restart", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, + { "reload-or-restart", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, + { "reload-or-try-restart", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, /* For compatbility with old systemctl <= 228 */ + { "try-reload-or-restart", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, + { "force-reload", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, /* For compatibility with SysV */ + { "condreload", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, /* For compatibility with ALTLinux */ + { "condrestart", 2, VERB_ANY, VERB_NOCHROOT, start_unit }, /* For compatibility with RH */ + { "isolate", 2, 2, VERB_NOCHROOT, start_unit }, + { "kill", 2, VERB_ANY, VERB_NOCHROOT, kill_unit }, + { "is-active", 2, VERB_ANY, VERB_NOCHROOT, check_unit_active }, + { "check", 2, VERB_ANY, VERB_NOCHROOT, check_unit_active }, + { "is-failed", 2, VERB_ANY, VERB_NOCHROOT, check_unit_failed }, + { "show", VERB_ANY, VERB_ANY, VERB_NOCHROOT, show }, + { "cat", 2, VERB_ANY, VERB_NOCHROOT, cat }, + { "status", VERB_ANY, VERB_ANY, VERB_NOCHROOT, show }, + { "help", VERB_ANY, VERB_ANY, VERB_NOCHROOT, show }, + { "daemon-reload", VERB_ANY, 1, VERB_NOCHROOT, daemon_reload }, + { "daemon-reexec", VERB_ANY, 1, VERB_NOCHROOT, daemon_reload }, + { "show-environment", VERB_ANY, 1, VERB_NOCHROOT, show_environment }, + { "set-environment", 2, VERB_ANY, VERB_NOCHROOT, set_environment }, + { "unset-environment", 2, VERB_ANY, VERB_NOCHROOT, set_environment }, + { "import-environment", VERB_ANY, VERB_ANY, VERB_NOCHROOT, import_environment }, + { "halt", VERB_ANY, 1, VERB_NOCHROOT, start_system_special }, + { "poweroff", VERB_ANY, 1, VERB_NOCHROOT, start_system_special }, + { "reboot", VERB_ANY, 2, VERB_NOCHROOT, start_system_special }, + { "kexec", VERB_ANY, 1, VERB_NOCHROOT, start_system_special }, + { "suspend", VERB_ANY, 1, VERB_NOCHROOT, start_system_special }, + { "hibernate", VERB_ANY, 1, VERB_NOCHROOT, start_system_special }, + { "hybrid-sleep", VERB_ANY, 1, VERB_NOCHROOT, start_system_special }, + { "default", VERB_ANY, 1, VERB_NOCHROOT, start_special }, + { "rescue", VERB_ANY, 1, VERB_NOCHROOT, start_system_special }, + { "emergency", VERB_ANY, 1, VERB_NOCHROOT, start_system_special }, + { "exit", VERB_ANY, 2, VERB_NOCHROOT, start_special }, + { "reset-failed", VERB_ANY, VERB_ANY, VERB_NOCHROOT, reset_failed }, + { "enable", 2, VERB_ANY, 0, enable_unit }, + { "disable", 2, VERB_ANY, 0, enable_unit }, + { "is-enabled", 2, VERB_ANY, 0, unit_is_enabled }, + { "reenable", 2, VERB_ANY, 0, enable_unit }, + { "preset", 2, VERB_ANY, 0, enable_unit }, + { "preset-all", VERB_ANY, 1, 0, preset_all }, + { "mask", 2, VERB_ANY, 0, enable_unit }, + { "unmask", 2, VERB_ANY, 0, enable_unit }, + { "link", 2, VERB_ANY, 0, enable_unit }, + { "revert", 2, VERB_ANY, 0, enable_unit }, + { "switch-root", 2, VERB_ANY, VERB_NOCHROOT, switch_root }, + { "list-dependencies", VERB_ANY, 2, VERB_NOCHROOT, list_dependencies }, + { "set-default", 2, 2, 0, set_default }, + { "get-default", VERB_ANY, 1, 0, get_default }, + { "set-property", 3, VERB_ANY, VERB_NOCHROOT, set_property }, + { "is-system-running", VERB_ANY, 1, 0, is_system_running }, + { "add-wants", 3, VERB_ANY, 0, add_dependency }, + { "add-requires", 3, VERB_ANY, 0, add_dependency }, + { "edit", 2, VERB_ANY, VERB_NOCHROOT, edit }, {} }; -- cgit v1.2.3-54-g00ecf From 51e22f8851fae578188bd190510a205f0e4487f4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 13 Jun 2016 18:54:36 +0200 Subject: systemctl: fix assertion hit when showing state of a unit without control group --- src/systemctl/systemctl.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 784c1cd7b5..74ef2d7ba1 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3847,14 +3847,13 @@ static void print_status_info( printf(" CPU: %s\n", format_timespan(buf, sizeof(buf), i->cpu_usage_nsec / NSEC_PER_USEC, USEC_PER_MSEC)); } - if (i->control_group) - printf(" CGroup: %s\n", i->control_group); - - { + if (i->control_group) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; static const char prefix[] = " "; unsigned c; + printf(" CGroup: %s\n", i->control_group); + c = columns(); if (c > sizeof(prefix) - 1) c -= sizeof(prefix) - 1; -- cgit v1.2.3-54-g00ecf From 3dced37b7c2c9a5c733817569d2bbbaa397adaf7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 13 Jun 2016 19:11:26 +0200 Subject: systemctl: rework "systemctl status" a bit This reworks "systemctl status" and "systemctl show" a bit. It removes the definition of the `property_info` structure, because we can simply reuse the existing UnitStatusInfo type for that. The "could not be found" message is now printed by show_one() itself (and not its caller), so that it is shown regardless by who the function is called. (This makes it necessary to pass the unit name to the function.) This also adds all properties found to a set, and then checks if any of the properties passed via "--property=" is mising in it, if so, a proper error is generated. Support for checking the PID file of a unit is removed, as this cannot be done reasonably client side (since the systemd instance we are talking to might sit on another host) Replaces: #3411 Fixes: #3425 Also see: #3504 --- src/systemctl/systemctl.c | 120 +++++++++++++++++++++++++--------------------- 1 file changed, 66 insertions(+), 54 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 74ef2d7ba1..feb93cad8e 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -4558,12 +4558,20 @@ static int show_one( const char *verb, sd_bus *bus, const char *path, + const char *unit, bool show_properties, bool *new_line, bool *ellipsized) { + static const struct bus_properties_map property_map[] = { + { "LoadState", "s", NULL, offsetof(UnitStatusInfo, load_state) }, + { "ActiveState", "s", NULL, offsetof(UnitStatusInfo, active_state) }, + {} + }; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_set_free_ Set *found_properties = NULL; UnitStatusInfo info = { .memory_current = (uint64_t) -1, .memory_high = CGROUP_LIMIT_MAX, @@ -4573,14 +4581,6 @@ static int show_one( .tasks_current = (uint64_t) -1, .tasks_max = (uint64_t) -1, }; - struct property_info { - const char *load_state, *active_state; - } property_info = {}; - static const struct bus_properties_map property_map[] = { - { "LoadState", "s", NULL, offsetof(struct property_info, load_state) }, - { "ActiveState", "s", NULL, offsetof(struct property_info, active_state) }, - {} - }; ExecStatusInfo *p; int r; @@ -4601,16 +4601,24 @@ static int show_one( if (r < 0) return log_error_errno(r, "Failed to get properties: %s", bus_error_message(&error, r)); - r = bus_message_map_all_properties(reply, property_map, &property_info); - if (r < 0) - return log_error_errno(r, "Failed to map properties: %s", bus_error_message(&error, r)); + if (unit) { + r = bus_message_map_all_properties(reply, property_map, &info); + if (r < 0) + return log_error_errno(r, "Failed to map properties: %s", bus_error_message(&error, r)); - if (streq_ptr(property_info.load_state, "not-found") && streq_ptr(property_info.active_state, "inactive")) - return EXIT_PROGRAM_OR_SERVICES_STATUS_UNKNOWN; + if (streq_ptr(info.load_state, "not-found") && streq_ptr(info.active_state, "inactive")) { + log_error("Unit %s could not be found.", unit); - r = sd_bus_message_rewind(reply, true); - if (r < 0) - return log_error_errno(r, "Failed to rewind: %s", bus_error_message(&error, r)); + if (streq(verb, "status")) + return EXIT_PROGRAM_OR_SERVICES_STATUS_UNKNOWN; + + return -ENOENT; + } + + r = sd_bus_message_rewind(reply, true); + if (r < 0) + return log_error_errno(r, "Failed to rewind: %s", bus_error_message(&error, r)); + } r = sd_bus_message_enter_container(reply, SD_BUS_TYPE_ARRAY, "{sv}"); if (r < 0) @@ -4636,9 +4644,17 @@ static int show_one( if (r < 0) return bus_log_parse_error(r); - if (show_properties) + if (show_properties) { + r = set_ensure_allocated(&found_properties, &string_hash_ops); + if (r < 0) + return log_oom(); + + r = set_put(found_properties, name); + if (r < 0 && r != EEXIST) + return log_oom(); + r = print_property(name, reply, contents); - else + } else r = status_property(name, reply, &info, contents); if (r < 0) return r; @@ -4660,35 +4676,30 @@ static int show_one( r = 0; - if (!show_properties) { - if (streq(verb, "help")) - show_unit_help(&info); + if (show_properties) { + char **pp; + + STRV_FOREACH(pp, arg_properties) { + if (!set_contains(found_properties, *pp)) { + log_warning("Property %s does not exist.", *pp); + r = -ENXIO; + } + } + } else if (streq(verb, "help")) + show_unit_help(&info); + else if (streq(verb, "status")) { + print_status_info(bus, &info, ellipsized); + + if (info.active_state && STR_IN_SET(info.active_state, "inactive", "failed")) + r = EXIT_PROGRAM_NOT_RUNNING; else - print_status_info(bus, &info, ellipsized); + r = EXIT_PROGRAM_RUNNING_OR_SERVICE_OK; } strv_free(info.documentation); strv_free(info.dropin_paths); strv_free(info.listen); - if (!streq_ptr(info.active_state, "active") && - !streq_ptr(info.active_state, "reloading") && - streq(verb, "status")) { - /* According to LSB: "program not running" */ - /* 0: program is running or service is OK - * 1: program is dead and /run PID file exists - * 2: program is dead and /run/lock lock file exists - * 3: program is not running - * 4: program or service status is unknown - */ - if (info.pid_file && access(info.pid_file, F_OK) == 0) - r = EXIT_PROGRAM_DEAD_AND_PID_EXISTS; - else if (streq_ptr(info.load_state, "not-found") && streq_ptr(info.active_state, "inactive")) - r = EXIT_PROGRAM_OR_SERVICES_STATUS_UNKNOWN; - else - r = EXIT_PROGRAM_NOT_RUNNING; - } - while ((p = info.exec)) { LIST_REMOVE(exec, info.exec, p); exec_status_info_free(p); @@ -4761,7 +4772,7 @@ static int show_all( if (!p) return log_oom(); - r = show_one(verb, bus, p, show_properties, new_line, ellipsized); + r = show_one(verb, bus, p, u->id, show_properties, new_line, ellipsized); if (r < 0) return r; else if (r > 0 && ret == 0) @@ -4857,7 +4868,7 @@ static int show(int argc, char *argv[], void *userdata) { /* If no argument is specified inspect the manager itself */ if (show_properties && argc <= 1) - return show_one(argv[0], bus, "/org/freedesktop/systemd1", show_properties, &new_line, &ellipsized); + return show_one(argv[0], bus, "/org/freedesktop/systemd1", NULL, show_properties, &new_line, &ellipsized); if (show_status && argc <= 1) { @@ -4872,7 +4883,7 @@ static int show(int argc, char *argv[], void *userdata) { char **name; STRV_FOREACH(name, strv_skip(argv, 1)) { - _cleanup_free_ char *unit = NULL; + _cleanup_free_ char *path = NULL, *unit = NULL; uint32_t id; if (safe_atou32(*name, &id) < 0) { @@ -4882,19 +4893,23 @@ static int show(int argc, char *argv[], void *userdata) { continue; } else if (show_properties) { /* Interpret as job id */ - if (asprintf(&unit, "/org/freedesktop/systemd1/job/%u", id) < 0) + if (asprintf(&path, "/org/freedesktop/systemd1/job/%u", id) < 0) return log_oom(); } else { /* Interpret as PID */ - r = get_unit_dbus_path_by_pid(bus, id, &unit); + r = get_unit_dbus_path_by_pid(bus, id, &path); if (r < 0) { ret = r; continue; } + + r = unit_name_from_dbus_path(path, &unit); + if (r < 0) + return log_oom(); } - r = show_one(argv[0], bus, unit, show_properties, &new_line, &ellipsized); + r = show_one(argv[0], bus, path, unit, show_properties, &new_line, &ellipsized); if (r < 0) return r; else if (r > 0 && ret == 0) @@ -4909,20 +4924,17 @@ static int show(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to expand names: %m"); STRV_FOREACH(name, names) { - _cleanup_free_ char *unit; + _cleanup_free_ char *path; - unit = unit_dbus_path_from_name(*name); - if (!unit) + path = unit_dbus_path_from_name(*name); + if (!path) return log_oom(); - r = show_one(argv[0], bus, unit, show_properties, &new_line, &ellipsized); + r = show_one(argv[0], bus, path, *name, show_properties, &new_line, &ellipsized); if (r < 0) return r; - else if (r > 0 && ret == 0) + if (r > 0 && ret == 0) ret = r; - - if (r == EXIT_PROGRAM_OR_SERVICES_STATUS_UNKNOWN) - log_error("Can't display property %s. Unit %s does not exist.", *patterns, *name); } } } -- cgit v1.2.3-54-g00ecf From 193edd61c30fab64662578f25d8cb9122c89c672 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Wed, 15 Jun 2016 08:21:15 -0400 Subject: systemctl: do not open pager twice Second attempt had no effect anyway. --- src/systemctl/systemctl.c | 1 - 1 file changed, 1 deletion(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index feb93cad8e..862eb17082 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -4872,7 +4872,6 @@ static int show(int argc, char *argv[], void *userdata) { if (show_status && argc <= 1) { - pager_open(arg_no_pager, false); show_system_status(bus); new_line = true; -- cgit v1.2.3-54-g00ecf From 33d52725f5e90f278fec675a8c34e3accaa6ad97 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Wed, 15 Jun 2016 10:03:33 -0400 Subject: systemctl: also fall back to ListUnitsFiltered on access denied When running systemctl from git on systemd from systemd-229-8.fc24.x86_64, ListUnitsByPatterns results in org.freedesktop.DBus.Error.AccessDenied. --- src/systemctl/systemctl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 862eb17082..ecd875fa2d 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -588,7 +588,8 @@ static int get_unit_list( return bus_log_create_error(r); r = sd_bus_call(bus, m, 0, &error, &reply); - if (r < 0 && sd_bus_error_has_name(&error, SD_BUS_ERROR_UNKNOWN_METHOD)) { + if (r < 0 && (sd_bus_error_has_name(&error, SD_BUS_ERROR_UNKNOWN_METHOD) || + sd_bus_error_has_name(&error, SD_BUS_ERROR_ACCESS_DENIED))) { /* Fallback to legacy ListUnitsFiltered method */ fallback = true; log_debug_errno(r, "Failed to list units: %s Falling back to ListUnitsFiltered method.", bus_error_message(&error, r)); -- cgit v1.2.3-54-g00ecf From cf647b69baee4c478d3909c327e3d917e1563f44 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 16 Jun 2016 15:29:16 +0200 Subject: systemctl: make sure we terminate the bus connection first, and then close the pager (#3550) If "systemctl -H" is used, let's make sure we first terminate the bus connection, and only then close the pager. If done in this order ssh will get an EOF on stdin (as we speak D-Bus through ssh's stdin/stdout), and then terminate. This makes sure the standard error we were invoked on is released by ssh, and only that makes sure we don't deadlock on the pager which waits for all clients closing its input pipe. (Similar fixes for the various other xyzctl tools that support both pagers and -H) Fixes: #3543 --- src/libsystemd/sd-bus/busctl.c | 3 ++- src/locale/localectl.c | 3 ++- src/login/loginctl.c | 4 +++- src/machine/machinectl.c | 3 ++- src/systemctl/systemctl.c | 4 ++-- src/timedate/timedatectl.c | 3 ++- 6 files changed, 13 insertions(+), 7 deletions(-) (limited to 'src/systemctl') diff --git a/src/libsystemd/sd-bus/busctl.c b/src/libsystemd/sd-bus/busctl.c index bfe967bfb0..eb042e9c81 100644 --- a/src/libsystemd/sd-bus/busctl.c +++ b/src/libsystemd/sd-bus/busctl.c @@ -1987,7 +1987,7 @@ static int busctl_main(sd_bus *bus, int argc, char *argv[]) { } int main(int argc, char *argv[]) { - _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + sd_bus *bus = NULL; int r; log_parse_environment(); @@ -2078,6 +2078,7 @@ int main(int argc, char *argv[]) { r = busctl_main(bus, argc, argv); finish: + sd_bus_flush_close_unref(bus); pager_close(); strv_free(arg_matches); diff --git a/src/locale/localectl.c b/src/locale/localectl.c index 4865335349..81afb4909f 100644 --- a/src/locale/localectl.c +++ b/src/locale/localectl.c @@ -656,7 +656,7 @@ static int localectl_main(sd_bus *bus, int argc, char *argv[]) { } int main(int argc, char*argv[]) { - _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + sd_bus *bus = NULL; int r; setlocale(LC_ALL, ""); @@ -676,6 +676,7 @@ int main(int argc, char*argv[]) { r = localectl_main(bus, argc, argv); finish: + sd_bus_flush_close_unref(bus); pager_close(); return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; diff --git a/src/login/loginctl.c b/src/login/loginctl.c index 1c75565636..0fc2720b43 100644 --- a/src/login/loginctl.c +++ b/src/login/loginctl.c @@ -1554,7 +1554,7 @@ static int loginctl_main(int argc, char *argv[], sd_bus *bus) { } int main(int argc, char *argv[]) { - _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + sd_bus *bus = NULL; int r; setlocale(LC_ALL, ""); @@ -1576,6 +1576,8 @@ int main(int argc, char *argv[]) { r = loginctl_main(argc, argv, bus); finish: + sd_bus_flush_close_unref(bus); + pager_close(); polkit_agent_close(); diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index afe5026373..583d2a21e7 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -2751,7 +2751,7 @@ static int machinectl_main(int argc, char *argv[], sd_bus *bus) { } int main(int argc, char*argv[]) { - _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + sd_bus *bus = NULL; int r; setlocale(LC_ALL, ""); @@ -2773,6 +2773,7 @@ int main(int argc, char*argv[]) { r = machinectl_main(argc, argv, bus); finish: + sd_bus_flush_close_unref(bus); pager_close(); polkit_agent_close(); diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index feb93cad8e..4b34d24ee8 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -7938,6 +7938,8 @@ int main(int argc, char*argv[]) { } finish: + release_busses(); + pager_close(); ask_password_agent_close(); polkit_agent_close(); @@ -7949,8 +7951,6 @@ finish: strv_free(arg_wall); free(arg_root); - release_busses(); - /* Note that we return r here, not EXIT_SUCCESS, so that we can implement the LSB-like return codes */ return r < 0 ? EXIT_FAILURE : r; } diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c index b7871f81aa..7f61cf0181 100644 --- a/src/timedate/timedatectl.c +++ b/src/timedate/timedatectl.c @@ -480,7 +480,7 @@ static int timedatectl_main(sd_bus *bus, int argc, char *argv[]) { } int main(int argc, char *argv[]) { - _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + sd_bus *bus = NULL; int r; setlocale(LC_ALL, ""); @@ -500,6 +500,7 @@ int main(int argc, char *argv[]) { r = timedatectl_main(bus, argc, argv); finish: + sd_bus_flush_close_unref(bus); pager_close(); return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; -- cgit v1.2.3-54-g00ecf From d2ad7e1ba5c9e68367e0eedbbfdadfc3adf94af2 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Wed, 15 Jun 2016 09:11:32 -0400 Subject: systemctl: delay pager/polkit agent opening as much as possible In https://github.com/systemd/systemd/issues/3543, we would open the pager before starting ssh, and the pipe fd was "leaked" into the ssh child as the stderr fd. Previous commit fixes bus-socket to nullify stderr before launching the child, but it seems reasonable to also delay starting the pager. If we are going to croak when trying to open the transport, it seems better to do this before starting the pager. This commit would also fix #3543 on its own. --- src/systemctl/systemctl.c | 100 +++++++++++++++++++++++----------------------- 1 file changed, 50 insertions(+), 50 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index ecd875fa2d..f69c858bd6 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -735,12 +735,12 @@ static int list_units(int argc, char *argv[], void *userdata) { sd_bus *bus; int r; - pager_open(arg_no_pager, false); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + pager_open(arg_no_pager, false); + r = get_unit_list_recursive(bus, strv_skip(argv, 1), &unit_infos, &replies, &machines); if (r < 0) return r; @@ -947,12 +947,12 @@ static int list_sockets(int argc, char *argv[], void *userdata) { int r = 0, n; sd_bus *bus; - pager_open(arg_no_pager, false); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + pager_open(arg_no_pager, false); + n = get_unit_list_recursive(bus, strv_skip(argv, 1), &unit_infos, &replies, &machines); if (n < 0) return n; @@ -1254,12 +1254,12 @@ static int list_timers(int argc, char *argv[], void *userdata) { sd_bus *bus; int r = 0; - pager_open(arg_no_pager, false); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + pager_open(arg_no_pager, false); + n = get_unit_list_recursive(bus, strv_skip(argv, 1), &unit_infos, &replies, &machines); if (n < 0) return n; @@ -1425,8 +1425,6 @@ static int list_unit_files(int argc, char *argv[], void *userdata) { int r; bool fallback = false; - pager_open(arg_no_pager, false); - if (install_client_side()) { Hashmap *h; UnitFileList *u; @@ -1541,6 +1539,8 @@ static int list_unit_files(int argc, char *argv[], void *userdata) { return bus_log_parse_error(r); } + pager_open(arg_no_pager, false); + qsort_safe(units, c, sizeof(UnitFileList), compare_unit_file_list); output_unit_file_list(units, c); @@ -1789,12 +1789,12 @@ static int list_dependencies(int argc, char *argv[], void *userdata) { } else u = SPECIAL_DEFAULT_TARGET; - pager_open(arg_no_pager, false); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + pager_open(arg_no_pager, false); + puts(u); return list_dependencies_one(bus, u, 0, &units, 0); @@ -2020,8 +2020,6 @@ static int list_machines(int argc, char *argv[], void *userdata) { return -EPERM; } - pager_open(arg_no_pager, false); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; @@ -2030,6 +2028,8 @@ static int list_machines(int argc, char *argv[], void *userdata) { if (r < 0) return r; + pager_open(arg_no_pager, false); + qsort_safe(machine_infos, r, sizeof(struct machine_info), compare_machine_info); output_machines_list(machine_infos, r); free_machines_list(machine_infos, r); @@ -2233,8 +2233,6 @@ static int list_jobs(int argc, char *argv[], void *userdata) { int r; bool skipped = false; - pager_open(arg_no_pager, false); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; @@ -2275,6 +2273,8 @@ static int list_jobs(int argc, char *argv[], void *userdata) { if (r < 0) return bus_log_parse_error(r); + pager_open(arg_no_pager, false); + output_jobs_list(jobs, c, skipped); return 0; } @@ -2287,12 +2287,12 @@ static int cancel_job(int argc, char *argv[], void *userdata) { if (argc <= 1) return trivial_method(argc, argv, userdata); - polkit_agent_open_if_enabled(); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + polkit_agent_open_if_enabled(); + STRV_FOREACH(name, strv_skip(argv, 1)) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; uint32_t id; @@ -2828,13 +2828,13 @@ static int start_unit(int argc, char *argv[], void *userdata) { char **name; int r = 0; - ask_password_agent_open_if_enabled(); - polkit_agent_open_if_enabled(); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + ask_password_agent_open_if_enabled(); + polkit_agent_open_if_enabled(); + if (arg_action == ACTION_SYSTEMCTL) { enum action action; @@ -2954,9 +2954,6 @@ static int logind_reboot(enum action a) { sd_bus *bus; int r; - polkit_agent_open_if_enabled(); - (void) logind_set_wall_message(); - r = acquire_bus(BUS_FULL, &bus); if (r < 0) return r; @@ -2992,6 +2989,9 @@ static int logind_reboot(enum action a) { return -EINVAL; } + polkit_agent_open_if_enabled(); + (void) logind_set_wall_message(); + r = sd_bus_call_method( bus, "org.freedesktop.login1", @@ -3338,12 +3338,12 @@ static int kill_unit(int argc, char *argv[], void *userdata) { sd_bus *bus; int r, q; - polkit_agent_open_if_enabled(); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + polkit_agent_open_if_enabled(); + if (!arg_kill_who) arg_kill_who = "all"; @@ -4855,6 +4855,10 @@ static int show(int argc, char *argv[], void *userdata) { return -EINVAL; } + r = acquire_bus(BUS_MANAGER, &bus); + if (r < 0) + return r; + pager_open(arg_no_pager, false); if (show_status) @@ -4863,10 +4867,6 @@ static int show(int argc, char *argv[], void *userdata) { * be split up into many files. */ setrlimit_closest(RLIMIT_NOFILE, &RLIMIT_MAKE_CONST(16384)); - r = acquire_bus(BUS_MANAGER, &bus); - if (r < 0) - return r; - /* If no argument is specified inspect the manager itself */ if (show_properties && argc <= 1) return show_one(argv[0], bus, "/org/freedesktop/systemd1", NULL, show_properties, &new_line, &ellipsized); @@ -5029,12 +5029,12 @@ static int set_property(int argc, char *argv[], void *userdata) { char **i; int r; - polkit_agent_open_if_enabled(); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + polkit_agent_open_if_enabled(); + r = sd_bus_message_new_method_call( bus, &m, @@ -5081,12 +5081,12 @@ static int daemon_reload(int argc, char *argv[], void *userdata) { sd_bus *bus; int r; - polkit_agent_open_if_enabled(); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + polkit_agent_open_if_enabled(); + switch (arg_action) { case ACTION_RELOAD: @@ -5141,12 +5141,12 @@ static int trivial_method(int argc, char *argv[], void *userdata) { sd_bus *bus; int r; - polkit_agent_open_if_enabled(); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + polkit_agent_open_if_enabled(); + method = streq(argv[0], "clear-jobs") || streq(argv[0], "cancel") ? "ClearJobs" : @@ -5184,12 +5184,12 @@ static int reset_failed(int argc, char *argv[], void *userdata) { if (argc <= 1) return trivial_method(argc, argv, userdata); - polkit_agent_open_if_enabled(); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + polkit_agent_open_if_enabled(); + r = expand_names(bus, strv_skip(argv, 1), NULL, &names); if (r < 0) return log_error_errno(r, "Failed to expand names: %m"); @@ -5223,12 +5223,12 @@ static int show_environment(int argc, char *argv[], void *userdata) { sd_bus *bus; int r; - pager_open(arg_no_pager, false); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + pager_open(arg_no_pager, false); + r = sd_bus_get_property( bus, "org.freedesktop.systemd1", @@ -5332,12 +5332,12 @@ static int set_environment(int argc, char *argv[], void *userdata) { assert(argc > 1); assert(argv); - polkit_agent_open_if_enabled(); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + polkit_agent_open_if_enabled(); + method = streq(argv[0], "set-environment") ? "SetEnvironment" : "UnsetEnvironment"; @@ -5369,12 +5369,12 @@ static int import_environment(int argc, char *argv[], void *userdata) { sd_bus *bus; int r; - polkit_agent_open_if_enabled(); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + polkit_agent_open_if_enabled(); + r = sd_bus_message_new_method_call( bus, &m, @@ -5666,12 +5666,12 @@ static int enable_unit(int argc, char *argv[], void *userdata) { const char *method; sd_bus *bus; - polkit_agent_open_if_enabled(); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + polkit_agent_open_if_enabled(); + if (streq(verb, "enable")) { method = "EnableUnitFiles"; expect_carries_install_info = true; @@ -5832,12 +5832,12 @@ static int add_dependency(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; sd_bus *bus; - polkit_agent_open_if_enabled(); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + polkit_agent_open_if_enabled(); + r = sd_bus_message_new_method_call( bus, &m, @@ -5894,12 +5894,12 @@ static int preset_all(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; sd_bus *bus; - polkit_agent_open_if_enabled(); - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; + polkit_agent_open_if_enabled(); + r = sd_bus_call_method( bus, "org.freedesktop.systemd1", @@ -7720,8 +7720,6 @@ static int logind_schedule_shutdown(void) { sd_bus *bus; int r; - (void) logind_set_wall_message(); - r = acquire_bus(BUS_FULL, &bus); if (r < 0) return r; @@ -7748,6 +7746,8 @@ static int logind_schedule_shutdown(void) { if (arg_dry) action = strjoina("dry-", action); + (void) logind_set_wall_message(); + r = sd_bus_call_method( bus, "org.freedesktop.login1", -- cgit v1.2.3-54-g00ecf From 79d4ace11e2813987d611aeae91b4f1237d98928 Mon Sep 17 00:00:00 2001 From: Douglas Christman Date: Wed, 22 Jun 2016 15:09:33 -0400 Subject: systemctl: Add missing "/" to files created by 'edit --runtime' --- src/systemctl/systemctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 0dfdae4538..38b5a7e082 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -6093,7 +6093,7 @@ static int get_file_to_edit( return log_oom(); if (arg_runtime) { - run = strjoin(paths->runtime_config, name, NULL); + run = strjoin(paths->runtime_config, "/", name, NULL); if (!run) return log_oom(); } -- cgit v1.2.3-54-g00ecf From 39c38ce17c3a15f7b709536a6d7f1f7e093ac450 Mon Sep 17 00:00:00 2001 From: Doug Christman Date: Fri, 24 Jun 2016 02:00:35 -0400 Subject: systemctl: Create new unit files with "edit --force" (#3584) --- TODO | 4 +--- man/systemctl.xml | 6 ++++++ src/systemctl/systemctl.c | 38 +++++++++++++++++++++++--------------- 3 files changed, 30 insertions(+), 18 deletions(-) (limited to 'src/systemctl') diff --git a/TODO b/TODO index 8de1029b9b..0d18e0734b 100644 --- a/TODO +++ b/TODO @@ -197,9 +197,7 @@ Features: * systemctl: if some operation fails, show log output? -* systemctl edit: -- allow creation of units from scratch -- use equvalent of cat() to insert existing config as a comment, prepended with #. +* systemctl edit: use equvalent of cat() to insert existing config as a comment, prepended with #. Upon editor exit, lines with one # are removed, lines with two # are left with one #, etc. * exponential backoff in timesyncd when we cannot reach a server diff --git a/man/systemctl.xml b/man/systemctl.xml index 914af929c8..742da81cfe 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -481,6 +481,9 @@ When used with enable, overwrite any existing conflicting symlinks. + When used with edit, create all of the + specified units which do not already exist. + When used with halt, poweroff, reboot or kexec, execute the selected operation without shutting down all units. However, all processes will be killed forcibly and all file systems are unmounted or remounted read-only. This is hence a @@ -1303,6 +1306,9 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service If is specified, this will copy the original units instead of creating drop-in files. + If is specified and any units do + not already exist, new unit files will be opened for editing. + If is specified, the changes will be made temporarily in /run and they will be lost on the next reboot. diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 38b5a7e082..0a8e60c195 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2499,7 +2499,7 @@ static int unit_find_paths( r = 1; } - if (r == 0) + if (r == 0 && !arg_force) log_error("No files found for %s.", unit_name); return r; @@ -6070,7 +6070,7 @@ static int create_edit_temp_file(const char *new_path, const char *original_path return log_error_errno(r, "Failed to create temporary file \"%s\": %m", t); } else if (r < 0) - return log_error_errno(r, "Failed to copy \"%s\" to \"%s\": %m", original_path, t); + return log_error_errno(r, "Failed to create temporary file for \"%s\": %m", new_path); *ret_tmp_fn = t; t = NULL; @@ -6114,9 +6114,10 @@ static int get_file_to_edit( return 0; } -static int unit_file_create_dropin( +static int unit_file_create_new( const LookupPaths *paths, const char *unit_name, + const char *suffix, char **ret_new_path, char **ret_tmp_path) { @@ -6127,7 +6128,7 @@ static int unit_file_create_dropin( assert(ret_new_path); assert(ret_tmp_path); - ending = strjoina(unit_name, ".d/override.conf"); + ending = strjoina(unit_name, suffix); r = get_file_to_edit(paths, ending, &tmp_new_path); if (r < 0) return r; @@ -6180,7 +6181,6 @@ static int unit_file_create_copy( r = create_edit_temp_file(tmp_new_path, fragment_path, &tmp_tmp_path); if (r < 0) { - log_error_errno(r, "Failed to create temporary file for \"%s\": %m", tmp_new_path); free(tmp_new_path); return r; } @@ -6291,18 +6291,26 @@ static int find_paths_to_edit(sd_bus *bus, char **names, char ***paths) { r = unit_find_paths(bus, *name, &lp, &path, NULL); if (r < 0) return r; - else if (r == 0) - return -ENOENT; - else if (!path) { - // FIXME: support units with path==NULL (no FragmentPath) - log_error("No fragment exists for %s.", *name); - return -ENOENT; + else if (!arg_force) { + if (r == 0) { + log_error("Run 'systemctl edit --force %s' to create a new unit.", *name); + return -ENOENT; + } else if (!path) { + // FIXME: support units with path==NULL (no FragmentPath) + log_error("No fragment exists for %s.", *name); + return -ENOENT; + } + } + + if (path) { + if (arg_full) + r = unit_file_create_copy(&lp, *name, path, &new_path, &tmp_path); + else + r = unit_file_create_new(&lp, *name, ".d/override.conf", &new_path, &tmp_path); + } else { + r = unit_file_create_new(&lp, *name, NULL, &new_path, &tmp_path); } - if (arg_full) - r = unit_file_create_copy(&lp, *name, path, &new_path, &tmp_path); - else - r = unit_file_create_dropin(&lp, *name, &new_path, &tmp_path); if (r < 0) return r; -- cgit v1.2.3-54-g00ecf From dbf43a1b3a10b8e50eca3866687989ca5b21dabd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 24 Jun 2016 21:03:47 +0200 Subject: systemctl: fix an error condition from "-1" to something meaningful We really shouldn't make up errors like "-1", but use proper errno definitions. --- src/systemctl/systemctl.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 0a8e60c195..c0b285b58f 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -6175,7 +6175,7 @@ static int unit_file_create_copy( if (response != 'y') { log_warning("%s ignored", unit_name); free(tmp_new_path); - return -1; + return -EKEYREJECTED; } } @@ -6307,10 +6307,8 @@ static int find_paths_to_edit(sd_bus *bus, char **names, char ***paths) { r = unit_file_create_copy(&lp, *name, path, &new_path, &tmp_path); else r = unit_file_create_new(&lp, *name, ".d/override.conf", &new_path, &tmp_path); - } else { + } else r = unit_file_create_new(&lp, *name, NULL, &new_path, &tmp_path); - } - if (r < 0) return r; -- cgit v1.2.3-54-g00ecf From a1abf5ea7fa1c3c3ef80937c8d349390ce3da39d Mon Sep 17 00:00:00 2001 From: Lénaïc Huard Date: Tue, 28 Jun 2016 20:15:33 +0200 Subject: Remove blank line in the output of “systemctl show” (#3614) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit “systemctl show” added an extra blank line after the dump of the EnvironmentFile property of the unit. --- src/systemctl/systemctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index c0b285b58f..e56b9b8957 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -4367,7 +4367,7 @@ static int print_property(const char *name, sd_bus_message *m, const char *conte return bus_log_parse_error(r); while ((r = sd_bus_message_read(m, "(sb)", &path, &ignore)) > 0) - print_prop("EnvironmentFile", "%s (ignore_errors=%s)\n", path, yes_no(ignore)); + print_prop("EnvironmentFile", "%s (ignore_errors=%s)", path, yes_no(ignore)); if (r < 0) return bus_log_parse_error(r); -- cgit v1.2.3-54-g00ecf From d6568222d74bdfbeb75e37e96b64248ec0145dea Mon Sep 17 00:00:00 2001 From: Susant Sahani Date: Wed, 29 Jun 2016 01:57:07 +0530 Subject: systemctl mask of an non-existent unit should print a warning (#3521) fixes https://bugzilla.redhat.com/show_bug.cgi?id=842060 --- src/systemctl/systemctl.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index e56b9b8957..b575437bcb 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -5605,6 +5605,46 @@ static int mangle_names(char **original_names, char ***mangled_names) { return 0; } +static int unit_exists(const char *unit) { + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL, *m = NULL; + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_free_ char *path = NULL; + static const struct bus_properties_map property_map[] = { + { "LoadState", "s", NULL, offsetof(UnitStatusInfo, load_state) }, + { "ActiveState", "s", NULL, offsetof(UnitStatusInfo, active_state)}, + {}, + }; + UnitStatusInfo info = {}; + sd_bus *bus; + int r; + + path = unit_dbus_path_from_name(unit); + if (!path) + return log_oom(); + + r = acquire_bus(BUS_MANAGER, &bus); + if (r < 0) + return r; + + r = sd_bus_call_method( + bus, + "org.freedesktop.systemd1", + path, + "org.freedesktop.DBus.Properties", + "GetAll", + &error, + &reply, + "s", ""); + if (r < 0) + return log_error_errno(r, "Failed to get properties: %s", bus_error_message(&error, r)); + + r = bus_message_map_all_properties(reply, property_map, &info); + if (r < 0) + return log_error_errno(r, "Failed to map properties: %s", bus_error_message(&error, r)); + + return !streq_ptr(info.load_state, "not-found") || !streq_ptr(info.active_state, "inactive"); +} + static int enable_unit(int argc, char *argv[], void *userdata) { _cleanup_strv_free_ char **names = NULL; const char *verb = argv[0]; @@ -5666,6 +5706,14 @@ static int enable_unit(int argc, char *argv[], void *userdata) { const char *method; sd_bus *bus; + if (STR_IN_SET(verb, "mask", "unmask")) { + r = unit_exists(*names); + if (r < 0) + return r; + if (r == 0) + log_notice("Unit %s does not exist, proceeding anyway.", *names); + } + r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; -- cgit v1.2.3-54-g00ecf From d5db7fe66a349574a000f47c4bb5623064d47d86 Mon Sep 17 00:00:00 2001 From: WaLyong Cho Date: Thu, 7 Jul 2016 22:45:05 +0900 Subject: systemctl: show failed condition list When unit has multiple condition list, systemctl is not showing which conditions were failed. When user want to know which conditions were failed, user has to check for each conditions. So, show failed condition list also. --- src/systemctl/systemctl.c | 85 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 69 insertions(+), 16 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index b575437bcb..84278e9bdb 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3455,6 +3455,24 @@ static int exec_status_info_deserialize(sd_bus_message *m, ExecStatusInfo *i) { return 1; } +typedef struct UnitCondition { + char *name; + bool trigger; + bool negate; + char *param; + int tristate; + + LIST_FIELDS(struct UnitCondition, condition); +} UnitCondition; + +static void unit_condition_free(UnitCondition *c) { + assert(c); + + free(c->name); + free(c->param); + free(c); +} + typedef struct UnitStatusInfo { const char *id; const char *load_state; @@ -3501,10 +3519,7 @@ typedef struct UnitStatusInfo { usec_t condition_timestamp; bool condition_result; - bool failed_condition_trigger; - bool failed_condition_negate; - const char *failed_condition; - const char *failed_condition_parameter; + LIST_HEAD(UnitCondition, condition); usec_t assert_timestamp; bool assert_result; @@ -3664,19 +3679,32 @@ static void print_status_info( printf("\n"); if (!i->condition_result && i->condition_timestamp > 0) { + UnitCondition *c; + int n = 0; + s1 = format_timestamp_relative(since1, sizeof(since1), i->condition_timestamp); s2 = format_timestamp(since2, sizeof(since2), i->condition_timestamp); printf("Condition: start %scondition failed%s at %s%s%s\n", ansi_highlight_yellow(), ansi_normal(), s2, s1 ? "; " : "", strempty(s1)); - if (i->failed_condition_trigger) - printf(" none of the trigger conditions were met\n"); - else if (i->failed_condition) - printf(" %s=%s%s was not met\n", - i->failed_condition, - i->failed_condition_negate ? "!" : "", - i->failed_condition_parameter); + + LIST_FOREACH(condition, c, i->condition) { + if (c->tristate < 0) + n++; + } + + LIST_FOREACH(condition, c, i->condition) { + if (c->tristate >= 0) + continue; + + printf(" %s %s=%s%s%s was not met\n", + --n ? special_glyph(TREE_BRANCH) : special_glyph(TREE_RIGHT), + c->name, + c->trigger ? "|" : "", + c->negate ? "!" : "", + c->param); + } } if (!i->assert_result && i->assert_timestamp > 0) { @@ -4169,13 +4197,32 @@ static int status_property(const char *name, sd_bus_message *m, UnitStatusInfo * return bus_log_parse_error(r); while ((r = sd_bus_message_read(m, "(sbbsi)", &cond, &trigger, &negate, ¶m, &state)) > 0) { + UnitCondition *c; + log_debug("%s %d %d %s %d", cond, trigger, negate, param, state); - if (state < 0 && (!trigger || !i->failed_condition)) { - i->failed_condition = cond; - i->failed_condition_trigger = trigger; - i->failed_condition_negate = negate; - i->failed_condition_parameter = param; + + c = new0(UnitCondition, 1); + if (!c) + return log_oom(); + + c->name = strdup(cond); + if (!c->name) { + free(c); + return log_oom(); } + + c->param = strdup(param); + if (!c->param) { + free(c->name); + free(c); + return log_oom(); + } + + c->trigger = trigger; + c->negate = negate; + c->tristate = state; + + LIST_PREPEND(condition, i->condition, c); } if (r < 0) return bus_log_parse_error(r); @@ -4583,6 +4630,7 @@ static int show_one( .tasks_max = (uint64_t) -1, }; ExecStatusInfo *p; + UnitCondition *c; int r; assert(path); @@ -4701,6 +4749,11 @@ static int show_one( strv_free(info.dropin_paths); strv_free(info.listen); + while ((c = info.condition)) { + LIST_REMOVE(condition, info.condition, c); + unit_condition_free(c); + } + while ((p = info.exec)) { LIST_REMOVE(exec, info.exec, p); exec_status_info_free(p); -- cgit v1.2.3-54-g00ecf From ba19c6e1819a89d3b90811ce4fd785a606dfd223 Mon Sep 17 00:00:00 2001 From: Thomas Hindoe Paaboel Andersen Date: Mon, 18 Jul 2016 22:31:34 +0200 Subject: treewide: remove unused variables --- src/machine/machinectl.c | 1 - src/nspawn/nspawn.c | 1 - src/systemctl/systemctl.c | 2 +- src/test/test-process-util.c | 2 +- 4 files changed, 2 insertions(+), 4 deletions(-) (limited to 'src/systemctl') diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index 161dd3922b..96e0ab4b8a 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -1551,7 +1551,6 @@ static int image_exists(sd_bus *bus, const char *name) { } static int make_service_name(const char *name, char **ret) { - _cleanup_free_ char *e = NULL; int r; assert(name); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index bedc5bf20b..e4be0a2251 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3125,7 +3125,6 @@ static int setup_uid_map(pid_t pid) { } static int nspawn_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata) { - _cleanup_fdset_free_ FDSet *fds = NULL; char buf[NOTIFY_BUFFER_MAX+1]; char *p = NULL; struct iovec iovec = { diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index b575437bcb..d3f437411a 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -5606,7 +5606,7 @@ static int mangle_names(char **original_names, char ***mangled_names) { } static int unit_exists(const char *unit) { - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL, *m = NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_free_ char *path = NULL; static const struct bus_properties_map property_map[] = { diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c index 99c92780b8..562ad4acb8 100644 --- a/src/test/test-process-util.c +++ b/src/test/test-process-util.c @@ -45,7 +45,7 @@ static void test_get_process_comm(pid_t pid) { struct stat st; - _cleanup_free_ char *a = NULL, *c = NULL, *d = NULL, *f = NULL, *i = NULL, *cwd = NULL, *root = NULL; + _cleanup_free_ char *a = NULL, *c = NULL, *d = NULL, *f = NULL, *i = NULL; _cleanup_free_ char *env = NULL; char path[strlen("/proc//comm") + DECIMAL_STR_MAX(pid_t)]; pid_t e; -- cgit v1.2.3-54-g00ecf From 1089dcd46966c83d415bb0b8353a8ce543a3bb7f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 21 Jul 2016 18:05:58 +0200 Subject: systemctl: fix output alignment in "systemctl status" If we show both a control and a main PID for a service fix this line in the output of "systemctl status": Main PID: 19670 (sleep); : 19671 (sleep) to become this: Main PID: 19670 (sleep); Control PID: 19671 (sleep) --- src/systemctl/systemctl.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index d3f437411a..ab3c4fb585 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3761,7 +3761,7 @@ static void print_status_info( if (i->running) { _cleanup_free_ char *comm = NULL; - get_process_comm(i->main_pid, &comm); + (void) get_process_comm(i->main_pid, &comm); if (comm) printf(" (%s)", comm); } else if (i->exit_code > 0) { @@ -3780,17 +3780,19 @@ static void print_status_info( printf("signal=%s", signal_to_string(i->exit_status)); printf(")"); } - - if (i->control_pid > 0) - printf(";"); } if (i->control_pid > 0) { _cleanup_free_ char *c = NULL; - printf(" %8s: "PID_FMT, i->main_pid ? "" : " Control", i->control_pid); + if (i->main_pid > 0) + fputs("; Control PID: ", stdout); + else + fputs("Cntrl PID: ", stdout); /* if first in column, abbreviated so it fits alignment */ + + printf(PID_FMT, i->control_pid); - get_process_comm(i->control_pid, &c); + (void) get_process_comm(i->control_pid, &c); if (c) printf(" (%s)", c); } -- cgit v1.2.3-54-g00ecf From 2a6736ddd080674170e9b9fe225009a0476c68e1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 21 Jul 2016 18:08:52 +0200 Subject: systemctl: fix format string for uint64_t field --- src/systemctl/systemctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index ab3c4fb585..ff76d8287a 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3809,7 +3809,7 @@ static void print_status_info( printf(" Tasks: %" PRIu64, i->tasks_current); if (i->tasks_max != (uint64_t) -1) - printf(" (limit: %" PRIi64 ")\n", i->tasks_max); + printf(" (limit: %" PRIu64 ")\n", i->tasks_max); else printf("\n"); } -- cgit v1.2.3-54-g00ecf From e08ab37902bbd6e46ad1365f8ffa87fd700683e2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Jul 2016 17:39:21 +0200 Subject: systemctl: never check inhibitors if -H or -M are used (#3781) Don't check inhibitors when operating remotely. The interactivity inhibitors imply can#t be provided anyway, and the current code checks for local sessions directly, via various sd_session_xyz() APIs, hence bypass it entirely if we operate on remote systems. Fixes: #3476 --- src/systemctl/systemctl.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index d3f437411a..1dbe35997a 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3033,6 +3033,9 @@ static int logind_check_inhibitors(enum action a) { if (!on_tty()) return 0; + if (arg_transport != BUS_TRANSPORT_LOCAL) + return 0; + r = acquire_bus(BUS_FULL, &bus); if (r < 0) return r; -- cgit v1.2.3-54-g00ecf From d710aaf7a5d74afb3135f2f79080bd4715790c59 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Fri, 22 Jul 2016 20:27:45 -0400 Subject: Use "return log_error_errno" in more places" --- src/coredump/coredump.c | 6 ++---- src/fstab-generator/fstab-generator.c | 26 ++++++++++++-------------- src/gpt-auto-generator/gpt-auto-generator.c | 6 ++---- src/login/logind-user.c | 3 +-- src/nspawn/nspawn-seccomp.c | 6 ++---- src/nspawn/nspawn-setuid.c | 7 ++----- src/shared/conf-parser.c | 3 +-- src/systemctl/systemctl.c | 6 ++---- src/tmpfiles/tmpfiles.c | 5 ++--- 9 files changed, 26 insertions(+), 42 deletions(-) (limited to 'src/systemctl') diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index e5719e67c3..043d785dd4 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -157,10 +157,8 @@ static int fix_acl(int fd, uid_t uid) { if (acl_create_entry(&acl, &entry) < 0 || acl_set_tag_type(entry, ACL_USER) < 0 || - acl_set_qualifier(entry, &uid) < 0) { - log_error_errno(errno, "Failed to patch ACL: %m"); - return -errno; - } + acl_set_qualifier(entry, &uid) < 0) + return log_error_errno(errno, "Failed to patch ACL: %m"); if (acl_get_permset(entry, &permset) < 0 || acl_add_perm(permset, ACL_READ) < 0) diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c index 5aeca7e2d5..33af553d0d 100644 --- a/src/fstab-generator/fstab-generator.c +++ b/src/fstab-generator/fstab-generator.c @@ -85,13 +85,12 @@ static int add_swap( return log_oom(); f = fopen(unit, "wxe"); - if (!f) { - if (errno == EEXIST) - log_error("Failed to create swap unit file %s, as it already exists. Duplicate entry in /etc/fstab?", unit); - else - log_error_errno(errno, "Failed to create unit file %s: %m", unit); - return -errno; - } + if (!f) + return log_error_errno(errno, + errno == EEXIST ? + "Failed to create swap unit file %s, as it already exists. Duplicate entry in /etc/fstab?" : + "Failed to create unit file %s: %m", + unit); fprintf(f, "# Automatically generated by systemd-fstab-generator\n\n" @@ -281,13 +280,12 @@ static int add_mount( return log_oom(); f = fopen(unit, "wxe"); - if (!f) { - if (errno == EEXIST) - log_error("Failed to create mount unit file %s, as it already exists. Duplicate entry in /etc/fstab?", unit); - else - log_error_errno(errno, "Failed to create unit file %s: %m", unit); - return -errno; - } + if (!f) + return log_error_errno(errno, + errno == EEXIST ? + "Failed to create mount unit file %s, as it already exists. Duplicate entry in /etc/fstab?" : + "Failed to create unit file %s: %m", + unit); fprintf(f, "# Automatically generated by systemd-fstab-generator\n\n" diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index a4938a7c3a..39355de953 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -489,10 +489,8 @@ static int add_boot(const char *what) { return 0; } - if (r < 0) { - log_error_errno(r, "Failed to read ESP partition UUID: %m"); - return r; - } + if (r < 0) + return log_error_errno(r, "Failed to read ESP partition UUID: %m"); errno = 0; b = blkid_new_probe_from_filename(what); diff --git a/src/login/logind-user.c b/src/login/logind-user.c index f7d83909a0..348e396292 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -311,8 +311,7 @@ int user_load(User *u) { if (r == -ENOENT) return 0; - log_error_errno(r, "Failed to read %s: %m", u->state_file); - return r; + return log_error_errno(r, "Failed to read %s: %m", u->state_file); } if (display) diff --git a/src/nspawn/nspawn-seccomp.c b/src/nspawn/nspawn-seccomp.c index 54db1b47f8..3ab7160ebe 100644 --- a/src/nspawn/nspawn-seccomp.c +++ b/src/nspawn/nspawn-seccomp.c @@ -119,10 +119,8 @@ static int seccomp_add_default_syscall_filter(scmp_filter_ctx ctx, r = seccomp_rule_add(ctx, SCMP_ACT_ERRNO(EPERM), blacklist[i].syscall_num, 0); if (r == -EFAULT) continue; /* unknown syscall */ - if (r < 0) { - log_error_errno(r, "Failed to block syscall: %m"); - return r; - } + if (r < 0) + return log_error_errno(r, "Failed to block syscall: %m"); } return 0; diff --git a/src/nspawn/nspawn-setuid.c b/src/nspawn/nspawn-setuid.c index ee15a47e93..b8e8e091c8 100644 --- a/src/nspawn/nspawn-setuid.c +++ b/src/nspawn/nspawn-setuid.c @@ -124,14 +124,12 @@ int change_uid_gid(const char *user, char **_home) { fd = -1; if (!fgets(line, sizeof(line), f)) { - if (!ferror(f)) { log_error("Failed to resolve user %s.", user); return -ESRCH; } - log_error_errno(errno, "Failed to read from getent: %m"); - return -errno; + return log_error_errno(errno, "Failed to read from getent: %m"); } truncate_nl(line); @@ -214,8 +212,7 @@ int change_uid_gid(const char *user, char **_home) { return -ESRCH; } - log_error_errno(errno, "Failed to read from getent: %m"); - return -errno; + return log_error_errno(errno, "Failed to read from getent: %m"); } truncate_nl(line); diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index d85ab5441e..7cf222e4d2 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -323,8 +323,7 @@ int config_parse(const char *unit, if (feof(f)) break; - log_error_errno(errno, "Failed to read configuration file '%s': %m", filename); - return -errno; + return log_error_errno(errno, "Failed to read configuration file '%s': %m", filename); } l = buf; diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 92ba6aa4e4..5298fcfb9c 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -5537,10 +5537,8 @@ static int enable_sysv_units(const char *verb, char **args) { } j = wait_for_terminate(pid, &status); - if (j < 0) { - log_error_errno(j, "Failed to wait for child: %m"); - return j; - } + if (j < 0) + return log_error_errno(j, "Failed to wait for child: %m"); if (status.si_code == CLD_EXITED) { if (streq(verb, "is-enabled")) { diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index bfb6293b3d..954f4aa985 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1575,13 +1575,12 @@ static int clean_item_instance(Item *i, const char* instance) { d = opendir_nomod(instance); if (!d) { - if (errno == ENOENT || errno == ENOTDIR) { + if (IN_SET(errno, ENOENT, ENOTDIR)) { log_debug_errno(errno, "Directory \"%s\": %m", instance); return 0; } - log_error_errno(errno, "Failed to open directory %s: %m", instance); - return -errno; + return log_error_errno(errno, "Failed to open directory %s: %m", instance); } if (fstat(dirfd(d), &s) < 0) -- cgit v1.2.3-54-g00ecf From 3990961df02a255cb75cc80445535d153fc7f165 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Jul 2016 15:10:15 +0200 Subject: man: update systemctl man page for unit file commands, in particular "systemctl enable" Clarify that "systemctl enable" can operate either on unit names or on unit file paths (also, adjust the --help text to clarify this). Say that "systemctl enable" on unit file paths also links the unit into the search path. Many other fixes. This should improve the documentation to avoid further confusion around #3706. --- man/systemctl.xml | 187 +++++++++++++++++++++------------------------- src/systemctl/systemctl.c | 2 +- 2 files changed, 88 insertions(+), 101 deletions(-) (limited to 'src/systemctl') diff --git a/man/systemctl.xml b/man/systemctl.xml index 742da81cfe..c7b830b7fb 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -973,70 +973,62 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service list-unit-files PATTERN... - List unit files installed in the file system and their enablement state - (as reported by is-enabled). If one or more - PATTERNs are specified, only units whose filename - (just the last component of the path) matches one of them are shown. + List unit files installed on the system, in combination with their enablement state (as reported by + is-enabled). If one or more PATTERNs are specified, only unit + files whose name matches one of them are shown (patterns matching unit file system paths are not + supported). enable NAME... + enable PATH... - Enable one or more unit files or unit file instances, - as specified on the command line. This will create a number - of symlinks as encoded in the [Install] - sections of the unit files. After the symlinks have been - created, the systemd configuration is reloaded (in a way that - is equivalent to daemon-reload) to ensure - the changes are taken into account immediately. Note that - this does not have the effect of also - starting any of the units being enabled. If this - is desired, either should be used - together with this command, or an additional start - command must be invoked for the unit. Also note that, in case of - instance enablement, symlinks named the same as instances - are created in the install location, however they all point to the - same template unit file. - - This command will print the actions executed. This - output may be suppressed by passing . + Enable one or more units or unit instances. This will create a set of symlinks, as encoded in the + [Install] sections of the indicated unit files. After the symlinks have been created, + the system manager configuration is reloaded (in a way equivalent to daemon-reload), in + order to ensure the changes are taken into account immediately. Note that this does + not have the effect of also starting any of the units being enabled. If this is + desired, combine this command with the switch, or invoke start + with appropriate arguments later. Note that in case of unit instance enablement (i.e. enablement of units of + the form foo@bar.service), symlinks named the same as instances are created in the + unit configuration diectory, however they point to the single template unit file they are instantiated + from. + + This command expects either valid unit names (in which case appropriate unit files for these names + are automatically searched in the various unit file directories), or absolute paths to unit files (in which + case these files are read directly). If a specified unit file is located outside of the unit file + directories searched automatically, an additional symlink is created, linking it into the unit + configuration path, thus ensuring it is automatically found when requested by commands such as + start. + + This command will print the file system operations executed. This output may be suppressed by passing + . - Note that this operation creates only the suggested - symlinks for the units. While this command is the - recommended way to manipulate the unit configuration - directory, the administrator is free to make additional - changes manually by placing or removing symlinks in the - directory. This is particularly useful to create - configurations that deviate from the suggested default - installation. In this case, the administrator must make sure - to invoke daemon-reload manually as - necessary to ensure the changes are taken into account. + Note that this operation creates only the symlinks suggested in the [Install] + section of the unit files. While this command is the recommended way to manipulate the unit configuration + directory, the administrator is free to make additional changes manually by placing or removing symlinks + below this directory. This is particularly useful to create configurations that deviate from the suggested + default installation. In this case, the administrator must make sure to invoke + daemon-reload manually as necessary, in order to ensure the changes are taken into + account. - Enabling units should not be confused with starting - (activating) units, as done by the start - command. Enabling and starting units is orthogonal: units - may be enabled without being started and started without - being enabled. Enabling simply hooks the unit into various - suggested places (for example, so that the unit is - automatically started on boot or when a particular kind of - hardware is plugged in). Starting actually spawns the daemon - process (in case of service units), or binds the socket (in - case of socket units), and so on. - - Depending on whether , - , , - or is specified, this enables the unit - for the system, for the calling user only, for only this boot of - the system, or for all future logins of all users, or only this - boot. Note that in the last case, no systemd daemon - configuration is reloaded. - - Using enable on masked units - results in an error. + Enabling units should not be confused with starting (activating) units, as done by the + start command. Enabling and starting units is orthogonal: units may be enabled without + being started and started without being enabled. Enabling simply hooks the unit into various suggested + places (for example, so that the unit is automatically started on boot or when a particular kind of + hardware is plugged in). Starting actually spawns the daemon process (in case of service units), or binds + the socket (in case of socket units), and so on. + + Depending on whether , , , + or is specified, this enables the unit for the system, for the calling user only, + for only this boot of the system, or for all future logins of all users, or only this boot. Note that in + the last case, no systemd daemon configuration is reloaded. + + Using enable on masked units is not supported and results in an error. @@ -1044,28 +1036,31 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service disable NAME... - Disables one or more units. This removes all symlinks - to the specified unit files from the unit configuration - directory, and hence undoes the changes made by - enable. Note however that this removes - all symlinks to the unit files (i.e. including manual - additions), not just those actually created by - enable. This call implicitly reloads the - systemd daemon configuration after completing the disabling - of the units. Note that this command does not implicitly - stop the units that are being disabled. If this is desired, either - should be used together with this command, or - an additional stop command should be executed - afterwards. - - This command will print the actions executed. This - output may be suppressed by passing . + Disables one or more units. This removes all symlinks to the unit files backing the specified units + from the unit configuration directory, and hence undoes any changes made by enable or + link. Note that this removes all symlinks to matching unit files, + including manually created symlinks, and not just those actually created by enable or + link. Note that while disable undoes the effect of + enable, the two commands are otherwise not symmetric, as disable may + remove more symlinks than a prior enable invocation of the same unit created. + + This command expects valid unit names only, it does not accept paths to unit files. + + In addition to the units specified as arguments, all units are disabled that are listed in the + Also= setting contained in the [Install] section of any of the unit + files being operated on. + + This command implicitly reloads the system manager configuration after completing the operation. Note + that this command does not implicitly stop the units that are being disabled. If this is desired, either + combine this command with the switch, or invoke the stop command + with appropriate arguments later. + + This command will print information about the file system operations (symlink removals) + executed. This output may be suppressed by passing . - This command honors , - , and - in a similar way as - enable. + This command honors , , + and in a similar way as enable. @@ -1073,12 +1068,10 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service reenable NAME... - Reenable one or more unit files, as specified on the - command line. This is a combination of - disable and enable and - is useful to reset the symlinks a unit is enabled with to - the defaults configured in the [Install] - section of the unit file. + Reenable one or more units, as specified on the command line. This is a combination of + disable and enable and is useful to reset the symlinks a unit file is + enabled with to the defaults configured in its [Install] section. This commands expects + a unit uname only, it does not accept paths to unit files. @@ -1209,16 +1202,13 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service mask NAME... - Mask one or more unit files, as specified on the - command line. This will link these units to - /dev/null, making it impossible to - start them. This is a stronger version of - disable, since it prohibits all kinds of - activation of the unit, including enablement and manual - activation. Use this option with care. This honors the - option to only mask temporarily - until the next reboot of the system. The - option can be used to ensure that the units are also stopped. + Mask one or more units, as specified on the command line. This will link these unit files to + /dev/null, making it impossible to start them. This is a stronger version of + disable, since it prohibits all kinds of activation of the unit, including enablement + and manual activation. Use this option with care. This honors the option to only + mask temporarily until the next reboot of the system. The option may be used to + ensure that the units are also stopped. This command expects valid unit names only, it does not accept unit + file paths. @@ -1226,23 +1216,20 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service unmask NAME... - Unmask one or more unit files, as specified on the - command line. This will undo the effect of - mask. + Unmask one or more unit files, as specified on the command line. This will undo the effect of + mask. This command expects valid unit names only, it does not accept unit file + paths. - link FILENAME... + link PATH... - Link a unit file that is not in the unit file search - paths into the unit file search path. This requires an - absolute path to a unit file. The effect of this can be - undone with disable. The effect of this - command is that a unit file is available for - start and other commands although it - is not installed directly in the unit search path. + Link a unit file that is not in the unit file search paths into the unit file search path. This + command expects an absolute path to a unit file. The effect of this may be undone with + disable. The effect of this command is that a unit file is made available for commands + such as start, even though it is not installed directly in the unit search path. diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 5298fcfb9c..bfd770e4df 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -6533,7 +6533,7 @@ static void systemctl_help(void) { " unit is required or wanted\n\n" "Unit File Commands:\n" " list-unit-files [PATTERN...] List installed unit files\n" - " enable NAME... Enable one or more unit files\n" + " enable [NAME...|PATH...] Enable one or more unit files\n" " disable NAME... Disable one or more unit files\n" " reenable NAME... Reenable one or more unit files\n" " preset NAME... Enable/disable one or more unit files\n" -- cgit v1.2.3-54-g00ecf From b1ed76ae19bbbe9b836f3dae700cf610ce5dd869 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Mon, 25 Jul 2016 11:20:58 -0400 Subject: systemctl: style tweaks for the new condition code --- src/systemctl/systemctl.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 84278e9bdb..d53450471e 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3457,12 +3457,12 @@ static int exec_status_info_deserialize(sd_bus_message *m, ExecStatusInfo *i) { typedef struct UnitCondition { char *name; + char *param; bool trigger; bool negate; - char *param; int tristate; - LIST_FIELDS(struct UnitCondition, condition); + LIST_FIELDS(struct UnitCondition, conditions); } UnitCondition; static void unit_condition_free(UnitCondition *c) { @@ -3519,7 +3519,7 @@ typedef struct UnitStatusInfo { usec_t condition_timestamp; bool condition_result; - LIST_HEAD(UnitCondition, condition); + LIST_HEAD(UnitCondition, conditions); usec_t assert_timestamp; bool assert_result; @@ -3689,22 +3689,18 @@ static void print_status_info( ansi_highlight_yellow(), ansi_normal(), s2, s1 ? "; " : "", strempty(s1)); - LIST_FOREACH(condition, c, i->condition) { + LIST_FOREACH(conditions, c, i->conditions) if (c->tristate < 0) n++; - } - - LIST_FOREACH(condition, c, i->condition) { - if (c->tristate >= 0) - continue; - printf(" %s %s=%s%s%s was not met\n", - --n ? special_glyph(TREE_BRANCH) : special_glyph(TREE_RIGHT), - c->name, - c->trigger ? "|" : "", - c->negate ? "!" : "", - c->param); - } + LIST_FOREACH(conditions, c, i->conditions) + if (c->tristate < 0) + printf(" %s %s=%s%s%s was not met\n", + --n ? special_glyph(TREE_BRANCH) : special_glyph(TREE_RIGHT), + c->name, + c->trigger ? "|" : "", + c->negate ? "!" : "", + c->param); } if (!i->assert_result && i->assert_timestamp > 0) { @@ -4199,7 +4195,7 @@ static int status_property(const char *name, sd_bus_message *m, UnitStatusInfo * while ((r = sd_bus_message_read(m, "(sbbsi)", &cond, &trigger, &negate, ¶m, &state)) > 0) { UnitCondition *c; - log_debug("%s %d %d %s %d", cond, trigger, negate, param, state); + log_debug("%s trigger=%d negate=%d %s →%d", cond, trigger, negate, param, state); c = new0(UnitCondition, 1); if (!c) @@ -4222,7 +4218,7 @@ static int status_property(const char *name, sd_bus_message *m, UnitStatusInfo * c->negate = negate; c->tristate = state; - LIST_PREPEND(condition, i->condition, c); + LIST_PREPEND(conditions, i->conditions, c); } if (r < 0) return bus_log_parse_error(r); @@ -4749,8 +4745,8 @@ static int show_one( strv_free(info.dropin_paths); strv_free(info.listen); - while ((c = info.condition)) { - LIST_REMOVE(condition, info.condition, c); + while ((c = info.conditions)) { + LIST_REMOVE(conditions, info.conditions, c); unit_condition_free(c); } -- cgit v1.2.3-54-g00ecf From a733551846bcb2c7f46cc68bbc2e5741e653fbef Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Mon, 25 Jul 2016 11:53:14 -0400 Subject: systemctl: use cleanup function for UnitStatusInfo There is no functional change, but clarity of the code is increased by splitting out the cleanup part and putting it next to the structure definition. --- src/systemctl/systemctl.c | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 7df2fa5421..e1bcef2d95 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3561,6 +3561,25 @@ typedef struct UnitStatusInfo { LIST_HEAD(ExecStatusInfo, exec); } UnitStatusInfo; +static void unit_status_info_free(UnitStatusInfo *info) { + ExecStatusInfo *p; + UnitCondition *c; + + strv_free(info->documentation); + strv_free(info->dropin_paths); + strv_free(info->listen); + + while ((c = info->conditions)) { + LIST_REMOVE(conditions, info->conditions, c); + unit_condition_free(c); + } + + while ((p = info->exec)) { + LIST_REMOVE(exec, info->exec, p); + exec_status_info_free(p); + } +} + static void print_status_info( sd_bus *bus, UnitStatusInfo *i, @@ -4621,7 +4640,7 @@ static int show_one( _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_set_free_ Set *found_properties = NULL; - UnitStatusInfo info = { + _cleanup_(unit_status_info_free) UnitStatusInfo info = { .memory_current = (uint64_t) -1, .memory_high = CGROUP_LIMIT_MAX, .memory_max = CGROUP_LIMIT_MAX, @@ -4630,8 +4649,6 @@ static int show_one( .tasks_current = (uint64_t) -1, .tasks_max = (uint64_t) -1, }; - ExecStatusInfo *p; - UnitCondition *c; int r; assert(path); @@ -4725,16 +4742,15 @@ static int show_one( return bus_log_parse_error(r); r = 0; - if (show_properties) { char **pp; - STRV_FOREACH(pp, arg_properties) { + STRV_FOREACH(pp, arg_properties) if (!set_contains(found_properties, *pp)) { log_warning("Property %s does not exist.", *pp); r = -ENXIO; } - } + } else if (streq(verb, "help")) show_unit_help(&info); else if (streq(verb, "status")) { @@ -4746,20 +4762,6 @@ static int show_one( r = EXIT_PROGRAM_RUNNING_OR_SERVICE_OK; } - strv_free(info.documentation); - strv_free(info.dropin_paths); - strv_free(info.listen); - - while ((c = info.conditions)) { - LIST_REMOVE(conditions, info.conditions, c); - unit_condition_free(c); - } - - while ((p = info.exec)) { - LIST_REMOVE(exec, info.exec, p); - exec_status_info_free(p); - } - return r; } -- cgit v1.2.3-54-g00ecf From 662bea6729d4147dfdd1501f81335adf5a2a6012 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Mon, 25 Jul 2016 12:15:57 -0400 Subject: systemctl: avoid "leaking" some strings in UnitStatusInfo % valgrind --leak-check=full systemctl status multipathd.service --no-pager -n0 ... ==431== 16 bytes in 2 blocks are definitely lost in loss record 1 of 2 ==431== at 0x4C2BBAD: malloc (vg_replace_malloc.c:299) ==431== by 0x534AF19: strdup (in /usr/lib64/libc-2.23.so) ==431== by 0x4E81AEE: free_and_strdup (string-util.c:794) ==431== by 0x4EF66C1: map_basic (bus-util.c:1030) ==431== by 0x4EF6A8E: bus_message_map_all_properties (bus-util.c:1153) ==431== by 0x120487: show_one (systemctl.c:4672) ==431== by 0x1218F3: show (systemctl.c:4990) ==431== by 0x4EC359E: dispatch_verb (verbs.c:92) ==431== by 0x12A3AE: systemctl_main (systemctl.c:7742) ==431== by 0x12B1A8: main (systemctl.c:8011) ==431== ==431== LEAK SUMMARY: ==431== definitely lost: 16 bytes in 2 blocks This happens because map_basic() strdups the strings. Other code in systemctl assigns strings to UnitStatusInfo without copying them, relying on the fact that the message is longer lived than UnitStatusInfo. Add a helper function that is similar to map_basic, but only accepts strings and does not copy them. The alternative of continuing to use map_basic() but adding proper cleanup to free fields in UnitStatusInfo seems less attractive because it'd require changing a lot of code and doing a lot of more allocations for little gain. (I put "leaking" in quotes, because systemctl is short lived anyway.) --- src/systemctl/systemctl.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index e1bcef2d95..d4ec1da290 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -224,6 +224,21 @@ static void release_busses(void) { busses[w] = sd_bus_flush_close_unref(busses[w]); } +static int map_string_no_copy(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { + char *s; + const char **p = userdata; + int r; + + r = sd_bus_message_read_basic(m, SD_BUS_TYPE_STRING, &s); + if (r < 0) + return r; + + if (!isempty(s)) + *p = s; + + return 0; +} + static void ask_password_agent_open_if_enabled(void) { /* Open the password agent as a child process if necessary */ @@ -4632,8 +4647,8 @@ static int show_one( bool *ellipsized) { static const struct bus_properties_map property_map[] = { - { "LoadState", "s", NULL, offsetof(UnitStatusInfo, load_state) }, - { "ActiveState", "s", NULL, offsetof(UnitStatusInfo, active_state) }, + { "LoadState", "s", map_string_no_copy, offsetof(UnitStatusInfo, load_state) }, + { "ActiveState", "s", map_string_no_copy, offsetof(UnitStatusInfo, active_state) }, {} }; @@ -5664,8 +5679,8 @@ static int unit_exists(const char *unit) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_free_ char *path = NULL; static const struct bus_properties_map property_map[] = { - { "LoadState", "s", NULL, offsetof(UnitStatusInfo, load_state) }, - { "ActiveState", "s", NULL, offsetof(UnitStatusInfo, active_state)}, + { "LoadState", "s", map_string_no_copy, offsetof(UnitStatusInfo, load_state) }, + { "ActiveState", "s", map_string_no_copy, offsetof(UnitStatusInfo, active_state)}, {}, }; UnitStatusInfo info = {}; -- cgit v1.2.3-54-g00ecf From f8654baa084e8888bc5ba11e8ef3d9784f955cff Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Mon, 25 Jul 2016 12:21:50 -0400 Subject: systemctl: simplify machine_info_clear It is only used with info allocated on the stack, so the pointer cannot be NULL. --- src/systemctl/systemctl.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index d4ec1da290..44a3708a62 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1835,12 +1835,12 @@ static const struct bus_properties_map machine_info_property_map[] = { }; static void machine_info_clear(struct machine_info *info) { - if (info) { - free(info->name); - free(info->state); - free(info->control_group); - zero(*info); - } + assert(info); + + free(info->name); + free(info->state); + free(info->control_group); + zero(*info); } static void free_machines_list(struct machine_info *machine_infos, int n) { -- cgit v1.2.3-54-g00ecf From 9bb7194019ab62b85f76df2be207d3dac58ed726 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Mon, 25 Jul 2016 12:29:20 -0400 Subject: systemctl: use _cleanup_ for UnitCondition --- src/systemctl/systemctl.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 44a3708a62..6a0ed79a53 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3484,13 +3484,16 @@ typedef struct UnitCondition { } UnitCondition; static void unit_condition_free(UnitCondition *c) { - assert(c); + if (!c) + return; free(c->name); free(c->param); free(c); } +DEFINE_TRIVIAL_CLEANUP_FUNC(UnitCondition*, unit_condition_free); + typedef struct UnitStatusInfo { const char *id; const char *load_state; @@ -4232,7 +4235,7 @@ static int status_property(const char *name, sd_bus_message *m, UnitStatusInfo * return bus_log_parse_error(r); while ((r = sd_bus_message_read(m, "(sbbsi)", &cond, &trigger, &negate, ¶m, &state)) > 0) { - UnitCondition *c; + _cleanup_(unit_condition_freep) UnitCondition *c = NULL; log_debug("%s trigger=%d negate=%d %s →%d", cond, trigger, negate, param, state); @@ -4241,23 +4244,16 @@ static int status_property(const char *name, sd_bus_message *m, UnitStatusInfo * return log_oom(); c->name = strdup(cond); - if (!c->name) { - free(c); - return log_oom(); - } - c->param = strdup(param); - if (!c->param) { - free(c->name); - free(c); + if (!c->name || !c->param) return log_oom(); - } c->trigger = trigger; c->negate = negate; c->tristate = state; LIST_PREPEND(conditions, i->conditions, c); + c = NULL; } if (r < 0) return bus_log_parse_error(r); -- cgit v1.2.3-54-g00ecf