diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 3c66d698932..0d4ee6e40e2 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -127,8 +127,10 @@ static int property_get_tainted( assert(bus); assert(reply); + assert(userdata); - _cleanup_free_ char *s = taint_string(); + Manager *m = userdata; + _cleanup_free_ char *s = taint_string(m); if (!s) return log_oom(); diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 03f25402a28..730ab3c20e2 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -939,6 +939,7 @@ const sd_bus_vtable bus_unit_vtable[] = { 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_PROPERTY("DebugInvocation", "b", bus_property_get_bool, offsetof(Unit, debug_invocation), 0), + SD_BUS_PROPERTY("WasOnDependencyCycle", "b", NULL, offsetof(Unit, was_on_dependency_cycle), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_METHOD_WITH_ARGS("Start", SD_BUS_ARGS("s", mode), diff --git a/src/core/manager.c b/src/core/manager.c index e75c760b6fd..e044abff037 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3853,7 +3853,7 @@ static void log_taint_string(Manager *m) { m->taint_logged = true; /* only check for taint once */ - _cleanup_free_ char *taint = taint_string(); + _cleanup_free_ char *taint = taint_string(m); if (isempty(taint)) return; @@ -5298,6 +5298,16 @@ int manager_allocate_idle_pipe(Manager *m) { return 1; } +bool manager_was_dependency_cycle(const Manager *m) { + const Unit *u; + + HASHMAP_FOREACH(u, m->units) + if (u->was_on_dependency_cycle) + return true; + + return false; +} + void unit_defaults_init(UnitDefaults *defaults, RuntimeScope scope) { assert(defaults); assert(scope >= 0); diff --git a/src/core/manager.h b/src/core/manager.h index e4cada80ff3..da7709154df 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -679,6 +679,8 @@ OOMPolicy oom_policy_from_string(const char *s) _pure_; void unit_defaults_init(UnitDefaults *defaults, RuntimeScope scope); void unit_defaults_done(UnitDefaults *defaults); +bool manager_was_dependency_cycle(const Manager *m); + enum { /* most important … */ EVENT_PRIORITY_USER_LOOKUP = SD_EVENT_PRIORITY_NORMAL-12, diff --git a/src/core/taint.c b/src/core/taint.c index b7a1c647a2b..65d8ab9e413 100644 --- a/src/core/taint.c +++ b/src/core/taint.c @@ -9,6 +9,7 @@ #include "fileio.h" #include "fs-util.h" #include "log.h" +#include "manager.h" #include "os-util.h" #include "path-util.h" #include "strv.h" @@ -31,8 +32,8 @@ static int short_uid_gid_range(UIDRangeUsernsMode mode) { return !uid_range_covers(p, 0, 65535); } -char** taint_strv(void) { - const char *stage[12] = {}; +char** taint_strv(const Manager *m) { + const char *stage[13] = {}; size_t n = 0; /* Returns a "taint string", e.g. "local-hwclock:var-run-bad". Only things that are detected at @@ -78,16 +79,18 @@ char** taint_strv(void) { stage[n++] = "short-uid-range"; if (short_uid_gid_range(GID_RANGE_USERNS_INSIDE) > 0) stage[n++] = "short-gid-range"; + if (manager_was_dependency_cycle(m)) + stage[n++] = "dependency-cycle"; assert(n < ELEMENTSOF(stage) - 1); /* One extra for NULL terminator */ return strv_copy((char *const *) stage); } -char* taint_string(void) { +char* taint_string(const Manager *m) { _cleanup_strv_free_ char **taints = NULL; - taints = taint_strv(); + taints = taint_strv(m); if (!taints) return NULL; diff --git a/src/core/taint.h b/src/core/taint.h index fb47156511c..eb0305c0482 100644 --- a/src/core/taint.h +++ b/src/core/taint.h @@ -1,5 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once -char** taint_strv(void); -char* taint_string(void); +typedef struct Manager Manager; + +char** taint_strv(const Manager *m); +char* taint_string(const Manager *m); diff --git a/src/core/transaction.c b/src/core/transaction.c index 377ea05983f..2177e65d878 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -366,8 +366,12 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi /* Have we seen this before? */ if (j->generation == generation) { - Job *k, *delete = NULL; + Job *k, *prev = NULL, *delete = NULL; _cleanup_free_ char **array = NULL, *unit_ids = NULL; + _cleanup_free_ char *dot = NULL; + + if (!strextend(&dot, "digraph {")) + log_oom(); /* If the marker is NULL we have been here already and decided the job was loop-free from * here. Hence shortcut things and return right-away. */ @@ -382,6 +386,18 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi /* For logging below */ if (strv_push_pair(&array, k->unit->id, (char*) job_type_to_string(k->type)) < 0) log_oom(); + /* Add edge to the graph when we already have part of it. */ + else if (dot) { + r = strextendf(&dot, "\"%s\" -> \"%s\";", k->unit->id, prev ? prev->unit->id : j->unit->id); + if (r < 0) { + log_oom(); + /* Dot output is not essential so let's get rid of it in case we ran into OOM. */ + dot = mfree(dot); + } + } + + /* Mark every unit along the cycle */ + k->unit->was_on_dependency_cycle = true; if (!delete && hashmap_contains(tr->jobs, k->unit) && !job_matters_to_anchor(k)) /* Ok, we can drop this one, so let's do so. */ @@ -390,8 +406,13 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi /* Check if this in fact was the beginning of the cycle */ if (k == j) break; + + prev = k; } + if (dot && !strextend(&dot, "}")) + log_oom(); + unit_ids = merge_unit_ids(j->manager->unit_log_field, array); /* ignore error */ STRV_FOREACH_PAIR(unit_id, job_type, array) @@ -403,6 +424,12 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi *unit_id, *job_type), "%s", strna(unit_ids)); + if (dot && IN_SET(log_get_target(), LOG_TARGET_AUTO, LOG_TARGET_JOURNAL, LOG_TARGET_JOURNAL_OR_KMSG)) { + log_struct(LOG_WARNING, + LOG_UNIT_MESSAGE(j->unit, "Ordering cycle generated, see TRANSACTION_CYCLE= for details. Note that each arc represents Before= dependency."), + "TRANSACTION_CYCLE=%s", dot); + } + if (delete) { const char *status; /* logging for j not k here to provide a consistent narrative */ diff --git a/src/core/unit.c b/src/core/unit.c index 0d88f4f6416..dbd6345d186 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3834,6 +3834,7 @@ void unit_reset_failed(Unit *u) { ratelimit_reset(&u->start_ratelimit); u->start_limit_hit = false; u->debug_invocation = false; + u->was_on_dependency_cycle = false; } Unit *unit_following(Unit *u) { diff --git a/src/core/unit.h b/src/core/unit.h index 8a3b812a4bf..85ecb70a0b4 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -464,6 +464,9 @@ typedef struct Unit { /* When writing transient unit files, stores which section we stored last. If < 0, we didn't write any yet. If * == 0 we are in the [Unit] section, if > 0 we are in the unit type-specific section. */ signed int last_section_private:2; + + /* Remember this unit was part of some dependency cycle. */ + bool was_on_dependency_cycle; } Unit; typedef struct UnitStatusMessageFormats { diff --git a/src/systemctl/systemctl-show.c b/src/systemctl/systemctl-show.c index fe35553e30c..c144db4e150 100644 --- a/src/systemctl/systemctl-show.c +++ b/src/systemctl/systemctl-show.c @@ -168,6 +168,7 @@ typedef struct UnitStatusInfo { const char *active_state; const char *freezer_state; const char *sub_state; + bool was_on_dependency_cycle; const char *unit_file_state; const char *unit_file_preset; @@ -374,12 +375,13 @@ static void print_status_info( bool show_preset = !isempty(i->unit_file_preset) && show_preset_for_state(unit_file_state_from_string(i->unit_file_state)); - printf(" Loaded: %s%s%s (%s; %s%s%s%s%s%s%s)\n", + printf(" Loaded: %s%s%s (%s; %s%s%s%s%s%s%s%s%s%s%s)\n", on, strna(i->load_state), off, path, enable_on, i->unit_file_state, enable_off, show_preset ? "; preset: " : "", - preset_on, show_preset ? i->unit_file_preset : "", preset_off); + preset_on, show_preset ? i->unit_file_preset : "", preset_off, + i->was_on_dependency_cycle ? ", " : "", ansi_highlight_red(), i->was_on_dependency_cycle ? "dependency-cycle" : "", ansi_normal()); } else if (path) printf(" Loaded: %s%s%s (%s)\n", @@ -2078,6 +2080,7 @@ static int show_one( { "ActiveState", "s", NULL, offsetof(UnitStatusInfo, active_state) }, { "FreezerState", "s", NULL, offsetof(UnitStatusInfo, freezer_state) }, { "SubState", "s", NULL, offsetof(UnitStatusInfo, sub_state) }, + { "WasOnDependencyCycle", "b", NULL, offsetof(UnitStatusInfo, was_on_dependency_cycle) }, { "Job", "(uo)", bus_map_job_id, offsetof(UnitStatusInfo, job_id) }, { "UnitFileState", "s", NULL, offsetof(UnitStatusInfo, unit_file_state) }, { "UnitFilePreset", "s", NULL, offsetof(UnitStatusInfo, unit_file_preset) }, diff --git a/src/test/test-taint.c b/src/test/test-taint.c index d9aa79dca57..a5d979af05f 100644 --- a/src/test/test-taint.c +++ b/src/test/test-taint.c @@ -1,10 +1,12 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include "manager.h" #include "taint.h" #include "tests.h" TEST(taint_string) { - _cleanup_free_ char *a = taint_string(); + Manager m = {}; + _cleanup_free_ char *a = taint_string(&m); assert_se(a); log_debug("taint string: '%s'", a);