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 e878f65de1a19f8813156de820f6acad456b5d35..5e1ed8c7e5b38cf51e6b8f31189c9c749c027ba8 100644 (file)
--- 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 {
 
     <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>

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 5a294f07b30556ed81a3d0d24195d7688bff7dc9..648728fdfbab0a234a93d398b49397d840be587f 100644 (file)
--- 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 fed151f7fef2001685980cb0d136e050b505342c..f7a1210a3451537df75aa80a5f0088530ced4ddd 100644 (file)
--- 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 a3ac320496906c7cd90d62dd8a729b5687c826c1..ccb379cee5501fa023b8ab455b54e9c76ca9a5ba 100644 (file)
--- 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 b7ac75ac71f306ab58060af551ce424470c479fb..94ab381626475d7a0519290dbda93903f0c4c353 100644 (file)
--- 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 c033c8a4faaa0e2d21615fcdac9e287c0364f672..a66e5985b8520d3cdea0cec1332c965cd7d4c713 100644 (file)
--- 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);