1
0
mirror of https://github.com/systemd/systemd.git synced 2024-10-27 01:55:22 +03:00

core: add StartUnitWithFlags DBUS method

When an activation job is skipped because of a Condition*= setting failing,
currently the JobRemoved signal lists 'done' as the result, just as with
a successful job.

This is a problem when doing dbus activation: dbus-broker will receive a
signal that says the job was successful, so then it moves into a state where
it waits for the requested name to appear on the bus, but that never happens
because the job was actually skipped.

Add a new StartUnitWithFlags that changes the behaviour of the JobRemoved
signal to list 'done' or 'skipped'.

Fixes #21520
This commit is contained in:
Luca Boccassi 2021-12-03 01:36:05 +00:00 committed by Zbigniew Jędrzejewski-Szmek
parent 72af88f231
commit f43282670b
6 changed files with 50 additions and 6 deletions

View File

@ -74,6 +74,10 @@ node /org/freedesktop/systemd1 {
StartUnit(in s name,
in s mode,
out o job);
StartUnitWithFlags(in s name,
in s mode,
in t flags,
out o job);
StartUnitReplace(in s old_unit,
in s new_unit,
in s mode,
@ -759,6 +763,8 @@ node /org/freedesktop/systemd1 {
<variablelist class="dbus-method" generated="True" extra-ref="StartUnit()"/>
<variablelist class="dbus-method" generated="True" extra-ref="StartUnitWithFlags()"/>
<variablelist class="dbus-method" generated="True" extra-ref="StartUnitReplace()"/>
<variablelist class="dbus-method" generated="True" extra-ref="StopUnit()"/>
@ -1174,6 +1180,13 @@ node /org/freedesktop/systemd1 {
<para><function>StartUnitReplace()</function> is similar to <function>StartUnit()</function> but
replaces a job that is queued for one unit by a job for another unit.</para>
<para><function>StartUnitWithFlags()</function> is similar to <function>StartUnit()</function> but
allows the caller to pass an extra <varname>flags</varname> parameter, which does not support any
flags for now, and is reserved for future extensions. The new method also changes the behaviour
of the <varname>JobRemoved</varname> signal and make it return <literal>skipped</literal> in case
the unit activation job is skipped because a <varname>Condition*=</varname> is not satisfied.
With the <varname>StartUnit</varname> method, <literal>done</literal> would be returned instead.</para>
<para><function>StopUnit()</function> is similar to <function>StartUnit()</function> but stops the
specified unit rather than starting it. Note that the <literal>isolate</literal> mode is invalid for this
method.</para>

View File

@ -2797,6 +2797,15 @@ const sd_bus_vtable bus_manager_vtable[] = {
SD_BUS_PARAM(job),
method_start_unit,
SD_BUS_VTABLE_UNPRIVILEGED),
SD_BUS_METHOD_WITH_NAMES("StartUnitWithFlags",
"sst",
SD_BUS_PARAM(name)
SD_BUS_PARAM(mode)
SD_BUS_PARAM(flags),
"o",
SD_BUS_PARAM(job),
method_start_unit,
SD_BUS_VTABLE_UNPRIVILEGED),
SD_BUS_METHOD_WITH_NAMES("StartUnitReplace",
"sss",
SD_BUS_PARAM(old_unit)

View File

@ -377,6 +377,7 @@ int bus_unit_method_start_generic(
bool reload_if_possible,
sd_bus_error *error) {
BusUnitQueueFlags job_flags = reload_if_possible ? BUS_UNIT_QUEUE_RELOAD_IF_POSSIBLE : 0;
const char *smode, *verb;
JobMode mode;
int r;
@ -405,6 +406,23 @@ int bus_unit_method_start_generic(
else
verb = job_type_to_string(job_type);
if (sd_bus_message_is_method_call(message, NULL, "StartUnitWithFlags")) {
uint64_t input_flags = 0;
r = sd_bus_message_read(message, "t", &input_flags);
if (r < 0)
return r;
/* Let clients know that this version doesn't support any flags at the moment. */
if (input_flags != 0)
return sd_bus_reply_method_errorf(message, SD_BUS_ERROR_INVALID_ARGS,
"Invalid 'flags' parameter '%" PRIu64 "'",
input_flags);
/* The new method unconditionally uses the new behaviour of returning 'skip' when
* a job is skipped. */
job_flags |= BUS_UNIT_QUEUE_RETURN_SKIP_ON_CONDITION_FAIL;
}
r = bus_verify_manage_units_async_full(
u,
verb,
@ -418,8 +436,7 @@ int bus_unit_method_start_generic(
if (r == 0)
return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
return bus_unit_queue_job(message, u, job_type, mode,
reload_if_possible ? BUS_UNIT_QUEUE_RELOAD_IF_POSSIBLE : 0, error);
return bus_unit_queue_job(message, u, job_type, mode, job_flags, error);
}
static int bus_unit_method_start(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@ -1781,6 +1798,9 @@ int bus_unit_queue_job_one(
if (r < 0)
return r;
if (FLAGS_SET(flags, BUS_UNIT_QUEUE_RETURN_SKIP_ON_CONDITION_FAIL))
j->return_skip_on_cond_failure = true;
r = bus_job_track_sender(j, message);
if (r < 0)
return r;

View File

@ -29,8 +29,9 @@ int bus_unit_method_freeze(sd_bus_message *message, void *userdata, sd_bus_error
int bus_unit_method_thaw(sd_bus_message *message, void *userdata, sd_bus_error *error);
typedef enum BusUnitQueueFlags {
BUS_UNIT_QUEUE_RELOAD_IF_POSSIBLE = 1 << 0,
BUS_UNIT_QUEUE_VERBOSE_REPLY = 1 << 1,
BUS_UNIT_QUEUE_RELOAD_IF_POSSIBLE = 1 << 0,
BUS_UNIT_QUEUE_VERBOSE_REPLY = 1 << 1,
BUS_UNIT_QUEUE_RETURN_SKIP_ON_CONDITION_FAIL = 1 << 2,
} BusUnitQueueFlags;
int bus_unit_queue_job_one(

View File

@ -889,8 +889,8 @@ int job_run_and_invalidate(Job *j) {
job_set_state(j, JOB_WAITING); /* Hmm, not ready after all, let's return to JOB_WAITING state */
else if (r == -EALREADY) /* already being executed */
r = job_finish_and_invalidate(j, JOB_DONE, true, true);
else if (r == -ECOMM) /* condition failed, but all is good */
r = job_finish_and_invalidate(j, JOB_DONE, true, false);
else if (r == -ECOMM) /* condition failed, but all is good. Return 'skip' if caller requested it. */
r = job_finish_and_invalidate(j, j->return_skip_on_cond_failure ? JOB_SKIPPED : JOB_DONE, true, false);
else if (r == -EBADR)
r = job_finish_and_invalidate(j, JOB_SKIPPED, true, false);
else if (r == -ENOEXEC)

View File

@ -160,6 +160,7 @@ struct Job {
bool irreversible:1;
bool in_gc_queue:1;
bool ref_by_private_bus:1;
bool return_skip_on_cond_failure:1;
};
Job* job_new(Unit *unit, JobType type);