From: Luca Boccassi Date: Fri, 3 Dec 2021 01:36:05 +0000 (+0000) Subject: core: add StartUnitWithFlags DBUS method X-Git-Tag: v250-rc1~35 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=f43282670bfd4c0e38c8c37dba9c91fff7d6232d;p=thirdparty%2Fsystemd.git 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 --- diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index e878f65de1a..5e1ed8c7e5b 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -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 { + + @@ -1174,6 +1180,13 @@ node /org/freedesktop/systemd1 { StartUnitReplace() is similar to StartUnit() but replaces a job that is queued for one unit by a job for another unit. + StartUnitWithFlags() is similar to StartUnit() but + allows the caller to pass an extra flags parameter, which does not support any + flags for now, and is reserved for future extensions. The new method also changes the behaviour + of the JobRemoved signal and make it return skipped in case + the unit activation job is skipped because a Condition*= is not satisfied. + With the StartUnit method, done would be returned instead. + StopUnit() is similar to StartUnit() but stops the specified unit rather than starting it. Note that the isolate mode is invalid for this method. diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 5a294f07b30..648728fdfba 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -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) diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index fed151f7fef..f7a1210a345 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -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; diff --git a/src/core/dbus-unit.h b/src/core/dbus-unit.h index a3ac3204969..ccb379cee55 100644 --- a/src/core/dbus-unit.h +++ b/src/core/dbus-unit.h @@ -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( diff --git a/src/core/job.c b/src/core/job.c index b7ac75ac71f..94ab3816264 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -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) diff --git a/src/core/job.h b/src/core/job.h index c033c8a4faa..a66e5985b85 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -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);