From 63ab06c4d281046b4a2424b6accd407470c91eb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Feb 2021 15:22:42 +0100 Subject: [PATCH 01/19] sd-bus: extend sd_bus_message_read_strv() to paths and signatures It's rather convenient to be able to read all three types with this function. Strictly speaking this change is not fully compatible, in case someone was relying on sd_bus_message_read_strv() returning an error for anything except "as", but I hope nobody was doing that. --- man/sd_bus_message_read_strv.xml | 19 ++++++++++++++----- src/libsystemd/sd-bus/bus-message.c | 15 ++++++++++++--- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/man/sd_bus_message_read_strv.xml b/man/sd_bus_message_read_strv.xml index a90ae84098..50580d86bc 100644 --- a/man/sd_bus_message_read_strv.xml +++ b/man/sd_bus_message_read_strv.xml @@ -36,11 +36,13 @@ Description - sd_bus_message_read_strv() gives access to an array of strings in message - m. The "read pointer" in the message must be right before an array of strings. On - success, a pointer to the NULL-terminated array of strings is returned in the output - parameter l. Note that ownership of this array is transferred to the caller. - Hence, the caller is responsible for freeing this array and its contents. + sd_bus_message_read_strv() gives access to an array of string-like items in + message m. The "read pointer" in the message must be right before an array of + strings (D-Bus type as), object paths (D-Bus type ao), or + signatures (D-Bus type ag). On success, a pointer to a + NULL-terminated array of strings is returned in the output parameter + l. Note that ownership of this array is transferred to the caller. Hence, the + caller is responsible for freeing this array and its contents. @@ -73,6 +75,13 @@ The message cannot be parsed. + + + -ENXIO + + The message "read pointer" is not right before an array of the appropriate type. + + diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index f358a8699e..894d681260 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -5587,17 +5587,26 @@ int bus_message_get_blob(sd_bus_message *m, void **buffer, size_t *sz) { } int bus_message_read_strv_extend(sd_bus_message *m, char ***l) { - const char *s; + char type; + const char *contents, *s; int r; assert(m); assert(l); - r = sd_bus_message_enter_container(m, 'a', "s"); + r = sd_bus_message_peek_type(m, &type, &contents); + if (r < 0) + return r; + + if (type != SD_BUS_TYPE_ARRAY || !STR_IN_SET(contents, "s", "o", "g")) + return -ENXIO; + + r = sd_bus_message_enter_container(m, 'a', NULL); if (r <= 0) return r; - while ((r = sd_bus_message_read_basic(m, 's', &s)) > 0) { + /* sd_bus_message_read_basic() does content validation for us. */ + while ((r = sd_bus_message_read_basic(m, *contents, &s)) > 0) { r = strv_extend(l, s); if (r < 0) return r; From 41b88bb8ad9474b1a215bbc1c730e6ae7e8d4fa9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Feb 2021 15:33:10 +0100 Subject: [PATCH 02/19] sd-bus: standarize on NULL for empty signature in method calls We would use sometimes "" and sometimes NULL. They are equivalent, so let's use NULL everywhere, except for a two places in tests. --- src/analyze/analyze.c | 2 +- src/busctl/busctl.c | 4 +- src/libsystemd/sd-bus/test-bus-objects.c | 89 ++++++++++++------------ 3 files changed, 47 insertions(+), 48 deletions(-) diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index c25d11e0de..72ebd25525 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -1277,7 +1277,7 @@ static int dot(int argc, char *argv[], void *userdata) { if (r < 0) return r; - r = bus_call_method(bus, bus_systemd_mgr, "ListUnits", &error, &reply, ""); + r = bus_call_method(bus, bus_systemd_mgr, "ListUnits", &error, &reply, NULL); if (r < 0) log_error_errno(r, "Failed to list units: %s", bus_error_message(&error, r)); diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index 7774aeac7c..c577ac2bab 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -442,7 +442,7 @@ static int find_nodes(sd_bus *bus, const char *service, const char *path, Set *p r = sd_bus_call_method(bus, service, path, "org.freedesktop.DBus.Introspectable", "Introspect", - &error, &reply, ""); + &error, &reply, NULL); if (r < 0) { printf("%sFailed to introspect object %s of service %s: %s%s\n", ansi_highlight_red(), @@ -981,7 +981,7 @@ static int introspect(int argc, char **argv, void *userdata) { r = sd_bus_call_method(bus, argv[1], argv[2], "org.freedesktop.DBus.Introspectable", "Introspect", - &error, &reply_xml, ""); + &error, &reply_xml, NULL); if (r < 0) return log_error_errno(r, "Failed to introspect object %s of service %s: %s", argv[2], argv[1], bus_error_message(&error, r)); diff --git a/src/libsystemd/sd-bus/test-bus-objects.c b/src/libsystemd/sd-bus/test-bus-objects.c index 80d2a5d98b..5ef3f30c70 100644 --- a/src/libsystemd/sd-bus/test-bus-objects.c +++ b/src/libsystemd/sd-bus/test-bus-objects.c @@ -297,8 +297,7 @@ static int client(struct context *c) { assert_se(r >= 0); assert_se(streq(s, "<<>>")); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.systemd.test", "Doesntexist", &error, &reply, ""); assert_se(r < 0); @@ -306,6 +305,12 @@ static int client(struct context *c) { sd_bus_error_free(&error); + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.systemd.test", "Doesntexist", &error, &reply, NULL); /* NULL and "" are equivalent */ + assert_se(r < 0); + assert_se(sd_bus_error_has_name(&error, SD_BUS_ERROR_UNKNOWN_METHOD)); + + sd_bus_error_free(&error); + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.systemd.test", "AlterSomething", &error, &reply, "as", 1, "hallo"); assert_se(r < 0); assert_se(sd_bus_error_has_name(&error, SD_BUS_ERROR_INVALID_ARGS)); @@ -319,8 +324,7 @@ static int client(struct context *c) { assert_se(r >= 0); assert_se(streq(s, "<<>>")); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); r = sd_bus_set_property(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.systemd.test", "Something", &error, "s", "test"); assert_se(r >= 0); @@ -332,8 +336,7 @@ static int client(struct context *c) { assert_se(r >= 0); assert_se(streq(s, "test")); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); r = sd_bus_set_property(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.systemd.test", "AutomaticIntegerProperty", &error, "u", 815); assert_se(r >= 0); @@ -352,8 +355,16 @@ static int client(struct context *c) { assert_se(r >= 0); fputs(s, stdout); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); + + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.DBus.Introspectable", "Introspect", &error, &reply, NULL); /* NULL and "" are equivalent */ + assert_se(r >= 0); + + r = sd_bus_message_read(reply, "s", &s); + assert_se(r >= 0); + fputs(s, stdout); + + reply = sd_bus_message_unref(reply); r = sd_bus_get_property(bus, "org.freedesktop.systemd.test", "/value/xuzz", "org.freedesktop.systemd.ValueTest", "Value", &error, &reply, "s"); assert_se(r >= 0); @@ -362,66 +373,60 @@ static int client(struct context *c) { assert_se(r >= 0); log_info("read %s", s); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); - r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/", "org.freedesktop.DBus.Introspectable", "Introspect", &error, &reply, ""); + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/", "org.freedesktop.DBus.Introspectable", "Introspect", &error, &reply, NULL); assert_se(r >= 0); r = sd_bus_message_read(reply, "s", &s); assert_se(r >= 0); fputs(s, stdout); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); - r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/value", "org.freedesktop.DBus.Introspectable", "Introspect", &error, &reply, ""); + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/value", "org.freedesktop.DBus.Introspectable", "Introspect", &error, &reply, NULL); assert_se(r >= 0); r = sd_bus_message_read(reply, "s", &s); assert_se(r >= 0); fputs(s, stdout); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); - r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/value/a", "org.freedesktop.DBus.Introspectable", "Introspect", &error, &reply, ""); + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/value/a", "org.freedesktop.DBus.Introspectable", "Introspect", &error, &reply, NULL); assert_se(r >= 0); r = sd_bus_message_read(reply, "s", &s); assert_se(r >= 0); fputs(s, stdout); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); - r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.DBus.Properties", "GetAll", &error, &reply, "s", ""); + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.DBus.Properties", "GetAll", &error, &reply, "s", NULL); assert_se(r >= 0); sd_bus_message_dump(reply, stdout, SD_BUS_MESSAGE_DUMP_WITH_HEADER); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/value/a", "org.freedesktop.DBus.Properties", "GetAll", &error, &reply, "s", "org.freedesktop.systemd.ValueTest2"); assert_se(r < 0); assert_se(sd_bus_error_has_name(&error, SD_BUS_ERROR_UNKNOWN_INTERFACE)); sd_bus_error_free(&error); - r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.DBus.ObjectManager", "GetManagedObjects", &error, &reply, ""); + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.DBus.ObjectManager", "GetManagedObjects", &error, &reply, NULL); assert_se(r < 0); assert_se(sd_bus_error_has_name(&error, SD_BUS_ERROR_UNKNOWN_METHOD)); sd_bus_error_free(&error); - r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/value", "org.freedesktop.DBus.ObjectManager", "GetManagedObjects", &error, &reply, ""); + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/value", "org.freedesktop.DBus.ObjectManager", "GetManagedObjects", &error, &reply, NULL); assert_se(r >= 0); sd_bus_message_dump(reply, stdout, SD_BUS_MESSAGE_DUMP_WITH_HEADER); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); - r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/value/a", "org.freedesktop.systemd.ValueTest", "NotifyTest", &error, NULL, ""); + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/value/a", "org.freedesktop.systemd.ValueTest", "NotifyTest", &error, NULL, NULL); assert_se(r >= 0); r = sd_bus_process(bus, &reply); @@ -430,10 +435,9 @@ static int client(struct context *c) { assert_se(sd_bus_message_is_signal(reply, "org.freedesktop.DBus.Properties", "PropertiesChanged")); sd_bus_message_dump(reply, stdout, SD_BUS_MESSAGE_DUMP_WITH_HEADER); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); - r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/value/a", "org.freedesktop.systemd.ValueTest", "NotifyTest2", &error, NULL, ""); + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/value/a", "org.freedesktop.systemd.ValueTest", "NotifyTest2", &error, NULL, NULL); assert_se(r >= 0); r = sd_bus_process(bus, &reply); @@ -442,10 +446,9 @@ static int client(struct context *c) { assert_se(sd_bus_message_is_signal(reply, "org.freedesktop.DBus.Properties", "PropertiesChanged")); sd_bus_message_dump(reply, stdout, SD_BUS_MESSAGE_DUMP_WITH_HEADER); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); - r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.systemd.test", "EmitInterfacesAdded", &error, NULL, ""); + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.systemd.test", "EmitInterfacesAdded", &error, NULL, NULL); assert_se(r >= 0); r = sd_bus_process(bus, &reply); @@ -454,10 +457,9 @@ static int client(struct context *c) { assert_se(sd_bus_message_is_signal(reply, "org.freedesktop.DBus.ObjectManager", "InterfacesAdded")); sd_bus_message_dump(reply, stdout, SD_BUS_MESSAGE_DUMP_WITH_HEADER); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); - r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.systemd.test", "EmitInterfacesRemoved", &error, NULL, ""); + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.systemd.test", "EmitInterfacesRemoved", &error, NULL, NULL); assert_se(r >= 0); r = sd_bus_process(bus, &reply); @@ -466,10 +468,9 @@ static int client(struct context *c) { assert_se(sd_bus_message_is_signal(reply, "org.freedesktop.DBus.ObjectManager", "InterfacesRemoved")); sd_bus_message_dump(reply, stdout, SD_BUS_MESSAGE_DUMP_WITH_HEADER); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); - r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.systemd.test", "EmitObjectAdded", &error, NULL, ""); + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.systemd.test", "EmitObjectAdded", &error, NULL, NULL); assert_se(r >= 0); r = sd_bus_process(bus, &reply); @@ -478,10 +479,9 @@ static int client(struct context *c) { assert_se(sd_bus_message_is_signal(reply, "org.freedesktop.DBus.ObjectManager", "InterfacesAdded")); sd_bus_message_dump(reply, stdout, SD_BUS_MESSAGE_DUMP_WITH_HEADER); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); - r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.systemd.test", "EmitObjectRemoved", &error, NULL, ""); + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.systemd.test", "EmitObjectRemoved", &error, NULL, NULL); assert_se(r >= 0); r = sd_bus_process(bus, &reply); @@ -490,10 +490,9 @@ static int client(struct context *c) { assert_se(sd_bus_message_is_signal(reply, "org.freedesktop.DBus.ObjectManager", "InterfacesRemoved")); sd_bus_message_dump(reply, stdout, SD_BUS_MESSAGE_DUMP_WITH_HEADER); - sd_bus_message_unref(reply); - reply = NULL; + reply = sd_bus_message_unref(reply); - r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.systemd.test", "Exit", &error, NULL, ""); + r = sd_bus_call_method(bus, "org.freedesktop.systemd.test", "/foo", "org.freedesktop.systemd.test", "Exit", &error, NULL, NULL); assert_se(r >= 0); sd_bus_flush(bus); From 2d3b784dfb6f59dbe95e47069664f9afb6b08871 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 6 Feb 2021 08:18:42 +0100 Subject: [PATCH 03/19] core: split out a few funcs into unit-serialize.[ch] Just a straightforward move and resulting include file adjustments. --- src/analyze/analyze-verify.c | 1 + src/core/execute.c | 2 +- src/core/fuzz-unit-file.c | 2 +- src/core/manager.c | 1 + src/core/meson.build | 2 + src/core/unit-serialize.c | 810 +++++++++++++++++++++++++++++++++++ src/core/unit-serialize.h | 13 + src/core/unit.c | 801 +--------------------------------- src/core/unit.h | 9 +- src/test/test-bpf-firewall.c | 2 +- 10 files changed, 835 insertions(+), 808 deletions(-) create mode 100644 src/core/unit-serialize.c create mode 100644 src/core/unit-serialize.h diff --git a/src/analyze/analyze-verify.c b/src/analyze/analyze-verify.c index bf28624d41..1b06e07019 100644 --- a/src/analyze/analyze-verify.c +++ b/src/analyze/analyze-verify.c @@ -13,6 +13,7 @@ #include "path-util.h" #include "strv.h" #include "unit-name.h" +#include "unit-serialize.h" static int prepare_filename(const char *filename, char **ret) { int r; diff --git a/src/core/execute.c b/src/core/execute.c index 29fe9f05b1..4b48802183 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -93,7 +93,7 @@ #include "terminal-util.h" #include "tmpfile-util.h" #include "umask-util.h" -#include "unit.h" +#include "unit-serialize.h" #include "user-util.h" #include "utmp-wtmp.h" diff --git a/src/core/fuzz-unit-file.c b/src/core/fuzz-unit-file.c index e67f6e9199..311ffcecc1 100644 --- a/src/core/fuzz-unit-file.c +++ b/src/core/fuzz-unit-file.c @@ -7,7 +7,7 @@ #include "install.h" #include "load-fragment.h" #include "string-util.h" -#include "unit.h" +#include "unit-serialize.h" #include "utf8.h" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { diff --git a/src/core/manager.c b/src/core/manager.c index 7458b0181b..c7cd1fbc6d 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -80,6 +80,7 @@ #include "transaction.h" #include "umask-util.h" #include "unit-name.h" +#include "unit-serialize.h" #include "user-util.h" #include "virt.h" #include "watchdog.h" diff --git a/src/core/meson.build b/src/core/meson.build index edfc73e627..b8089c51ce 100644 --- a/src/core/meson.build +++ b/src/core/meson.build @@ -115,6 +115,8 @@ libcore_sources = ''' transaction.h unit-printf.c unit-printf.h + unit-serialize.c + unit-serialize.h unit.c unit.h '''.split() diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c new file mode 100644 index 0000000000..5cab175021 --- /dev/null +++ b/src/core/unit-serialize.c @@ -0,0 +1,810 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "bus-util.h" +#include "dbus.h" +#include "fileio-label.h" +#include "fileio.h" +#include "format-util.h" +#include "parse-util.h" +#include "serialize.h" +#include "string-table.h" +#include "unit-serialize.h" +#include "user-util.h" + +static int serialize_cgroup_mask(FILE *f, const char *key, CGroupMask mask) { + _cleanup_free_ char *s = NULL; + int r; + + assert(f); + assert(key); + + if (mask == 0) + return 0; + + r = cg_mask_to_string(mask, &s); + if (r < 0) + return log_error_errno(r, "Failed to format cgroup mask: %m"); + + return serialize_item(f, key, s); +} + +static const char *const ip_accounting_metric_field[_CGROUP_IP_ACCOUNTING_METRIC_MAX] = { + [CGROUP_IP_INGRESS_BYTES] = "ip-accounting-ingress-bytes", + [CGROUP_IP_INGRESS_PACKETS] = "ip-accounting-ingress-packets", + [CGROUP_IP_EGRESS_BYTES] = "ip-accounting-egress-bytes", + [CGROUP_IP_EGRESS_PACKETS] = "ip-accounting-egress-packets", +}; + +static const char *const io_accounting_metric_field_base[_CGROUP_IO_ACCOUNTING_METRIC_MAX] = { + [CGROUP_IO_READ_BYTES] = "io-accounting-read-bytes-base", + [CGROUP_IO_WRITE_BYTES] = "io-accounting-write-bytes-base", + [CGROUP_IO_READ_OPERATIONS] = "io-accounting-read-operations-base", + [CGROUP_IO_WRITE_OPERATIONS] = "io-accounting-write-operations-base", +}; + +static const char *const io_accounting_metric_field_last[_CGROUP_IO_ACCOUNTING_METRIC_MAX] = { + [CGROUP_IO_READ_BYTES] = "io-accounting-read-bytes-last", + [CGROUP_IO_WRITE_BYTES] = "io-accounting-write-bytes-last", + [CGROUP_IO_READ_OPERATIONS] = "io-accounting-read-operations-last", + [CGROUP_IO_WRITE_OPERATIONS] = "io-accounting-write-operations-last", +}; + +int unit_serialize(Unit *u, FILE *f, FDSet *fds, bool serialize_jobs) { + int r; + + assert(u); + assert(f); + assert(fds); + + if (unit_can_serialize(u)) { + r = UNIT_VTABLE(u)->serialize(u, f, fds); + if (r < 0) + return r; + } + + (void) serialize_dual_timestamp(f, "state-change-timestamp", &u->state_change_timestamp); + + (void) serialize_dual_timestamp(f, "inactive-exit-timestamp", &u->inactive_exit_timestamp); + (void) serialize_dual_timestamp(f, "active-enter-timestamp", &u->active_enter_timestamp); + (void) serialize_dual_timestamp(f, "active-exit-timestamp", &u->active_exit_timestamp); + (void) serialize_dual_timestamp(f, "inactive-enter-timestamp", &u->inactive_enter_timestamp); + + (void) serialize_dual_timestamp(f, "condition-timestamp", &u->condition_timestamp); + (void) serialize_dual_timestamp(f, "assert-timestamp", &u->assert_timestamp); + + if (dual_timestamp_is_set(&u->condition_timestamp)) + (void) serialize_bool(f, "condition-result", u->condition_result); + + if (dual_timestamp_is_set(&u->assert_timestamp)) + (void) serialize_bool(f, "assert-result", u->assert_result); + + (void) serialize_bool(f, "transient", u->transient); + (void) serialize_bool(f, "in-audit", u->in_audit); + + (void) serialize_bool(f, "exported-invocation-id", u->exported_invocation_id); + (void) serialize_bool(f, "exported-log-level-max", u->exported_log_level_max); + (void) serialize_bool(f, "exported-log-extra-fields", u->exported_log_extra_fields); + (void) serialize_bool(f, "exported-log-rate-limit-interval", u->exported_log_ratelimit_interval); + (void) serialize_bool(f, "exported-log-rate-limit-burst", u->exported_log_ratelimit_burst); + + (void) serialize_item_format(f, "cpu-usage-base", "%" PRIu64, u->cpu_usage_base); + if (u->cpu_usage_last != NSEC_INFINITY) + (void) serialize_item_format(f, "cpu-usage-last", "%" PRIu64, u->cpu_usage_last); + + if (u->managed_oom_kill_last > 0) + (void) serialize_item_format(f, "managed-oom-kill-last", "%" PRIu64, u->managed_oom_kill_last); + + if (u->oom_kill_last > 0) + (void) serialize_item_format(f, "oom-kill-last", "%" PRIu64, u->oom_kill_last); + + for (CGroupIOAccountingMetric im = 0; im < _CGROUP_IO_ACCOUNTING_METRIC_MAX; im++) { + (void) serialize_item_format(f, io_accounting_metric_field_base[im], "%" PRIu64, u->io_accounting_base[im]); + + if (u->io_accounting_last[im] != UINT64_MAX) + (void) serialize_item_format(f, io_accounting_metric_field_last[im], "%" PRIu64, u->io_accounting_last[im]); + } + + if (u->cgroup_path) + (void) serialize_item(f, "cgroup", u->cgroup_path); + + (void) serialize_bool(f, "cgroup-realized", u->cgroup_realized); + (void) serialize_cgroup_mask(f, "cgroup-realized-mask", u->cgroup_realized_mask); + (void) serialize_cgroup_mask(f, "cgroup-enabled-mask", u->cgroup_enabled_mask); + (void) serialize_cgroup_mask(f, "cgroup-invalidated-mask", u->cgroup_invalidated_mask); + + if (uid_is_valid(u->ref_uid)) + (void) serialize_item_format(f, "ref-uid", UID_FMT, u->ref_uid); + if (gid_is_valid(u->ref_gid)) + (void) serialize_item_format(f, "ref-gid", GID_FMT, u->ref_gid); + + if (!sd_id128_is_null(u->invocation_id)) + (void) serialize_item_format(f, "invocation-id", SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(u->invocation_id)); + + (void) serialize_item_format(f, "freezer-state", "%s", freezer_state_to_string(unit_freezer_state(u))); + + bus_track_serialize(u->bus_track, f, "ref"); + + for (CGroupIPAccountingMetric m = 0; m < _CGROUP_IP_ACCOUNTING_METRIC_MAX; m++) { + uint64_t v; + + r = unit_get_ip_accounting(u, m, &v); + if (r >= 0) + (void) serialize_item_format(f, ip_accounting_metric_field[m], "%" PRIu64, v); + } + + if (serialize_jobs) { + if (u->job) { + fputs("job\n", f); + job_serialize(u->job, f); + } + + if (u->nop_job) { + fputs("job\n", f); + job_serialize(u->nop_job, f); + } + } + + /* End marker */ + fputc('\n', f); + return 0; +} + +static int unit_deserialize_job(Unit *u, FILE *f) { + _cleanup_(job_freep) Job *j = NULL; + int r; + + assert(u); + assert(f); + + j = job_new_raw(u); + if (!j) + return log_oom(); + + r = job_deserialize(j, f); + if (r < 0) + return r; + + r = job_install_deserialized(j); + if (r < 0) + return r; + + TAKE_PTR(j); + return 0; +} + +int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { + int r; + + assert(u); + assert(f); + assert(fds); + + for (;;) { + _cleanup_free_ char *line = NULL; + char *l, *v; + ssize_t m; + size_t k; + + r = read_line(f, LONG_LINE_MAX, &line); + if (r < 0) + return log_error_errno(r, "Failed to read serialization line: %m"); + if (r == 0) /* eof */ + break; + + l = strstrip(line); + if (isempty(l)) /* End marker */ + break; + + k = strcspn(l, "="); + + if (l[k] == '=') { + l[k] = 0; + v = l+k+1; + } else + v = l+k; + + if (streq(l, "job")) { + if (v[0] == '\0') { + /* New-style serialized job */ + r = unit_deserialize_job(u, f); + if (r < 0) + return r; + } else /* Legacy for pre-44 */ + log_unit_warning(u, "Update from too old systemd versions are unsupported, cannot deserialize job: %s", v); + continue; + } else if (streq(l, "state-change-timestamp")) { + (void) deserialize_dual_timestamp(v, &u->state_change_timestamp); + continue; + } else if (streq(l, "inactive-exit-timestamp")) { + (void) deserialize_dual_timestamp(v, &u->inactive_exit_timestamp); + continue; + } else if (streq(l, "active-enter-timestamp")) { + (void) deserialize_dual_timestamp(v, &u->active_enter_timestamp); + continue; + } else if (streq(l, "active-exit-timestamp")) { + (void) deserialize_dual_timestamp(v, &u->active_exit_timestamp); + continue; + } else if (streq(l, "inactive-enter-timestamp")) { + (void) deserialize_dual_timestamp(v, &u->inactive_enter_timestamp); + continue; + } else if (streq(l, "condition-timestamp")) { + (void) deserialize_dual_timestamp(v, &u->condition_timestamp); + continue; + } else if (streq(l, "assert-timestamp")) { + (void) deserialize_dual_timestamp(v, &u->assert_timestamp); + continue; + } else if (streq(l, "condition-result")) { + + r = parse_boolean(v); + if (r < 0) + log_unit_debug(u, "Failed to parse condition result value %s, ignoring.", v); + else + u->condition_result = r; + + continue; + + } else if (streq(l, "assert-result")) { + + r = parse_boolean(v); + if (r < 0) + log_unit_debug(u, "Failed to parse assert result value %s, ignoring.", v); + else + u->assert_result = r; + + continue; + + } else if (streq(l, "transient")) { + + r = parse_boolean(v); + if (r < 0) + log_unit_debug(u, "Failed to parse transient bool %s, ignoring.", v); + else + u->transient = r; + + continue; + + } else if (streq(l, "in-audit")) { + + r = parse_boolean(v); + if (r < 0) + log_unit_debug(u, "Failed to parse in-audit bool %s, ignoring.", v); + else + u->in_audit = r; + + continue; + + } else if (streq(l, "exported-invocation-id")) { + + r = parse_boolean(v); + if (r < 0) + log_unit_debug(u, "Failed to parse exported invocation ID bool %s, ignoring.", v); + else + u->exported_invocation_id = r; + + continue; + + } else if (streq(l, "exported-log-level-max")) { + + r = parse_boolean(v); + if (r < 0) + log_unit_debug(u, "Failed to parse exported log level max bool %s, ignoring.", v); + else + u->exported_log_level_max = r; + + continue; + + } else if (streq(l, "exported-log-extra-fields")) { + + r = parse_boolean(v); + if (r < 0) + log_unit_debug(u, "Failed to parse exported log extra fields bool %s, ignoring.", v); + else + u->exported_log_extra_fields = r; + + continue; + + } else if (streq(l, "exported-log-rate-limit-interval")) { + + r = parse_boolean(v); + if (r < 0) + log_unit_debug(u, "Failed to parse exported log rate limit interval %s, ignoring.", v); + else + u->exported_log_ratelimit_interval = r; + + continue; + + } else if (streq(l, "exported-log-rate-limit-burst")) { + + r = parse_boolean(v); + if (r < 0) + log_unit_debug(u, "Failed to parse exported log rate limit burst %s, ignoring.", v); + else + u->exported_log_ratelimit_burst = r; + + continue; + + } else if (STR_IN_SET(l, "cpu-usage-base", "cpuacct-usage-base")) { + + r = safe_atou64(v, &u->cpu_usage_base); + if (r < 0) + log_unit_debug(u, "Failed to parse CPU usage base %s, ignoring.", v); + + continue; + + } else if (streq(l, "cpu-usage-last")) { + + r = safe_atou64(v, &u->cpu_usage_last); + if (r < 0) + log_unit_debug(u, "Failed to read CPU usage last %s, ignoring.", v); + + continue; + + } else if (streq(l, "managed-oom-kill-last")) { + + r = safe_atou64(v, &u->managed_oom_kill_last); + if (r < 0) + log_unit_debug(u, "Failed to read managed OOM kill last %s, ignoring.", v); + + continue; + + } else if (streq(l, "oom-kill-last")) { + + r = safe_atou64(v, &u->oom_kill_last); + if (r < 0) + log_unit_debug(u, "Failed to read OOM kill last %s, ignoring.", v); + + continue; + + } else if (streq(l, "cgroup")) { + + r = unit_set_cgroup_path(u, v); + if (r < 0) + log_unit_debug_errno(u, r, "Failed to set cgroup path %s, ignoring: %m", v); + + (void) unit_watch_cgroup(u); + (void) unit_watch_cgroup_memory(u); + + continue; + } else if (streq(l, "cgroup-realized")) { + int b; + + b = parse_boolean(v); + if (b < 0) + log_unit_debug(u, "Failed to parse cgroup-realized bool %s, ignoring.", v); + else + u->cgroup_realized = b; + + continue; + + } else if (streq(l, "cgroup-realized-mask")) { + + r = cg_mask_from_string(v, &u->cgroup_realized_mask); + if (r < 0) + log_unit_debug(u, "Failed to parse cgroup-realized-mask %s, ignoring.", v); + continue; + + } else if (streq(l, "cgroup-enabled-mask")) { + + r = cg_mask_from_string(v, &u->cgroup_enabled_mask); + if (r < 0) + log_unit_debug(u, "Failed to parse cgroup-enabled-mask %s, ignoring.", v); + continue; + + } else if (streq(l, "cgroup-invalidated-mask")) { + + r = cg_mask_from_string(v, &u->cgroup_invalidated_mask); + if (r < 0) + log_unit_debug(u, "Failed to parse cgroup-invalidated-mask %s, ignoring.", v); + continue; + + } else if (streq(l, "ref-uid")) { + uid_t uid; + + r = parse_uid(v, &uid); + if (r < 0) + log_unit_debug(u, "Failed to parse referenced UID %s, ignoring.", v); + else + unit_ref_uid_gid(u, uid, GID_INVALID); + + continue; + + } else if (streq(l, "ref-gid")) { + gid_t gid; + + r = parse_gid(v, &gid); + if (r < 0) + log_unit_debug(u, "Failed to parse referenced GID %s, ignoring.", v); + else + unit_ref_uid_gid(u, UID_INVALID, gid); + + continue; + + } else if (streq(l, "ref")) { + + r = strv_extend(&u->deserialized_refs, v); + if (r < 0) + return log_oom(); + + continue; + } else if (streq(l, "invocation-id")) { + sd_id128_t id; + + r = sd_id128_from_string(v, &id); + if (r < 0) + log_unit_debug(u, "Failed to parse invocation id %s, ignoring.", v); + else { + r = unit_set_invocation_id(u, id); + if (r < 0) + log_unit_warning_errno(u, r, "Failed to set invocation ID for unit: %m"); + } + + continue; + } else if (streq(l, "freezer-state")) { + FreezerState s; + + s = freezer_state_from_string(v); + if (s < 0) + log_unit_debug(u, "Failed to deserialize freezer-state '%s', ignoring.", v); + else + u->freezer_state = s; + + continue; + } + + /* Check if this is an IP accounting metric serialization field */ + m = string_table_lookup(ip_accounting_metric_field, ELEMENTSOF(ip_accounting_metric_field), l); + if (m >= 0) { + uint64_t c; + + r = safe_atou64(v, &c); + if (r < 0) + log_unit_debug(u, "Failed to parse IP accounting value %s, ignoring.", v); + else + u->ip_accounting_extra[m] = c; + continue; + } + + m = string_table_lookup(io_accounting_metric_field_base, ELEMENTSOF(io_accounting_metric_field_base), l); + if (m >= 0) { + uint64_t c; + + r = safe_atou64(v, &c); + if (r < 0) + log_unit_debug(u, "Failed to parse IO accounting base value %s, ignoring.", v); + else + u->io_accounting_base[m] = c; + continue; + } + + m = string_table_lookup(io_accounting_metric_field_last, ELEMENTSOF(io_accounting_metric_field_last), l); + if (m >= 0) { + uint64_t c; + + r = safe_atou64(v, &c); + if (r < 0) + log_unit_debug(u, "Failed to parse IO accounting last value %s, ignoring.", v); + else + u->io_accounting_last[m] = c; + continue; + } + + if (unit_can_serialize(u)) { + r = exec_runtime_deserialize_compat(u, l, v, fds); + if (r < 0) { + log_unit_warning(u, "Failed to deserialize runtime parameter '%s', ignoring.", l); + continue; + } + + /* Returns positive if key was handled by the call */ + if (r > 0) + continue; + + r = UNIT_VTABLE(u)->deserialize_item(u, l, v, fds); + if (r < 0) + log_unit_warning(u, "Failed to deserialize unit parameter '%s', ignoring.", l); + } + } + + /* Versions before 228 did not carry a state change timestamp. In this case, take the current + * time. This is useful, so that timeouts based on this timestamp don't trigger too early, and is + * in-line with the logic from before 228 where the base for timeouts was not persistent across + * reboots. */ + + if (!dual_timestamp_is_set(&u->state_change_timestamp)) + dual_timestamp_get(&u->state_change_timestamp); + + /* Let's make sure that everything that is deserialized also gets any potential new cgroup settings + * applied after we are done. For that we invalidate anything already realized, so that we can + * realize it again. */ + unit_invalidate_cgroup(u, _CGROUP_MASK_ALL); + unit_invalidate_cgroup_bpf(u); + + return 0; +} + +int unit_deserialize_skip(FILE *f) { + int r; + assert(f); + + /* Skip serialized data for this unit. We don't know what it is. */ + + for (;;) { + _cleanup_free_ char *line = NULL; + char *l; + + r = read_line(f, LONG_LINE_MAX, &line); + if (r < 0) + return log_error_errno(r, "Failed to read serialization line: %m"); + if (r == 0) + return 0; + + l = strstrip(line); + + /* End marker */ + if (isempty(l)) + return 1; + } +} + +static void print_unit_dependency_mask(FILE *f, const char *kind, UnitDependencyMask mask, bool *space) { + const struct { + UnitDependencyMask mask; + const char *name; + } table[] = { + { UNIT_DEPENDENCY_FILE, "file" }, + { UNIT_DEPENDENCY_IMPLICIT, "implicit" }, + { UNIT_DEPENDENCY_DEFAULT, "default" }, + { UNIT_DEPENDENCY_UDEV, "udev" }, + { UNIT_DEPENDENCY_PATH, "path" }, + { UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT, "mountinfo-implicit" }, + { UNIT_DEPENDENCY_MOUNTINFO_DEFAULT, "mountinfo-default" }, + { UNIT_DEPENDENCY_PROC_SWAP, "proc-swap" }, + }; + + assert(f); + assert(kind); + assert(space); + + for (size_t i = 0; i < ELEMENTSOF(table); i++) { + + if (mask == 0) + break; + + if (FLAGS_SET(mask, table[i].mask)) { + if (*space) + fputc(' ', f); + else + *space = true; + + fputs(kind, f); + fputs("-", f); + fputs(table[i].name, f); + + mask &= ~table[i].mask; + } + } + + assert(mask == 0); +} + +void unit_dump(Unit *u, FILE *f, const char *prefix) { + char *t, **j; + const char *prefix2; + char timestamp[5][FORMAT_TIMESTAMP_MAX], timespan[FORMAT_TIMESPAN_MAX]; + Unit *following; + _cleanup_set_free_ Set *following_set = NULL; + CGroupMask m; + int r; + + assert(u); + assert(u->type >= 0); + + prefix = strempty(prefix); + prefix2 = strjoina(prefix, "\t"); + + fprintf(f, + "%s-> Unit %s:\n", + prefix, u->id); + + SET_FOREACH(t, u->aliases) + fprintf(f, "%s\tAlias: %s\n", prefix, t); + + fprintf(f, + "%s\tDescription: %s\n" + "%s\tInstance: %s\n" + "%s\tUnit Load State: %s\n" + "%s\tUnit Active State: %s\n" + "%s\tState Change Timestamp: %s\n" + "%s\tInactive Exit Timestamp: %s\n" + "%s\tActive Enter Timestamp: %s\n" + "%s\tActive Exit Timestamp: %s\n" + "%s\tInactive Enter Timestamp: %s\n" + "%s\tMay GC: %s\n" + "%s\tNeed Daemon Reload: %s\n" + "%s\tTransient: %s\n" + "%s\tPerpetual: %s\n" + "%s\tGarbage Collection Mode: %s\n" + "%s\tSlice: %s\n" + "%s\tCGroup: %s\n" + "%s\tCGroup realized: %s\n", + prefix, unit_description(u), + prefix, strna(u->instance), + prefix, unit_load_state_to_string(u->load_state), + prefix, unit_active_state_to_string(unit_active_state(u)), + prefix, strna(format_timestamp(timestamp[0], sizeof(timestamp[0]), u->state_change_timestamp.realtime)), + prefix, strna(format_timestamp(timestamp[1], sizeof(timestamp[1]), u->inactive_exit_timestamp.realtime)), + prefix, strna(format_timestamp(timestamp[2], sizeof(timestamp[2]), u->active_enter_timestamp.realtime)), + prefix, strna(format_timestamp(timestamp[3], sizeof(timestamp[3]), u->active_exit_timestamp.realtime)), + prefix, strna(format_timestamp(timestamp[4], sizeof(timestamp[4]), u->inactive_enter_timestamp.realtime)), + prefix, yes_no(unit_may_gc(u)), + prefix, yes_no(unit_need_daemon_reload(u)), + prefix, yes_no(u->transient), + prefix, yes_no(u->perpetual), + prefix, collect_mode_to_string(u->collect_mode), + prefix, strna(unit_slice_name(u)), + prefix, strna(u->cgroup_path), + prefix, yes_no(u->cgroup_realized)); + + if (u->cgroup_realized_mask != 0) { + _cleanup_free_ char *s = NULL; + (void) cg_mask_to_string(u->cgroup_realized_mask, &s); + fprintf(f, "%s\tCGroup realized mask: %s\n", prefix, strnull(s)); + } + + if (u->cgroup_enabled_mask != 0) { + _cleanup_free_ char *s = NULL; + (void) cg_mask_to_string(u->cgroup_enabled_mask, &s); + fprintf(f, "%s\tCGroup enabled mask: %s\n", prefix, strnull(s)); + } + + m = unit_get_own_mask(u); + if (m != 0) { + _cleanup_free_ char *s = NULL; + (void) cg_mask_to_string(m, &s); + fprintf(f, "%s\tCGroup own mask: %s\n", prefix, strnull(s)); + } + + m = unit_get_members_mask(u); + if (m != 0) { + _cleanup_free_ char *s = NULL; + (void) cg_mask_to_string(m, &s); + fprintf(f, "%s\tCGroup members mask: %s\n", prefix, strnull(s)); + } + + m = unit_get_delegate_mask(u); + if (m != 0) { + _cleanup_free_ char *s = NULL; + (void) cg_mask_to_string(m, &s); + fprintf(f, "%s\tCGroup delegate mask: %s\n", prefix, strnull(s)); + } + + if (!sd_id128_is_null(u->invocation_id)) + fprintf(f, "%s\tInvocation ID: " SD_ID128_FORMAT_STR "\n", + prefix, SD_ID128_FORMAT_VAL(u->invocation_id)); + + STRV_FOREACH(j, u->documentation) + fprintf(f, "%s\tDocumentation: %s\n", prefix, *j); + + following = unit_following(u); + if (following) + fprintf(f, "%s\tFollowing: %s\n", prefix, following->id); + + r = unit_following_set(u, &following_set); + if (r >= 0) { + Unit *other; + + SET_FOREACH(other, following_set) + fprintf(f, "%s\tFollowing Set Member: %s\n", prefix, other->id); + } + + if (u->fragment_path) + fprintf(f, "%s\tFragment Path: %s\n", prefix, u->fragment_path); + + if (u->source_path) + fprintf(f, "%s\tSource Path: %s\n", prefix, u->source_path); + + STRV_FOREACH(j, u->dropin_paths) + fprintf(f, "%s\tDropIn Path: %s\n", prefix, *j); + + if (u->failure_action != EMERGENCY_ACTION_NONE) + fprintf(f, "%s\tFailure Action: %s\n", prefix, emergency_action_to_string(u->failure_action)); + if (u->failure_action_exit_status >= 0) + fprintf(f, "%s\tFailure Action Exit Status: %i\n", prefix, u->failure_action_exit_status); + if (u->success_action != EMERGENCY_ACTION_NONE) + fprintf(f, "%s\tSuccess Action: %s\n", prefix, emergency_action_to_string(u->success_action)); + if (u->success_action_exit_status >= 0) + fprintf(f, "%s\tSuccess Action Exit Status: %i\n", prefix, u->success_action_exit_status); + + if (u->job_timeout != USEC_INFINITY) + fprintf(f, "%s\tJob Timeout: %s\n", prefix, format_timespan(timespan, sizeof(timespan), u->job_timeout, 0)); + + if (u->job_timeout_action != EMERGENCY_ACTION_NONE) + fprintf(f, "%s\tJob Timeout Action: %s\n", prefix, emergency_action_to_string(u->job_timeout_action)); + + if (u->job_timeout_reboot_arg) + fprintf(f, "%s\tJob Timeout Reboot Argument: %s\n", prefix, u->job_timeout_reboot_arg); + + condition_dump_list(u->conditions, f, prefix, condition_type_to_string); + condition_dump_list(u->asserts, f, prefix, assert_type_to_string); + + if (dual_timestamp_is_set(&u->condition_timestamp)) + fprintf(f, + "%s\tCondition Timestamp: %s\n" + "%s\tCondition Result: %s\n", + prefix, strna(format_timestamp(timestamp[0], sizeof(timestamp[0]), u->condition_timestamp.realtime)), + prefix, yes_no(u->condition_result)); + + if (dual_timestamp_is_set(&u->assert_timestamp)) + fprintf(f, + "%s\tAssert Timestamp: %s\n" + "%s\tAssert Result: %s\n", + prefix, strna(format_timestamp(timestamp[0], sizeof(timestamp[0]), u->assert_timestamp.realtime)), + prefix, yes_no(u->assert_result)); + + for (UnitDependency d = 0; d < _UNIT_DEPENDENCY_MAX; d++) { + UnitDependencyInfo di; + Unit *other; + + HASHMAP_FOREACH_KEY(di.data, other, u->dependencies[d]) { + bool space = false; + + fprintf(f, "%s\t%s: %s (", prefix, unit_dependency_to_string(d), other->id); + + print_unit_dependency_mask(f, "origin", di.origin_mask, &space); + print_unit_dependency_mask(f, "destination", di.destination_mask, &space); + + fputs(")\n", f); + } + } + + if (!hashmap_isempty(u->requires_mounts_for)) { + UnitDependencyInfo di; + const char *path; + + HASHMAP_FOREACH_KEY(di.data, path, u->requires_mounts_for) { + bool space = false; + + fprintf(f, "%s\tRequiresMountsFor: %s (", prefix, path); + + print_unit_dependency_mask(f, "origin", di.origin_mask, &space); + print_unit_dependency_mask(f, "destination", di.destination_mask, &space); + + fputs(")\n", f); + } + } + + if (u->load_state == UNIT_LOADED) { + + fprintf(f, + "%s\tStopWhenUnneeded: %s\n" + "%s\tRefuseManualStart: %s\n" + "%s\tRefuseManualStop: %s\n" + "%s\tDefaultDependencies: %s\n" + "%s\tOnFailureJobMode: %s\n" + "%s\tIgnoreOnIsolate: %s\n", + prefix, yes_no(u->stop_when_unneeded), + prefix, yes_no(u->refuse_manual_start), + prefix, yes_no(u->refuse_manual_stop), + prefix, yes_no(u->default_dependencies), + prefix, job_mode_to_string(u->on_failure_job_mode), + prefix, yes_no(u->ignore_on_isolate)); + + if (UNIT_VTABLE(u)->dump) + UNIT_VTABLE(u)->dump(u, f, prefix2); + + } else if (u->load_state == UNIT_MERGED) + fprintf(f, + "%s\tMerged into: %s\n", + prefix, u->merged_into->id); + else if (u->load_state == UNIT_ERROR) + fprintf(f, "%s\tLoad Error Code: %s\n", prefix, strerror_safe(u->load_error)); + + for (const char *n = sd_bus_track_first(u->bus_track); n; n = sd_bus_track_next(u->bus_track)) + fprintf(f, "%s\tBus Ref: %s\n", prefix, n); + + if (u->job) + job_dump(u->job, f, prefix2); + + if (u->nop_job) + job_dump(u->nop_job, f, prefix2); +} diff --git a/src/core/unit-serialize.h b/src/core/unit-serialize.h new file mode 100644 index 0000000000..599d883dec --- /dev/null +++ b/src/core/unit-serialize.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +#include + +#include "unit.h" +#include "fdset.h" + +int unit_serialize(Unit *u, FILE *f, FDSet *fds, bool serialize_jobs); +int unit_deserialize(Unit *u, FILE *f, FDSet *fds); +int unit_deserialize_skip(FILE *f); + +void unit_dump(Unit *u, FILE *f, const char *prefix); diff --git a/src/core/unit.c b/src/core/unit.c index 53273b5984..eacebe796a 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -26,8 +26,8 @@ #include "fileio.h" #include "format-util.h" #include "id128-util.h" -#include "io-util.h" #include "install.h" +#include "io-util.h" #include "label.h" #include "load-dropin.h" #include "load-fragment.h" @@ -35,11 +35,9 @@ #include "macro.h" #include "missing_audit.h" #include "mkdir.h" -#include "parse-util.h" #include "path-util.h" #include "process-util.h" #include "rm-rf.h" -#include "serialize.h" #include "set.h" #include "signal-util.h" #include "sparse-endian.h" @@ -1166,269 +1164,6 @@ const char *unit_status_string(Unit *u) { return unit_description(u); } -static void print_unit_dependency_mask(FILE *f, const char *kind, UnitDependencyMask mask, bool *space) { - const struct { - UnitDependencyMask mask; - const char *name; - } table[] = { - { UNIT_DEPENDENCY_FILE, "file" }, - { UNIT_DEPENDENCY_IMPLICIT, "implicit" }, - { UNIT_DEPENDENCY_DEFAULT, "default" }, - { UNIT_DEPENDENCY_UDEV, "udev" }, - { UNIT_DEPENDENCY_PATH, "path" }, - { UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT, "mountinfo-implicit" }, - { UNIT_DEPENDENCY_MOUNTINFO_DEFAULT, "mountinfo-default" }, - { UNIT_DEPENDENCY_PROC_SWAP, "proc-swap" }, - }; - - assert(f); - assert(kind); - assert(space); - - for (size_t i = 0; i < ELEMENTSOF(table); i++) { - - if (mask == 0) - break; - - if (FLAGS_SET(mask, table[i].mask)) { - if (*space) - fputc(' ', f); - else - *space = true; - - fputs(kind, f); - fputs("-", f); - fputs(table[i].name, f); - - mask &= ~table[i].mask; - } - } - - assert(mask == 0); -} - -void unit_dump(Unit *u, FILE *f, const char *prefix) { - char *t, **j; - const char *prefix2; - char timestamp[5][FORMAT_TIMESTAMP_MAX], timespan[FORMAT_TIMESPAN_MAX]; - Unit *following; - _cleanup_set_free_ Set *following_set = NULL; - CGroupMask m; - int r; - - assert(u); - assert(u->type >= 0); - - prefix = strempty(prefix); - prefix2 = strjoina(prefix, "\t"); - - fprintf(f, - "%s-> Unit %s:\n", - prefix, u->id); - - SET_FOREACH(t, u->aliases) - fprintf(f, "%s\tAlias: %s\n", prefix, t); - - fprintf(f, - "%s\tDescription: %s\n" - "%s\tInstance: %s\n" - "%s\tUnit Load State: %s\n" - "%s\tUnit Active State: %s\n" - "%s\tState Change Timestamp: %s\n" - "%s\tInactive Exit Timestamp: %s\n" - "%s\tActive Enter Timestamp: %s\n" - "%s\tActive Exit Timestamp: %s\n" - "%s\tInactive Enter Timestamp: %s\n" - "%s\tMay GC: %s\n" - "%s\tNeed Daemon Reload: %s\n" - "%s\tTransient: %s\n" - "%s\tPerpetual: %s\n" - "%s\tGarbage Collection Mode: %s\n" - "%s\tSlice: %s\n" - "%s\tCGroup: %s\n" - "%s\tCGroup realized: %s\n", - prefix, unit_description(u), - prefix, strna(u->instance), - prefix, unit_load_state_to_string(u->load_state), - prefix, unit_active_state_to_string(unit_active_state(u)), - prefix, strna(format_timestamp(timestamp[0], sizeof(timestamp[0]), u->state_change_timestamp.realtime)), - prefix, strna(format_timestamp(timestamp[1], sizeof(timestamp[1]), u->inactive_exit_timestamp.realtime)), - prefix, strna(format_timestamp(timestamp[2], sizeof(timestamp[2]), u->active_enter_timestamp.realtime)), - prefix, strna(format_timestamp(timestamp[3], sizeof(timestamp[3]), u->active_exit_timestamp.realtime)), - prefix, strna(format_timestamp(timestamp[4], sizeof(timestamp[4]), u->inactive_enter_timestamp.realtime)), - prefix, yes_no(unit_may_gc(u)), - prefix, yes_no(unit_need_daemon_reload(u)), - prefix, yes_no(u->transient), - prefix, yes_no(u->perpetual), - prefix, collect_mode_to_string(u->collect_mode), - prefix, strna(unit_slice_name(u)), - prefix, strna(u->cgroup_path), - prefix, yes_no(u->cgroup_realized)); - - if (u->cgroup_realized_mask != 0) { - _cleanup_free_ char *s = NULL; - (void) cg_mask_to_string(u->cgroup_realized_mask, &s); - fprintf(f, "%s\tCGroup realized mask: %s\n", prefix, strnull(s)); - } - - if (u->cgroup_enabled_mask != 0) { - _cleanup_free_ char *s = NULL; - (void) cg_mask_to_string(u->cgroup_enabled_mask, &s); - fprintf(f, "%s\tCGroup enabled mask: %s\n", prefix, strnull(s)); - } - - m = unit_get_own_mask(u); - if (m != 0) { - _cleanup_free_ char *s = NULL; - (void) cg_mask_to_string(m, &s); - fprintf(f, "%s\tCGroup own mask: %s\n", prefix, strnull(s)); - } - - m = unit_get_members_mask(u); - if (m != 0) { - _cleanup_free_ char *s = NULL; - (void) cg_mask_to_string(m, &s); - fprintf(f, "%s\tCGroup members mask: %s\n", prefix, strnull(s)); - } - - m = unit_get_delegate_mask(u); - if (m != 0) { - _cleanup_free_ char *s = NULL; - (void) cg_mask_to_string(m, &s); - fprintf(f, "%s\tCGroup delegate mask: %s\n", prefix, strnull(s)); - } - - if (!sd_id128_is_null(u->invocation_id)) - fprintf(f, "%s\tInvocation ID: " SD_ID128_FORMAT_STR "\n", - prefix, SD_ID128_FORMAT_VAL(u->invocation_id)); - - STRV_FOREACH(j, u->documentation) - fprintf(f, "%s\tDocumentation: %s\n", prefix, *j); - - following = unit_following(u); - if (following) - fprintf(f, "%s\tFollowing: %s\n", prefix, following->id); - - r = unit_following_set(u, &following_set); - if (r >= 0) { - Unit *other; - - SET_FOREACH(other, following_set) - fprintf(f, "%s\tFollowing Set Member: %s\n", prefix, other->id); - } - - if (u->fragment_path) - fprintf(f, "%s\tFragment Path: %s\n", prefix, u->fragment_path); - - if (u->source_path) - fprintf(f, "%s\tSource Path: %s\n", prefix, u->source_path); - - STRV_FOREACH(j, u->dropin_paths) - fprintf(f, "%s\tDropIn Path: %s\n", prefix, *j); - - if (u->failure_action != EMERGENCY_ACTION_NONE) - fprintf(f, "%s\tFailure Action: %s\n", prefix, emergency_action_to_string(u->failure_action)); - if (u->failure_action_exit_status >= 0) - fprintf(f, "%s\tFailure Action Exit Status: %i\n", prefix, u->failure_action_exit_status); - if (u->success_action != EMERGENCY_ACTION_NONE) - fprintf(f, "%s\tSuccess Action: %s\n", prefix, emergency_action_to_string(u->success_action)); - if (u->success_action_exit_status >= 0) - fprintf(f, "%s\tSuccess Action Exit Status: %i\n", prefix, u->success_action_exit_status); - - if (u->job_timeout != USEC_INFINITY) - fprintf(f, "%s\tJob Timeout: %s\n", prefix, format_timespan(timespan, sizeof(timespan), u->job_timeout, 0)); - - if (u->job_timeout_action != EMERGENCY_ACTION_NONE) - fprintf(f, "%s\tJob Timeout Action: %s\n", prefix, emergency_action_to_string(u->job_timeout_action)); - - if (u->job_timeout_reboot_arg) - fprintf(f, "%s\tJob Timeout Reboot Argument: %s\n", prefix, u->job_timeout_reboot_arg); - - condition_dump_list(u->conditions, f, prefix, condition_type_to_string); - condition_dump_list(u->asserts, f, prefix, assert_type_to_string); - - if (dual_timestamp_is_set(&u->condition_timestamp)) - fprintf(f, - "%s\tCondition Timestamp: %s\n" - "%s\tCondition Result: %s\n", - prefix, strna(format_timestamp(timestamp[0], sizeof(timestamp[0]), u->condition_timestamp.realtime)), - prefix, yes_no(u->condition_result)); - - if (dual_timestamp_is_set(&u->assert_timestamp)) - fprintf(f, - "%s\tAssert Timestamp: %s\n" - "%s\tAssert Result: %s\n", - prefix, strna(format_timestamp(timestamp[0], sizeof(timestamp[0]), u->assert_timestamp.realtime)), - prefix, yes_no(u->assert_result)); - - for (UnitDependency d = 0; d < _UNIT_DEPENDENCY_MAX; d++) { - UnitDependencyInfo di; - Unit *other; - - HASHMAP_FOREACH_KEY(di.data, other, u->dependencies[d]) { - bool space = false; - - fprintf(f, "%s\t%s: %s (", prefix, unit_dependency_to_string(d), other->id); - - print_unit_dependency_mask(f, "origin", di.origin_mask, &space); - print_unit_dependency_mask(f, "destination", di.destination_mask, &space); - - fputs(")\n", f); - } - } - - if (!hashmap_isempty(u->requires_mounts_for)) { - UnitDependencyInfo di; - const char *path; - - HASHMAP_FOREACH_KEY(di.data, path, u->requires_mounts_for) { - bool space = false; - - fprintf(f, "%s\tRequiresMountsFor: %s (", prefix, path); - - print_unit_dependency_mask(f, "origin", di.origin_mask, &space); - print_unit_dependency_mask(f, "destination", di.destination_mask, &space); - - fputs(")\n", f); - } - } - - if (u->load_state == UNIT_LOADED) { - - fprintf(f, - "%s\tStopWhenUnneeded: %s\n" - "%s\tRefuseManualStart: %s\n" - "%s\tRefuseManualStop: %s\n" - "%s\tDefaultDependencies: %s\n" - "%s\tOnFailureJobMode: %s\n" - "%s\tIgnoreOnIsolate: %s\n", - prefix, yes_no(u->stop_when_unneeded), - prefix, yes_no(u->refuse_manual_start), - prefix, yes_no(u->refuse_manual_stop), - prefix, yes_no(u->default_dependencies), - prefix, job_mode_to_string(u->on_failure_job_mode), - prefix, yes_no(u->ignore_on_isolate)); - - if (UNIT_VTABLE(u)->dump) - UNIT_VTABLE(u)->dump(u, f, prefix2); - - } else if (u->load_state == UNIT_MERGED) - fprintf(f, - "%s\tMerged into: %s\n", - prefix, u->merged_into->id); - else if (u->load_state == UNIT_ERROR) - fprintf(f, "%s\tLoad Error Code: %s\n", prefix, strerror_safe(u->load_error)); - - for (const char *n = sd_bus_track_first(u->bus_track); n; n = sd_bus_track_next(u->bus_track)) - fprintf(f, "%s\tBus Ref: %s\n", prefix, n); - - if (u->job) - job_dump(u->job, f, prefix2); - - if (u->nop_job) - job_dump(u->nop_job, f, prefix2); -} - /* Common implementation for multiple backends */ int unit_load_fragment_and_dropin(Unit *u, bool fragment_required) { int r; @@ -3241,7 +2976,7 @@ char *unit_dbus_path_invocation_id(Unit *u) { return unit_dbus_path_from_name(u->invocation_id_string); } -static int unit_set_invocation_id(Unit *u, sd_id128_t id) { +int unit_set_invocation_id(Unit *u, sd_id128_t id) { int r; assert(u); @@ -3526,538 +3261,6 @@ bool unit_can_serialize(Unit *u) { return UNIT_VTABLE(u)->serialize && UNIT_VTABLE(u)->deserialize_item; } -static int serialize_cgroup_mask(FILE *f, const char *key, CGroupMask mask) { - _cleanup_free_ char *s = NULL; - int r; - - assert(f); - assert(key); - - if (mask == 0) - return 0; - - r = cg_mask_to_string(mask, &s); - if (r < 0) - return log_error_errno(r, "Failed to format cgroup mask: %m"); - - return serialize_item(f, key, s); -} - -static const char *const ip_accounting_metric_field[_CGROUP_IP_ACCOUNTING_METRIC_MAX] = { - [CGROUP_IP_INGRESS_BYTES] = "ip-accounting-ingress-bytes", - [CGROUP_IP_INGRESS_PACKETS] = "ip-accounting-ingress-packets", - [CGROUP_IP_EGRESS_BYTES] = "ip-accounting-egress-bytes", - [CGROUP_IP_EGRESS_PACKETS] = "ip-accounting-egress-packets", -}; - -static const char *const io_accounting_metric_field_base[_CGROUP_IO_ACCOUNTING_METRIC_MAX] = { - [CGROUP_IO_READ_BYTES] = "io-accounting-read-bytes-base", - [CGROUP_IO_WRITE_BYTES] = "io-accounting-write-bytes-base", - [CGROUP_IO_READ_OPERATIONS] = "io-accounting-read-operations-base", - [CGROUP_IO_WRITE_OPERATIONS] = "io-accounting-write-operations-base", -}; - -static const char *const io_accounting_metric_field_last[_CGROUP_IO_ACCOUNTING_METRIC_MAX] = { - [CGROUP_IO_READ_BYTES] = "io-accounting-read-bytes-last", - [CGROUP_IO_WRITE_BYTES] = "io-accounting-write-bytes-last", - [CGROUP_IO_READ_OPERATIONS] = "io-accounting-read-operations-last", - [CGROUP_IO_WRITE_OPERATIONS] = "io-accounting-write-operations-last", -}; - -int unit_serialize(Unit *u, FILE *f, FDSet *fds, bool serialize_jobs) { - int r; - - assert(u); - assert(f); - assert(fds); - - if (unit_can_serialize(u)) { - r = UNIT_VTABLE(u)->serialize(u, f, fds); - if (r < 0) - return r; - } - - (void) serialize_dual_timestamp(f, "state-change-timestamp", &u->state_change_timestamp); - - (void) serialize_dual_timestamp(f, "inactive-exit-timestamp", &u->inactive_exit_timestamp); - (void) serialize_dual_timestamp(f, "active-enter-timestamp", &u->active_enter_timestamp); - (void) serialize_dual_timestamp(f, "active-exit-timestamp", &u->active_exit_timestamp); - (void) serialize_dual_timestamp(f, "inactive-enter-timestamp", &u->inactive_enter_timestamp); - - (void) serialize_dual_timestamp(f, "condition-timestamp", &u->condition_timestamp); - (void) serialize_dual_timestamp(f, "assert-timestamp", &u->assert_timestamp); - - if (dual_timestamp_is_set(&u->condition_timestamp)) - (void) serialize_bool(f, "condition-result", u->condition_result); - - if (dual_timestamp_is_set(&u->assert_timestamp)) - (void) serialize_bool(f, "assert-result", u->assert_result); - - (void) serialize_bool(f, "transient", u->transient); - (void) serialize_bool(f, "in-audit", u->in_audit); - - (void) serialize_bool(f, "exported-invocation-id", u->exported_invocation_id); - (void) serialize_bool(f, "exported-log-level-max", u->exported_log_level_max); - (void) serialize_bool(f, "exported-log-extra-fields", u->exported_log_extra_fields); - (void) serialize_bool(f, "exported-log-rate-limit-interval", u->exported_log_ratelimit_interval); - (void) serialize_bool(f, "exported-log-rate-limit-burst", u->exported_log_ratelimit_burst); - - (void) serialize_item_format(f, "cpu-usage-base", "%" PRIu64, u->cpu_usage_base); - if (u->cpu_usage_last != NSEC_INFINITY) - (void) serialize_item_format(f, "cpu-usage-last", "%" PRIu64, u->cpu_usage_last); - - if (u->managed_oom_kill_last > 0) - (void) serialize_item_format(f, "managed-oom-kill-last", "%" PRIu64, u->managed_oom_kill_last); - - if (u->oom_kill_last > 0) - (void) serialize_item_format(f, "oom-kill-last", "%" PRIu64, u->oom_kill_last); - - for (CGroupIOAccountingMetric im = 0; im < _CGROUP_IO_ACCOUNTING_METRIC_MAX; im++) { - (void) serialize_item_format(f, io_accounting_metric_field_base[im], "%" PRIu64, u->io_accounting_base[im]); - - if (u->io_accounting_last[im] != UINT64_MAX) - (void) serialize_item_format(f, io_accounting_metric_field_last[im], "%" PRIu64, u->io_accounting_last[im]); - } - - if (u->cgroup_path) - (void) serialize_item(f, "cgroup", u->cgroup_path); - - (void) serialize_bool(f, "cgroup-realized", u->cgroup_realized); - (void) serialize_cgroup_mask(f, "cgroup-realized-mask", u->cgroup_realized_mask); - (void) serialize_cgroup_mask(f, "cgroup-enabled-mask", u->cgroup_enabled_mask); - (void) serialize_cgroup_mask(f, "cgroup-invalidated-mask", u->cgroup_invalidated_mask); - - if (uid_is_valid(u->ref_uid)) - (void) serialize_item_format(f, "ref-uid", UID_FMT, u->ref_uid); - if (gid_is_valid(u->ref_gid)) - (void) serialize_item_format(f, "ref-gid", GID_FMT, u->ref_gid); - - if (!sd_id128_is_null(u->invocation_id)) - (void) serialize_item_format(f, "invocation-id", SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(u->invocation_id)); - - (void) serialize_item_format(f, "freezer-state", "%s", freezer_state_to_string(unit_freezer_state(u))); - - bus_track_serialize(u->bus_track, f, "ref"); - - for (CGroupIPAccountingMetric m = 0; m < _CGROUP_IP_ACCOUNTING_METRIC_MAX; m++) { - uint64_t v; - - r = unit_get_ip_accounting(u, m, &v); - if (r >= 0) - (void) serialize_item_format(f, ip_accounting_metric_field[m], "%" PRIu64, v); - } - - if (serialize_jobs) { - if (u->job) { - fputs("job\n", f); - job_serialize(u->job, f); - } - - if (u->nop_job) { - fputs("job\n", f); - job_serialize(u->nop_job, f); - } - } - - /* End marker */ - fputc('\n', f); - return 0; -} - -static int unit_deserialize_job(Unit *u, FILE *f) { - _cleanup_(job_freep) Job *j = NULL; - int r; - - assert(u); - assert(f); - - j = job_new_raw(u); - if (!j) - return log_oom(); - - r = job_deserialize(j, f); - if (r < 0) - return r; - - r = job_install_deserialized(j); - if (r < 0) - return r; - - TAKE_PTR(j); - return 0; -} - -int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { - int r; - - assert(u); - assert(f); - assert(fds); - - for (;;) { - _cleanup_free_ char *line = NULL; - char *l, *v; - ssize_t m; - size_t k; - - r = read_line(f, LONG_LINE_MAX, &line); - if (r < 0) - return log_error_errno(r, "Failed to read serialization line: %m"); - if (r == 0) /* eof */ - break; - - l = strstrip(line); - if (isempty(l)) /* End marker */ - break; - - k = strcspn(l, "="); - - if (l[k] == '=') { - l[k] = 0; - v = l+k+1; - } else - v = l+k; - - if (streq(l, "job")) { - if (v[0] == '\0') { - /* New-style serialized job */ - r = unit_deserialize_job(u, f); - if (r < 0) - return r; - } else /* Legacy for pre-44 */ - log_unit_warning(u, "Update from too old systemd versions are unsupported, cannot deserialize job: %s", v); - continue; - } else if (streq(l, "state-change-timestamp")) { - (void) deserialize_dual_timestamp(v, &u->state_change_timestamp); - continue; - } else if (streq(l, "inactive-exit-timestamp")) { - (void) deserialize_dual_timestamp(v, &u->inactive_exit_timestamp); - continue; - } else if (streq(l, "active-enter-timestamp")) { - (void) deserialize_dual_timestamp(v, &u->active_enter_timestamp); - continue; - } else if (streq(l, "active-exit-timestamp")) { - (void) deserialize_dual_timestamp(v, &u->active_exit_timestamp); - continue; - } else if (streq(l, "inactive-enter-timestamp")) { - (void) deserialize_dual_timestamp(v, &u->inactive_enter_timestamp); - continue; - } else if (streq(l, "condition-timestamp")) { - (void) deserialize_dual_timestamp(v, &u->condition_timestamp); - continue; - } else if (streq(l, "assert-timestamp")) { - (void) deserialize_dual_timestamp(v, &u->assert_timestamp); - continue; - } else if (streq(l, "condition-result")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse condition result value %s, ignoring.", v); - else - u->condition_result = r; - - continue; - - } else if (streq(l, "assert-result")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse assert result value %s, ignoring.", v); - else - u->assert_result = r; - - continue; - - } else if (streq(l, "transient")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse transient bool %s, ignoring.", v); - else - u->transient = r; - - continue; - - } else if (streq(l, "in-audit")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse in-audit bool %s, ignoring.", v); - else - u->in_audit = r; - - continue; - - } else if (streq(l, "exported-invocation-id")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse exported invocation ID bool %s, ignoring.", v); - else - u->exported_invocation_id = r; - - continue; - - } else if (streq(l, "exported-log-level-max")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse exported log level max bool %s, ignoring.", v); - else - u->exported_log_level_max = r; - - continue; - - } else if (streq(l, "exported-log-extra-fields")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse exported log extra fields bool %s, ignoring.", v); - else - u->exported_log_extra_fields = r; - - continue; - - } else if (streq(l, "exported-log-rate-limit-interval")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse exported log rate limit interval %s, ignoring.", v); - else - u->exported_log_ratelimit_interval = r; - - continue; - - } else if (streq(l, "exported-log-rate-limit-burst")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse exported log rate limit burst %s, ignoring.", v); - else - u->exported_log_ratelimit_burst = r; - - continue; - - } else if (STR_IN_SET(l, "cpu-usage-base", "cpuacct-usage-base")) { - - r = safe_atou64(v, &u->cpu_usage_base); - if (r < 0) - log_unit_debug(u, "Failed to parse CPU usage base %s, ignoring.", v); - - continue; - - } else if (streq(l, "cpu-usage-last")) { - - r = safe_atou64(v, &u->cpu_usage_last); - if (r < 0) - log_unit_debug(u, "Failed to read CPU usage last %s, ignoring.", v); - - continue; - - } else if (streq(l, "managed-oom-kill-last")) { - - r = safe_atou64(v, &u->managed_oom_kill_last); - if (r < 0) - log_unit_debug(u, "Failed to read managed OOM kill last %s, ignoring.", v); - - continue; - - } else if (streq(l, "oom-kill-last")) { - - r = safe_atou64(v, &u->oom_kill_last); - if (r < 0) - log_unit_debug(u, "Failed to read OOM kill last %s, ignoring.", v); - - continue; - - } else if (streq(l, "cgroup")) { - - r = unit_set_cgroup_path(u, v); - if (r < 0) - log_unit_debug_errno(u, r, "Failed to set cgroup path %s, ignoring: %m", v); - - (void) unit_watch_cgroup(u); - (void) unit_watch_cgroup_memory(u); - - continue; - } else if (streq(l, "cgroup-realized")) { - int b; - - b = parse_boolean(v); - if (b < 0) - log_unit_debug(u, "Failed to parse cgroup-realized bool %s, ignoring.", v); - else - u->cgroup_realized = b; - - continue; - - } else if (streq(l, "cgroup-realized-mask")) { - - r = cg_mask_from_string(v, &u->cgroup_realized_mask); - if (r < 0) - log_unit_debug(u, "Failed to parse cgroup-realized-mask %s, ignoring.", v); - continue; - - } else if (streq(l, "cgroup-enabled-mask")) { - - r = cg_mask_from_string(v, &u->cgroup_enabled_mask); - if (r < 0) - log_unit_debug(u, "Failed to parse cgroup-enabled-mask %s, ignoring.", v); - continue; - - } else if (streq(l, "cgroup-invalidated-mask")) { - - r = cg_mask_from_string(v, &u->cgroup_invalidated_mask); - if (r < 0) - log_unit_debug(u, "Failed to parse cgroup-invalidated-mask %s, ignoring.", v); - continue; - - } else if (streq(l, "ref-uid")) { - uid_t uid; - - r = parse_uid(v, &uid); - if (r < 0) - log_unit_debug(u, "Failed to parse referenced UID %s, ignoring.", v); - else - unit_ref_uid_gid(u, uid, GID_INVALID); - - continue; - - } else if (streq(l, "ref-gid")) { - gid_t gid; - - r = parse_gid(v, &gid); - if (r < 0) - log_unit_debug(u, "Failed to parse referenced GID %s, ignoring.", v); - else - unit_ref_uid_gid(u, UID_INVALID, gid); - - continue; - - } else if (streq(l, "ref")) { - - r = strv_extend(&u->deserialized_refs, v); - if (r < 0) - return log_oom(); - - continue; - } else if (streq(l, "invocation-id")) { - sd_id128_t id; - - r = sd_id128_from_string(v, &id); - if (r < 0) - log_unit_debug(u, "Failed to parse invocation id %s, ignoring.", v); - else { - r = unit_set_invocation_id(u, id); - if (r < 0) - log_unit_warning_errno(u, r, "Failed to set invocation ID for unit: %m"); - } - - continue; - } else if (streq(l, "freezer-state")) { - FreezerState s; - - s = freezer_state_from_string(v); - if (s < 0) - log_unit_debug(u, "Failed to deserialize freezer-state '%s', ignoring.", v); - else - u->freezer_state = s; - - continue; - } - - /* Check if this is an IP accounting metric serialization field */ - m = string_table_lookup(ip_accounting_metric_field, ELEMENTSOF(ip_accounting_metric_field), l); - if (m >= 0) { - uint64_t c; - - r = safe_atou64(v, &c); - if (r < 0) - log_unit_debug(u, "Failed to parse IP accounting value %s, ignoring.", v); - else - u->ip_accounting_extra[m] = c; - continue; - } - - m = string_table_lookup(io_accounting_metric_field_base, ELEMENTSOF(io_accounting_metric_field_base), l); - if (m >= 0) { - uint64_t c; - - r = safe_atou64(v, &c); - if (r < 0) - log_unit_debug(u, "Failed to parse IO accounting base value %s, ignoring.", v); - else - u->io_accounting_base[m] = c; - continue; - } - - m = string_table_lookup(io_accounting_metric_field_last, ELEMENTSOF(io_accounting_metric_field_last), l); - if (m >= 0) { - uint64_t c; - - r = safe_atou64(v, &c); - if (r < 0) - log_unit_debug(u, "Failed to parse IO accounting last value %s, ignoring.", v); - else - u->io_accounting_last[m] = c; - continue; - } - - if (unit_can_serialize(u)) { - r = exec_runtime_deserialize_compat(u, l, v, fds); - if (r < 0) { - log_unit_warning(u, "Failed to deserialize runtime parameter '%s', ignoring.", l); - continue; - } - - /* Returns positive if key was handled by the call */ - if (r > 0) - continue; - - r = UNIT_VTABLE(u)->deserialize_item(u, l, v, fds); - if (r < 0) - log_unit_warning(u, "Failed to deserialize unit parameter '%s', ignoring.", l); - } - } - - /* Versions before 228 did not carry a state change timestamp. In this case, take the current time. This is - * useful, so that timeouts based on this timestamp don't trigger too early, and is in-line with the logic from - * before 228 where the base for timeouts was not persistent across reboots. */ - - if (!dual_timestamp_is_set(&u->state_change_timestamp)) - dual_timestamp_get(&u->state_change_timestamp); - - /* Let's make sure that everything that is deserialized also gets any potential new cgroup settings applied - * after we are done. For that we invalidate anything already realized, so that we can realize it again. */ - unit_invalidate_cgroup(u, _CGROUP_MASK_ALL); - unit_invalidate_cgroup_bpf(u); - - return 0; -} - -int unit_deserialize_skip(FILE *f) { - int r; - assert(f); - - /* Skip serialized data for this unit. We don't know what it is. */ - - for (;;) { - _cleanup_free_ char *line = NULL; - char *l; - - r = read_line(f, LONG_LINE_MAX, &line); - if (r < 0) - return log_error_errno(r, "Failed to read serialization line: %m"); - if (r == 0) - return 0; - - l = strstrip(line); - - /* End marker */ - if (isempty(l)) - return 1; - } -} int unit_add_node_dependency(Unit *u, const char *what, UnitDependency dep, UnitDependencyMask mask) { _cleanup_free_ char *e = NULL; diff --git a/src/core/unit.h b/src/core/unit.h index 1b3b146369..1af0e50312 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -5,6 +5,8 @@ #include #include +#include "sd-id128.h" + #include "bpf-program.h" #include "condition.h" #include "emergency-action.h" @@ -721,8 +723,6 @@ int unit_freezer_state_kernel(Unit *u, FreezerState *ret); const char* unit_sub_state_to_string(Unit *u); -void unit_dump(Unit *u, FILE *f, const char *prefix); - bool unit_can_reload(Unit *u) _pure_; bool unit_can_start(Unit *u) _pure_; bool unit_can_stop(Unit *u) _pure_; @@ -764,10 +764,6 @@ int unit_load_related_unit(Unit *u, const char *type, Unit **_found); bool unit_can_serialize(Unit *u) _pure_; -int unit_serialize(Unit *u, FILE *f, FDSet *fds, bool serialize_jobs); -int unit_deserialize(Unit *u, FILE *f, FDSet *fds); -int unit_deserialize_skip(FILE *f); - int unit_add_node_dependency(Unit *u, const char *what, UnitDependency d, UnitDependencyMask mask); int unit_add_blockdev_dependency(Unit *u, const char *what, UnitDependencyMask mask); @@ -847,6 +843,7 @@ void unit_unref_uid_gid(Unit *u, bool destroy_now); void unit_notify_user_lookup(Unit *u, uid_t uid, gid_t gid); +int unit_set_invocation_id(Unit *u, sd_id128_t id); int unit_acquire_invocation_id(Unit *u); bool unit_shall_confirm_spawn(Unit *u); diff --git a/src/test/test-bpf-firewall.c b/src/test/test-bpf-firewall.c index b6fd22904f..b29c0d7844 100644 --- a/src/test/test-bpf-firewall.c +++ b/src/test/test-bpf-firewall.c @@ -12,7 +12,7 @@ #include "rm-rf.h" #include "service.h" #include "tests.h" -#include "unit.h" +#include "unit-serialize.h" #include "virt.h" int main(int argc, char *argv[]) { From 9466ec13a531288db2b893b0c57cb5949f582a7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 30 Jan 2021 20:50:19 +0100 Subject: [PATCH 04/19] core: add helper macros for deserialization A helper function would seem more natural, but there are two reasons why a macro is needed: - many bool fields are bitfields, so we can't take a pointer, and using a macro allows us to avoid taking a pointer. - we have a few diffent types (bool, uint64_t, FreezerState), and we can have type safety without specifying the type by using the macro. This also makes the error messages more informative: they print the exact field identifier that failed, which is more useful for debugging than a description. --- src/core/unit-serialize.c | 188 +++++++++++--------------------------- src/core/unit.c | 1 - 2 files changed, 51 insertions(+), 138 deletions(-) diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c index 5cab175021..ea4a57907e 100644 --- a/src/core/unit-serialize.c +++ b/src/core/unit-serialize.c @@ -172,6 +172,32 @@ static int unit_deserialize_job(Unit *u, FILE *f) { return 0; } +#define MATCH_DESERIALIZE(key, l, v, parse_func, target) \ + ({ \ + bool _deserialize_matched = streq(l, key); \ + if (_deserialize_matched) { \ + int _deserialize_r = parse_func(v); \ + if (_deserialize_r < 0) \ + log_unit_debug_errno(u, _deserialize_r, \ + "Failed to parse \"%s=%s\", ignoring.", l, v); \ + else \ + target = _deserialize_r; \ + }; \ + _deserialize_matched; \ + }) + +#define MATCH_DESERIALIZE_IMMEDIATE(key, l, v, parse_func, target) \ + ({ \ + bool _deserialize_matched = streq(l, key); \ + if (_deserialize_matched) { \ + int _deserialize_r = parse_func(v, &target); \ + if (_deserialize_r < 0) \ + log_unit_debug_errno(u, _deserialize_r, \ + "Failed to parse \"%s=%s\", ignoring", l, v); \ + }; \ + _deserialize_matched; \ + }) + int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { int r; @@ -233,130 +259,48 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { } else if (streq(l, "assert-timestamp")) { (void) deserialize_dual_timestamp(v, &u->assert_timestamp); continue; - } else if (streq(l, "condition-result")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse condition result value %s, ignoring.", v); - else - u->condition_result = r; + } else if (MATCH_DESERIALIZE("condition-result", l, v, parse_boolean, u->condition_result)) continue; - } else if (streq(l, "assert-result")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse assert result value %s, ignoring.", v); - else - u->assert_result = r; - + else if (MATCH_DESERIALIZE("assert-result", l, v, parse_boolean, u->assert_result)) continue; - } else if (streq(l, "transient")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse transient bool %s, ignoring.", v); - else - u->transient = r; - + else if (MATCH_DESERIALIZE("transient", l, v, parse_boolean, u->transient)) continue; - } else if (streq(l, "in-audit")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse in-audit bool %s, ignoring.", v); - else - u->in_audit = r; - + else if (MATCH_DESERIALIZE("in-audit", l, v, parse_boolean, u->in_audit)) continue; - } else if (streq(l, "exported-invocation-id")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse exported invocation ID bool %s, ignoring.", v); - else - u->exported_invocation_id = r; - + else if (MATCH_DESERIALIZE("exported-invocation-id", l, v, parse_boolean, u->exported_invocation_id)) continue; - } else if (streq(l, "exported-log-level-max")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse exported log level max bool %s, ignoring.", v); - else - u->exported_log_level_max = r; - + else if (MATCH_DESERIALIZE("exported-log-level-max", l, v, parse_boolean, u->exported_log_level_max)) continue; - } else if (streq(l, "exported-log-extra-fields")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse exported log extra fields bool %s, ignoring.", v); - else - u->exported_log_extra_fields = r; - + else if (MATCH_DESERIALIZE("exported-log-extra-fields", l, v, parse_boolean, u->exported_log_extra_fields)) continue; - } else if (streq(l, "exported-log-rate-limit-interval")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse exported log rate limit interval %s, ignoring.", v); - else - u->exported_log_ratelimit_interval = r; - + else if (MATCH_DESERIALIZE("exported-log-rate-limit-interval", l, v, parse_boolean, u->exported_log_ratelimit_interval)) continue; - } else if (streq(l, "exported-log-rate-limit-burst")) { - - r = parse_boolean(v); - if (r < 0) - log_unit_debug(u, "Failed to parse exported log rate limit burst %s, ignoring.", v); - else - u->exported_log_ratelimit_burst = r; - + else if (MATCH_DESERIALIZE("exported-log-rate-limit-burst", l, v, parse_boolean, u->exported_log_ratelimit_burst)) continue; - } else if (STR_IN_SET(l, "cpu-usage-base", "cpuacct-usage-base")) { - - r = safe_atou64(v, &u->cpu_usage_base); - if (r < 0) - log_unit_debug(u, "Failed to parse CPU usage base %s, ignoring.", v); - + else if (MATCH_DESERIALIZE_IMMEDIATE("cpu-usage-base", l, v, safe_atou64, u->cpu_usage_base) || + MATCH_DESERIALIZE_IMMEDIATE("cpuacct-usage-base", l, v, safe_atou64, u->cpu_usage_base)) continue; - } else if (streq(l, "cpu-usage-last")) { - - r = safe_atou64(v, &u->cpu_usage_last); - if (r < 0) - log_unit_debug(u, "Failed to read CPU usage last %s, ignoring.", v); - + else if (MATCH_DESERIALIZE_IMMEDIATE("cpu-usage-last", l, v, safe_atou64, u->cpu_usage_last)) continue; - } else if (streq(l, "managed-oom-kill-last")) { - - r = safe_atou64(v, &u->managed_oom_kill_last); - if (r < 0) - log_unit_debug(u, "Failed to read managed OOM kill last %s, ignoring.", v); - + else if (MATCH_DESERIALIZE_IMMEDIATE("managed-oom-kill-last", l, v, safe_atou64, u->managed_oom_kill_last)) continue; - } else if (streq(l, "oom-kill-last")) { - - r = safe_atou64(v, &u->oom_kill_last); - if (r < 0) - log_unit_debug(u, "Failed to read OOM kill last %s, ignoring.", v); - + else if (MATCH_DESERIALIZE_IMMEDIATE("oom-kill-last", l, v, safe_atou64, u->oom_kill_last)) continue; - } else if (streq(l, "cgroup")) { - + else if (streq(l, "cgroup")) { r = unit_set_cgroup_path(u, v); if (r < 0) log_unit_debug_errno(u, r, "Failed to set cgroup path %s, ignoring: %m", v); @@ -365,47 +309,27 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { (void) unit_watch_cgroup_memory(u); continue; - } else if (streq(l, "cgroup-realized")) { - int b; - - b = parse_boolean(v); - if (b < 0) - log_unit_debug(u, "Failed to parse cgroup-realized bool %s, ignoring.", v); - else - u->cgroup_realized = b; + } else if (MATCH_DESERIALIZE("cgroup-realized", l, v, parse_boolean, u->cgroup_realized)) continue; - } else if (streq(l, "cgroup-realized-mask")) { - - r = cg_mask_from_string(v, &u->cgroup_realized_mask); - if (r < 0) - log_unit_debug(u, "Failed to parse cgroup-realized-mask %s, ignoring.", v); + else if (MATCH_DESERIALIZE_IMMEDIATE("cgroup-realized-mask", l, v, cg_mask_from_string, u->cgroup_realized_mask)) continue; - } else if (streq(l, "cgroup-enabled-mask")) { - - r = cg_mask_from_string(v, &u->cgroup_enabled_mask); - if (r < 0) - log_unit_debug(u, "Failed to parse cgroup-enabled-mask %s, ignoring.", v); + else if (MATCH_DESERIALIZE_IMMEDIATE("cgroup-enabled-mask", l, v, cg_mask_from_string, u->cgroup_enabled_mask)) continue; - } else if (streq(l, "cgroup-invalidated-mask")) { - - r = cg_mask_from_string(v, &u->cgroup_invalidated_mask); - if (r < 0) - log_unit_debug(u, "Failed to parse cgroup-invalidated-mask %s, ignoring.", v); + else if (MATCH_DESERIALIZE_IMMEDIATE("cgroup-invalidated-mask", l, v, cg_mask_from_string, u->cgroup_invalidated_mask)) continue; - } else if (streq(l, "ref-uid")) { + else if (streq(l, "ref-uid")) { uid_t uid; r = parse_uid(v, &uid); if (r < 0) - log_unit_debug(u, "Failed to parse referenced UID %s, ignoring.", v); + log_unit_debug(u, "Failed to parse \"%s=%s\", ignoring.", l, v); else unit_ref_uid_gid(u, uid, GID_INVALID); - continue; } else if (streq(l, "ref-gid")) { @@ -413,25 +337,23 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { r = parse_gid(v, &gid); if (r < 0) - log_unit_debug(u, "Failed to parse referenced GID %s, ignoring.", v); + log_unit_debug(u, "Failed to parse \"%s=%s\", ignoring.", l, v); else unit_ref_uid_gid(u, UID_INVALID, gid); - continue; } else if (streq(l, "ref")) { - r = strv_extend(&u->deserialized_refs, v); if (r < 0) return log_oom(); - continue; + } else if (streq(l, "invocation-id")) { sd_id128_t id; r = sd_id128_from_string(v, &id); if (r < 0) - log_unit_debug(u, "Failed to parse invocation id %s, ignoring.", v); + log_unit_debug(u, "Failed to parse \"%s=%s\", ignoring.", l, v); else { r = unit_set_invocation_id(u, id); if (r < 0) @@ -439,17 +361,9 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { } continue; - } else if (streq(l, "freezer-state")) { - FreezerState s; - - s = freezer_state_from_string(v); - if (s < 0) - log_unit_debug(u, "Failed to deserialize freezer-state '%s', ignoring.", v); - else - u->freezer_state = s; + } else if (MATCH_DESERIALIZE("freezer-state", l, v, freezer_state_from_string, u->freezer_state)) continue; - } /* Check if this is an IP accounting metric serialization field */ m = string_table_lookup(ip_accounting_metric_field, ELEMENTSOF(ip_accounting_metric_field), l); diff --git a/src/core/unit.c b/src/core/unit.c index eacebe796a..d1525f5553 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3261,7 +3261,6 @@ bool unit_can_serialize(Unit *u) { return UNIT_VTABLE(u)->serialize && UNIT_VTABLE(u)->deserialize_item; } - int unit_add_node_dependency(Unit *u, const char *what, UnitDependency dep, UnitDependencyMask mask) { _cleanup_free_ char *e = NULL; Unit *device; From ef3fc326b9cb080134916683b5ad7095d9e3db13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 30 Jan 2021 17:27:45 +0100 Subject: [PATCH 05/19] manager: remove unnecessary conditional --- src/core/manager.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index c7cd1fbc6d..ee28bf4e98 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1188,18 +1188,15 @@ static void unit_gc_sweep(Unit *u, unsigned gc_marker) { is_bad = false; } - if (u->refs_by_target) { - const UnitRef *ref; + const UnitRef *ref; + LIST_FOREACH(refs_by_target, ref, u->refs_by_target) { + unit_gc_sweep(ref->source, gc_marker); - LIST_FOREACH(refs_by_target, ref, u->refs_by_target) { - unit_gc_sweep(ref->source, gc_marker); + if (ref->source->gc_marker == gc_marker + GC_OFFSET_GOOD) + goto good; - if (ref->source->gc_marker == gc_marker + GC_OFFSET_GOOD) - goto good; - - if (ref->source->gc_marker != gc_marker + GC_OFFSET_BAD) - is_bad = false; - } + if (ref->source->gc_marker != gc_marker + GC_OFFSET_BAD) + is_bad = false; } if (is_bad) From 8c2524c7fdb46d801594030504c6059afcde89fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 10 Feb 2021 17:32:31 +0100 Subject: [PATCH 06/19] core: pahole optimization of struct Unit We had a lone 'bool job_running_timeout_set:1', which generated a hole. Let's move things around a bit. The structure is a tiny bit smaller and has less holes: /* size: 1192, cachelines: 19, members: 149 */ /* sum members: 1175, holes: 3, sum holes: 11 */ /* sum bitfield members: 27 bits, bit holes: 1, sum bit holes: 7 bits */ /* bit_padding: 14 bits */ /* last cacheline: 40 bytes */ /* size: 1184, cachelines: 19, members: 149 */ /* sum members: 1175, holes: 1, sum holes: 4 */ /* sum bitfield members: 27 bits (3 bytes) */ /* bit_padding: 13 bits */ /* last cacheline: 32 bytes */ --- src/core/unit.h | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/core/unit.h b/src/core/unit.h index 1af0e50312..457eba44a1 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -120,9 +120,6 @@ typedef struct Unit { UnitLoadState load_state; Unit *merged_into; - FreezerState freezer_state; - sd_bus_message *pending_freezer_message; - char *id; /* The one special name that we use for identification */ char *instance; @@ -150,6 +147,16 @@ typedef struct Unit { /* If this is a transient unit we are currently writing, this is where we are writing it to */ FILE *transient_file; + /* Freezer state */ + sd_bus_message *pending_freezer_message; + FreezerState freezer_state; + + /* Job timeout and action to take */ + EmergencyAction job_timeout_action; + usec_t job_timeout; + usec_t job_running_timeout; + char *job_timeout_reboot_arg; + /* If there is something to do with this unit, then this is the installed job for it */ Job *job; @@ -164,13 +171,6 @@ typedef struct Unit { sd_bus_track *bus_track; char **deserialized_refs; - /* Job timeout and action to take */ - usec_t job_timeout; - usec_t job_running_timeout; - bool job_running_timeout_set:1; - EmergencyAction job_timeout_action; - char *job_timeout_reboot_arg; - /* References to this */ LIST_HEAD(UnitRef, refs_by_target); @@ -359,6 +359,8 @@ typedef struct Unit { bool sent_dbus_new_signal:1; + bool job_running_timeout_set:1; + bool in_audit:1; bool on_console:1; From ff68472a20c208121b69ea13586f3105a219bc14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 30 Jan 2021 16:58:19 +0100 Subject: [PATCH 07/19] core: add Unit.Markers property The property is never set by systemd, only reset after a stop or restart or reload. It may externally be set to mark the unit for a later restart/reload. I wasn't sure whether to configure the property only for the types where this makes sense (Service, Swap, etc). But Restart() method is defined on the unit, and also having this always under the same property name is more convenient. --- man/org.freedesktop.systemd1.xml | 16 +++++++-- src/basic/unit-def.c | 7 ++++ src/basic/unit-def.h | 10 ++++++ src/core/dbus-unit.c | 34 +++++++++++++++++++ src/core/unit-serialize.c | 56 ++++++++++++++++++++++++++++++++ src/core/unit.c | 6 +++- src/core/unit.h | 3 ++ src/udev/test-udev-builtin.c | 2 +- 8 files changed, 130 insertions(+), 4 deletions(-) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index 7543a617b7..13771a459a 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -1685,6 +1685,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { readonly b IgnoreOnIsolate = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly b NeedDaemonReload = ...; + @org.freedesktop.DBus.Property.EmitsChangedSignal("false") + readonly as Markers = ['...', ...]; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly t JobTimeoutUSec = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") @@ -1969,6 +1971,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + @@ -2160,8 +2164,16 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { NeedDaemonReload is a boolean that indicates whether the configuration file this unit is loaded from (i.e. FragmentPath or SourcePath) has - changed since the configuration was read and hence whether a configuration reload is - recommended. + changed since the configuration was read and hence whether a configuration reload is recommended. + + + Markers is an array of string flags that can be set using + SetUnitProperties() to indicate that the service should be reloaded or + restarted. Currently known values are needs-restart and + needs-reload. Package scripts may use the first to mark units for later restart when + a new version of the package is installed. Configuration management scripts may use the second to mark + units for a later reload when the configuration is adjusted. Those flags are not set by the manager, + except to unset as appropriate when when the unit is stopped, restarted, or reloaded. JobTimeoutUSec maps directly to the corresponding configuration setting in the unit file. diff --git a/src/basic/unit-def.c b/src/basic/unit-def.c index bb4fa1ec6b..6fbb947f09 100644 --- a/src/basic/unit-def.c +++ b/src/basic/unit-def.c @@ -117,6 +117,13 @@ static const char* const freezer_state_table[_FREEZER_STATE_MAX] = { DEFINE_STRING_TABLE_LOOKUP(freezer_state, FreezerState); +static const char* const unit_marker_table[_UNIT_MARKER_MAX] = { + [UNIT_MARKER_NEEDS_RELOAD] = "needs-reload", + [UNIT_MARKER_NEEDS_RESTART] = "needs-restart", +}; + +DEFINE_STRING_TABLE_LOOKUP(unit_marker, UnitMarker); + static const char* const automount_state_table[_AUTOMOUNT_STATE_MAX] = { [AUTOMOUNT_DEAD] = "dead", [AUTOMOUNT_WAITING] = "waiting", diff --git a/src/basic/unit-def.h b/src/basic/unit-def.h index ff05799c1d..7742c5c078 100644 --- a/src/basic/unit-def.h +++ b/src/basic/unit-def.h @@ -58,6 +58,13 @@ typedef enum FreezerState { _FREEZER_STATE_INVALID = -EINVAL, } FreezerState; +typedef enum UnitMarker { + UNIT_MARKER_NEEDS_RELOAD, + UNIT_MARKER_NEEDS_RESTART, + _UNIT_MARKER_MAX, + _UNIT_MARKER_INVALID = -1 +} UnitMarker; + typedef enum AutomountState { AUTOMOUNT_DEAD, AUTOMOUNT_WAITING, @@ -267,6 +274,9 @@ UnitActiveState unit_active_state_from_string(const char *s) _pure_; const char *freezer_state_to_string(FreezerState i) _const_; FreezerState freezer_state_from_string(const char *s) _pure_; +const char *unit_marker_to_string(UnitMarker m) _const_; +UnitMarker unit_marker_from_string(const char *s) _pure_; + const char* automount_state_to_string(AutomountState i) _const_; AutomountState automount_state_from_string(const char *s) _pure_; diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 90bf5514a3..c9837df9a2 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -323,6 +323,39 @@ static int property_get_load_error( return sd_bus_message_append(reply, "(ss)", NULL, NULL); } +static int property_get_markers( + sd_bus *bus, + const char *path, + const char *interface, + const char *property, + sd_bus_message *reply, + void *userdata, + sd_bus_error *error) { + + unsigned *markers = userdata; + int r; + + assert(bus); + assert(reply); + assert(markers); + + r = sd_bus_message_open_container(reply, 'a', "s"); + if (r < 0) + return r; + + /* Make sure out values fit in the bitfield. */ + assert_cc(_UNIT_MARKER_MAX <= sizeof(((Unit){}).markers) * 8); + + for (UnitMarker m = 0; m < _UNIT_MARKER_MAX; m++) + if (FLAGS_SET(*markers, 1u << m)) { + r = sd_bus_message_append(reply, "s", unit_marker_to_string(m)); + if (r < 0) + return r; + } + + return sd_bus_message_close_container(reply); +} + static const char *const polkit_message_for_job[_JOB_TYPE_MAX] = { [JOB_START] = N_("Authentication is required to start '$(unit)'."), [JOB_STOP] = N_("Authentication is required to stop '$(unit)'."), @@ -864,6 +897,7 @@ const sd_bus_vtable bus_unit_vtable[] = { SD_BUS_PROPERTY("OnFailureJobMode", "s", property_get_job_mode, offsetof(Unit, on_failure_job_mode), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("IgnoreOnIsolate", "b", bus_property_get_bool, offsetof(Unit, ignore_on_isolate), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("NeedDaemonReload", "b", property_get_need_daemon_reload, 0, SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("Markers", "as", property_get_markers, offsetof(Unit, markers), 0), SD_BUS_PROPERTY("JobTimeoutUSec", "t", bus_property_get_usec, offsetof(Unit, job_timeout), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("JobRunningTimeoutUSec", "t", bus_property_get_usec, offsetof(Unit, job_running_timeout), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("JobTimeoutAction", "s", property_get_emergency_action, offsetof(Unit, job_timeout_action), SD_BUS_VTABLE_PROPERTY_CONST), diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c index ea4a57907e..3f099248ce 100644 --- a/src/core/unit-serialize.c +++ b/src/core/unit-serialize.c @@ -28,6 +28,45 @@ static int serialize_cgroup_mask(FILE *f, const char *key, CGroupMask mask) { return serialize_item(f, key, s); } +/* Make sure out values fit in the bitfield. */ +assert_cc(_UNIT_MARKER_MAX <= sizeof(((Unit){}).markers) * 8); + +static int serialize_markers(FILE *f, unsigned markers) { + assert(f); + + if (markers == 0) + return 0; + + fputs("markers=", f); + for (UnitMarker m = 0; m < _UNIT_MARKER_MAX; m++) + if (FLAGS_SET(markers, 1u << m)) + fputs(unit_marker_to_string(m), f); + fputc('\n', f); + return 0; +} + +static int deserialize_markers(Unit *u, const char *value) { + assert(u); + assert(value); + int r; + + for (const char *p = value;;) { + _cleanup_free_ char *word = NULL; + + r = extract_first_word(&p, &word, NULL, 0); + if (r <= 0) + return r; + + UnitMarker m = unit_marker_from_string(word); + if (m < 0) { + log_unit_debug_errno(u, m, "Unknown unit marker \"%s\", ignoring.", word); + continue; + } + + u->markers |= 1u << m; + } +} + static const char *const ip_accounting_metric_field[_CGROUP_IP_ACCOUNTING_METRIC_MAX] = { [CGROUP_IP_INGRESS_BYTES] = "ip-accounting-ingress-bytes", [CGROUP_IP_INGRESS_PACKETS] = "ip-accounting-ingress-packets", @@ -121,6 +160,7 @@ int unit_serialize(Unit *u, FILE *f, FDSet *fds, bool serialize_jobs) { (void) serialize_item_format(f, "invocation-id", SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(u->invocation_id)); (void) serialize_item_format(f, "freezer-state", "%s", freezer_state_to_string(unit_freezer_state(u))); + (void) serialize_markers(f, u->markers); bus_track_serialize(u->bus_track, f, "ref"); @@ -365,6 +405,13 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { } else if (MATCH_DESERIALIZE("freezer-state", l, v, freezer_state_from_string, u->freezer_state)) continue; + else if (streq(l, "markers")) { + r = deserialize_markers(u, v); + if (r < 0) + log_unit_debug_errno(u, r, "Failed to deserialize \"%s=%s\", ignoring: %m", l, v); + continue; + } + /* Check if this is an IP accounting metric serialization field */ m = string_table_lookup(ip_accounting_metric_field, ELEMENTSOF(ip_accounting_metric_field), l); if (m >= 0) { @@ -559,6 +606,15 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { prefix, strna(u->cgroup_path), prefix, yes_no(u->cgroup_realized)); + if (u->markers != 0) { + fprintf(f, "%s\tMarkers:", prefix); + + for (UnitMarker marker = 0; marker < _UNIT_MARKER_MAX; marker++) + if (FLAGS_SET(u->markers, 1u << marker)) + fprintf(f, " %s", unit_marker_to_string(marker)); + fputs("\n", f); + } + if (u->cgroup_realized_mask != 0) { _cleanup_free_ char *s = NULL; (void) cg_mask_to_string(u->cgroup_realized_mask, &s); diff --git a/src/core/unit.c b/src/core/unit.c index d1525f5553..9c7f3dcdc0 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -2390,9 +2390,13 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, UnitNotifyFlag /* Make sure the cgroup and state files are always removed when we become inactive */ if (UNIT_IS_INACTIVE_OR_FAILED(ns)) { + SET_FLAG(u->markers, + (1u << UNIT_MARKER_NEEDS_RELOAD)|(1u << UNIT_MARKER_NEEDS_RESTART), + false); unit_prune_cgroup(u); unit_unlink_state_files(u); - } + } else if (ns != os && ns == UNIT_RELOADING) + SET_FLAG(u->markers, 1u << UNIT_MARKER_NEEDS_RELOAD, false); unit_update_on_console(u); diff --git a/src/core/unit.h b/src/core/unit.h index 457eba44a1..7c13e50878 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -242,6 +242,9 @@ typedef struct Unit { RateLimit start_ratelimit; EmergencyAction start_limit_action; + /* The unit has been marked for reload, restart, etc. Stored as 1u << marker1 | 1u << marker2. */ + unsigned markers; + /* What to do on failure or success */ EmergencyAction success_action, failure_action; int success_action_exit_status, failure_action_exit_status; diff --git a/src/udev/test-udev-builtin.c b/src/udev/test-udev-builtin.c index 1bd1dbddf5..21a8ea3fa6 100644 --- a/src/udev/test-udev-builtin.c +++ b/src/udev/test-udev-builtin.c @@ -6,7 +6,7 @@ static void test_udev_builtin_cmd_to_ptr(void) { log_info("/* %s */", __func__); - /* Those could have been static_assert()s, but ({}) is not allowed there. */ + /* Those could have been static asserts, but ({}) is not allowed there. */ #if HAVE_BLKID assert_se(UDEV_BUILTIN_CMD_TO_PTR(UDEV_BUILTIN_BLKID)); assert_se(PTR_TO_UDEV_BUILTIN_CMD(UDEV_BUILTIN_CMD_TO_PTR(UDEV_BUILTIN_BLKID)) == UDEV_BUILTIN_BLKID); From 95d1e21e9071f3c6c0be87f238dbea86c93231fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 12 Feb 2021 10:25:13 +0100 Subject: [PATCH 08/19] core/dbus-unit: reduce scope of iterator variables --- src/core/dbus-unit.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index c9837df9a2..561a4e887f 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -811,7 +811,6 @@ static int property_get_refs( sd_bus_error *error) { Unit *u = userdata; - const char *i; int r; assert(bus); @@ -821,15 +820,15 @@ static int property_get_refs( if (r < 0) return r; - for (i = sd_bus_track_first(u->bus_track); i; i = sd_bus_track_next(u->bus_track)) { - int c, k; + for (const char *i = sd_bus_track_first(u->bus_track); i; i = sd_bus_track_next(u->bus_track)) { + int c; c = sd_bus_track_count_name(u->bus_track, i); if (c < 0) return c; /* Add the item multiple times if the ref count for each is above 1 */ - for (k = 0; k < c; k++) { + for (int k = 0; k < c; k++) { r = sd_bus_message_append(reply, "s", i); if (r < 0) return r; From 0dd3c0907f42774dccd79658d744a5a75c72ef58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 6 Feb 2021 11:39:55 +0100 Subject: [PATCH 09/19] core: allow Markers to be set using set-property --- src/core/dbus-unit.c | 57 ++++++++++++++++++++++++++++++++++++++ src/shared/bus-unit-util.c | 3 +- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 561a4e887f..1e7e51ffba 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -1871,6 +1871,63 @@ static int bus_unit_set_live_property( return 1; } + /* A setting that only applies to active units. We don't actually write this to /run, this state is + * managed internally. "+foo" sets flag foo, "-foo" unsets flag foo, just "foo" resets flags to + * foo. The last type cannot be mixed with "+" or "-". */ + + if (streq(name, "Markers")) { + unsigned settings = 0, mask = 0; + bool some_plus_minus = false, some_absolute = false; + + r = sd_bus_message_enter_container(message, 'a', "s"); + if (r < 0) + return r; + + for (;;) { + const char *word; + bool b; + + r = sd_bus_message_read(message, "s", &word); + if (r < 0) + return r; + if (r == 0) + break; + + if (IN_SET(word[0], '+', '-')) { + b = word[0] == '+'; + word++; + some_plus_minus = true; + } else { + b = true; + some_absolute = true; + } + + UnitMarker m = unit_marker_from_string(word); + if (m < 0) + return sd_bus_error_setf(error, BUS_ERROR_BAD_UNIT_SETTING, + "Unknown marker \"%s\".", word); + + SET_FLAG(settings, 1u << m, b); + SET_FLAG(mask, 1u << m, true); + } + + r = sd_bus_message_exit_container(message); + if (r < 0) + return r; + + if (some_plus_minus && some_absolute) + return sd_bus_error_setf(error, BUS_ERROR_BAD_UNIT_SETTING, "Bad marker syntax."); + + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { + if (some_absolute) + u->markers = settings; + else + u->markers = settings | (u->markers & ~mask); + } + + return 1; + } + return 0; } diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 84f57d94d2..80345ef46a 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -2191,7 +2191,8 @@ static int bus_append_unit_property(sd_bus_message *m, const char *field, const if (unit_dependency_from_string(field) >= 0 || STR_IN_SET(field, "Documentation", - "RequiresMountsFor")) + "RequiresMountsFor", + "Markers")) return bus_append_strv(m, field, eq, EXTRACT_UNQUOTE); t = condition_type_from_string(field); From 70666e28a1affe241a9c4a6fb26aecdf7ce5d167 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 30 Jan 2021 19:44:15 +0100 Subject: [PATCH 10/19] core: add EnqueueMarkedJobs method to reload/restart marked units We support two return types for methods that start jobs. EnqueueJob support the full-monty mode with affected jobs. I didn't do this here, since it seems unlikely to be used. In the common case there'd be a huge list of jobs and affected jobs. EnqueueMarkedJobs() just returns a list of jobs that we can wait upon. The name of the method is generic in case we decide to add something other than just reload/restart later on. When errors occur, resource errors are treated as fatal, but for other error types we queue up other jobs, and only return an error at the end. The assumption is that the caller will ignore the result error anyway, so it's better to try to reload/restart as much as possible. --- man/org.freedesktop.systemd1.xml | 8 ++ src/core/dbus-manager.c | 75 +++++++++++++++ src/core/dbus-unit.c | 154 +++++++++++++++++-------------- src/core/dbus-unit.h | 16 +++- 4 files changed, 182 insertions(+), 71 deletions(-) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index 13771a459a..9e64e58fe3 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -176,6 +176,7 @@ node /org/freedesktop/systemd1 { UnsetEnvironment(in as names); UnsetAndSetEnvironment(in as names, in as assignments); + EnqueueMarkedJobs(out ao jobs); ListUnitFiles(out a(ss) unit_files); ListUnitFilesByPatterns(in as states, in as patterns, @@ -848,6 +849,8 @@ node /org/freedesktop/systemd1 { + + @@ -1171,6 +1174,11 @@ node /org/freedesktop/systemd1 { the "Try" flavor is used in which case a service that isn't running is not affected by the restart. The "ReloadOrRestart" flavors attempt a reload if the unit supports it and use a restart otherwise. + EnqueueMarkedJobs() creates reload/restart jobs for units which have been + appropriately marked, see Marks property above. This is equivalent to calling + TryRestartUnit() or ReloadOrTryRestartUnit() for the marked + units. + BindMountUnit() can be used to bind mount new files or directories into a running service mount namespace. diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 9053d48149..c4b7aa427b 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -1807,6 +1807,75 @@ static int method_get_dynamic_users(sd_bus_message *message, void *userdata, sd_ return sd_bus_send(NULL, reply, NULL); } +static int method_enqueue_marked_jobs(sd_bus_message *message, void *userdata, sd_bus_error *error) { + Manager *m = userdata; + int r; + + assert(message); + assert(m); + + r = mac_selinux_access_check(message, "start", error); + if (r < 0) + return r; + + r = bus_verify_manage_units_async(m, message, error); + if (r < 0) + return r; + if (r == 0) + return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ + + log_info("Queuing reload/restart jobs for marked units…"); + + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + r = sd_bus_message_new_method_return(message, &reply); + if (r < 0) + return r; + + r = sd_bus_message_open_container(reply, 'a', "o"); + if (r < 0) + return r; + + Unit *u; + char *k; + int ret = 0; + HASHMAP_FOREACH_KEY(u, k, m->units) { + /* ignore aliases */ + if (u->id != k) + continue; + + BusUnitQueueFlags flags; + if (FLAGS_SET(u->markers, 1u << UNIT_MARKER_NEEDS_RESTART)) + flags = 0; + else if (FLAGS_SET(u->markers, 1u << UNIT_MARKER_NEEDS_RELOAD)) + flags = BUS_UNIT_QUEUE_RELOAD_IF_POSSIBLE; + else + continue; + + r = mac_selinux_unit_access_check(u, message, "start", error); + if (r >= 0) + r = bus_unit_queue_job_one(message, u, + JOB_TRY_RESTART, JOB_FAIL, flags, + reply, error); + if (r < 0) { + if (ERRNO_IS_RESOURCE(r)) + return r; + if (ret >= 0) + ret = r; + sd_bus_error_free(error); + } + } + + if (ret < 0) + return sd_bus_error_set_errnof(error, ret, + "Failed to enqueue some jobs, see logs for details: %m"); + + r = sd_bus_message_close_container(reply); + if (r < 0) + return r; + + return sd_bus_send(NULL, reply, NULL); +} + static int list_unit_files_by_patterns(sd_bus_message *message, void *userdata, sd_bus_error *error, char **states, char **patterns) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; Manager *m = userdata; @@ -3007,6 +3076,12 @@ const sd_bus_vtable bus_manager_vtable[] = { NULL,, method_unset_and_set_environment, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD_WITH_NAMES("EnqueueMarkedJobs", + NULL,, + "ao", + SD_BUS_PARAM(jobs), + method_enqueue_marked_jobs, + SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD_WITH_NAMES("ListUnitFiles", NULL,, "a(ss)", diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 1e7e51ffba..d8351a4d43 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -1716,6 +1716,89 @@ void bus_unit_send_removed_signal(Unit *u) { log_unit_debug_errno(u, r, "Failed to send unit remove signal for %s: %m", u->id); } +int bus_unit_queue_job_one( + sd_bus_message *message, + Unit *u, + JobType type, + JobMode mode, + BusUnitQueueFlags flags, + sd_bus_message *reply, + sd_bus_error *error) { + + _cleanup_set_free_ Set *affected = NULL; + _cleanup_free_ char *job_path = NULL, *unit_path = NULL; + Job *j, *a; + int r; + + if (FLAGS_SET(flags, BUS_UNIT_QUEUE_VERBOSE_REPLY)) { + affected = set_new(NULL); + if (!affected) + return -ENOMEM; + } + + r = manager_add_job(u->manager, type, u, mode, affected, error, &j); + if (r < 0) + return r; + + r = bus_job_track_sender(j, message); + if (r < 0) + return r; + + /* Before we send the method reply, force out the announcement JobNew for this job */ + bus_job_send_pending_change_signal(j, true); + + job_path = job_dbus_path(j); + if (!job_path) + return -ENOMEM; + + /* The classic response is just a job object path */ + if (!FLAGS_SET(flags, BUS_UNIT_QUEUE_VERBOSE_REPLY)) + return sd_bus_message_append(reply, "o", job_path); + + /* In verbose mode respond with the anchor job plus everything that has been affected */ + + unit_path = unit_dbus_path(j->unit); + if (!unit_path) + return -ENOMEM; + + r = sd_bus_message_append(reply, "uosos", + j->id, job_path, + j->unit->id, unit_path, + job_type_to_string(j->type)); + if (r < 0) + return r; + + r = sd_bus_message_open_container(reply, 'a', "(uosos)"); + if (r < 0) + return r; + + SET_FOREACH(a, affected) { + if (a->id == j->id) + continue; + + /* Free paths from previous iteration */ + job_path = mfree(job_path); + unit_path = mfree(unit_path); + + job_path = job_dbus_path(a); + if (!job_path) + return -ENOMEM; + + unit_path = unit_dbus_path(a->unit); + if (!unit_path) + return -ENOMEM; + + r = sd_bus_message_append(reply, "(uosos)", + a->id, job_path, + a->unit->id, unit_path, + job_type_to_string(a->type)); + if (r < 0) + return r; + } + + return sd_bus_message_close_container(reply); +} + int bus_unit_queue_job( sd_bus_message *message, Unit *u, @@ -1725,9 +1808,6 @@ int bus_unit_queue_job( sd_bus_error *error) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - _cleanup_free_ char *job_path = NULL, *unit_path = NULL; - _cleanup_set_free_ Set *affected = NULL; - Job *j, *a; int r; assert(message); @@ -1760,77 +1840,11 @@ int bus_unit_queue_job( (type == JOB_RELOAD_OR_START && job_type_collapse(type, u) == JOB_START && u->refuse_manual_start)) return sd_bus_error_setf(error, BUS_ERROR_ONLY_BY_DEPENDENCY, "Operation refused, unit %s may be requested by dependency only (it is configured to refuse manual start/stop).", u->id); - if (FLAGS_SET(flags, BUS_UNIT_QUEUE_VERBOSE_REPLY)) { - affected = set_new(NULL); - if (!affected) - return -ENOMEM; - } - - r = manager_add_job(u->manager, type, u, mode, affected, error, &j); - if (r < 0) - return r; - - r = bus_job_track_sender(j, message); - if (r < 0) - return r; - - /* Before we send the method reply, force out the announcement JobNew for this job */ - bus_job_send_pending_change_signal(j, true); - - job_path = job_dbus_path(j); - if (!job_path) - return -ENOMEM; - - /* The classic response is just a job object path */ - if (!FLAGS_SET(flags, BUS_UNIT_QUEUE_VERBOSE_REPLY)) - return sd_bus_reply_method_return(message, "o", job_path); - - /* In verbose mode respond with the anchor job plus everything that has been affected */ r = sd_bus_message_new_method_return(message, &reply); if (r < 0) return r; - unit_path = unit_dbus_path(j->unit); - if (!unit_path) - return -ENOMEM; - - r = sd_bus_message_append(reply, "uosos", - j->id, job_path, - j->unit->id, unit_path, - job_type_to_string(j->type)); - if (r < 0) - return r; - - r = sd_bus_message_open_container(reply, 'a', "(uosos)"); - if (r < 0) - return r; - - SET_FOREACH(a, affected) { - - if (a->id == j->id) - continue; - - /* Free paths from previous iteration */ - job_path = mfree(job_path); - unit_path = mfree(unit_path); - - job_path = job_dbus_path(a); - if (!job_path) - return -ENOMEM; - - unit_path = unit_dbus_path(a->unit); - if (!unit_path) - return -ENOMEM; - - r = sd_bus_message_append(reply, "(uosos)", - a->id, job_path, - a->unit->id, unit_path, - job_type_to_string(a->type)); - if (r < 0) - return r; - } - - r = sd_bus_message_close_container(reply); + r = bus_unit_queue_job_one(message, u, type, mode, flags, reply, error); if (r < 0) return r; diff --git a/src/core/dbus-unit.h b/src/core/dbus-unit.h index 1da3cfeb96..a3ac320496 100644 --- a/src/core/dbus-unit.h +++ b/src/core/dbus-unit.h @@ -33,7 +33,21 @@ typedef enum BusUnitQueueFlags { BUS_UNIT_QUEUE_VERBOSE_REPLY = 1 << 1, } BusUnitQueueFlags; -int bus_unit_queue_job(sd_bus_message *message, Unit *u, JobType type, JobMode mode, BusUnitQueueFlags flags, sd_bus_error *error); +int bus_unit_queue_job_one( + sd_bus_message *message, + Unit *u, + JobType type, + JobMode mode, + BusUnitQueueFlags flags, + sd_bus_message *reply, + sd_bus_error *error); +int bus_unit_queue_job( + sd_bus_message *message, + Unit *u, + JobType type, + JobMode mode, + BusUnitQueueFlags flags, + sd_bus_error *error); int bus_unit_validate_load_state(Unit *u, sd_bus_error *error); int bus_unit_track_add_name(Unit *u, const char *name); From cb31470f40283f4806a9fd50fb612800e99ef75d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 30 Jan 2021 19:53:58 +0100 Subject: [PATCH 11/19] core: wrap long lines --- src/core/dbus-manager.c | 130 +++++++++++++++++++++++++++------------- src/core/dbus-unit.c | 18 +++--- 2 files changed, 97 insertions(+), 51 deletions(-) diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index c4b7aa427b..86d0b3744d 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -38,8 +38,8 @@ #include "virt.h" #include "watchdog.h" -/* Require 16MiB free in /run/systemd for reloading/reexecing. After all we need to serialize our state there, and if - * we can't we'll fail badly. */ +/* Require 16MiB free in /run/systemd for reloading/reexecing. After all we need to serialize our state + * there, and if we can't we'll fail badly. */ #define RELOAD_DISK_SPACE_MIN (UINT64_C(16) * UINT64_C(1024) * UINT64_C(1024)) static UnitFileFlags unit_file_bools_to_flags(bool runtime, bool force) { @@ -363,8 +363,8 @@ static int bus_get_unit_by_name(Manager *m, sd_bus_message *message, const char assert(message); assert(ret_unit); - /* More or less a wrapper around manager_get_unit() that generates nice errors and has one trick up its sleeve: - * if the name is specified empty we use the client's unit. */ + /* More or less a wrapper around manager_get_unit() that generates nice errors and has one trick up + * its sleeve: if the name is specified empty we use the client's unit. */ if (isempty(name)) { _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; @@ -520,7 +520,8 @@ static int method_get_unit_by_invocation_id(sd_bus_message *message, void *userd u = manager_get_unit_by_pid(m, pid); if (!u) - return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "Client " PID_FMT " not member of any unit.", pid); + return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, + "Client " PID_FMT " not member of any unit.", pid); } else { u = hashmap_get(m->units_by_invocation_id, &id); if (!u) @@ -531,8 +532,9 @@ static int method_get_unit_by_invocation_id(sd_bus_message *message, void *userd if (r < 0) return r; - /* So here's a special trick: the bus path we return actually references the unit by its invocation ID instead - * of the unit name. This means it stays valid only as long as the invocation ID stays the same. */ + /* So here's a special trick: the bus path we return actually references the unit by its invocation + * ID instead of the unit name. This means it stays valid only as long as the invocation ID stays the + * same. */ path = unit_dbus_path_invocation_id(u); if (!path) return -ENOMEM; @@ -552,7 +554,9 @@ static int method_get_unit_by_control_group(sd_bus_message *message, void *userd u = manager_get_unit_by_cgroup(m, cgroup); if (!u) - return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "Control group '%s' is not valid or not managed by this instance", cgroup); + return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, + "Control group '%s' is not valid or not managed by this instance", + cgroup); return reply_unit_path(u, message, error); } @@ -851,17 +855,21 @@ static int transient_unit_from_message( t = unit_name_to_type(name); if (t < 0) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid unit name or type."); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Invalid unit name or type."); if (!unit_vtable[t]->can_transient) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unit type %s does not support transient units.", unit_type_to_string(t)); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Unit type %s does not support transient units.", + unit_type_to_string(t)); r = manager_load_unit(m, name, NULL, error, &u); if (r < 0) return r; if (!unit_is_pristine(u)) - return sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS, "Unit %s already exists.", name); + return sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS, + "Unit %s already exists.", name); /* OK, the unit failed to load and is unreferenced, now let's * fill in the transient data instead */ @@ -1435,7 +1443,8 @@ static int method_reboot(sd_bus_message *message, void *userdata, sd_bus_error * return r; if (!MANAGER_IS_SYSTEM(m)) - return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Reboot is only supported for system managers."); + return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, + "Reboot is only supported for system managers."); m->objective = MANAGER_REBOOT; @@ -1454,7 +1463,8 @@ static int method_poweroff(sd_bus_message *message, void *userdata, sd_bus_error return r; if (!MANAGER_IS_SYSTEM(m)) - return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Powering off is only supported for system managers."); + return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, + "Powering off is only supported for system managers."); m->objective = MANAGER_POWEROFF; @@ -1473,7 +1483,8 @@ static int method_halt(sd_bus_message *message, void *userdata, sd_bus_error *er return r; if (!MANAGER_IS_SYSTEM(m)) - return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Halt is only supported for system managers."); + return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, + "Halt is only supported for system managers."); m->objective = MANAGER_HALT; @@ -1492,7 +1503,8 @@ static int method_kexec(sd_bus_message *message, void *userdata, sd_bus_error *e return r; if (!MANAGER_IS_SYSTEM(m)) - return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "KExec is only supported for system managers."); + return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, + "KExec is only supported for system managers."); m->objective = MANAGER_KEXEC; @@ -1517,7 +1529,7 @@ static int method_switch_root(sd_bus_message *message, void *userdata, sd_bus_er if (available < RELOAD_DISK_SPACE_MIN) { char fb_available[FORMAT_BYTES_MAX], fb_need[FORMAT_BYTES_MAX]; - log_warning("Dangerously low amount of free space on /run/systemd, root switching operation might not complete successfully. " + log_warning("Dangerously low amount of free space on /run/systemd, root switching might fail.\n" "Currently, %s are free, but %s are suggested. Proceeding anyway.", format_bytes(fb_available, sizeof(fb_available), available), format_bytes(fb_need, sizeof(fb_need), RELOAD_DISK_SPACE_MIN)); @@ -1528,41 +1540,53 @@ static int method_switch_root(sd_bus_message *message, void *userdata, sd_bus_er return r; if (!MANAGER_IS_SYSTEM(m)) - return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Root switching is only supported by system manager."); + return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, + "Root switching is only supported by system manager."); r = sd_bus_message_read(message, "ss", &root, &init); if (r < 0) return r; if (isempty(root)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "New root directory may not be the empty string."); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "New root directory may not be the empty string."); if (!path_is_absolute(root)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "New root path '%s' is not absolute.", root); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "New root path '%s' is not absolute.", root); if (path_equal(root, "/")) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "New root directory cannot be the old root directory."); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "New root directory cannot be the old root directory."); /* Safety check */ if (isempty(init)) { r = path_is_os_tree(root); if (r < 0) - return sd_bus_error_set_errnof(error, r, "Failed to determine whether root path '%s' contains an OS tree: %m", root); + return sd_bus_error_set_errnof(error, r, + "Failed to determine whether root path '%s' contains an OS tree: %m", + root); if (r == 0) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Specified switch root path '%s' does not seem to be an OS tree. os-release file is missing.", root); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Specified switch root path '%s' does not seem to be an OS tree. os-release file is missing.", + root); } else { _cleanup_free_ char *chased = NULL; if (!path_is_absolute(init)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Path to init binary '%s' not absolute.", init); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Path to init binary '%s' not absolute.", init); r = chase_symlinks(init, root, CHASE_PREFIX_ROOT|CHASE_TRAIL_SLASH, &chased, NULL); if (r < 0) - return sd_bus_error_set_errnof(error, r, "Could not resolve init executable %s: %m", init); + return sd_bus_error_set_errnof(error, r, + "Could not resolve init executable %s: %m", init); if (laccess(chased, X_OK) < 0) { if (errno == EACCES) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Init binary %s is not executable.", init); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Init binary %s is not executable.", init); - return sd_bus_error_set_errnof(error, r, "Could not check whether init binary %s is executable: %m", init); + return sd_bus_error_set_errnof(error, r, + "Could not check whether init binary %s is executable: %m", init); } } @@ -1632,7 +1656,8 @@ static int method_unset_environment(sd_bus_message *message, void *userdata, sd_ return r; if (!strv_env_name_or_assignment_is_valid(minus)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid environment variable names or assignments"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Invalid environment variable names or assignments"); r = bus_verify_set_environment_async(m, message, error); if (r < 0) @@ -1668,9 +1693,11 @@ static int method_unset_and_set_environment(sd_bus_message *message, void *userd return r; if (!strv_env_name_or_assignment_is_valid(minus)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid environment variable names or assignments"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Invalid environment variable names or assignments"); if (!strv_env_is_valid(plus)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid environment assignments"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Invalid environment assignments"); r = bus_verify_set_environment_async(m, message, error); if (r < 0) @@ -1723,13 +1750,16 @@ static int method_lookup_dynamic_user_by_name(sd_bus_message *message, void *use return r; if (!MANAGER_IS_SYSTEM(m)) - return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Dynamic users are only supported in the system instance."); + return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, + "Dynamic users are only supported in the system instance."); if (!valid_user_group_name(name, VALID_USER_RELAX)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "User name invalid: %s", name); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "User name invalid: %s", name); r = dynamic_user_lookup_name(m, name, &uid); if (r == -ESRCH) - return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_DYNAMIC_USER, "Dynamic user %s does not exist.", name); + return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_DYNAMIC_USER, + "Dynamic user %s does not exist.", name); if (r < 0) return r; @@ -1751,13 +1781,16 @@ static int method_lookup_dynamic_user_by_uid(sd_bus_message *message, void *user return r; if (!MANAGER_IS_SYSTEM(m)) - return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Dynamic users are only supported in the system instance."); + return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, + "Dynamic users are only supported in the system instance."); if (!uid_is_valid(uid)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "User ID invalid: " UID_FMT, uid); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "User ID invalid: " UID_FMT, uid); r = dynamic_user_lookup_uid(m, uid, &name); if (r == -ESRCH) - return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_DYNAMIC_USER, "Dynamic user ID " UID_FMT " does not exist.", uid); + return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_DYNAMIC_USER, + "Dynamic user ID " UID_FMT " does not exist.", uid); if (r < 0) return r; @@ -1776,7 +1809,8 @@ static int method_get_dynamic_users(sd_bus_message *message, void *userdata, sd_ assert_cc(sizeof(uid_t) == sizeof(uint32_t)); if (!MANAGER_IS_SYSTEM(m)) - return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Dynamic users are only supported in the system instance."); + return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, + "Dynamic users are only supported in the system instance."); r = sd_bus_message_new_method_return(message, &reply); if (r < 0) @@ -1793,7 +1827,8 @@ static int method_get_dynamic_users(sd_bus_message *message, void *userdata, sd_ if (r == -EAGAIN) /* not realized yet? */ continue; if (r < 0) - return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Failed to look up a dynamic user."); + return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, + "Failed to look up a dynamic user."); r = sd_bus_message_append(reply, "(us)", uid, d->name); if (r < 0) @@ -2001,7 +2036,10 @@ static int send_unit_files_changed(sd_bus *bus, void *userdata) { assert(bus); - r = sd_bus_message_new_signal(bus, &message, "/org/freedesktop/systemd1", "org.freedesktop.systemd1.Manager", "UnitFilesChanged"); + r = sd_bus_message_new_signal(bus, &message, + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "UnitFilesChanged"); if (r < 0) return r; @@ -2129,8 +2167,8 @@ static int reply_unit_file_changes_and_free( good = true; } - /* If there was a failed change, and no successful change, then return the first failure as proper method call - * error. */ + /* If there was a failed change, and no successful change, then return the first failure as proper + * method call error. */ if (bad && !good) return install_error(error, 0, changes, n_changes); @@ -2555,7 +2593,8 @@ static int method_abandon_scope(sd_bus_message *message, void *userdata, sd_bus_ return r; if (u->type != UNIT_SCOPE) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unit '%s' is not a scope unit, refusing.", name); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Unit '%s' is not a scope unit, refusing.", name); return bus_scope_method_abandon(message, u, error); } @@ -2576,7 +2615,8 @@ static int method_set_show_status(sd_bus_message *message, void *userdata, sd_bu if (!isempty(t)) { mode = show_status_from_string(t); if (mode < 0) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid show status '%s'", t); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Invalid show status '%s'", t); } manager_override_show_status(m, mode, "bus"); @@ -3335,7 +3375,11 @@ static int send_finished(sd_bus *bus, void *userdata) { assert(bus); assert(times); - r = sd_bus_message_new_signal(bus, &message, "/org/freedesktop/systemd1", "org.freedesktop.systemd1.Manager", "StartupFinished"); + r = sd_bus_message_new_signal(bus, + &message, + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "StartupFinished"); if (r < 0) return r; diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index d8351a4d43..d10a121b7a 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -1864,8 +1864,8 @@ static int bus_unit_set_live_property( assert(name); assert(message); - /* Handles setting properties both "live" (i.e. at any time during runtime), and during creation (for transient - * units that are being created). */ + /* Handles setting properties both "live" (i.e. at any time during runtime), and during creation (for + * transient units that are being created). */ if (streq(name, "Description")) { const char *d; @@ -2093,8 +2093,8 @@ static int bus_unit_set_transient_property( assert(name); assert(message); - /* Handles settings when transient units are created. This settings cannot be altered anymore after the unit - * has been created. */ + /* Handles settings when transient units are created. This settings cannot be altered anymore after + * the unit has been created. */ if (streq(name, "SourcePath")) return bus_set_transient_path(u, name, &u->source_path, message, flags, error); @@ -2402,7 +2402,8 @@ int bus_unit_set_properties( return r; if (!UNIT_VTABLE(u)->bus_set_property) - return sd_bus_error_setf(error, SD_BUS_ERROR_PROPERTY_READ_ONLY, "Objects of this type do not support setting properties."); + return sd_bus_error_setf(error, SD_BUS_ERROR_PROPERTY_READ_ONLY, + "Objects of this type do not support setting properties."); r = sd_bus_message_enter_container(message, 'v', NULL); if (r < 0) @@ -2420,7 +2421,8 @@ int bus_unit_set_properties( return r; if (r == 0) - return sd_bus_error_setf(error, SD_BUS_ERROR_PROPERTY_READ_ONLY, "Cannot set property %s, or unknown property.", name); + return sd_bus_error_setf(error, SD_BUS_ERROR_PROPERTY_READ_ONLY, + "Cannot set property %s, or unknown property.", name); r = sd_bus_message_exit_container(message); if (r < 0) @@ -2538,8 +2540,8 @@ int bus_unit_track_add_sender(Unit *u, sd_bus_message *m) { int bus_unit_track_remove_sender(Unit *u, sd_bus_message *m) { assert(u); - /* If we haven't allocated the bus track object yet, then there's definitely no reference taken yet, return an - * error */ + /* If we haven't allocated the bus track object yet, then there's definitely no reference taken yet, + * return an error */ if (!u->bus_track) return -EUNATCH; From deaf4b863bfab0ce1fb396fb273f716359bb0517 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Feb 2021 11:33:48 +0100 Subject: [PATCH 12/19] systemctl: reduce scope of iterator variables --- src/systemctl/systemctl-is-active.c | 4 +- src/systemctl/systemctl-is-enabled.c | 4 +- src/systemctl/systemctl-list-dependencies.c | 3 +- src/systemctl/systemctl-list-jobs.c | 3 +- src/systemctl/systemctl-list-machines.c | 7 +-- src/systemctl/systemctl-list-unit-files.c | 3 +- src/systemctl/systemctl-list-units.c | 68 ++++++++++----------- src/systemctl/systemctl-show.c | 9 ++- src/systemctl/systemctl-start-unit.c | 12 +--- src/systemctl/systemctl-util.c | 8 +-- 10 files changed, 50 insertions(+), 71 deletions(-) diff --git a/src/systemctl/systemctl-is-active.c b/src/systemctl/systemctl-is-active.c index 3d99b0deb6..d83736e94a 100644 --- a/src/systemctl/systemctl-is-active.c +++ b/src/systemctl/systemctl-is-active.c @@ -14,7 +14,7 @@ static int check_unit_generic(int code, const UnitActiveState good_states[], int UnitActiveState active_state; sd_bus *bus; char **name; - int r, i; + int r; bool found = false; r = acquire_bus(BUS_MANAGER, &bus); @@ -33,7 +33,7 @@ static int check_unit_generic(int code, const UnitActiveState good_states[], int if (!arg_quiet) puts(unit_active_state_to_string(active_state)); - for (i = 0; i < nb_states; ++i) + for (int i = 0; i < nb_states; ++i) if (good_states[i] == active_state) found = true; } diff --git a/src/systemctl/systemctl-is-enabled.c b/src/systemctl/systemctl-is-enabled.c index babd5902c9..de1d016bb3 100644 --- a/src/systemctl/systemctl-is-enabled.c +++ b/src/systemctl/systemctl-is-enabled.c @@ -9,7 +9,7 @@ static int show_installation_targets_client_side(const char *name) { UnitFileChange *changes = NULL; - size_t n_changes = 0, i; + size_t n_changes = 0; UnitFileFlags flags; char **p; int r; @@ -22,7 +22,7 @@ static int show_installation_targets_client_side(const char *name) { if (r < 0) return log_error_errno(r, "Failed to get file links for %s: %m", name); - for (i = 0; i < n_changes; i++) + for (size_t i = 0; i < n_changes; i++) if (changes[i].type == UNIT_FILE_UNLINK) printf(" %s\n", changes[i].path); diff --git a/src/systemctl/systemctl-list-dependencies.c b/src/systemctl/systemctl-list-dependencies.c index fbb81d90fa..5d19f338d8 100644 --- a/src/systemctl/systemctl-list-dependencies.c +++ b/src/systemctl/systemctl-list-dependencies.c @@ -12,11 +12,10 @@ static int list_dependencies_print(const char *name, int level, unsigned branche _cleanup_free_ char *n = NULL; size_t max_len = MAX(columns(),20u); size_t len = 0; - int i; if (!arg_plain) { - for (i = level - 1; i >= 0; i--) { + for (int i = level - 1; i >= 0; i--) { len += 2; if (len > max_len - 3 && !arg_full) { printf("%s...\n",max_len % 2 ? "" : " "); diff --git a/src/systemctl/systemctl-list-jobs.c b/src/systemctl/systemctl-list-jobs.c index 8b028c013a..27eecb548c 100644 --- a/src/systemctl/systemctl-list-jobs.c +++ b/src/systemctl/systemctl-list-jobs.c @@ -58,7 +58,6 @@ struct job_info { static int output_jobs_list(sd_bus *bus, const struct job_info* jobs, unsigned n, bool skipped) { _cleanup_(table_unrefp) Table *table = NULL; - const struct job_info *j; const char *on, *off; int r; @@ -86,7 +85,7 @@ static int output_jobs_list(sd_bus *bus, const struct job_info* jobs, unsigned n (void) table_set_empty_string(table, "-"); - for (j = jobs; j < jobs + n; j++) { + for (const struct job_info *j = jobs; j < jobs + n; j++) { if (streq(j->state, "running")) on = ansi_highlight(); else diff --git a/src/systemctl/systemctl-list-machines.c b/src/systemctl/systemctl-list-machines.c index 48d0e8bde4..63adcec5f3 100644 --- a/src/systemctl/systemctl-list-machines.c +++ b/src/systemctl/systemctl-list-machines.c @@ -33,12 +33,10 @@ void machine_info_clear(struct machine_info *info) { } static void free_machines_list(struct machine_info *machine_infos, int n) { - int i; - if (!machine_infos) return; - for (i = 0; i < n; i++) + for (int i = 0; i < n; i++) machine_info_clear(&machine_infos[i]); free(machine_infos); @@ -150,7 +148,6 @@ static int get_machine_list( static int output_machines_list(struct machine_info *machine_infos, unsigned n) { _cleanup_(table_unrefp) Table *table = NULL; - struct machine_info *m; bool state_missing = false; int r; @@ -172,7 +169,7 @@ static int output_machines_list(struct machine_info *machine_infos, unsigned n) (void) table_set_empty_string(table, "-"); - for (m = machine_infos; m < machine_infos + n; m++) { + for (struct machine_info *m = machine_infos; m < machine_infos + n; m++) { _cleanup_free_ char *mname = NULL; const char *on_state = "", *on_failed = ""; bool circle = false; diff --git a/src/systemctl/systemctl-list-unit-files.c b/src/systemctl/systemctl-list-unit-files.c index e1bf876620..6da2914eff 100644 --- a/src/systemctl/systemctl-list-unit-files.c +++ b/src/systemctl/systemctl-list-unit-files.c @@ -136,7 +136,6 @@ static int output_unit_file_list(const UnitFileList *units, unsigned c) { int list_unit_files(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_free_ UnitFileList *units = NULL; - UnitFileList *unit; size_t size = 0; unsigned c = 0; const char *state; @@ -265,7 +264,7 @@ int list_unit_files(int argc, char *argv[], void *userdata) { return r; if (install_client_side()) - for (unit = units; unit < units + c; unit++) + for (UnitFileList *unit = units; unit < units + c; unit++) free(unit->path); if (c == 0) diff --git a/src/systemctl/systemctl-list-units.c b/src/systemctl/systemctl-list-units.c index 7f0e79eedd..a413ef6d5b 100644 --- a/src/systemctl/systemctl-list-units.c +++ b/src/systemctl/systemctl-list-units.c @@ -351,7 +351,6 @@ static int socket_info_compare(const struct socket_info *a, const struct socket_ static int output_sockets_list(struct socket_info *socket_infos, unsigned cs) { _cleanup_(table_unrefp) Table *table = NULL; - struct socket_info *s; const char *on, *off; int r; @@ -373,7 +372,7 @@ static int output_sockets_list(struct socket_info *socket_infos, unsigned cs) { (void) table_set_empty_string(table, "-"); if (cs) { - for (s = socket_infos; s < socket_infos + cs; s++) { + for (struct socket_info *s = socket_infos; s < socket_infos + cs; s++) { _cleanup_free_ char *j = NULL; const char *path; @@ -432,8 +431,6 @@ int list_sockets(int argc, char *argv[], void *userdata) { _cleanup_strv_free_ char **sockets_with_suffix = NULL; _cleanup_free_ UnitInfo *unit_infos = NULL; _cleanup_free_ struct socket_info *socket_infos = NULL; - const UnitInfo *u; - struct socket_info *s; unsigned cs = 0; size_t size = 0; int r, n; @@ -454,9 +451,9 @@ int list_sockets(int argc, char *argv[], void *userdata) { if (n < 0) return n; - for (u = unit_infos; u < unit_infos + n; u++) { + for (const UnitInfo *u = unit_infos; u < unit_infos + n; u++) { _cleanup_strv_free_ char **listening = NULL, **triggered = NULL; - int i, c; + int c; if (!endswith(u->id, ".socket")) continue; @@ -476,7 +473,7 @@ int list_sockets(int argc, char *argv[], void *userdata) { goto cleanup; } - for (i = 0; i < c; i++) + for (int i = 0; i < c; i++) socket_infos[cs + i] = (struct socket_info) { .machine = u->machine, .id = u->id, @@ -499,7 +496,7 @@ int list_sockets(int argc, char *argv[], void *userdata) { cleanup: assert(cs == 0 || socket_infos); - for (s = socket_infos; s < socket_infos + cs; s++) { + for (struct socket_info *s = socket_infos; s < socket_infos + cs; s++) { free(s->type); free(s->path); if (s->own_triggered) @@ -604,7 +601,6 @@ static int timer_info_compare(const struct timer_info *a, const struct timer_inf static int output_timers_list(struct timer_info *timer_infos, unsigned n) { _cleanup_(table_unrefp) Table *table = NULL; - struct timer_info *t; const char *on, *off; int r; @@ -620,34 +616,34 @@ static int output_timers_list(struct timer_info *timer_infos, unsigned n) { (void) table_set_empty_string(table, "-"); - if (n > 0) { - for (t = timer_infos; t < timer_infos + n; t++) { - _cleanup_free_ char *j = NULL, *activates = NULL; - const char *unit; + for (struct timer_info *t = timer_infos; t < timer_infos + n; t++) { + _cleanup_free_ char *j = NULL, *activates = NULL; + const char *unit; - if (t->machine) { - j = strjoin(t->machine, ":", t->id); - if (!j) - return log_oom(); - unit = j; - } else - unit = t->id; - - activates = strv_join(t->triggered, ", "); - if (!activates) + if (t->machine) { + j = strjoin(t->machine, ":", t->id); + if (!j) return log_oom(); + unit = j; + } else + unit = t->id; - r = table_add_many(table, - TABLE_TIMESTAMP, t->next_elapse, - TABLE_TIMESTAMP_RELATIVE, t->next_elapse, - TABLE_TIMESTAMP, t->last_trigger, - TABLE_TIMESTAMP_RELATIVE, t->last_trigger, - TABLE_STRING, unit, - TABLE_STRING, activates); - if (r < 0) - return table_log_add_error(r); - } + activates = strv_join(t->triggered, ", "); + if (!activates) + return log_oom(); + r = table_add_many(table, + TABLE_TIMESTAMP, t->next_elapse, + TABLE_TIMESTAMP_RELATIVE, t->next_elapse, + TABLE_TIMESTAMP, t->last_trigger, + TABLE_TIMESTAMP_RELATIVE, t->last_trigger, + TABLE_STRING, unit, + TABLE_STRING, activates); + if (r < 0) + return table_log_add_error(r); + } + + if (n > 0) { on = ansi_highlight(); off = ansi_normal(); } else { @@ -699,8 +695,6 @@ int list_timers(int argc, char *argv[], void *userdata) { _cleanup_strv_free_ char **timers_with_suffix = NULL; _cleanup_free_ struct timer_info *timer_infos = NULL; _cleanup_free_ UnitInfo *unit_infos = NULL; - struct timer_info *t; - const UnitInfo *u; size_t size = 0; int n, c = 0; dual_timestamp nw; @@ -724,7 +718,7 @@ int list_timers(int argc, char *argv[], void *userdata) { dual_timestamp_get(&nw); - for (u = unit_infos; u < unit_infos + n; u++) { + for (const UnitInfo *u = unit_infos; u < unit_infos + n; u++) { _cleanup_strv_free_ char **triggered = NULL; dual_timestamp next = DUAL_TIMESTAMP_NULL; usec_t m, last = 0; @@ -764,7 +758,7 @@ int list_timers(int argc, char *argv[], void *userdata) { output_timers_list(timer_infos, c); cleanup: - for (t = timer_infos; t < timer_infos + c; t++) + for (struct timer_info *t = timer_infos; t < timer_infos + c; t++) strv_free(t->triggered); return r; diff --git a/src/systemctl/systemctl-show.c b/src/systemctl/systemctl-show.c index 9b04b698be..57e708095d 100644 --- a/src/systemctl/systemctl-show.c +++ b/src/systemctl/systemctl-show.c @@ -1103,7 +1103,7 @@ static int print_property(const char *name, const char *expected_value, sd_bus_m } else if (endswith(name, "ExitStatus") && streq(contents, "aiai")) { const int32_t *status, *signal; - size_t n_status, n_signal, i; + size_t n_status, n_signal; r = sd_bus_message_enter_container(m, 'r', "aiai"); if (r < 0) @@ -1132,7 +1132,7 @@ static int print_property(const char *name, const char *expected_value, sd_bus_m fputc('=', stdout); } - for (i = 0; i < n_status; i++) { + for (size_t i = 0; i < n_status; i++) { if (first) first = false; else @@ -1141,7 +1141,7 @@ static int print_property(const char *name, const char *expected_value, sd_bus_m printf("%"PRIi32, status[i]); } - for (i = 0; i < n_signal; i++) { + for (size_t i = 0; i < n_signal; i++) { const char *str; str = signal_to_string((int) signal[i]); @@ -1933,7 +1933,6 @@ static int show_all( _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_free_ UnitInfo *unit_infos = NULL; - const UnitInfo *u; unsigned c; int r, ret = 0; @@ -1947,7 +1946,7 @@ static int show_all( typesafe_qsort(unit_infos, c, unit_info_compare); - for (u = unit_infos; u < unit_infos + c; u++) { + for (const UnitInfo *u = unit_infos; u < unit_infos + c; u++) { _cleanup_free_ char *p = NULL; p = unit_dbus_path_from_name(u->id); diff --git a/src/systemctl/systemctl-start-unit.c b/src/systemctl/systemctl-start-unit.c index b398e77eb2..d08e363c9f 100644 --- a/src/systemctl/systemctl-start-unit.c +++ b/src/systemctl/systemctl-start-unit.c @@ -36,9 +36,7 @@ static const struct { }; static const char *verb_to_method(const char *verb) { - size_t i; - - for (i = 0; i < ELEMENTSOF(unit_actions); i++) + for (size_t i = 0; i < ELEMENTSOF(unit_actions); i++) if (streq_ptr(unit_actions[i].verb, verb)) return unit_actions[i].method; @@ -46,9 +44,7 @@ static const char *verb_to_method(const char *verb) { } static const char *verb_to_job_type(const char *verb) { - size_t i; - - for (i = 0; i < ELEMENTSOF(unit_actions); i++) + for (size_t i = 0; i < ELEMENTSOF(unit_actions); i++) if (streq_ptr(unit_actions[i].verb, verb)) return unit_actions[i].job_type; @@ -200,9 +196,7 @@ const struct action_metadata action_table[_ACTION_MAX] = { }; enum action verb_to_action(const char *verb) { - enum action i; - - for (i = 0; i < _ACTION_MAX; i++) + for (enum action i = 0; i < _ACTION_MAX; i++) if (streq_ptr(action_table[i].verb, verb)) return i; diff --git a/src/systemctl/systemctl-util.c b/src/systemctl/systemctl-util.c index 89c342a0b1..d1490c906d 100644 --- a/src/systemctl/systemctl-util.c +++ b/src/systemctl/systemctl-util.c @@ -62,9 +62,7 @@ int acquire_bus(BusFocus focus, sd_bus **ret) { } void release_busses(void) { - BusFocus w; - - for (w = 0; w < _BUS_FOCUS_MAX; w++) + for (BusFocus w = 0; w < _BUS_FOCUS_MAX; w++) buses[w] = sd_bus_flush_close_unref(buses[w]); } @@ -237,7 +235,7 @@ int get_unit_list( int expand_unit_names(sd_bus *bus, char **names, const char* suffix, char ***ret, bool *ret_expanded) { _cleanup_strv_free_ char **mangled = NULL, **globs = NULL; char **name; - int r, i; + int r; assert(bus); assert(ret); @@ -272,7 +270,7 @@ int expand_unit_names(sd_bus *bus, char **names, const char* suffix, char ***ret n = strv_length(mangled); allocated = n + 1; - for (i = 0; i < r; i++) { + for (int i = 0; i < r; i++) { if (!GREEDY_REALLOC(mangled, allocated, n+2)) return log_oom(); From c9615f73521986b3607b852c139036d58973043c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Feb 2021 13:54:33 +0100 Subject: [PATCH 13/19] systemctl: add "reload-or-restart --marked" This is almost equivalent to 'busctl call-method org.freedesktop.systemd1 /org/freedesktop/systemd1 org.freedesktop.systemd1.Manager EnqueueMarkedJobs', but waits for the jobs to finish. --- man/systemctl.xml | 12 +++++ src/systemctl/systemctl-start-unit.c | 65 +++++++++++++++++++++++----- src/systemctl/systemctl.c | 31 ++++++++++++- src/systemctl/systemctl.h | 1 + 4 files changed, 96 insertions(+), 13 deletions(-) diff --git a/man/systemctl.xml b/man/systemctl.xml index db4e2b2c65..642a5ecf2d 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -2316,6 +2316,18 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err a directory, but a regular file, device node, socket or FIFO. + + + + Only allowed with reload-or-restart. Enqueues restart jobs for all + units that have the needs-restart mark, and reload jobs for units that have the + needs-reload mark. When a unit marked for reload does not support reload, restart + will be queued. Those properties can be set using set-property Marks. + + Unless is used, systemctl will wait for the + queued jobs to finish. + + diff --git a/src/systemctl/systemctl-start-unit.c b/src/systemctl/systemctl-start-unit.c index d08e363c9f..c40e807212 100644 --- a/src/systemctl/systemctl-start-unit.c +++ b/src/systemctl/systemctl-start-unit.c @@ -176,6 +176,43 @@ fail: return r; } +static int enqueue_marked_jobs( + sd_bus *bus, + BusWaitForJobs *w) { + + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + int r; + + log_debug("%s dbus call org.freedesktop.systemd1.Manager EnqueueMarkedJobs()", + arg_dry_run ? "Would execute" : "Executing"); + + if (arg_dry_run) + return 0; + + r = bus_call_method(bus, bus_systemd_mgr, "EnqueueMarkedJobs", &error, &reply, NULL); + if (r < 0) + return log_error_errno(r, "Failed to start jobs: %s", bus_error_message(&error, r)); + + _cleanup_strv_free_ char **paths = NULL; + r = sd_bus_message_read_strv(reply, &paths); + if (r < 0) + return bus_log_parse_error(r); + + if (w) { + char **path; + + STRV_FOREACH(path, paths) { + log_debug("Adding %s to the set", *path); + r = bus_wait_for_jobs_add(w, *path); + if (r < 0) + return log_error_errno(r, "Failed to watch job %s: %m", *path); + } + } + + return 0; +} + const struct action_metadata action_table[_ACTION_MAX] = { [ACTION_HALT] = { SPECIAL_HALT_TARGET, "halt", "replace-irreversibly" }, [ACTION_POWEROFF] = { SPECIAL_POWEROFF_TARGET, "poweroff", "replace-irreversibly" }, @@ -265,7 +302,7 @@ int start_unit(int argc, char *argv[], void *userdata) { job_type = "start"; mode = "isolate"; suffix = ".target"; - } else { + } else if (!arg_marked) { /* A command in style of "systemctl start …", "sysemctl stop …" and so on */ method = verb_to_method(argv[0]); job_type = verb_to_job_type(argv[0]); @@ -289,7 +326,7 @@ int start_unit(int argc, char *argv[], void *userdata) { names = strv_new(one_name); if (!names) return log_oom(); - } else { + } else if (!arg_marked) { bool expanded; r = expand_unit_names(bus, strv_skip(argv, 1), suffix, &names, &expanded); @@ -322,19 +359,23 @@ int start_unit(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to allocate unit watch context: %m"); } - STRV_FOREACH(name, names) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + if (arg_marked) + ret = enqueue_marked_jobs(bus, w); - r = start_unit_one(bus, method, job_type, *name, mode, &error, w, wu); - if (ret == EXIT_SUCCESS && r < 0) - ret = translate_bus_error_to_exit_status(r, &error); + else + STRV_FOREACH(name, names) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - if (r >= 0 && streq(method, "StopUnit")) { - r = strv_push(&stopped_units, *name); - if (r < 0) - return log_oom(); + r = start_unit_one(bus, method, job_type, *name, mode, &error, w, wu); + if (ret == EXIT_SUCCESS && r < 0) + ret = translate_bus_error_to_exit_status(r, &error); + + if (r >= 0 && streq(method, "StopUnit")) { + r = strv_push(&stopped_units, *name); + if (r < 0) + return log_oom(); + } } - } if (!arg_no_block) { const char* extra_args[4]; diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 16d0555c7c..315202a64b 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -108,6 +108,7 @@ char **arg_clean_what = NULL; TimestampStyle arg_timestamp_style = TIMESTAMP_PRETTY; bool arg_read_only = false; bool arg_mkdir = false; +bool arg_marked = false; STATIC_DESTRUCTOR_REGISTER(arg_wall, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_root, freep); @@ -295,6 +296,7 @@ static int systemctl_help(void) { " 'us+utc': 'Day YYYY-MM-DD HH:MM:SS.UUUUUU UTC\n" " --read-only Create read-only bind mount\n" " --mkdir Create directory before mounting, if missing\n" + " --marked Restart/reload previously marked units\n" "\nSee the %2$s for details.\n", program_invocation_short_name, link, @@ -413,6 +415,7 @@ static int systemctl_parse_argv(int argc, char *argv[]) { ARG_TIMESTAMP_STYLE, ARG_READ_ONLY, ARG_MKDIR, + ARG_MARKED, }; static const struct option options[] = { @@ -471,6 +474,7 @@ static int systemctl_parse_argv(int argc, char *argv[]) { { "timestamp", required_argument, NULL, ARG_TIMESTAMP_STYLE }, { "read-only", no_argument, NULL, ARG_READ_ONLY }, { "mkdir", no_argument, NULL, ARG_MKDIR }, + { "marked", no_argument, NULL, ARG_MARKED }, {} }; @@ -900,6 +904,10 @@ static int systemctl_parse_argv(int argc, char *argv[]) { arg_mkdir = true; break; + case ARG_MARKED: + arg_marked = true; + break; + case '.': /* Output an error mimicking getopt, and print a hint afterwards */ log_error("%s: invalid option -- '.'", program_invocation_name); @@ -923,6 +931,27 @@ static int systemctl_parse_argv(int argc, char *argv[]) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--wait may not be combined with --no-block."); + bool do_reload_or_restart = streq_ptr(argv[optind], "reload-or-restart"); + if (arg_marked) { + if (!do_reload_or_restart) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "--marked may only be used with 'reload-or-restart'."); + if (optind + 1 < argc) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "No additional arguments allowed with 'reload-or-restart --marked'."); + if (arg_wait) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "--marked --wait is not supported."); + if (arg_show_transaction) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "--marked --show-transaction is not supported."); + + } else if (do_reload_or_restart) { + if (optind + 1 >= argc) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "List of units to restart/reload is required."); + } + return 1; } @@ -1004,7 +1033,7 @@ static int systemctl_main(int argc, char *argv[]) { { "reload", 2, VERB_ANY, VERB_ONLINE_ONLY, start_unit }, { "restart", 2, VERB_ANY, VERB_ONLINE_ONLY, start_unit }, { "try-restart", 2, VERB_ANY, VERB_ONLINE_ONLY, start_unit }, - { "reload-or-restart", 2, VERB_ANY, VERB_ONLINE_ONLY, start_unit }, + { "reload-or-restart", VERB_ANY, VERB_ANY, VERB_ONLINE_ONLY, start_unit }, { "reload-or-try-restart", 2, VERB_ANY, VERB_ONLINE_ONLY, start_unit }, /* For compatibility with old systemctl <= 228 */ { "try-reload-or-restart", 2, VERB_ANY, VERB_ONLINE_ONLY, start_unit }, { "force-reload", 2, VERB_ANY, VERB_ONLINE_ONLY, start_unit }, /* For compatibility with SysV */ diff --git a/src/systemctl/systemctl.h b/src/systemctl/systemctl.h index 0ebe4580c6..3c5a4023a9 100644 --- a/src/systemctl/systemctl.h +++ b/src/systemctl/systemctl.h @@ -93,5 +93,6 @@ extern char **arg_clean_what; extern TimestampStyle arg_timestamp_style; extern bool arg_read_only; extern bool arg_mkdir; +extern bool arg_marked; int systemctl_dispatch_parse_argv(int argc, char *argv[]); From 2a9b4bbe3ec3e414dbc02e909c11c9403dba57b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 31 Jan 2021 17:16:32 +0100 Subject: [PATCH 14/19] Move rpm stuff into a separate src/rpm/ directory It is only of interest to rpm-based distros, we can move it out of src/core/ which is pretty busy. --- meson.build | 1 + src/core/meson.build | 6 ++---- src/{core => rpm}/macros.systemd.in | 0 src/rpm/meson.build | 14 ++++++++++++++ src/{core => rpm}/triggers.systemd.in | 0 5 files changed, 17 insertions(+), 4 deletions(-) rename src/{core => rpm}/macros.systemd.in (100%) create mode 100644 src/rpm/meson.build rename src/{core => rpm}/triggers.systemd.in (100%) diff --git a/meson.build b/meson.build index 09d8122091..226dc90f9e 100644 --- a/meson.build +++ b/meson.build @@ -1746,6 +1746,7 @@ subdir('src/partition') subdir('src/portable') subdir('src/pstore') subdir('src/resolve') +subdir('src/rpm') subdir('src/shutdown') subdir('src/sysext') subdir('src/systemctl') diff --git a/src/core/meson.build b/src/core/meson.build index b8089c51ce..a389c906b3 100644 --- a/src/core/meson.build +++ b/src/core/meson.build @@ -164,11 +164,9 @@ core_includes = [includes, include_directories('.')] systemd_sources = files('main.c') -in_files = [['macros.systemd', rpmmacrosdir], - ['system.conf', pkgsysconfdir], +in_files = [['system.conf', pkgsysconfdir], ['user.conf', pkgsysconfdir], - ['systemd.pc', pkgconfigdatadir], - ['triggers.systemd', '']] + ['systemd.pc', pkgconfigdatadir]] foreach item : in_files file = item[0] diff --git a/src/core/macros.systemd.in b/src/rpm/macros.systemd.in similarity index 100% rename from src/core/macros.systemd.in rename to src/rpm/macros.systemd.in diff --git a/src/rpm/meson.build b/src/rpm/meson.build new file mode 100644 index 0000000000..8e5283ff55 --- /dev/null +++ b/src/rpm/meson.build @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later + +configure_file( + input : 'macros.systemd.in', + output : 'macros.systemd', + configuration : substs, + install_dir : rpmmacrosdir == 'no' ? '' : rpmmacrosdir) + +configure_file( + input : 'triggers.systemd.in', + output : 'triggers.systemd', + configuration : substs) +# This doesn't get installed anywhere, it needs to included in the rpm spec +# file definition. diff --git a/src/core/triggers.systemd.in b/src/rpm/triggers.systemd.in similarity index 100% rename from src/core/triggers.systemd.in rename to src/rpm/triggers.systemd.in From 8e5658010dbb929d129539521acb7de3dc6cdf69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 31 Jan 2021 17:30:49 +0100 Subject: [PATCH 15/19] rpm: pull in the alternative trigger implementation in sh From https://src.fedoraproject.org/rpms/systemd/blob/master/f/triggers.systemd. In 12dde791d519bc80d5cca4ab6f088763cd481015 scriptlets were converted to lua. This is not only faster and cleaner, but also avoids a nasty dependency loop: rpm implements the lua scripting internally, so we don't need a working shell for the scriplets. This is nice and all, but unfortunately ostree wants to capture scriptlets and execute them at a later time and does not support lua. So in Fedora we ended up with a revert back to a shell-based implementation [1]. At the time I hoped this would only be a temporary workaround, but three years later I think it's fair to assume that this will not happen any time soon. But carrying the upstream lua version and the downstream sh version is error prone. So let's import the other version into our tree too so that they can be kept in sync. [1] https://src.fedoraproject.org/rpms/systemd/c/8e6b39457b3e2660793821e0524855226e33e306 --- src/rpm/meson.build | 8 ++- src/rpm/triggers.systemd.sh.in | 111 +++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 src/rpm/triggers.systemd.sh.in diff --git a/src/rpm/meson.build b/src/rpm/meson.build index 8e5283ff55..d299b18019 100644 --- a/src/rpm/meson.build +++ b/src/rpm/meson.build @@ -6,9 +6,13 @@ configure_file( configuration : substs, install_dir : rpmmacrosdir == 'no' ? '' : rpmmacrosdir) +# Those doesn't get installed anywhere, one of them needs to included in the +# rpm spec file definition. configure_file( input : 'triggers.systemd.in', output : 'triggers.systemd', configuration : substs) -# This doesn't get installed anywhere, it needs to included in the rpm spec -# file definition. +configure_file( + input : 'triggers.systemd.sh.in', + output : 'triggers.systemd.sh', + configuration : substs) diff --git a/src/rpm/triggers.systemd.sh.in b/src/rpm/triggers.systemd.sh.in new file mode 100644 index 0000000000..7a7e792da0 --- /dev/null +++ b/src/rpm/triggers.systemd.sh.in @@ -0,0 +1,111 @@ +# -*- Mode: rpm-spec; indent-tabs-mode: nil -*- */ +# SPDX-License-Identifier: LGPL-2.1+ +# +# This file is part of systemd. +# +# Copyright 2015 Zbigniew Jędrzejewski-Szmek +# Copyright 2018 Neal Gompa +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. +# +# systemd is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with systemd; If not, see . + +# The contents of this are an example to be copied into systemd.spec. +# +# Minimum rpm version supported: 4.13.0 + +%transfiletriggerin -P 900900 -- /usr/lib/systemd/system /etc/systemd/system +# This script will run after any package is initially installed or +# upgraded. We care about the case where a package is initially +# installed, because other cases are covered by the *un scriptlets, +# so sometimes we will reload needlessly. +if test -d /run/systemd/system; then + %{_bindir}/systemctl daemon-reload +fi + +%transfiletriggerun -- /usr/lib/systemd/system /etc/systemd/system +# On removal, we need to run daemon-reload after any units have been +# removed. %transfiletriggerpostun would be ideal, but it does not get +# executed for some reason. +# On upgrade, we need to run daemon-reload after any new unit files +# have been installed, but before %postun scripts in packages get +# executed. %transfiletriggerun gets the right list of files +# but it is invoked too early (before changes happen). +# %filetriggerpostun happens at the right time, but it fires for +# every package. +# To execute the reload at the right time, we create a state +# file in %transfiletriggerun and execute the daemon-reload in +# the first %filetriggerpostun. + +if test -d "/run/systemd/system"; then + mkdir -p "%{_localstatedir}/lib/rpm-state/systemd" + touch "%{_localstatedir}/lib/rpm-state/systemd/needs-reload" +fi + +%filetriggerpostun -P 1000100 -- /usr/lib/systemd/system /etc/systemd/system +if test -f "%{_localstatedir}/lib/rpm-state/systemd/needs-reload"; then + rm -rf "%{_localstatedir}/lib/rpm-state/systemd" + %{_bindir}/systemctl daemon-reload +fi + +%transfiletriggerin -P 100700 -- /usr/lib/sysusers.d +# This script will process files installed in /usr/lib/sysusers.d to create +# specified users automatically. The priority is set such that it +# will run before the tmpfiles file trigger. +if test -d /run/systemd/system; then + %{_bindir}/systemd-sysusers || : +fi + +%transfiletriggerin -P 100500 -- /usr/lib/tmpfiles.d +# This script will process files installed in /usr/lib/tmpfiles.d to create +# tmpfiles automatically. The priority is set such that it will run +# after the sysusers file trigger, but before any other triggers. +if test -d /run/systemd/system; then + %{_bindir}/systemd-tmpfiles --create || : +fi + +%transfiletriggerin udev -- /usr/lib/udev/hwdb.d +# This script will automatically invoke hwdb update if files have been +# installed or updated in /usr/lib/udev/hwdb.d. +if test -d /run/systemd/system; then + %{_bindir}/systemd-hwdb update || : +fi + +%transfiletriggerin -- /usr/lib/systemd/catalog +# This script will automatically invoke journal catalog update if files +# have been installed or updated in /usr/lib/systemd/catalog. +if test -d /run/systemd/system; then + %{_bindir}/journalctl --update-catalog || : +fi + +%transfiletriggerin udev -- /usr/lib/udev/rules.d +# This script will automatically update udev with new rules if files +# have been installed or updated in /usr/lib/udev/rules.d. +if test -e /run/udev/control; then + %{_bindir}/udevadm control --reload || : +fi + +%transfiletriggerin -- /usr/lib/sysctl.d +# This script will automatically apply sysctl rules if files have been +# installed or updated in /usr/lib/sysctl.d. +if test -d /run/systemd/system; then + /usr/lib/systemd/systemd-sysctl || : +fi + +%transfiletriggerin -- /usr/lib/binfmt.d +# This script will automatically apply binfmt rules if files have been +# installed or updated in /usr/lib/binfmt.d. +if test -d /run/systemd/system; then + # systemd-binfmt might fail if binfmt_misc kernel module is not loaded + # during install + /usr/lib/systemd/systemd-binfmt || : +fi From addf2c0ecdedaf3863f1b670ca5ca17d058446c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 31 Jan 2021 17:56:11 +0100 Subject: [PATCH 16/19] rpm: sync the shell version of triggers.systemd with the lua version Note that this goes both ways: in particular the lua version had udev scriptlets in the wrong package, fixed in https://src.fedoraproject.org/rpms/systemd/c/3c9433d7cf4afc8d76660402f6c3d9d991596b83. Add missing "|| :" so the scriptlets never fail. --- src/rpm/triggers.systemd.in | 4 +- src/rpm/triggers.systemd.sh.in | 78 ++++++++++++++-------------------- 2 files changed, 34 insertions(+), 48 deletions(-) diff --git a/src/rpm/triggers.systemd.in b/src/rpm/triggers.systemd.in index 2d25db3696..9391ed45a4 100644 --- a/src/rpm/triggers.systemd.in +++ b/src/rpm/triggers.systemd.in @@ -82,7 +82,7 @@ if posix.access("/run/systemd/system") then end end -%transfiletriggerin -p -- @udevhwdbdir@ +%transfiletriggerin udev -p -- @udevhwdbdir@ -- This script will automatically invoke hwdb update if files have been -- installed or updated in @udevhwdbdir@. if posix.access("/run/systemd/system") then @@ -106,7 +106,7 @@ if posix.access("/run/systemd/system") then end end -%transfiletriggerin -p -- @udevrulesdir@ +%transfiletriggerin udev -p -- @udevrulesdir@ -- This script will automatically update udev with new rules if files -- have been installed or updated in @udevrulesdir@. if posix.access("/run/systemd/system") then diff --git a/src/rpm/triggers.systemd.sh.in b/src/rpm/triggers.systemd.sh.in index 7a7e792da0..13f7cb631d 100644 --- a/src/rpm/triggers.systemd.sh.in +++ b/src/rpm/triggers.systemd.sh.in @@ -1,38 +1,24 @@ # -*- Mode: rpm-spec; indent-tabs-mode: nil -*- */ -# SPDX-License-Identifier: LGPL-2.1+ +# SPDX-License-Identifier: LGPL-2.1-or-later # # This file is part of systemd. # -# Copyright 2015 Zbigniew Jędrzejewski-Szmek # Copyright 2018 Neal Gompa -# -# systemd is free software; you can redistribute it and/or modify it -# under the terms of the GNU Lesser General Public License as published by -# the Free Software Foundation; either version 2.1 of the License, or -# (at your option) any later version. -# -# systemd is distributed in the hope that it will be useful, but -# WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -# Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public License -# along with systemd; If not, see . # The contents of this are an example to be copied into systemd.spec. # # Minimum rpm version supported: 4.13.0 -%transfiletriggerin -P 900900 -- /usr/lib/systemd/system /etc/systemd/system +%transfiletriggerin -P 900900 -- @systemunitdir@ /etc/systemd/system # This script will run after any package is initially installed or # upgraded. We care about the case where a package is initially # installed, because other cases are covered by the *un scriptlets, # so sometimes we will reload needlessly. -if test -d /run/systemd/system; then - %{_bindir}/systemctl daemon-reload +if test -d "/run/systemd/system"; then + %{_bindir}/systemctl daemon-reload || : fi -%transfiletriggerun -- /usr/lib/systemd/system /etc/systemd/system +%transfiletriggerun -- @systemunitdir@ /etc/systemd/system # On removal, we need to run daemon-reload after any units have been # removed. %transfiletriggerpostun would be ideal, but it does not get # executed for some reason. @@ -47,65 +33,65 @@ fi # the first %filetriggerpostun. if test -d "/run/systemd/system"; then - mkdir -p "%{_localstatedir}/lib/rpm-state/systemd" - touch "%{_localstatedir}/lib/rpm-state/systemd/needs-reload" + mkdir -p "%{_localstatedir}/lib/rpm-state/systemd" + touch "%{_localstatedir}/lib/rpm-state/systemd/needs-reload" fi -%filetriggerpostun -P 1000100 -- /usr/lib/systemd/system /etc/systemd/system +%filetriggerpostun -P 1000100 -- @systemunitdir@ /etc/systemd/system if test -f "%{_localstatedir}/lib/rpm-state/systemd/needs-reload"; then - rm -rf "%{_localstatedir}/lib/rpm-state/systemd" - %{_bindir}/systemctl daemon-reload + rm -rf "%{_localstatedir}/lib/rpm-state/systemd" + %{_bindir}/systemctl daemon-reload || : fi -%transfiletriggerin -P 100700 -- /usr/lib/sysusers.d -# This script will process files installed in /usr/lib/sysusers.d to create +%transfiletriggerin -P 100700 -- @sysusersdir@ +# This script will process files installed in @sysusersdir@ to create # specified users automatically. The priority is set such that it # will run before the tmpfiles file trigger. -if test -d /run/systemd/system; then +if test -d "/run/systemd/system"; then %{_bindir}/systemd-sysusers || : fi -%transfiletriggerin -P 100500 -- /usr/lib/tmpfiles.d -# This script will process files installed in /usr/lib/tmpfiles.d to create +%transfiletriggerin -P 100500 -- @tmpfilesdir@ +# This script will process files installed in @tmpfilesdir@ to create # tmpfiles automatically. The priority is set such that it will run # after the sysusers file trigger, but before any other triggers. -if test -d /run/systemd/system; then +if test -d "/run/systemd/system"; then %{_bindir}/systemd-tmpfiles --create || : fi -%transfiletriggerin udev -- /usr/lib/udev/hwdb.d +%transfiletriggerin udev -- @udevhwdbdir@ # This script will automatically invoke hwdb update if files have been -# installed or updated in /usr/lib/udev/hwdb.d. -if test -d /run/systemd/system; then +# installed or updated in @udevhwdbdir@. +if test -d "/run/systemd/system"; then %{_bindir}/systemd-hwdb update || : fi -%transfiletriggerin -- /usr/lib/systemd/catalog +%transfiletriggerin -- @catalogdir@ # This script will automatically invoke journal catalog update if files -# have been installed or updated in /usr/lib/systemd/catalog. -if test -d /run/systemd/system; then +# have been installed or updated in @catalogdir@. +if test -d "/run/systemd/system"; then %{_bindir}/journalctl --update-catalog || : fi -%transfiletriggerin udev -- /usr/lib/udev/rules.d +%transfiletriggerin udev -- @udevrulesdir@ # This script will automatically update udev with new rules if files -# have been installed or updated in /usr/lib/udev/rules.d. +# have been installed or updated in @udevrulesdir@. if test -e /run/udev/control; then %{_bindir}/udevadm control --reload || : fi -%transfiletriggerin -- /usr/lib/sysctl.d +%transfiletriggerin -- @sysctldir@ # This script will automatically apply sysctl rules if files have been -# installed or updated in /usr/lib/sysctl.d. -if test -d /run/systemd/system; then - /usr/lib/systemd/systemd-sysctl || : +# installed or updated in @sysctldir@. +if test -d "/run/systemd/system"; then + @rootlibexecdir@/systemd-sysctl || : fi -%transfiletriggerin -- /usr/lib/binfmt.d +%transfiletriggerin -- @binfmtdir@ # This script will automatically apply binfmt rules if files have been -# installed or updated in /usr/lib/binfmt.d. -if test -d /run/systemd/system; then +# installed or updated in @binfmtdir@. +if test -d "/run/systemd/system"; then # systemd-binfmt might fail if binfmt_misc kernel module is not loaded # during install - /usr/lib/systemd/systemd-binfmt || : + @rootlibexecdir@/systemd-binfmt || : fi From b492db99d60ac0f0403decb3d291d3ddcc435db6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 31 Jan 2021 14:24:46 +0100 Subject: [PATCH 17/19] rpm: simplify daemon-reload trigger The trigger was initially written to use %transfiletriggerun instead of %transfiletriggerpostun because the latter would not fire. It turned out to a buffer overread in rpm that since has been long fixed: https://bugzilla.redhat.com/show_bug.cgi?id=1284645 https://github.com/rpm-software-management/rpm/commit/f6521c50f6836374a0f7995f8f393aaf36e178ea --- src/rpm/triggers.systemd.in | 27 ++++----------------------- src/rpm/triggers.systemd.sh.in | 23 ++++------------------- 2 files changed, 8 insertions(+), 42 deletions(-) diff --git a/src/rpm/triggers.systemd.in b/src/rpm/triggers.systemd.in index 9391ed45a4..1e925cf974 100644 --- a/src/rpm/triggers.systemd.in +++ b/src/rpm/triggers.systemd.in @@ -6,14 +6,13 @@ # The contents of this are an example to be copied into systemd.spec. # -# Minimum rpm version supported: 4.13.0 +# Minimum rpm version supported: 4.14.0 %transfiletriggerin -P 900900 -p -- @systemunitdir@ /etc/systemd/system -- This script will run after any package is initially installed or -- upgraded. We care about the case where a package is initially -- installed, because other cases are covered by the *un scriptlets, -- so sometimes we will reload needlessly. - if posix.access("/run/systemd/system") then pid = posix.fork() if pid == 0 then @@ -23,31 +22,13 @@ if posix.access("/run/systemd/system") then end end -%transfiletriggerun -p -- @systemunitdir@ /etc/systemd/system +%transfiletriggerpostun -P 1000100 -p -- @systemunitdir@ /etc/systemd/system -- On removal, we need to run daemon-reload after any units have been --- removed. %transfiletriggerpostun would be ideal, but it does not get --- executed for some reason. +-- removed. -- On upgrade, we need to run daemon-reload after any new unit files -- have been installed, but before %postun scripts in packages get --- executed. %transfiletriggerun gets the right list of files --- but it is invoked too early (before changes happen). --- %filetriggerpostun happens at the right time, but it fires for --- every package. --- To execute the reload at the right time, we create a state --- file in %transfiletriggerun and execute the daemon-reload in --- the first %filetriggerpostun. - +-- executed. if posix.access("/run/systemd/system") then - posix.mkdir("%{_localstatedir}/lib") - posix.mkdir("%{_localstatedir}/lib/rpm-state") - posix.mkdir("%{_localstatedir}/lib/rpm-state/systemd") - io.open("%{_localstatedir}/lib/rpm-state/systemd/needs-reload", "w") -end - -%filetriggerpostun -P 1000100 -p -- @systemunitdir@ /etc/systemd/system -if posix.access("%{_localstatedir}/lib/rpm-state/systemd/needs-reload") then - posix.unlink("%{_localstatedir}/lib/rpm-state/systemd/needs-reload") - posix.rmdir("%{_localstatedir}/lib/rpm-state/systemd") pid = posix.fork() if pid == 0 then assert(posix.exec("%{_bindir}/systemctl", "daemon-reload")) diff --git a/src/rpm/triggers.systemd.sh.in b/src/rpm/triggers.systemd.sh.in index 13f7cb631d..641afd9ef7 100644 --- a/src/rpm/triggers.systemd.sh.in +++ b/src/rpm/triggers.systemd.sh.in @@ -7,7 +7,7 @@ # The contents of this are an example to be copied into systemd.spec. # -# Minimum rpm version supported: 4.13.0 +# Minimum rpm version supported: 4.14.0 %transfiletriggerin -P 900900 -- @systemunitdir@ /etc/systemd/system # This script will run after any package is initially installed or @@ -18,28 +18,13 @@ if test -d "/run/systemd/system"; then %{_bindir}/systemctl daemon-reload || : fi -%transfiletriggerun -- @systemunitdir@ /etc/systemd/system +%transfiletriggerpostun -P 1000100 -- @systemunitdir@ /etc/systemd/system # On removal, we need to run daemon-reload after any units have been -# removed. %transfiletriggerpostun would be ideal, but it does not get -# executed for some reason. +# removed. # On upgrade, we need to run daemon-reload after any new unit files # have been installed, but before %postun scripts in packages get -# executed. %transfiletriggerun gets the right list of files -# but it is invoked too early (before changes happen). -# %filetriggerpostun happens at the right time, but it fires for -# every package. -# To execute the reload at the right time, we create a state -# file in %transfiletriggerun and execute the daemon-reload in -# the first %filetriggerpostun. - +# executed. if test -d "/run/systemd/system"; then - mkdir -p "%{_localstatedir}/lib/rpm-state/systemd" - touch "%{_localstatedir}/lib/rpm-state/systemd/needs-reload" -fi - -%filetriggerpostun -P 1000100 -- @systemunitdir@ /etc/systemd/system -if test -f "%{_localstatedir}/lib/rpm-state/systemd/needs-reload"; then - rm -rf "%{_localstatedir}/lib/rpm-state/systemd" %{_bindir}/systemctl daemon-reload || : fi From cd621954ed643c6ee0d869132293e26056a48826 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Feb 2021 17:03:43 +0100 Subject: [PATCH 18/19] rpm: order sysctl/sysusers/tmpfiles execution before package scriptlets P>1000000 is *before* "normal" scriptlets, P<1000000 is *after*. I think it makes sense to do stuff like execution of sysctl/sysusers/tmpfiles configuration before package scriptlets. I think that was the intent, but a single digit got dropped ;( Also, let's reorder the scriptlets in the file to match execution order, to make it easier to see what is going on. Most of those may happen in any order, but there are some exceptions: tmpfiles should be after sysusers, udevadm --reload should be after hwdb. --- src/rpm/triggers.systemd.in | 78 +++++++++++++++++----------------- src/rpm/triggers.systemd.sh.in | 52 +++++++++++------------ 2 files changed, 65 insertions(+), 65 deletions(-) diff --git a/src/rpm/triggers.systemd.in b/src/rpm/triggers.systemd.in index 1e925cf974..37f62cb1b7 100644 --- a/src/rpm/triggers.systemd.in +++ b/src/rpm/triggers.systemd.in @@ -50,7 +50,43 @@ if posix.access("/run/systemd/system") then end end -%transfiletriggerin -P 100500 -p -- @tmpfilesdir@ +%transfiletriggerin -P 1000700 udev -p -- @udevhwdbdir@ +-- This script will automatically invoke hwdb update if files have been +-- installed or updated in @udevhwdbdir@. +if posix.access("/run/systemd/system") then + pid = posix.fork() + if pid == 0 then + assert(posix.exec("%{_bindir}/systemd-hwdb", "update")) + elseif pid > 0 then + posix.wait(pid) + end +end + +%transfiletriggerin -P 1000700 -p -- @catalogdir@ +-- This script will automatically invoke journal catalog update if files +-- have been installed or updated in @catalogdir@. +if posix.access("/run/systemd/system") then + pid = posix.fork() + if pid == 0 then + assert(posix.exec("%{_bindir}/journalctl", "--update-catalog")) + elseif pid > 0 then + posix.wait(pid) + end +end + +%transfiletriggerin -P 1000700 -p -- @binfmtdir@ +-- This script will automatically apply binfmt rules if files have been +-- installed or updated in @binfmtdir@. +if posix.access("/run/systemd/system") then + pid = posix.fork() + if pid == 0 then + assert(posix.exec("@rootlibexecdir@/systemd-binfmt")) + elseif pid > 0 then + posix.wait(pid) + end +end + +%transfiletriggerin -P 1000600 -p -- @tmpfilesdir@ -- This script will process files installed in @tmpfilesdir@ to create -- tmpfiles automatically. The priority is set such that it will run -- after the sysusers file trigger, but before any other triggers. @@ -63,31 +99,7 @@ if posix.access("/run/systemd/system") then end end -%transfiletriggerin udev -p -- @udevhwdbdir@ --- This script will automatically invoke hwdb update if files have been --- installed or updated in @udevhwdbdir@. -if posix.access("/run/systemd/system") then - pid = posix.fork() - if pid == 0 then - assert(posix.exec("%{_bindir}/systemd-hwdb", "update")) - elseif pid > 0 then - posix.wait(pid) - end -end - -%transfiletriggerin -p -- @catalogdir@ --- This script will automatically invoke journal catalog update if files --- have been installed or updated in @catalogdir@. -if posix.access("/run/systemd/system") then - pid = posix.fork() - if pid == 0 then - assert(posix.exec("%{_bindir}/journalctl", "--update-catalog")) - elseif pid > 0 then - posix.wait(pid) - end -end - -%transfiletriggerin udev -p -- @udevrulesdir@ +%transfiletriggerin -P 1000600 udev -p -- @udevrulesdir@ -- This script will automatically update udev with new rules if files -- have been installed or updated in @udevrulesdir@. if posix.access("/run/systemd/system") then @@ -99,7 +111,7 @@ if posix.access("/run/systemd/system") then end end -%transfiletriggerin -p -- @sysctldir@ +%transfiletriggerin -P 1000500 -p -- @sysctldir@ -- This script will automatically apply sysctl rules if files have been -- installed or updated in @sysctldir@. if posix.access("/run/systemd/system") then @@ -110,15 +122,3 @@ if posix.access("/run/systemd/system") then posix.wait(pid) end end - -%transfiletriggerin -p -- @binfmtdir@ --- This script will automatically apply binfmt rules if files have been --- installed or updated in @binfmtdir@. -if posix.access("/run/systemd/system") then - pid = posix.fork() - if pid == 0 then - assert(posix.exec("@rootlibexecdir@/systemd-binfmt")) - elseif pid > 0 then - posix.wait(pid) - end -end diff --git a/src/rpm/triggers.systemd.sh.in b/src/rpm/triggers.systemd.sh.in index 641afd9ef7..c1af5f43d3 100644 --- a/src/rpm/triggers.systemd.sh.in +++ b/src/rpm/triggers.systemd.sh.in @@ -28,7 +28,7 @@ if test -d "/run/systemd/system"; then %{_bindir}/systemctl daemon-reload || : fi -%transfiletriggerin -P 100700 -- @sysusersdir@ +%transfiletriggerin -P 1000700 -- @sysusersdir@ # This script will process files installed in @sysusersdir@ to create # specified users automatically. The priority is set such that it # will run before the tmpfiles file trigger. @@ -36,43 +36,21 @@ if test -d "/run/systemd/system"; then %{_bindir}/systemd-sysusers || : fi -%transfiletriggerin -P 100500 -- @tmpfilesdir@ -# This script will process files installed in @tmpfilesdir@ to create -# tmpfiles automatically. The priority is set such that it will run -# after the sysusers file trigger, but before any other triggers. -if test -d "/run/systemd/system"; then - %{_bindir}/systemd-tmpfiles --create || : -fi - -%transfiletriggerin udev -- @udevhwdbdir@ +%transfiletriggerin -P 1000700 udev -- @udevhwdbdir@ # This script will automatically invoke hwdb update if files have been # installed or updated in @udevhwdbdir@. if test -d "/run/systemd/system"; then %{_bindir}/systemd-hwdb update || : fi -%transfiletriggerin -- @catalogdir@ +%transfiletriggerin -P 1000700 -- @catalogdir@ # This script will automatically invoke journal catalog update if files # have been installed or updated in @catalogdir@. if test -d "/run/systemd/system"; then %{_bindir}/journalctl --update-catalog || : fi -%transfiletriggerin udev -- @udevrulesdir@ -# This script will automatically update udev with new rules if files -# have been installed or updated in @udevrulesdir@. -if test -e /run/udev/control; then - %{_bindir}/udevadm control --reload || : -fi - -%transfiletriggerin -- @sysctldir@ -# This script will automatically apply sysctl rules if files have been -# installed or updated in @sysctldir@. -if test -d "/run/systemd/system"; then - @rootlibexecdir@/systemd-sysctl || : -fi - -%transfiletriggerin -- @binfmtdir@ +%transfiletriggerin -P 1000700 -- @binfmtdir@ # This script will automatically apply binfmt rules if files have been # installed or updated in @binfmtdir@. if test -d "/run/systemd/system"; then @@ -80,3 +58,25 @@ if test -d "/run/systemd/system"; then # during install @rootlibexecdir@/systemd-binfmt || : fi + +%transfiletriggerin -P 1000600 -- @tmpfilesdir@ +# This script will process files installed in @tmpfilesdir@ to create +# tmpfiles automatically. The priority is set such that it will run +# after the sysusers file trigger, but before any other triggers. +if test -d "/run/systemd/system"; then + %{_bindir}/systemd-tmpfiles --create || : +fi + +%transfiletriggerin -P 1000600 udev -- @udevrulesdir@ +# This script will automatically update udev with new rules if files +# have been installed or updated in @udevrulesdir@. +if test -e /run/udev/control; then + %{_bindir}/udevadm control --reload || : +fi + +%transfiletriggerin -P 1000500 -- @sysctldir@ +# This script will automatically apply sysctl rules if files have been +# installed or updated in @sysctldir@. +if test -d "/run/systemd/system"; then + @rootlibexecdir@/systemd-sysctl || : +fi From fa97d2fcf64e0558054bee673f734f523373b146 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 31 Jan 2021 12:11:36 +0100 Subject: [PATCH 19/19] rpm: restart services in %posttrans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes a long-standing issue in packaging scriptlets: daemon-reload was moved to the end of the transaction, but restarting services was still straightaway after package installation. https://bugzilla.redhat.com/show_bug.cgi?id=1614751 Note that daemon-reload is called twice. This wouldn't be hardly noticable, except that now a bunch of units (at least in Fedora) generate very verbose warnings about deprecated features. So we get those warnings twice… reload-or-restart --needing-restart is also called twice, but the second call is usually a noop, because the first clears the flag for restarted units. The second call is necessary for the case where we only uninstall packages, and the %transfiletriggerpostun trigger fires, but not the %transfiletriggerin scriptlet. Also note that this assumes that units are marked only for restart if paths under @systemunitdir@ or /etc/systemd/system have been touched. I would prefer make the trigger that does 'restart --needing-restart' fire always, but it seems rpm doesn't have such functionality. (Except as a %transfiletrigger that would trigger on "/*" to catch all transactions, but that seems ineffiecient and ugly.) --- src/rpm/macros.systemd.in | 4 +++- src/rpm/triggers.systemd.in | 18 ++++++++++++++++++ src/rpm/triggers.systemd.sh.in | 7 +++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/rpm/macros.systemd.in b/src/rpm/macros.systemd.in index 1c40328db0..24996de10a 100644 --- a/src/rpm/macros.systemd.in +++ b/src/rpm/macros.systemd.in @@ -82,7 +82,9 @@ fi \ %{expand:%%{?__systemd_someargs_%#:%%__systemd_someargs_%# systemd_postun_with_restart}} \ if [ $1 -ge 1 ] && [ -x @bindir@/systemctl ]; then \ # Package upgrade, not uninstall \ - @bindir@/systemctl try-restart %{?*} || : \ + for unit in %{?*}; do \ + @bindir@/systemctl set-property $unit Markers=+needs-restart || : \ + done \ fi \ %{nil} diff --git a/src/rpm/triggers.systemd.in b/src/rpm/triggers.systemd.in index 37f62cb1b7..3a89f9169d 100644 --- a/src/rpm/triggers.systemd.in +++ b/src/rpm/triggers.systemd.in @@ -20,6 +20,13 @@ if posix.access("/run/systemd/system") then elseif pid > 0 then posix.wait(pid) end + + pid = posix.fork() + if pid == 0 then + assert(posix.exec("%{_bindir}/systemctl", "reload-or-restart", "--marked")) + elseif pid > 0 then + posix.wait(pid) + end end %transfiletriggerpostun -P 1000100 -p -- @systemunitdir@ /etc/systemd/system @@ -37,6 +44,17 @@ if posix.access("/run/systemd/system") then end end +%transfiletriggerpostun -P 10000 -p -- @systemunitdir@ /etc/systemd/system +-- We restart remaining services that should be restarted here. +if posix.access("/run/systemd/system") then + pid = posix.fork() + if pid == 0 then + assert(posix.exec("%{_bindir}/systemctl", "reload-or-restart", "--marked")) + elseif pid > 0 then + posix.wait(pid) + end +end + %transfiletriggerin -P 100700 -p -- @sysusersdir@ -- This script will process files installed in @sysusersdir@ to create -- specified users automatically. The priority is set such that it diff --git a/src/rpm/triggers.systemd.sh.in b/src/rpm/triggers.systemd.sh.in index c1af5f43d3..0080040de4 100644 --- a/src/rpm/triggers.systemd.sh.in +++ b/src/rpm/triggers.systemd.sh.in @@ -16,6 +16,7 @@ # so sometimes we will reload needlessly. if test -d "/run/systemd/system"; then %{_bindir}/systemctl daemon-reload || : + %{_bindir}/systemctl reload-or-restart --marked || : fi %transfiletriggerpostun -P 1000100 -- @systemunitdir@ /etc/systemd/system @@ -28,6 +29,12 @@ if test -d "/run/systemd/system"; then %{_bindir}/systemctl daemon-reload || : fi +%transfiletriggerpostun -P 10000 -- @systemunitdir@ /etc/systemd/system +# We restart remaining services that should be restarted here. +if test -d "/run/systemd/system"; then + %{_bindir}/systemctl reload-or-restart --marked || : +fi + %transfiletriggerin -P 1000700 -- @sysusersdir@ # This script will process files installed in @sysusersdir@ to create # specified users automatically. The priority is set such that it