From: Mike Yuan Date: Mon, 14 Oct 2024 21:49:09 +0000 (+0200) Subject: core: make refuse_late_merge a proper attr of Job and introduce TRANSACTION_REENQUEUE... X-Git-Tag: v257-rc1~53^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=569269d02deed18ac4605c277220e84a150d6a56;p=thirdparty%2Fsystemd.git core: make refuse_late_merge a proper attr of Job and introduce TRANSACTION_REENQUEUE_ANCHOR --- diff --git a/src/core/job.c b/src/core/job.c index 49cda53f22f..8fbbe757feb 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -186,25 +186,37 @@ void job_uninstall(Job *j) { j->installed = false; } -static bool job_type_allows_late_merge(JobType t) { - /* Tells whether it is OK to merge a job of type 't' with an already - * running job. - * Reloads cannot be merged this way. Think of the sequence: - * 1. Reload of a daemon is in progress; the daemon has already loaded - * its config file, but hasn't completed the reload operation yet. +static bool jobs_may_late_merge(const Job *j, const Job *uj) { + assert(j); + assert(!j->installed); + assert(uj); + assert(uj->installed); + assert(uj->state == JOB_RUNNING); + + /* Tells whether it is OK to merge a job with an already running job. */ + + if (j->refuse_late_merge) /* refused when constructing transaction? */ + return false; + + /* Reloads cannot be merged this way. Think of the sequence: + * 1. Reload of a daemon is in progress; the daemon has already loaded its config file, but hasn't + * completed the reload operation yet. * 2. Edit foo's config file. * 3. Trigger another reload to have the daemon use the new config. - * Should the second reload job be merged into the first one, the daemon - * would not know about the new config. - * JOB_RESTART jobs on the other hand can be merged, because they get - * patched into JOB_START after stopping the unit. So if we see a - * JOB_RESTART running, it means the unit hasn't stopped yet and at - * this time the merge is still allowed. */ - return t != JOB_RELOAD; + * Should the second reload job be merged into the first one, the daemon would not know about the new config. + * JOB_RESTART jobs on the other hand can be merged, because they get patched into JOB_START + * after stopping the unit. So if we see a JOB_RESTART running, it means the unit hasn't stopped yet + * and at this time the merge is still allowed. */ + if (j->type == JOB_RELOAD) + return false; + + return job_type_is_superset(uj->type, j->type); } static void job_merge_into_installed(Job *j, Job *other) { + assert(j); assert(j->installed); + assert(other); assert(j->unit == other->unit); if (j->type != JOB_NOP) { @@ -220,13 +232,13 @@ static void job_merge_into_installed(Job *j, Job *other) { j->ignore_order = j->ignore_order || other->ignore_order; } -Job* job_install(Job *j, bool refuse_late_merge) { +Job* job_install(Job *j) { Job **pj; Job *uj; assert(j); assert(!j->installed); - assert(j->type < _JOB_TYPE_MAX_IN_TRANSACTION); + assert(j->type >= 0 && j->type < _JOB_TYPE_MAX_IN_TRANSACTION); assert(j->state == JOB_WAITING); pj = j->type == JOB_NOP ? &j->unit->nop_job : &j->unit->job; @@ -238,8 +250,7 @@ Job* job_install(Job *j, bool refuse_late_merge) { else { /* not conflicting, i.e. mergeable */ - if (uj->state == JOB_WAITING || - (!refuse_late_merge && job_type_allows_late_merge(j->type) && job_type_is_superset(uj->type, j->type))) { + if (uj->state == JOB_WAITING || jobs_may_late_merge(j, uj)) { job_merge_into_installed(uj, j); log_unit_debug(uj->unit, "Merged %s/%s into installed job %s/%s as %"PRIu32, diff --git a/src/core/job.h b/src/core/job.h index 7603f311f2d..f3daf9d094a 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -158,13 +158,18 @@ struct Job { bool installed:1; bool in_run_queue:1; + bool matters_to_anchor:1; - bool in_dbus_queue:1; - bool sent_dbus_new_signal:1; + bool refuse_late_merge:1; bool ignore_order:1; bool irreversible:1; - bool in_gc_queue:1; + + bool in_dbus_queue:1; + bool sent_dbus_new_signal:1; + bool ref_by_private_bus:1; + + bool in_gc_queue:1; }; Job* job_new(Unit *unit, JobType type); @@ -173,7 +178,7 @@ void job_unlink(Job *job); Job* job_free(Job *job); DEFINE_TRIVIAL_CLEANUP_FUNC(Job*, job_free); -Job* job_install(Job *j, bool refuse_late_merge); +Job* job_install(Job *j); int job_install_deserialized(Job *j); void job_uninstall(Job *j); diff --git a/src/core/service.c b/src/core/service.c index b9c12140d38..1185e2fbcff 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -47,6 +47,7 @@ #include "string-table.h" #include "string-util.h" #include "strv.h" +#include "transaction.h" #include "unit-name.h" #include "unit.h" #include "utf8.h" @@ -2646,8 +2647,18 @@ static void service_enter_restart(Service *s, bool shortcut) { } /* Any units that are bound to this service must also be restarted. We use JOB_START for ourselves - * but then set JOB_RESTART_DEPENDENCIES which will enqueue JOB_RESTART for those dependency jobs. */ - r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT(s), JOB_RESTART_DEPENDENCIES, &error, /* ret = */ NULL); + * but then set JOB_RESTART_DEPENDENCIES which will enqueue JOB_RESTART for those dependency jobs. + * + * When RestartMode=direct is used, the service being restarted don't enter the inactive/failed state, + * i.e. unit_process_job -> job_finish_and_invalidate is never called, and the previous job might still + * be running (especially for Type=oneshot services). + * We need to refuse late merge and re-enqueue the anchor job. */ + r = manager_add_job_full(UNIT(s)->manager, + JOB_START, UNIT(s), + JOB_RESTART_DEPENDENCIES, + TRANSACTION_REENQUEUE_ANCHOR, + /* affected_jobs = */ NULL, + &error, /* ret = */ NULL); if (r < 0) { log_unit_warning(UNIT(s), "Failed to schedule restart job: %s", bus_error_message(&error, r)); return service_enter_dead(s, SERVICE_FAILURE_RESOURCES, /* allow_restart= */ false); diff --git a/src/core/transaction.c b/src/core/transaction.c index 1ab59b823e5..377ea05983f 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -620,6 +620,9 @@ static int transaction_apply( Job *j; int r; + assert(tr); + assert(m); + /* Moves the transaction jobs to the set of active jobs */ if (IN_SET(mode, JOB_ISOLATE, JOB_FLUSH)) { @@ -658,12 +661,7 @@ static int transaction_apply( /* Clean the job dependencies */ transaction_unlink_job(tr, j, false); - /* When RestartMode=direct is used, the service being restarted don't enter the inactive/failed - * state, i.e. unit_process_job -> job_finish_and_invalidate is never called, and the previous - * job might still be running (especially for Type=oneshot services). We need to refuse - * late merge and re-enqueue the anchor job. */ - installed_job = job_install(j, - /* refuse_late_merge = */ mode == JOB_RESTART_DEPENDENCIES && j == tr->anchor_job); + installed_job = job_install(j); if (installed_job != j) { /* j has been merged into a previously installed job */ if (tr->anchor_job == j) @@ -705,10 +703,10 @@ int transaction_activate( int r; unsigned generation = 1; - assert(tr); + /* This applies the changes recorded in tr->jobs to the actual list of jobs, if possible. */ - /* This applies the changes recorded in tr->jobs to - * the actual list of jobs, if possible. */ + assert(tr); + assert(m); /* Reset the generation counter of all installed jobs. The detection of cycles * looks at installed jobs. If they had a non-zero generation from some previous @@ -739,7 +737,6 @@ int transaction_activate( r = transaction_verify_order(tr, &generation, e); if (r >= 0) break; - if (r != -EAGAIN) return log_warning_errno(r, "Requested transaction contains an unfixable cyclic ordering dependency: %s", bus_error_message(e, r)); @@ -754,7 +751,6 @@ int transaction_activate( r = transaction_merge_jobs(tr, e); if (r >= 0) break; - if (r != -EAGAIN) return log_warning_errno(r, "Requested transaction contains unmergeable jobs: %s", bus_error_message(e, r)); @@ -817,9 +813,6 @@ static Job* transaction_add_one_job(Transaction *tr, JobType type, Unit *unit, b if (!j) return NULL; - j->generation = 0; - j->marker = NULL; - j->matters_to_anchor = false; j->irreversible = tr->irreversible; LIST_PREPEND(transaction, f, j); @@ -944,6 +937,7 @@ int transaction_add_job_and_dependencies( int r; assert(tr); + assert(type >= 0); assert(type < _JOB_TYPE_MAX); assert(type < _JOB_TYPE_MAX_IN_TRANSACTION); assert(unit); @@ -1008,6 +1002,9 @@ int transaction_add_job_and_dependencies( /* If the job has no parent job, it is the anchor job. */ assert(!tr->anchor_job); tr->anchor_job = ret; + + if (FLAGS_SET(flags, TRANSACTION_REENQUEUE_ANCHOR)) + ret->refuse_late_merge = true; } if (!is_new || FLAGS_SET(flags, TRANSACTION_IGNORE_REQUIREMENTS) || type == JOB_NOP) diff --git a/src/core/transaction.h b/src/core/transaction.h index f2bf047f10e..b6728a35d7b 100644 --- a/src/core/transaction.h +++ b/src/core/transaction.h @@ -33,7 +33,10 @@ typedef enum TransactionAddFlags { /* Indicate that we're in the recursion for processing UNIT_ATOM_PROPAGATE_STOP_GRACEFUL units */ TRANSACTION_PROCESS_PROPAGATE_STOP_GRACEFUL = 1 << 5, - _TRANSACTION_FLAGS_MASK_PUBLIC = 0, + /* Always re-enqueue anchor job (refuse late merge) */ + TRANSACTION_REENQUEUE_ANCHOR = 1 << 6, + + _TRANSACTION_FLAGS_MASK_PUBLIC = TRANSACTION_REENQUEUE_ANCHOR, } TransactionAddFlags; void transaction_add_propagate_reload_jobs(