From 3d083b2245b0b8e52f2d8ccc3e55246f41f1f544 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 25 Jan 2018 15:14:25 +0100 Subject: [PATCH 1/2] systemctl: load unit if needed in "systemctl is-active" Previously, we'd explicitly use "GetUnit()" on the server side to convert a unit name into a bus path, as that function will return an error if the unit is not currently loaded. If we'd convert the path on the client side, and access the unit this way directly the unit would be loaded automatically in the background. The old logic was done in order to minimize the effect of "is-active" on the system, i.e. that a monoitoring command does not itself alter the state of the system. however, this is problematic as this can lead to confusing results if the queried unit name is an alias that currently is not loaded: we'd claim the unit wasn't active even though this isn't strictly true: the unit the name is an alias for might be. Hence, let's simplify the code, and accept that we might end up loading a unit briefly here, and let's make "systemctl is-active" skip the GetUnit() thing and calculate the unit path right away. Fixes: #7875 --- src/systemctl/systemctl.c | 55 ++++++++++++--------------------------- 1 file changed, 16 insertions(+), 39 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 30077d319d..5732d88a17 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2650,55 +2650,32 @@ static int unit_find_paths( static int get_state_one_unit(sd_bus *bus, const char *name, UnitActiveState *active_state) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - _cleanup_free_ char *buf = NULL; + _cleanup_free_ char *buf = NULL, *path = NULL; UnitActiveState state; - const char *path; int r; assert(name); assert(active_state); - /* We don't use unit_dbus_path_from_name() directly since we don't want to load the unit unnecessarily, if it - * isn't loaded. */ - r = sd_bus_call_method( + path = unit_dbus_path_from_name(name); + if (!path) + return log_oom(); + + r = sd_bus_get_property_string( bus, "org.freedesktop.systemd1", - "/org/freedesktop/systemd1", - "org.freedesktop.systemd1.Manager", - "GetUnit", + path, + "org.freedesktop.systemd1.Unit", + "ActiveState", &error, - &reply, - "s", name); - if (r < 0) { - if (!sd_bus_error_has_name(&error, BUS_ERROR_NO_SUCH_UNIT)) - return log_error_errno(r, "Failed to retrieve unit: %s", bus_error_message(&error, r)); + &buf); + if (r < 0) + return log_error_errno(r, "Failed to retrieve unit state: %s", bus_error_message(&error, r)); - /* The unit is currently not loaded, hence say it's "inactive", since all units that aren't loaded are - * considered inactive. */ - state = UNIT_INACTIVE; - - } else { - r = sd_bus_message_read(reply, "o", &path); - if (r < 0) - return bus_log_parse_error(r); - - r = sd_bus_get_property_string( - bus, - "org.freedesktop.systemd1", - path, - "org.freedesktop.systemd1.Unit", - "ActiveState", - &error, - &buf); - if (r < 0) - return log_error_errno(r, "Failed to retrieve unit state: %s", bus_error_message(&error, r)); - - state = unit_active_state_from_string(buf); - if (state == _UNIT_ACTIVE_STATE_INVALID) { - log_error("Invalid unit state '%s' for: %s", buf, name); - return -EINVAL; - } + state = unit_active_state_from_string(buf); + if (state == _UNIT_ACTIVE_STATE_INVALID) { + log_error("Invalid unit state '%s' for: %s", buf, name); + return -EINVAL; } *active_state = state; From 71c9f49d730c8e8d072258f5ba30fc1ff360ce91 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 25 Jan 2018 15:18:27 +0100 Subject: [PATCH 2/2] Revert "man: mention that systemctl is-active or is-failed do not load units" This reverts commit c7612b20052d9151f60a96623b8743cbac88390d. --- man/systemctl.xml | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/man/systemctl.xml b/man/systemctl.xml index 375b91a9bb..60882e5aa3 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -877,10 +877,6 @@ Sun 2017-02-26 20:57:49 EST 2h 3min left Sun 2017-02-26 11:56:36 EST 6h ago non-zero otherwise. Unless is specified, this will also print the current unit state to standard output. - - Unlike status or show commands, this does not - load units. So, when a specified unit is an alias of another unit and is not loaded, - then this outputs "inactive", even if the aliased unit is active. @@ -893,10 +889,6 @@ Sun 2017-02-26 20:57:49 EST 2h 3min left Sun 2017-02-26 11:56:36 EST 6h ago non-zero otherwise. Unless is specified, this will also print the current unit state to standard output. - - Unlike status or show commands, this does not - load units. So, when a specified unit is an alias of another unit and is not loaded, - then this outputs "inactive", even if the aliased unit is failed. @@ -995,9 +987,6 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err are always exposed as properties ending in the …USec suffix even if a matching configuration options end in …Sec, because microseconds is the normalized time unit used by the system and service manager. - - As similar to status command, systemd implicitly loads units as necessary. - See also status command for the detail.