From: Mike Yuan Date: Mon, 3 Jul 2023 23:28:33 +0000 (+0800) Subject: core: introduce UNIT_ATOM_PROPAGATE_STOP_GRACEFUL for PropagatesStopTo= X-Git-Tag: v254-rc1~32^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=48cb073db81fa73f64bc5aa9a1b81ebf627235fa;p=thirdparty%2Fsystemd.git core: introduce UNIT_ATOM_PROPAGATE_STOP_GRACEFUL for PropagatesStopTo= Follow-up for 017a7ba4f406adcf69d6b3ec15b9f2d9ed5ad853 Before this commit, when a unit that is restarting propagates stop to other units, it can also depend on them, which results in job type conflict and thus failure to pull in the dependencies. So, let's introduce a new dependency atom UNIT_ATOM_PROPAGATE_STOP_GRACEFUL, and use it for PropagatesStopTo=. It will enqueue a restart job if there's already a start job, which meets the ultimate goal and avoids job type conflict. Fixes #26839 --- diff --git a/src/core/transaction.c b/src/core/transaction.c index ba8f19bca17..aedd24949ed 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -1055,21 +1055,15 @@ int transaction_add_job_and_dependencies( } } - _cleanup_set_free_ Set *propagated_restart = NULL; + if (IN_SET(type, JOB_RESTART, JOB_STOP) || (type == JOB_START && FLAGS_SET(flags, TRANSACTION_PROPAGATE_START_AS_RESTART))) { + bool is_stop = type == JOB_STOP; - if (type == JOB_RESTART || (type == JOB_START && FLAGS_SET(flags, TRANSACTION_PROPAGATE_START_AS_RESTART))) { - - /* We propagate RESTART only as TRY_RESTART, in order not to start dependencies that - * are not around. */ - - UNIT_FOREACH_DEPENDENCY(dep, ret->unit, UNIT_ATOM_PROPAGATE_RESTART) { + UNIT_FOREACH_DEPENDENCY(dep, ret->unit, is_stop ? UNIT_ATOM_PROPAGATE_STOP : UNIT_ATOM_PROPAGATE_RESTART) { + /* We propagate RESTART only as TRY_RESTART, in order not to start dependencies that + * are not around. */ JobType nt; - r = set_ensure_put(&propagated_restart, NULL, dep); - if (r < 0) - return r; - - nt = job_type_collapse(JOB_TRY_RESTART, dep); + nt = job_type_collapse(is_stop ? JOB_STOP : JOB_TRY_RESTART, dep); if (nt == JOB_NOP) continue; @@ -1081,24 +1075,49 @@ int transaction_add_job_and_dependencies( sd_bus_error_free(e); } } - } - if (type == JOB_STOP) { - /* The 'stop' part of a restart job is also propagated to units with UNIT_ATOM_PROPAGATE_STOP. */ - - UNIT_FOREACH_DEPENDENCY(dep, ret->unit, UNIT_ATOM_PROPAGATE_STOP) { - /* Units experienced restart propagation are skipped */ - if (set_contains(propagated_restart, dep)) - continue; + /* Process UNIT_ATOM_PROPAGATE_STOP_GRACEFUL (PropagatesStopTo=) units. We need to wait until + * all other dependencies are processed, i.e. we're the anchor job or already in the recursion + * that handles it. */ + if (!by || FLAGS_SET(flags, TRANSACTION_PROCESS_PROPAGATE_STOP_GRACEFUL)) + UNIT_FOREACH_DEPENDENCY(dep, ret->unit, UNIT_ATOM_PROPAGATE_STOP_GRACEFUL) { + JobType nt = JOB_STOP; + Job *j; + + j = hashmap_get(tr->jobs, dep); + if (j) + LIST_FOREACH(transaction, i, j) + switch (i->type) { + + case JOB_STOP: + case JOB_RESTART: + /* Nothing to worry about, an appropriate job is in-place */ + nt = JOB_NOP; + break; + + case JOB_START: + /* This unit is pulled in by other dependency types in + * this transaction. We will run into job type conflict + * if we enqueue a stop job, so let's enqueue a restart + * job instead. */ + nt = JOB_RESTART; + + default: /* We don't care about others */ + ; + + } + + if (nt == JOB_NOP) + continue; - r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, ret, TRANSACTION_MATTERS | (flags & TRANSACTION_IGNORE_ORDER), e); - if (r < 0) { - if (r != -EBADR) /* job type not applicable */ - return r; + r = transaction_add_job_and_dependencies(tr, nt, dep, ret, TRANSACTION_MATTERS | (flags & TRANSACTION_IGNORE_ORDER) | TRANSACTION_PROCESS_PROPAGATE_STOP_GRACEFUL, e); + if (r < 0) { + if (r != -EBADR) /* job type not applicable */ + return r; - sd_bus_error_free(e); + sd_bus_error_free(e); + } } - } } if (type == JOB_RELOAD) diff --git a/src/core/transaction.h b/src/core/transaction.h index db2d0566499..151e02dd605 100644 --- a/src/core/transaction.h +++ b/src/core/transaction.h @@ -21,13 +21,16 @@ Transaction *transaction_abort_and_free(Transaction *tr); DEFINE_TRIVIAL_CLEANUP_FUNC(Transaction*, transaction_abort_and_free); typedef enum TransactionAddFlags { - TRANSACTION_MATTERS = 1 << 0, - TRANSACTION_CONFLICTS = 1 << 1, - TRANSACTION_IGNORE_REQUIREMENTS = 1 << 2, - TRANSACTION_IGNORE_ORDER = 1 << 3, + TRANSACTION_MATTERS = 1 << 0, + TRANSACTION_CONFLICTS = 1 << 1, + TRANSACTION_IGNORE_REQUIREMENTS = 1 << 2, + TRANSACTION_IGNORE_ORDER = 1 << 3, /* Propagate a START job to other units like a RESTART */ - TRANSACTION_PROPAGATE_START_AS_RESTART = 1 << 4, + TRANSACTION_PROPAGATE_START_AS_RESTART = 1 << 4, + + /* Indicate that we're in the recursion for processing UNIT_ATOM_PROPAGATE_STOP_GRACEFUL units */ + TRANSACTION_PROCESS_PROPAGATE_STOP_GRACEFUL = 1 << 5, } TransactionAddFlags; void transaction_add_propagate_reload_jobs( diff --git a/src/core/unit-dependency-atom.c b/src/core/unit-dependency-atom.c index 81333523e73..35b279b5c74 100644 --- a/src/core/unit-dependency-atom.c +++ b/src/core/unit-dependency-atom.c @@ -80,7 +80,7 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = { UNIT_ATOM_PROPAGATE_STOP_FAILURE, [UNIT_PROPAGATES_STOP_TO] = UNIT_ATOM_RETROACTIVE_STOP_ON_STOP | - UNIT_ATOM_PROPAGATE_STOP, + UNIT_ATOM_PROPAGATE_STOP_GRACEFUL, /* These are simple dependency types: they consist of a single atom only */ [UNIT_ON_FAILURE] = UNIT_ATOM_ON_FAILURE, @@ -196,6 +196,11 @@ UnitDependency unit_dependency_from_unique_atom(UnitDependencyAtom atom) { case UNIT_ATOM_PROPAGATE_STOP_FAILURE: return UNIT_CONFLICTED_BY; + case UNIT_ATOM_RETROACTIVE_STOP_ON_STOP | + UNIT_ATOM_PROPAGATE_STOP_GRACEFUL: + case UNIT_ATOM_PROPAGATE_STOP_GRACEFUL: + return UNIT_PROPAGATES_STOP_TO; + /* And now, the simple ones */ case UNIT_ATOM_ON_FAILURE: diff --git a/src/core/unit-dependency-atom.h b/src/core/unit-dependency-atom.h index 02532e57d6a..96f00ca39a8 100644 --- a/src/core/unit-dependency-atom.h +++ b/src/core/unit-dependency-atom.h @@ -58,30 +58,33 @@ typedef enum UnitDependencyAtom { UNIT_ATOM_PROPAGATE_INACTIVE_START_AS_FAILURE = UINT64_C(1) << 17, /* When putting together a transaction, propagate JOB_STOP from our unit to the other. */ UNIT_ATOM_PROPAGATE_STOP = UINT64_C(1) << 18, + /* Like UNIT_ATOM_PROPAGATE_STOP, but enqueues a restart job if there's already a start job (avoids + * job type conflict). */ + UNIT_ATOM_PROPAGATE_STOP_GRACEFUL = UINT64_C(1) << 19, /* When putting together a transaction, propagate JOB_RESTART from our unit to the other. */ - UNIT_ATOM_PROPAGATE_RESTART = UINT64_C(1) << 19, + UNIT_ATOM_PROPAGATE_RESTART = UINT64_C(1) << 20, /* Add the other unit to the default target dependency queue */ - UNIT_ATOM_ADD_DEFAULT_TARGET_DEPENDENCY_QUEUE = UINT64_C(1) << 20, + UNIT_ATOM_ADD_DEFAULT_TARGET_DEPENDENCY_QUEUE = UINT64_C(1) << 21, /* Recheck default target deps on other units (which are target units) */ - UNIT_ATOM_DEFAULT_TARGET_DEPENDENCIES = UINT64_C(1) << 21, + UNIT_ATOM_DEFAULT_TARGET_DEPENDENCIES = UINT64_C(1) << 22, /* The remaining atoms map 1:1 to the equally named high-level deps */ - UNIT_ATOM_ON_FAILURE = UINT64_C(1) << 22, - UNIT_ATOM_ON_SUCCESS = UINT64_C(1) << 23, - UNIT_ATOM_ON_FAILURE_OF = UINT64_C(1) << 24, - UNIT_ATOM_ON_SUCCESS_OF = UINT64_C(1) << 25, - UNIT_ATOM_BEFORE = UINT64_C(1) << 26, - UNIT_ATOM_AFTER = UINT64_C(1) << 27, - UNIT_ATOM_TRIGGERS = UINT64_C(1) << 28, - UNIT_ATOM_TRIGGERED_BY = UINT64_C(1) << 29, - UNIT_ATOM_PROPAGATES_RELOAD_TO = UINT64_C(1) << 30, - UNIT_ATOM_JOINS_NAMESPACE_OF = UINT64_C(1) << 31, - UNIT_ATOM_REFERENCES = UINT64_C(1) << 32, - UNIT_ATOM_REFERENCED_BY = UINT64_C(1) << 33, - UNIT_ATOM_IN_SLICE = UINT64_C(1) << 34, - UNIT_ATOM_SLICE_OF = UINT64_C(1) << 35, - _UNIT_DEPENDENCY_ATOM_MAX = (UINT64_C(1) << 36) - 1, + UNIT_ATOM_ON_FAILURE = UINT64_C(1) << 23, + UNIT_ATOM_ON_SUCCESS = UINT64_C(1) << 24, + UNIT_ATOM_ON_FAILURE_OF = UINT64_C(1) << 25, + UNIT_ATOM_ON_SUCCESS_OF = UINT64_C(1) << 26, + UNIT_ATOM_BEFORE = UINT64_C(1) << 27, + UNIT_ATOM_AFTER = UINT64_C(1) << 28, + UNIT_ATOM_TRIGGERS = UINT64_C(1) << 29, + UNIT_ATOM_TRIGGERED_BY = UINT64_C(1) << 30, + UNIT_ATOM_PROPAGATES_RELOAD_TO = UINT64_C(1) << 31, + UNIT_ATOM_JOINS_NAMESPACE_OF = UINT64_C(1) << 32, + UNIT_ATOM_REFERENCES = UINT64_C(1) << 33, + UNIT_ATOM_REFERENCED_BY = UINT64_C(1) << 34, + UNIT_ATOM_IN_SLICE = UINT64_C(1) << 35, + UNIT_ATOM_SLICE_OF = UINT64_C(1) << 36, + _UNIT_DEPENDENCY_ATOM_MAX = (UINT64_C(1) << 37) - 1, _UNIT_DEPENDENCY_ATOM_INVALID = -EINVAL, } UnitDependencyAtom;