From d4f38ff036ce001081175292e87198ee4b19c364 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Wed, 3 Aug 2022 16:50:39 +0100 Subject: [PATCH 1/5] test-63: convert to full shell script and generalize Will add more path unit tests later --- .../Makefile | 0 .../test.sh | 0 test/units/testsuite-63.service | 26 ++------------- test/units/testsuite-63.sh | 33 +++++++++++++++++++ 4 files changed, 35 insertions(+), 24 deletions(-) rename test/{TEST-63-ISSUE-17433 => TEST-63-PATH}/Makefile (100%) rename test/{TEST-63-ISSUE-17433 => TEST-63-PATH}/test.sh (100%) create mode 100755 test/units/testsuite-63.sh diff --git a/test/TEST-63-ISSUE-17433/Makefile b/test/TEST-63-PATH/Makefile similarity index 100% rename from test/TEST-63-ISSUE-17433/Makefile rename to test/TEST-63-PATH/Makefile diff --git a/test/TEST-63-ISSUE-17433/test.sh b/test/TEST-63-PATH/test.sh similarity index 100% rename from test/TEST-63-ISSUE-17433/test.sh rename to test/TEST-63-PATH/test.sh diff --git a/test/units/testsuite-63.service b/test/units/testsuite-63.service index 40422127ff..483c6a859b 100644 --- a/test/units/testsuite-63.service +++ b/test/units/testsuite-63.service @@ -1,30 +1,8 @@ # SPDX-License-Identifier: LGPL-2.1-or-later [Unit] -Description=TEST-63-ISSUE-17433 +Description=TEST-63-PATH [Service] ExecStartPre=rm -f /failed /testok +ExecStart=/usr/lib/systemd/tests/testdata/units/%N.sh Type=oneshot - -# Test that a path unit continuously triggering a service that fails condition checks eventually fails with -# the trigger-limit-hit error. -ExecStart=rm -f /tmp/nonexistent -ExecStart=systemctl start test63.path -ExecStart=touch /tmp/test63 -# Make sure systemd has sufficient time to hit the trigger limit for test63.path. -ExecStart=sleep 2 -ExecStart=sh -x -c 'test "$(systemctl show test63.service -P ActiveState)" = inactive' -ExecStart=sh -x -c 'test "$(systemctl show test63.service -P Result)" = success' -ExecStart=sh -x -c 'test "$(systemctl show test63.path -P ActiveState)" = failed' -ExecStart=sh -x -c 'test "$(systemctl show test63.path -P Result)" = trigger-limit-hit' - -# Test that starting the service manually doesn't affect the path unit. -ExecStart=rm -f /tmp/test63 -ExecStart=systemctl reset-failed -ExecStart=systemctl start test63.path -ExecStart=systemctl start test63.service -ExecStart=sh -x -c 'test "$(systemctl show test63.service -P ActiveState)" = inactive' -ExecStart=sh -x -c 'test "$(systemctl show test63.service -P Result)" = success' -ExecStart=sh -x -c 'test "$(systemctl show test63.path -P ActiveState)" = active' -ExecStart=sh -x -c 'test "$(systemctl show test63.path -P Result)" = success' -ExecStart=sh -x -c 'echo OK >/testok' diff --git a/test/units/testsuite-63.sh b/test/units/testsuite-63.sh new file mode 100755 index 0000000000..20d93936b3 --- /dev/null +++ b/test/units/testsuite-63.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later +set -ex +set -o pipefail + +systemctl log-level debug + +# Test that a path unit continuously triggering a service that fails condition checks eventually fails with +# the trigger-limit-hit error. +rm -f /tmp/nonexistent +systemctl start test63.path +touch /tmp/test63 + +# Make sure systemd has sufficient time to hit the trigger limit for test63.path. +sleep 2 +test "$(systemctl show test63.service -P ActiveState)" = inactive +test "$(systemctl show test63.service -P Result)" = success +test "$(systemctl show test63.path -P ActiveState)" = failed +test "$(systemctl show test63.path -P Result)" = trigger-limit-hit + +# Test that starting the service manually doesn't affect the path unit. +rm -f /tmp/test63 +systemctl reset-failed +systemctl start test63.path +systemctl start test63.service +test "$(systemctl show test63.service -P ActiveState)" = inactive +test "$(systemctl show test63.service -P Result)" = success +test "$(systemctl show test63.path -P ActiveState)" = active +test "$(systemctl show test63.path -P Result)" = success + +systemctl log-level info + +echo OK >/testok From f52faaf923acfe6fe3f0955d1ad66840b13babfc Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Wed, 3 Aug 2022 14:20:41 +0100 Subject: [PATCH 2/5] glob: add glob_first(), returns first match Note that which match is returned depends on the system and is not guaranteed to be stable --- src/basic/glob-util.c | 15 +++++++++++++-- src/basic/glob-util.h | 4 +++- src/test/test-glob-util.c | 21 +++++++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/basic/glob-util.c b/src/basic/glob-util.c index e026b29478..fd60a6eda2 100644 --- a/src/basic/glob-util.c +++ b/src/basic/glob-util.c @@ -47,17 +47,28 @@ int safe_glob(const char *path, int flags, glob_t *pglob) { return 0; } -int glob_exists(const char *path) { +int glob_first(const char *path, char **ret_first) { _cleanup_globfree_ glob_t g = {}; int k; assert(path); k = safe_glob(path, GLOB_NOSORT|GLOB_BRACE, &g); - if (k == -ENOENT) + if (k == -ENOENT) { + if (ret_first) + *ret_first = NULL; return false; + } if (k < 0) return k; + + if (ret_first) { + char *first = strdup(g.gl_pathv[0]); + if (!first) + return log_oom_debug(); + *ret_first = first; + } + return true; } diff --git a/src/basic/glob-util.h b/src/basic/glob-util.h index fc86e990dd..7ca26cc27f 100644 --- a/src/basic/glob-util.h +++ b/src/basic/glob-util.h @@ -10,7 +10,9 @@ /* Note: this function modifies pglob to set various functions. */ int safe_glob(const char *path, int flags, glob_t *pglob); -int glob_exists(const char *path); +/* Note: which match is returned depends on the implementation/system and not guaranteed to be stable */ +int glob_first(const char *path, char **ret_first); +#define glob_exists(path) glob_first(path, NULL) int glob_extend(char ***strv, const char *path, int flags); int glob_non_glob_prefix(const char *path, char **ret); diff --git a/src/test/test-glob-util.c b/src/test/test-glob-util.c index ec8b74f48f..566b68b589 100644 --- a/src/test/test-glob-util.c +++ b/src/test/test-glob-util.c @@ -13,6 +13,27 @@ #include "tests.h" #include "tmpfile-util.h" +TEST(glob_first) { + char *first, name[] = "/tmp/test-glob_first.XXXXXX"; + int fd = -1; + int r; + + fd = mkostemp_safe(name); + assert_se(fd >= 0); + close(fd); + + r = glob_first("/tmp/test-glob_first*", &first); + assert_se(r == 1); + assert_se(streq(name, first)); + first = mfree(first); + + r = unlink(name); + assert_se(r == 0); + r = glob_first("/tmp/test-glob_first*", &first); + assert_se(r == 0); + assert_se(first == NULL); +} + TEST(glob_exists) { char name[] = "/tmp/test-glob_exists.XXXXXX"; int fd = -1; From 48b92b37acbd2bf9c36ac5e67961b15723243cda Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Tue, 2 Aug 2022 19:49:20 +0100 Subject: [PATCH 3/5] core: add basic infrastructure to record unit activation information Not wired in by any unit type yet, just the basic to allocate, ref, deref and plug in to other unit types. Includes recording the trigger unit name and passing it to the triggered unit as TRIGGER_UNIT= env var. --- man/org.freedesktop.systemd1.xml | 22 ++++ src/core/dbus-job.c | 2 + src/core/dbus-unit.c | 1 + src/core/dbus-util.c | 32 ++++++ src/core/dbus-util.h | 2 + src/core/job.c | 29 ++++- src/core/job.h | 6 + src/core/service.c | 10 ++ src/core/unit.c | 162 ++++++++++++++++++++++++++- src/core/unit.h | 74 +++++++++++- src/test/test-bpf-firewall.c | 4 +- src/test/test-bpf-foreign-programs.c | 2 +- src/test/test-bpf-lsm.c | 2 +- src/test/test-execute.c | 4 +- src/test/test-path.c | 14 +-- src/test/test-socket-bind.c | 2 +- 16 files changed, 348 insertions(+), 20 deletions(-) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index 120ffbc8ef..689ca3ec88 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -1898,6 +1898,7 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { readonly s CollectMode = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly as Refs = ['...', ...]; + readonly a(ss) ActivationDetails = [...]; }; interface org.freedesktop.DBus.Peer { ... }; interface org.freedesktop.DBus.Introspectable { ... }; @@ -2229,6 +2230,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + @@ -2406,6 +2409,18 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { Transient contains a boolean that indicates whether the unit was created as a transient unit (i.e. via CreateTransientUnit() on the manager object). + + ActivationDetails contains a list of string pairs, key and value, that + describe the event that caused the unit to be activated, if any. The key describes the information + (e.g.: trigger_unit, with value foo.service). This is only filled + in if the unit was triggered by a Path or Timer unit, and it is + only provided in a best effort fashion: it is not guaranteed to be set, and it is not guaranteed to be + the only trigger. It is only guaranteed to be a valid trigger that caused the activation job to be + enqueued and complete successfully. The key value pairs correspond (in lowercase) to the environment + variables described in the Environment Variables Set on Triggered Units section in + systemd.exec1. + Note that new key value pair may be added at any time in future versions. Existing entries will not be + removed. @@ -10650,6 +10665,8 @@ node /org/freedesktop/systemd1/job/666 { @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly s JobType = '...'; readonly s State = '...'; + @org.freedesktop.DBus.Property.EmitsChangedSignal("const") + readonly a(ss) ActivationDetails = [...]; }; interface org.freedesktop.DBus.Peer { ... }; interface org.freedesktop.DBus.Introspectable { ... }; @@ -10681,6 +10698,8 @@ node /org/freedesktop/systemd1/job/666 { + + @@ -10709,6 +10728,9 @@ node /org/freedesktop/systemd1/job/666 { State refers to the job's state and is one of waiting and running. The former indicates that a job is currently queued but has not begun to execute yet. The latter indicates that a job is currently being executed. + + ActivationDetails has the same content as the property of the same name under + the org.freedesktop.systemd1.Unit interface. diff --git a/src/core/dbus-job.c b/src/core/dbus-job.c index de474e6d4e..7b1438266b 100644 --- a/src/core/dbus-job.c +++ b/src/core/dbus-job.c @@ -7,6 +7,7 @@ #include "bus-util.h" #include "dbus-job.h" #include "dbus-unit.h" +#include "dbus-util.h" #include "dbus.h" #include "job.h" #include "log.h" @@ -136,6 +137,7 @@ const sd_bus_vtable bus_job_vtable[] = { SD_BUS_PROPERTY("Unit", "(so)", property_get_unit, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("JobType", "s", property_get_type, offsetof(Job, type), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("State", "s", property_get_state, offsetof(Job, state), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_PROPERTY("ActivationDetails", "a(ss)", bus_property_get_activation_details, offsetof(Job, activation_details), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_VTABLE_END }; diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index ee013e1bc5..7a16471758 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -951,6 +951,7 @@ const sd_bus_vtable bus_unit_vtable[] = { SD_BUS_PROPERTY("InvocationID", "ay", bus_property_get_id128, offsetof(Unit, invocation_id), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("CollectMode", "s", property_get_collect_mode, offsetof(Unit, collect_mode), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Refs", "as", property_get_refs, 0, 0), + SD_BUS_PROPERTY("ActivationDetails", "a(ss)", bus_property_get_activation_details, offsetof(Unit, activation_details), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_METHOD_WITH_ARGS("Start", SD_BUS_ARGS("s", mode), diff --git a/src/core/dbus-util.c b/src/core/dbus-util.c index 264a4f55b6..edfa0eb69a 100644 --- a/src/core/dbus-util.c +++ b/src/core/dbus-util.c @@ -228,3 +228,35 @@ int bus_read_mount_options( return 0; } + +int bus_property_get_activation_details( + sd_bus *bus, + const char *path, + const char *interface, + const char *property, + sd_bus_message *reply, + void *userdata, + sd_bus_error *error) { + + ActivationDetails **details = ASSERT_PTR(userdata); + _cleanup_strv_free_ char **pairs = NULL; + int r; + + assert(reply); + + r = activation_details_append_pair(*details, &pairs); + if (r < 0) + return r; + + r = sd_bus_message_open_container(reply, 'a', "(ss)"); + if (r < 0) + return r; + + STRV_FOREACH_PAIR(key, value, pairs) { + r = sd_bus_message_append(reply, "(ss)", *key, *value); + if (r < 0) + return r; + } + + return sd_bus_message_close_container(reply); +} diff --git a/src/core/dbus-util.h b/src/core/dbus-util.h index 799136737b..e12631a0e2 100644 --- a/src/core/dbus-util.h +++ b/src/core/dbus-util.h @@ -251,3 +251,5 @@ static inline int bus_set_transient_usec_fix_0(Unit *u, const char *name, usec_t int bus_verify_manage_units_async_full(Unit *u, const char *verb, int capability, const char *polkit_message, bool interactive, sd_bus_message *call, sd_bus_error *error); int bus_read_mount_options(sd_bus_message *message, sd_bus_error *error, MountOptions **ret_options, char **ret_format_str, const char *separator); + +int bus_property_get_activation_details(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error); diff --git a/src/core/job.c b/src/core/job.c index 6653dbde84..dd8d858bd2 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -104,6 +104,8 @@ Job* job_free(Job *j) { sd_bus_track_unref(j->bus_track); strv_free(j->deserialized_clients); + activation_details_unref(j->activation_details); + return mfree(j); } @@ -180,9 +182,13 @@ static void job_merge_into_installed(Job *j, Job *other) { assert(j->installed); assert(j->unit == other->unit); - if (j->type != JOB_NOP) + if (j->type != JOB_NOP) { assert_se(job_type_merge_and_collapse(&j->type, other->type, j->unit) == 0); - else + + /* Keep the oldest ActivationDetails, if any */ + if (!j->activation_details) + j->activation_details = TAKE_PTR(other->activation_details); + } else assert(other->type == JOB_NOP); j->irreversible = j->irreversible || other->irreversible; @@ -776,6 +782,7 @@ static void job_emit_done_message(Unit *u, uint32_t job_id, JobType t, JobResult } static int job_perform_on_unit(Job **j) { + ActivationDetails *a; uint32_t id; Manager *m; JobType t; @@ -795,10 +802,11 @@ static int job_perform_on_unit(Job **j) { u = (*j)->unit; t = (*j)->type; id = (*j)->id; + a = (*j)->activation_details; switch (t) { case JOB_START: - r = unit_start(u); + r = unit_start(u, a); break; case JOB_RESTART: @@ -1160,6 +1168,8 @@ int job_serialize(Job *j, FILE *f) { bus_track_serialize(j->bus_track, f, "subscribed"); + activation_details_serialize(j->activation_details, f); + /* End marker */ fputc('\n', f); return 0; @@ -1257,6 +1267,11 @@ int job_deserialize(Job *j, FILE *f) { else if (streq(l, "subscribed")) { if (strv_extend(&j->deserialized_clients, v) < 0) return log_oom(); + + } else if (startswith(l, "activation-details")) { + if (activation_details_deserialize(l, v, &j->activation_details) < 0) + log_debug("Failed to parse job ActivationDetails element: %s", v); + } else log_debug("Unknown job serialization key: %s", l); } @@ -1636,3 +1651,11 @@ int job_compare(Job *a, Job *b, UnitDependencyAtom assume_dep) { else return -1; } + +void job_set_activation_details(Job *j, ActivationDetails *info) { + /* Existing (older) ActivationDetails win, newer ones are discarded. */ + if (!j || j->activation_details || !info) + return; /* Nothing to do. */ + + j->activation_details = activation_details_ref(info); +} diff --git a/src/core/job.h b/src/core/job.h index c033c8a4fa..0305e0ea44 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -10,6 +10,7 @@ #include "unit-name.h" #include "unit.h" +typedef struct ActivationDetails ActivationDetails; typedef struct Job Job; typedef struct JobDependency JobDependency; typedef enum JobType JobType; @@ -151,6 +152,9 @@ struct Job { unsigned run_queue_idx; + /* If the job had a specific trigger that needs to be advertised (eg: a path unit), store it. */ + ActivationDetails *activation_details; + bool installed:1; bool in_run_queue:1; bool matters_to_anchor:1; @@ -243,3 +247,5 @@ JobResult job_result_from_string(const char *s) _pure_; const char* job_type_to_access_method(JobType t); int job_compare(Job *a, Job *b, UnitDependencyAtom assume_dep); + +void job_set_activation_details(Job *j, ActivationDetails *info); diff --git a/src/core/service.c b/src/core/service.c index a715a1d1dc..fa37207dcc 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1641,6 +1641,16 @@ static int service_spawn_internal( } } + if (UNIT(s)->activation_details) { + r = activation_details_append_env(UNIT(s)->activation_details, &our_env); + if (r < 0) + return r; + /* The number of env vars added here can vary, rather than keeping the allocation block in + * sync manually, these functions simply use the strv methods to append to it, so we need + * to update n_env when we are done in case of future usage. */ + n_env += r; + } + r = unit_set_exec_params(UNIT(s), &exec_params); if (r < 0) return r; diff --git a/src/core/unit.c b/src/core/unit.c index 6242f045b4..18d9ba85de 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -41,6 +41,7 @@ #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" @@ -807,6 +808,8 @@ Unit* unit_free(Unit *u) { set_free_free(u->aliases); free(u->id); + activation_details_unref(u->activation_details); + return mfree(u); } @@ -1188,6 +1191,9 @@ int unit_merge(Unit *u, Unit *other) { other->load_state = UNIT_MERGED; other->merged_into = u; + if (!u->activation_details) + u->activation_details = activation_details_ref(other->activation_details); + /* If there is still some data attached to the other node, we * don't need it anymore, and can free it. */ if (other->load_state != UNIT_STUB) @@ -1861,7 +1867,7 @@ static bool unit_verify_deps(Unit *u) { * -ESTALE: This unit has been started before and can't be started a second time * -ENOENT: This is a triggering unit and unit to trigger is not loaded */ -int unit_start(Unit *u) { +int unit_start(Unit *u, ActivationDetails *details) { UnitActiveState state; Unit *following; int r; @@ -1918,7 +1924,7 @@ int unit_start(Unit *u) { following = unit_following(u); if (following) { log_unit_debug(u, "Redirecting start request from %s to %s.", u->id, following->id); - return unit_start(following); + return unit_start(following, details); } /* Check our ability to start early so that failure conditions don't cause us to enter a busy loop. */ @@ -1939,6 +1945,9 @@ int unit_start(Unit *u) { unit_add_to_dbus_queue(u); unit_cgroup_freezer_action(u, FREEZER_THAW); + if (!u->activation_details) /* Older details object wins */ + u->activation_details = activation_details_ref(details); + return UNIT_VTABLE(u)->start(u); } @@ -5919,3 +5928,152 @@ int unit_get_dependency_array(const Unit *u, UnitDependencyAtom atom, Unit ***re assert(n <= INT_MAX); return (int) n; } + +const ActivationDetailsVTable * const activation_details_vtable[_UNIT_TYPE_MAX] = { +}; + +ActivationDetails *activation_details_new(Unit *trigger_unit) { + _cleanup_free_ ActivationDetails *details = NULL; + + assert(trigger_unit); + assert(trigger_unit->type != _UNIT_TYPE_INVALID); + assert(trigger_unit->id); + + details = malloc0(activation_details_vtable[trigger_unit->type]->object_size); + if (!details) + return NULL; + + *details = (ActivationDetails) { + .n_ref = 1, + .trigger_unit_type = trigger_unit->type, + }; + + details->trigger_unit_name = strdup(trigger_unit->id); + if (!details->trigger_unit_name) + return NULL; + + if (ACTIVATION_DETAILS_VTABLE(details)->init) + ACTIVATION_DETAILS_VTABLE(details)->init(details, trigger_unit); + + return TAKE_PTR(details); +} + +static ActivationDetails *activation_details_free(ActivationDetails *details) { + if (!details) + return NULL; + + if (ACTIVATION_DETAILS_VTABLE(details)->done) + ACTIVATION_DETAILS_VTABLE(details)->done(details); + + free(details->trigger_unit_name); + + return mfree(details); +} + +void activation_details_serialize(ActivationDetails *details, FILE *f) { + if (!details || details->trigger_unit_type == _UNIT_TYPE_INVALID) + return; + + (void) serialize_item(f, "activation-details-unit-type", unit_type_to_string(details->trigger_unit_type)); + if (details->trigger_unit_name) + (void) serialize_item(f, "activation-details-unit-name", details->trigger_unit_name); + if (ACTIVATION_DETAILS_VTABLE(details)->serialize) + ACTIVATION_DETAILS_VTABLE(details)->serialize(details, f); +} + +int activation_details_deserialize(const char *key, const char *value, ActivationDetails **details) { + assert(key); + assert(value); + assert(details); + + if (!*details) { + UnitType t; + + if (!streq(key, "activation-details-unit-type")) + return -EINVAL; + + t = unit_type_from_string(value); + if (t == _UNIT_TYPE_INVALID) + return -EINVAL; + + *details = malloc0(activation_details_vtable[t]->object_size); + if (!*details) + return -ENOMEM; + + **details = (ActivationDetails) { + .n_ref = 1, + .trigger_unit_type = t, + }; + + return 0; + } + + if (streq(key, "activation-details-unit-name")) { + (*details)->trigger_unit_name = strdup(value); + if (!(*details)->trigger_unit_name) + return -ENOMEM; + + return 0; + } + + if (ACTIVATION_DETAILS_VTABLE(*details)->deserialize) + return ACTIVATION_DETAILS_VTABLE(*details)->deserialize(key, value, details); + + return -EINVAL; +} + +int activation_details_append_env(ActivationDetails *details, char ***strv) { + int r = 0; + + assert(strv); + + if (!details) + return 0; + + if (!isempty(details->trigger_unit_name)) { + char *s = strjoin("TRIGGER_UNIT=", details->trigger_unit_name); + if (!s) + return -ENOMEM; + + r = strv_consume(strv, TAKE_PTR(s)); + if (r < 0) + return r; + } + + if (ACTIVATION_DETAILS_VTABLE(details)->append_env) { + r = ACTIVATION_DETAILS_VTABLE(details)->append_env(details, strv); + if (r < 0) + return r; + } + + return r + !isempty(details->trigger_unit_name); /* Return the number of variables added to the env block */ +} + +int activation_details_append_pair(ActivationDetails *details, char ***strv) { + int r = 0; + + assert(strv); + + if (!details) + return 0; + + if (!isempty(details->trigger_unit_name)) { + r = strv_extend(strv, "trigger_unit"); + if (r < 0) + return r; + + r = strv_extend(strv, details->trigger_unit_name); + if (r < 0) + return r; + } + + if (ACTIVATION_DETAILS_VTABLE(details)->append_env) { + r = ACTIVATION_DETAILS_VTABLE(details)->append_pair(details, strv); + if (r < 0) + return r; + } + + return r + !isempty(details->trigger_unit_name); /* Return the number of pairs added to the strv */ +} + +DEFINE_TRIVIAL_REF_UNREF_FUNC(ActivationDetails, activation_details, activation_details_free); diff --git a/src/core/unit.h b/src/core/unit.h index 4dabbb9a3c..fc8edaade5 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -110,6 +110,75 @@ typedef union UnitDependencyInfo { } _packed_; } UnitDependencyInfo; +/* Store information about why a unit was activated. + * We start with trigger units (.path/.timer), eventually it will be expanded to include more metadata. */ +typedef struct ActivationDetails { + unsigned n_ref; + UnitType trigger_unit_type; + char *trigger_unit_name; +} ActivationDetails; + +/* For casting an activation event into the various unit-specific types */ +#define DEFINE_ACTIVATION_DETAILS_CAST(UPPERCASE, MixedCase, UNIT_TYPE) \ + static inline MixedCase* UPPERCASE(ActivationDetails *a) { \ + if (_unlikely_(!a || a->trigger_unit_type != UNIT_##UNIT_TYPE)) \ + return NULL; \ + \ + return (MixedCase*) a; \ + } + +/* For casting the various unit types into a unit */ +#define ACTIVATION_DETAILS(u) \ + ({ \ + typeof(u) _u_ = (u); \ + ActivationDetails *_w_ = _u_ ? &(_u_)->meta : NULL; \ + _w_; \ + }) + +ActivationDetails *activation_details_new(Unit *trigger_unit); +ActivationDetails *activation_details_ref(ActivationDetails *p); +ActivationDetails *activation_details_unref(ActivationDetails *p); +void activation_details_serialize(ActivationDetails *p, FILE *f); +int activation_details_deserialize(const char *key, const char *value, ActivationDetails **info); +int activation_details_append_env(ActivationDetails *info, char ***strv); +int activation_details_append_pair(ActivationDetails *info, char ***strv); +DEFINE_TRIVIAL_CLEANUP_FUNC(ActivationDetails*, activation_details_unref); + +typedef struct ActivationDetailsVTable { + /* How much memory does an object of this activation type need */ + size_t object_size; + + /* This should reset all type-specific variables. This should not allocate memory, and is called + * with zero-initialized data. It should hence only initialize variables that need to be set != 0. */ + void (*init)(ActivationDetails *info, Unit *trigger_unit); + + /* This should free all type-specific variables. It should be idempotent. */ + void (*done)(ActivationDetails *info); + + /* This should serialize all type-specific variables. */ + void (*serialize)(ActivationDetails *info, FILE *f); + + /* This should deserialize all type-specific variables, one at a time. */ + int (*deserialize)(const char *key, const char *value, ActivationDetails **info); + + /* This should format the type-specific variables for the env block of the spawned service, + * and return the number of added items. */ + int (*append_env)(ActivationDetails *info, char ***strv); + + /* This should append type-specific variables as key/value pairs for the D-Bus property of the job, + * and return the number of added pairs. */ + int (*append_pair)(ActivationDetails *info, char ***strv); +} ActivationDetailsVTable; + +extern const ActivationDetailsVTable * const activation_details_vtable[_UNIT_TYPE_MAX]; + +static inline const ActivationDetailsVTable* ACTIVATION_DETAILS_VTABLE(const ActivationDetails *a) { + assert(a); + assert(a->trigger_unit_type < _UNIT_TYPE_MAX); + + return activation_details_vtable[a->trigger_unit_type]; +} + /* Newer LLVM versions don't like implicit casts from large pointer types to smaller enums, hence let's add * explicit type-safe helpers for that. */ static inline UnitDependency UNIT_DEPENDENCY_FROM_PTR(const void *p) { @@ -363,6 +432,9 @@ typedef struct Unit { JobMode on_success_job_mode; JobMode on_failure_job_mode; + /* If the job had a specific trigger that needs to be advertised (eg: a path unit), store it. */ + ActivationDetails *activation_details; + /* Tweaking the GC logic */ CollectMode collect_mode; @@ -813,7 +885,7 @@ bool unit_can_start(Unit *u) _pure_; bool unit_can_stop(Unit *u) _pure_; bool unit_can_isolate(Unit *u) _pure_; -int unit_start(Unit *u); +int unit_start(Unit *u, ActivationDetails *details); int unit_stop(Unit *u); int unit_reload(Unit *u); diff --git a/src/test/test-bpf-firewall.c b/src/test/test-bpf-firewall.c index 3b99c5aaec..d655058d3d 100644 --- a/src/test/test-bpf-firewall.c +++ b/src/test/test-bpf-firewall.c @@ -176,7 +176,7 @@ int main(int argc, char *argv[]) { assert_se(r >= 0); - assert_se(unit_start(u) >= 0); + assert_se(unit_start(u, NULL) >= 0); while (!IN_SET(SERVICE(u)->state, SERVICE_DEAD, SERVICE_FAILED)) assert_se(sd_event_run(m->event, UINT64_MAX) >= 0); @@ -201,7 +201,7 @@ int main(int argc, char *argv[]) { SERVICE(u)->type = SERVICE_ONESHOT; u->load_state = UNIT_LOADED; - assert_se(unit_start(u) >= 0); + assert_se(unit_start(u, NULL) >= 0); while (!IN_SET(SERVICE(u)->state, SERVICE_DEAD, SERVICE_FAILED)) assert_se(sd_event_run(m->event, UINT64_MAX) >= 0); diff --git a/src/test/test-bpf-foreign-programs.c b/src/test/test-bpf-foreign-programs.c index 061426f287..0445c39855 100644 --- a/src/test/test-bpf-foreign-programs.c +++ b/src/test/test-bpf-foreign-programs.c @@ -246,7 +246,7 @@ static int test_bpf_cgroup_programs(Manager *m, const char *unit_name, const Tes SERVICE(u)->type = SERVICE_ONESHOT; u->load_state = UNIT_LOADED; - r = unit_start(u); + r = unit_start(u, NULL); if (r < 0) return log_error_errno(r, "Unit start failed %m"); diff --git a/src/test/test-bpf-lsm.c b/src/test/test-bpf-lsm.c index 9709053d0a..e1df62f1a6 100644 --- a/src/test/test-bpf-lsm.c +++ b/src/test/test-bpf-lsm.c @@ -39,7 +39,7 @@ static int test_restrict_filesystems(Manager *m, const char *unit_name, const ch SERVICE(u)->type = SERVICE_ONESHOT; u->load_state = UNIT_LOADED; - r = unit_start(u); + r = unit_start(u, NULL); if (r < 0) return log_error_errno(r, "Unit start failed %m"); diff --git a/src/test/test-execute.c b/src/test/test-execute.c index 82e4cf5e54..0283caeca6 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -210,7 +210,7 @@ static void _test(const char *file, unsigned line, const char *func, assert_se(unit_name); assert_se(manager_load_startable_unit_or_warn(m, unit_name, NULL, &unit) >= 0); - assert_se(unit_start(unit) >= 0); + assert_se(unit_start(unit, NULL) >= 0); check_main_result(file, line, func, m, unit, status_expected, code_expected); } #define test(m, unit_name, status_expected, code_expected) \ @@ -223,7 +223,7 @@ static void _test_service(const char *file, unsigned line, const char *func, assert_se(unit_name); assert_se(manager_load_startable_unit_or_warn(m, unit_name, NULL, &unit) >= 0); - assert_se(unit_start(unit) >= 0); + assert_se(unit_start(unit, NULL) >= 0); check_service_result(file, line, func, m, unit, result_expected); } #define test_service(m, unit_name, result_expected) \ diff --git a/src/test/test-path.c b/src/test/test-path.c index 7fb1f7363c..a440ca08b4 100644 --- a/src/test/test-path.c +++ b/src/test/test-path.c @@ -136,7 +136,7 @@ static void test_path_exists(Manager *m) { path = PATH(unit); service = service_for_path(m, path, NULL); - assert_se(unit_start(unit) >= 0); + assert_se(unit_start(unit, NULL) >= 0); if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0) return; @@ -170,7 +170,7 @@ static void test_path_existsglob(Manager *m) { path = PATH(unit); service = service_for_path(m, path, NULL); - assert_se(unit_start(unit) >= 0); + assert_se(unit_start(unit, NULL) >= 0); if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0) return; @@ -205,7 +205,7 @@ static void test_path_changed(Manager *m) { path = PATH(unit); service = service_for_path(m, path, NULL); - assert_se(unit_start(unit) >= 0); + assert_se(unit_start(unit, NULL) >= 0); if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0) return; @@ -247,7 +247,7 @@ static void test_path_modified(Manager *m) { path = PATH(unit); service = service_for_path(m, path, NULL); - assert_se(unit_start(unit) >= 0); + assert_se(unit_start(unit, NULL) >= 0); if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0) return; @@ -288,7 +288,7 @@ static void test_path_unit(Manager *m) { path = PATH(unit); service = service_for_path(m, path, "path-mycustomunit.service"); - assert_se(unit_start(unit) >= 0); + assert_se(unit_start(unit, NULL) >= 0); if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0) return; @@ -319,7 +319,7 @@ static void test_path_directorynotempty(Manager *m) { assert_se(access(test_path, F_OK) < 0); - assert_se(unit_start(unit) >= 0); + assert_se(unit_start(unit, NULL) >= 0); if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0) return; @@ -356,7 +356,7 @@ static void test_path_makedirectory_directorymode(Manager *m) { assert_se(access(test_path, F_OK) < 0); - assert_se(unit_start(unit) >= 0); + assert_se(unit_start(unit, NULL) >= 0); /* Check if the directory has been created */ assert_se(access(test_path, F_OK) >= 0); diff --git a/src/test/test-socket-bind.c b/src/test/test-socket-bind.c index dadd2c0dff..d7d9110343 100644 --- a/src/test/test-socket-bind.c +++ b/src/test/test-socket-bind.c @@ -78,7 +78,7 @@ static int test_socket_bind( SERVICE(u)->type = SERVICE_ONESHOT; u->load_state = UNIT_LOADED; - r = unit_start(u); + r = unit_start(u, NULL); if (r < 0) return log_error_errno(r, "Unit start failed %m"); From 4c420328549ed98c6ca170d8ccc89cbee2f38ccc Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Tue, 2 Aug 2022 20:07:35 +0100 Subject: [PATCH 4/5] service: set TRIGGER_UNIT= and TRIGGER_PATH= on activation by path unit When a service is triggered by a path unit, pass the path unit name and the path that triggered it via env vars to the spawned processes. Note that this is best-effort, as there might be many triggers at the same time, but we only get woken up by one. --- man/systemd.exec.xml | 13 ++ man/systemd.path.xml | 4 + src/core/path.c | 144 ++++++++++++++++++-- src/core/path.h | 8 ++ src/core/unit.c | 1 + test/testsuite-63.units/test63-glob.path | 3 + test/testsuite-63.units/test63-glob.service | 7 + test/testsuite-63.units/test63.service | 2 + test/units/testsuite-63.sh | 13 ++ 9 files changed, 184 insertions(+), 11 deletions(-) create mode 100644 test/testsuite-63.units/test63-glob.path create mode 100644 test/testsuite-63.units/test63-glob.service diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index d9da864bc2..2acb737c38 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -3695,6 +3695,19 @@ StandardInputData=V2XigLJyZSBubyBzdHJhbmdlcnMgdG8gbG92ZQpZb3Uga25vdyB0aGUgcnVsZX system. + + $TRIGGER_UNIT + $TRIGGER_PATH + + If the unit was activated dynamically (e.g.: a corresponding path unit), the + unit that triggered it and other type-dependent information will be passed via these variables. Note that + this information is provided in a best-effort way. For example, multiple triggers happening one after + another will be coalesced and only one will be reported, with no guarantee as to which one it will be. + Because of this, in most cases this variable will be primarily informational, i.e. useful for debugging + purposes, is lossy, and should not be relied upon to propagate a comprehensive reason for activation. + + + For system services, when PAMName= is enabled and pam_systemd is part diff --git a/man/systemd.path.xml b/man/systemd.path.xml index 4e4cd9137a..f143208cb4 100644 --- a/man/systemd.path.xml +++ b/man/systemd.path.xml @@ -208,6 +208,10 @@ See Also + Environment variables with details on the trigger will be set for triggered units. See the + Environment Variables Set on Triggered Units section in + systemd.exec1 + for more details. systemd1, systemctl1, diff --git a/src/core/path.c b/src/core/path.c index 69bbddf158..2810e30573 100644 --- a/src/core/path.c +++ b/src/core/path.c @@ -197,9 +197,13 @@ int path_spec_fd_event(PathSpec *s, uint32_t revents) { return 0; } -static bool path_spec_check_good(PathSpec *s, bool initial, bool from_trigger_notify) { +static bool path_spec_check_good(PathSpec *s, bool initial, bool from_trigger_notify, char **ret_trigger_path) { + _cleanup_free_ char *trigger = NULL; bool b, good = false; + assert(s); + assert(ret_trigger_path); + switch (s->type) { case PATH_EXISTS: @@ -207,7 +211,7 @@ static bool path_spec_check_good(PathSpec *s, bool initial, bool from_trigger_no break; case PATH_EXISTS_GLOB: - good = glob_exists(s->path) > 0; + good = glob_first(s->path, &trigger) > 0; break; case PATH_DIRECTORY_NOT_EMPTY: { @@ -229,6 +233,15 @@ static bool path_spec_check_good(PathSpec *s, bool initial, bool from_trigger_no ; } + if (good) { + if (!trigger) { + trigger = strdup(s->path); + if (!trigger) + (void) log_oom_debug(); + } + *ret_trigger_path = TAKE_PTR(trigger); + } + return good; } @@ -494,9 +507,11 @@ static void path_enter_dead(Path *p, PathResult f) { path_set_state(p, p->result != PATH_SUCCESS ? PATH_FAILED : PATH_DEAD); } -static void path_enter_running(Path *p) { +static void path_enter_running(Path *p, char *trigger_path) { + _cleanup_(activation_details_unrefp) ActivationDetails *details = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; Unit *trigger; + Job *job; int r; assert(p); @@ -518,10 +533,22 @@ static void path_enter_running(Path *p) { return; } - r = manager_add_job(UNIT(p)->manager, JOB_START, trigger, JOB_REPLACE, NULL, &error, NULL); + details = activation_details_new(UNIT(p)); + if (!details) { + r = -ENOMEM; + goto fail; + } + + r = free_and_strdup(&(ACTIVATION_DETAILS_PATH(details))->trigger_path_filename, trigger_path); if (r < 0) goto fail; + r = manager_add_job(UNIT(p)->manager, JOB_START, trigger, JOB_REPLACE, NULL, &error, &job); + if (r < 0) + goto fail; + + job_set_activation_details(job, details); + path_set_state(p, PATH_RUNNING); path_unwatch(p); @@ -532,17 +559,19 @@ fail: path_enter_dead(p, PATH_FAILURE_RESOURCES); } -static bool path_check_good(Path *p, bool initial, bool from_trigger_notify) { +static bool path_check_good(Path *p, bool initial, bool from_trigger_notify, char **ret_trigger_path) { assert(p); + assert(ret_trigger_path); LIST_FOREACH(spec, s, p->specs) - if (path_spec_check_good(s, initial, from_trigger_notify)) + if (path_spec_check_good(s, initial, from_trigger_notify, ret_trigger_path)) return true; return false; } static void path_enter_waiting(Path *p, bool initial, bool from_trigger_notify) { + _cleanup_free_ char *trigger_path = NULL; Unit *trigger; int r; @@ -554,9 +583,9 @@ static void path_enter_waiting(Path *p, bool initial, bool from_trigger_notify) return; } - if (path_check_good(p, initial, from_trigger_notify)) { + if (path_check_good(p, initial, from_trigger_notify, &trigger_path)) { log_unit_debug(UNIT(p), "Got triggered."); - path_enter_running(p); + path_enter_running(p, trigger_path); return; } @@ -568,9 +597,9 @@ static void path_enter_waiting(Path *p, bool initial, bool from_trigger_notify) * might have appeared/been removed by now, so we must * recheck */ - if (path_check_good(p, false, from_trigger_notify)) { + if (path_check_good(p, false, from_trigger_notify, &trigger_path)) { log_unit_debug(UNIT(p), "Got triggered."); - path_enter_running(p); + path_enter_running(p, trigger_path); return; } @@ -759,7 +788,7 @@ static int path_dispatch_io(sd_event_source *source, int fd, uint32_t revents, v goto fail; if (changed) - path_enter_running(p); + path_enter_running(p, found->path); else path_enter_waiting(p, false, false); @@ -832,6 +861,89 @@ static int path_can_start(Unit *u) { return 1; } +static void activation_details_path_done(ActivationDetails *details) { + ActivationDetailsPath *p = ASSERT_PTR(ACTIVATION_DETAILS_PATH(details)); + + p->trigger_path_filename = mfree(p->trigger_path_filename); +} + +static void activation_details_path_serialize(ActivationDetails *details, FILE *f) { + ActivationDetailsPath *p = ASSERT_PTR(ACTIVATION_DETAILS_PATH(details)); + + assert(f); + + if (p->trigger_path_filename) + (void) serialize_item(f, "activation-details-path-filename", p->trigger_path_filename); +} + +static int activation_details_path_deserialize(const char *key, const char *value, ActivationDetails **details) { + int r; + + assert(key); + assert(value); + + if (!details || !*details) + return -EINVAL; + + ActivationDetailsPath *p = ACTIVATION_DETAILS_PATH(*details); + if (!p) + return -EINVAL; + + if (!streq(key, "activation-details-path-filename")) + return -EINVAL; + + r = free_and_strdup(&p->trigger_path_filename, value); + if (r < 0) + return r; + + return 0; +} + +static int activation_details_path_append_env(ActivationDetails *details, char ***strv) { + ActivationDetailsPath *p = ACTIVATION_DETAILS_PATH(details); + char *s; + int r; + + assert(details); + assert(strv); + assert(p); + + if (isempty(p->trigger_path_filename)) + return 0; + + s = strjoin("TRIGGER_PATH=", p->trigger_path_filename); + if (!s) + return -ENOMEM; + + r = strv_consume(strv, TAKE_PTR(s)); + if (r < 0) + return r; + + return 1; /* Return the number of variables added to the env block */ +} + +static int activation_details_path_append_pair(ActivationDetails *details, char ***strv) { + ActivationDetailsPath *p = ACTIVATION_DETAILS_PATH(details); + int r; + + assert(details); + assert(strv); + assert(p); + + if (isempty(p->trigger_path_filename)) + return 0; + + r = strv_extend(strv, "trigger_path"); + if (r < 0) + return r; + + r = strv_extend(strv, p->trigger_path_filename); + if (r < 0) + return r; + + return 1; /* Return the number of pairs added to the env block */ +} + static const char* const path_type_table[_PATH_TYPE_MAX] = { [PATH_EXISTS] = "PathExists", [PATH_EXISTS_GLOB] = "PathExistsGlob", @@ -890,3 +1002,13 @@ const UnitVTable path_vtable = { .can_start = path_can_start, }; + +const ActivationDetailsVTable activation_details_path_vtable = { + .object_size = sizeof(ActivationDetailsPath), + + .done = activation_details_path_done, + .serialize = activation_details_path_serialize, + .deserialize = activation_details_path_deserialize, + .append_env = activation_details_path_append_env, + .append_pair = activation_details_path_append_pair, +}; diff --git a/src/core/path.h b/src/core/path.h index d835c24166..c76103cc12 100644 --- a/src/core/path.h +++ b/src/core/path.h @@ -3,6 +3,7 @@ typedef struct Path Path; typedef struct PathSpec PathSpec; +typedef struct ActivationDetailsPath ActivationDetailsPath; #include "unit.h" @@ -66,9 +67,15 @@ struct Path { RateLimit trigger_limit; }; +struct ActivationDetailsPath { + ActivationDetails meta; + char *trigger_path_filename; +}; + void path_free_specs(Path *p); extern const UnitVTable path_vtable; +extern const ActivationDetailsVTable activation_details_path_vtable; const char* path_type_to_string(PathType i) _const_; PathType path_type_from_string(const char *s) _pure_; @@ -77,3 +84,4 @@ const char* path_result_to_string(PathResult i) _const_; PathResult path_result_from_string(const char *s) _pure_; DEFINE_CAST(PATH, Path); +DEFINE_ACTIVATION_DETAILS_CAST(ACTIVATION_DETAILS_PATH, ActivationDetailsPath, PATH); diff --git a/src/core/unit.c b/src/core/unit.c index 18d9ba85de..4255e0cf1f 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -5930,6 +5930,7 @@ int unit_get_dependency_array(const Unit *u, UnitDependencyAtom atom, Unit ***re } const ActivationDetailsVTable * const activation_details_vtable[_UNIT_TYPE_MAX] = { + [UNIT_PATH] = &activation_details_path_vtable, }; ActivationDetails *activation_details_new(Unit *trigger_unit) { diff --git a/test/testsuite-63.units/test63-glob.path b/test/testsuite-63.units/test63-glob.path new file mode 100644 index 0000000000..5f237a9fcf --- /dev/null +++ b/test/testsuite-63.units/test63-glob.path @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Path] +PathExistsGlob=/tmp/test63-glob* diff --git a/test/testsuite-63.units/test63-glob.service b/test/testsuite-63.units/test63-glob.service new file mode 100644 index 0000000000..3f49dd4620 --- /dev/null +++ b/test/testsuite-63.units/test63-glob.service @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Service] +ExecStartPre=sh -c 'test "$TRIGGER_PATH" = /tmp/test63-glob-foo' +ExecStartPre=sh -c 'test "$TRIGGER_UNIT" = test63-glob.path' +ExecStart=systemd-notify --ready +RemainAfterExit=yes +Type=notify diff --git a/test/testsuite-63.units/test63.service b/test/testsuite-63.units/test63.service index 1a8721d82c..01a928b8d6 100644 --- a/test/testsuite-63.units/test63.service +++ b/test/testsuite-63.units/test63.service @@ -3,4 +3,6 @@ ConditionPathExists=/tmp/nonexistent [Service] +ExecStartPre=sh -c 'test "$TRIGGER_PATH" = /tmp/test63' +ExecStartPre=sh -c 'test "$TRIGGER_UNIT" = test63.path' ExecStart=true diff --git a/test/units/testsuite-63.sh b/test/units/testsuite-63.sh index 20d93936b3..7ee7fc1513 100755 --- a/test/units/testsuite-63.sh +++ b/test/units/testsuite-63.sh @@ -28,6 +28,19 @@ test "$(systemctl show test63.service -P Result)" = success test "$(systemctl show test63.path -P ActiveState)" = active test "$(systemctl show test63.path -P Result)" = success +# Test that glob matching works too, with $TRIGGER_PATH +systemctl start test63-glob.path +touch /tmp/test63-glob-foo +timeout 60 bash -c 'while ! systemctl -q is-active test63-glob.service; do sleep .2; done' +test "$(systemctl show test63-glob.service -P ActiveState)" = active +test "$(systemctl show test63-glob.service -P Result)" = success + +test "$(busctl --json=short get-property org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/test63_2dglob_2eservice org.freedesktop.systemd1.Unit ActivationDetails)" = '{"type":"a(ss)","data":[["trigger_unit","test63-glob.path"],["trigger_path","/tmp/test63-glob-foo"]]}' + +systemctl stop test63-glob.path test63-glob.service + +test "$(busctl --json=short get-property org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/test63_2dglob_2eservice org.freedesktop.systemd1.Unit ActivationDetails)" = '{"type":"a(ss)","data":[]}' + systemctl log-level info echo OK >/testok From c8bc7519c888a99134f88f8c82353246d3c0cc5d Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Tue, 2 Aug 2022 20:15:12 +0100 Subject: [PATCH 5/5] service: set TRIGGER_UNIT= and TRIGGER_TIMER_REALTIME_USEC/MONOTONIC_USEC on activation by timer unit Same as path unit, best effort. --- man/systemd.exec.xml | 4 +- man/systemd.timer.xml | 4 ++ src/core/timer.c | 107 ++++++++++++++++++++++++++++++++++++- src/core/timer.h | 8 +++ src/core/unit.c | 1 + test/units/testsuite-07.sh | 3 ++ 6 files changed, 125 insertions(+), 2 deletions(-) diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index 2acb737c38..2e843be2e5 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -3698,8 +3698,10 @@ StandardInputData=V2XigLJyZSBubyBzdHJhbmdlcnMgdG8gbG92ZQpZb3Uga25vdyB0aGUgcnVsZX $TRIGGER_UNIT $TRIGGER_PATH + $TRIGGER_TIMER_REALTIME_USEC + $TRIGGER_TIMER_MONOTONIC_USEC - If the unit was activated dynamically (e.g.: a corresponding path unit), the + If the unit was activated dynamically (e.g.: a corresponding path unit or timer unit), the unit that triggered it and other type-dependent information will be passed via these variables. Note that this information is provided in a best-effort way. For example, multiple triggers happening one after another will be coalesced and only one will be reported, with no guarantee as to which one it will be. diff --git a/man/systemd.timer.xml b/man/systemd.timer.xml index 49bcb18be5..953faa9b33 100644 --- a/man/systemd.timer.xml +++ b/man/systemd.timer.xml @@ -366,6 +366,10 @@ See Also + Environment variables with details on the trigger will be set for triggered units. See the + Environment Variables Set on Triggered Units section in + systemd.exec1 + for more details. systemd1, systemctl1, diff --git a/src/core/timer.c b/src/core/timer.c index 9de325ba66..b89d593b75 100644 --- a/src/core/timer.c +++ b/src/core/timer.c @@ -576,8 +576,10 @@ fail: } static void timer_enter_running(Timer *t) { + _cleanup_(activation_details_unrefp) ActivationDetails *details = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; Unit *trigger; + Job *job; int r; assert(t); @@ -593,11 +595,20 @@ static void timer_enter_running(Timer *t) { return; } - r = manager_add_job(UNIT(t)->manager, JOB_START, trigger, JOB_REPLACE, NULL, &error, NULL); + details = activation_details_new(UNIT(t)); + if (!details) { + r = -ENOMEM; + goto fail; + } + + r = manager_add_job(UNIT(t)->manager, JOB_START, trigger, JOB_REPLACE, NULL, &error, &job); if (r < 0) goto fail; dual_timestamp_get(&t->last_trigger); + ACTIVATION_DETAILS_TIMER(details)->last_trigger = t->last_trigger; + + job_set_activation_details(job, details); if (t->stamp_path) touch_file(t->stamp_path, true, t->last_trigger.realtime, UID_INVALID, GID_INVALID, MODE_INVALID); @@ -892,6 +903,91 @@ static int timer_can_start(Unit *u) { return 1; } +static void activation_details_timer_serialize(ActivationDetails *details, FILE *f) { + ActivationDetailsTimer *t = ACTIVATION_DETAILS_TIMER(details); + + assert(details); + assert(f); + assert(t); + + (void) serialize_dual_timestamp(f, "activation-details-timer-last-trigger", &t->last_trigger); +} + +static int activation_details_timer_deserialize(const char *key, const char *value, ActivationDetails **details) { + int r; + + assert(key); + assert(value); + + if (!details || !*details) + return -EINVAL; + + ActivationDetailsTimer *t = ACTIVATION_DETAILS_TIMER(*details); + if (!t) + return -EINVAL; + + if (!streq(key, "activation-details-timer-last-trigger")) + return -EINVAL; + + r = deserialize_dual_timestamp(value, &t->last_trigger); + if (r < 0) + return r; + + return 0; +} + +static int activation_details_timer_append_env(ActivationDetails *details, char ***strv) { + ActivationDetailsTimer *t = ACTIVATION_DETAILS_TIMER(details); + int r; + + assert(details); + assert(strv); + assert(t); + + if (!dual_timestamp_is_set(&t->last_trigger)) + return 0; + + r = strv_extendf(strv, "TRIGGER_TIMER_REALTIME_USEC=%" USEC_FMT, t->last_trigger.realtime); + if (r < 0) + return r; + + r = strv_extendf(strv, "TRIGGER_TIMER_MONOTONIC_USEC=%" USEC_FMT, t->last_trigger.monotonic); + if (r < 0) + return r; + + return 2; /* Return the number of variables added to the env block */ +} + +static int activation_details_timer_append_pair(ActivationDetails *details, char ***strv) { + ActivationDetailsTimer *t = ACTIVATION_DETAILS_TIMER(details); + int r; + + assert(details); + assert(strv); + assert(t); + + if (!dual_timestamp_is_set(&t->last_trigger)) + return 0; + + r = strv_extend(strv, "trigger_timer_realtime_usec"); + if (r < 0) + return r; + + r = strv_extendf(strv, "%" USEC_FMT, t->last_trigger.realtime); + if (r < 0) + return r; + + r = strv_extend(strv, "trigger_timer_monotonic_usec"); + if (r < 0) + return r; + + r = strv_extendf(strv, "%" USEC_FMT, t->last_trigger.monotonic); + if (r < 0) + return r; + + return 2; /* Return the number of pairs added to the env block */ +} + static const char* const timer_base_table[_TIMER_BASE_MAX] = { [TIMER_ACTIVE] = "OnActiveSec", [TIMER_BOOT] = "OnBootSec", @@ -954,3 +1050,12 @@ const UnitVTable timer_vtable = { .can_start = timer_can_start, }; + +const ActivationDetailsVTable activation_details_timer_vtable = { + .object_size = sizeof(ActivationDetailsTimer), + + .serialize = activation_details_timer_serialize, + .deserialize = activation_details_timer_deserialize, + .append_env = activation_details_timer_append_env, + .append_pair = activation_details_timer_append_pair, +}; diff --git a/src/core/timer.h b/src/core/timer.h index 551e283341..914e42580e 100644 --- a/src/core/timer.h +++ b/src/core/timer.h @@ -2,6 +2,7 @@ #pragma once typedef struct Timer Timer; +typedef struct ActivationDetailsTimer ActivationDetailsTimer; #include "calendarspec.h" #include "unit.h" @@ -64,11 +65,17 @@ struct Timer { char *stamp_path; }; +struct ActivationDetailsTimer { + ActivationDetails meta; + dual_timestamp last_trigger; +}; + #define TIMER_MONOTONIC_CLOCK(t) ((t)->wake_system ? CLOCK_BOOTTIME_ALARM : CLOCK_MONOTONIC) void timer_free_values(Timer *t); extern const UnitVTable timer_vtable; +extern const ActivationDetailsVTable activation_details_timer_vtable; const char *timer_base_to_string(TimerBase i) _const_; TimerBase timer_base_from_string(const char *s) _pure_; @@ -77,3 +84,4 @@ const char* timer_result_to_string(TimerResult i) _const_; TimerResult timer_result_from_string(const char *s) _pure_; DEFINE_CAST(TIMER, Timer); +DEFINE_ACTIVATION_DETAILS_CAST(ACTIVATION_DETAILS_TIMER, ActivationDetailsTimer, TIMER); diff --git a/src/core/unit.c b/src/core/unit.c index 4255e0cf1f..5f1c6109b0 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -5931,6 +5931,7 @@ int unit_get_dependency_array(const Unit *u, UnitDependencyAtom atom, Unit ***re const ActivationDetailsVTable * const activation_details_vtable[_UNIT_TYPE_MAX] = { [UNIT_PATH] = &activation_details_path_vtable, + [UNIT_TIMER] = &activation_details_timer_vtable, }; ActivationDetails *activation_details_new(Unit *trigger_unit) { diff --git a/test/units/testsuite-07.sh b/test/units/testsuite-07.sh index 5e9fe64ea9..95ebe3876f 100755 --- a/test/units/testsuite-07.sh +++ b/test/units/testsuite-07.sh @@ -8,6 +8,9 @@ set -o pipefail cat >/lib/systemd/system/my.service <