Merge d11e70d92f into 3cbf00a30c

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();
 

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),

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);

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,

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) {
+char** taint_strv(const Manager *m) {
-        const char *stage[12] = {};
+        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;
 

src/core/taint.h

@@ -1,5 +1,7 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 #pragma once
 
-char** taint_strv(void);
+typedef struct Manager Manager;
-char* taint_string(void);
+
+char** taint_strv(const Manager *m);
+char* taint_string(const Manager *m);

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 */

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) {

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 {

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)                  },

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);