Merge pull request #4733 from poettering/binds-to

When we are about to start a unit, check the deps again.

4 files changed, 98 insertions, 70 deletions

diff --git a/NEWS b/NEWS
index ec50c0c403..23f555fc53 100644
--- a/NEWS
+++ b/NEWS
@@ -35,6 +35,10 @@ CHANGES WITH 233 in spe
           names of remote hosts and to reply to mDNS's A and AAAA requests
           from the hosts.
 
+        * When units are about to be started an additional check is now done to
+          ensure that all dependencies of type BindsTo= (when used in
+          combination with After=) have been started.
+
 CHANGES WITH 232:
 
         * The new RemoveIPC= option can be used to remove IPC objects owned by
diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml
index 417840e6c2..2208b6a287 100644
--- a/man/systemd.unit.xml
+++ b/man/systemd.unit.xml
@@ -436,32 +436,30 @@
       <varlistentry>
         <term><varname>Requires=</varname></term>
 
-        <listitem><para>Configures requirement dependencies on other
-        units. If this unit gets activated, the units listed here will
-        be activated as well. If one of the other units gets
-        deactivated or its activation fails, this unit will be
-        deactivated. This option may be specified more than once or
-        multiple space-separated units may be specified in one option
-        in which case requirement dependencies for all listed names
-        will be created. Note that requirement dependencies do not
-        influence the order in which services are started or stopped.
-        This has to be configured independently with the
-        <varname>After=</varname> or <varname>Before=</varname>
-        options. If a unit <filename>foo.service</filename> requires a
-        unit <filename>bar.service</filename> as configured with
-        <varname>Requires=</varname> and no ordering is configured
-        with <varname>After=</varname> or <varname>Before=</varname>,
-        then both units will be started simultaneously and without any
-        delay between them if <filename>foo.service</filename> is
-        activated. Often, it is a better choice to use
-        <varname>Wants=</varname> instead of
-        <varname>Requires=</varname> in order to achieve a system that
-        is more robust when dealing with failing services.</para>
-
-        <para>Note that dependencies of this type may also be
-        configured outside of the unit configuration file by adding a
-        symlink to a <filename>.requires/</filename> directory
-        accompanying the unit file. For details, see
+        <listitem><para>Configures requirement dependencies on other units. If this unit gets activated, the units
+        listed here will be activated as well. If one of the other units gets deactivated or its activation fails, this
+        unit will be deactivated. This option may be specified more than once or multiple space-separated units may be
+        specified in one option in which case requirement dependencies for all listed names will be created. Note that
+        requirement dependencies do not influence the order in which services are started or stopped.  This has to be
+        configured independently with the <varname>After=</varname> or <varname>Before=</varname> options. If a unit
+        <filename>foo.service</filename> requires a unit <filename>bar.service</filename> as configured with
+        <varname>Requires=</varname> and no ordering is configured with <varname>After=</varname> or
+        <varname>Before=</varname>, then both units will be started simultaneously and without any delay between them
+        if <filename>foo.service</filename> is activated. Often, it is a better choice to use <varname>Wants=</varname>
+        instead of <varname>Requires=</varname> in order to achieve a system that is more robust when dealing with
+        failing services.</para>
+
+        <para>Note that this dependency type does not imply that the other unit always has to be in active state when
+        this unit is running. Specifically: failing condition checks (such as <varname>ConditionPathExists=</varname>,
+        <varname>ConditionPathExists=</varname>, … — see below) do not cause the start job of a unit with a
+        <varname>Requires=</varname> dependency on it to fail. Also, some unit types may deactivate on their own (for
+        example, a service process may decide to exit cleanly, or a device may be unplugged by the user), which is not
+        propagated to units having a <varname>Requires=</varname> dependency. Use the <varname>BindsTo=</varname>
+        dependency type together with <varname>After=</varname> to ensure that a unit may never be in active state
+        without a specific other unit also in active state (see below).</para>
+
+        <para>Note that dependencies of this type may also be configured outside of the unit configuration file by
+        adding a symlink to a <filename>.requires/</filename> directory accompanying the unit file. For details, see
         above.</para></listitem>
       </varlistentry>
 
@@ -495,14 +493,21 @@
       <varlistentry>
         <term><varname>BindsTo=</varname></term>
 
-        <listitem><para>Configures requirement dependencies, very
-        similar in style to <varname>Requires=</varname>, however in
-        addition to this behavior, it also declares that this unit is
-        stopped when any of the units listed suddenly disappears.
-        Units can suddenly, unexpectedly disappear if a service
-        terminates on its own choice, a device is unplugged or a mount
-        point unmounted without involvement of
-        systemd.</para></listitem>
+        <listitem><para>Configures requirement dependencies, very similar in style to
+        <varname>Requires=</varname>. However, this dependency type is stronger: in addition to the effect of
+        <varname>Requires=</varname> it declares that if the unit bound to is stopped, this unit will be stopped
+        too. This means a unit bound to another unit that suddenly enters inactive state will be stopped too.
+        Units can suddenly, unexpectedly enter inactive state for different reasons: the main process of a service unit
+        might terminate on its own choice, the backing device of a device unit might be unplugged or the mount point of
+        a mount unit might be unmounted without involvement of the system and service manager.</para>
+
+        <para>When used in conjunction with <varname>After=</varname> on the same unit the behaviour of
+        <varname>BindsTo=</varname> is even stronger. In this case, the unit bound to strictly has to be in active
+        state for this unit to also be in active state. This not only means a unit bound to another unit that suddenly
+        enters inactive state, but also one that is bound to another unit that gets skipped due to a failed condition
+        check (such as <varname>ConditionPathExists=</varname>, <varname>ConditionPathIsSymbolicLink=</varname>, … —
+        see below) will be stopped, should it be running. Hence, in many cases it is best to combine
+        <varname>BindsTo=</varname> with <varname>After=</varname>.</para></listitem>
       </varlistentry>
 
       <varlistentry>
@@ -541,42 +546,26 @@
         <term><varname>Before=</varname></term>
         <term><varname>After=</varname></term>
 
-        <listitem><para>A space-separated list of unit names.
-        Configures ordering dependencies between units. If a unit
-        <filename>foo.service</filename> contains a setting
-        <option>Before=bar.service</option> and both units are being
-        started, <filename>bar.service</filename>'s start-up is
-        delayed until <filename>foo.service</filename> is started up.
-        Note that this setting is independent of and orthogonal to the
-        requirement dependencies as configured by
-        <varname>Requires=</varname>. It is a common pattern to
-        include a unit name in both the <varname>After=</varname> and
-        <varname>Requires=</varname> option, in which case the unit
-        listed will be started before the unit that is configured with
-        these options. This option may be specified more than once, in
-        which case ordering dependencies for all listed names are
-        created. <varname>After=</varname> is the inverse of
-        <varname>Before=</varname>, i.e. while
-        <varname>After=</varname> ensures that the configured unit is
-        started after the listed unit finished starting up,
-        <varname>Before=</varname> ensures the opposite, i.e. that the
-        configured unit is fully started up before the listed unit is
-        started. Note that when two units with an ordering dependency
-        between them are shut down, the inverse of the start-up order
-        is applied. i.e. if a unit is configured with
-        <varname>After=</varname> on another unit, the former is
-        stopped before the latter if both are shut down. Given two units
-        with any ordering dependency between them, if one unit is shut
-        down and the other is started up, the shutdown is ordered
-        before the start-up. It doesn't matter if the ordering
-        dependency is <varname>After=</varname> or
-        <varname>Before=</varname>. It also doesn't matter which of the
-        two is shut down, as long as one is shut down and the other is
-        started up. The shutdown is ordered before the start-up in all
-        cases. If two units have no ordering dependencies between them,
-        they are shut down or started up simultaneously, and no ordering
-        takes place.
-        </para></listitem>
+        <listitem><para>A space-separated list of unit names.  Configures ordering dependencies between units. If a
+        unit <filename>foo.service</filename> contains a setting <option>Before=bar.service</option> and both units are
+        being started, <filename>bar.service</filename>'s start-up is delayed until <filename>foo.service</filename> is
+        started up.  Note that this setting is independent of and orthogonal to the requirement dependencies as
+        configured by <varname>Requires=</varname>, <varname>Wants=</varname> or <varname>BindsTo=</varname>. It is a
+        common pattern to include a unit name in both the <varname>After=</varname> and <varname>Requires=</varname>
+        option, in which case the unit listed will be started before the unit that is configured with these
+        options. This option may be specified more than once, in which case ordering dependencies for all listed names
+        are created. <varname>After=</varname> is the inverse of <varname>Before=</varname>, i.e. while
+        <varname>After=</varname> ensures that the configured unit is started after the listed unit finished starting
+        up, <varname>Before=</varname> ensures the opposite, i.e. that the configured unit is fully started up before
+        the listed unit is started. Note that when two units with an ordering dependency between them are shut down,
+        the inverse of the start-up order is applied. i.e. if a unit is configured with <varname>After=</varname> on
+        another unit, the former is stopped before the latter if both are shut down. Given two units with any ordering
+        dependency between them, if one unit is shut down and the other is started up, the shutdown is ordered before
+        the start-up. It doesn't matter if the ordering dependency is <varname>After=</varname> or
+        <varname>Before=</varname>, in this case. It also doesn't matter which of the two is shut down, as long as one
+        is shut down and the other is started up. The shutdown is ordered before the start-up in all cases. If two
+        units have no ordering dependencies between them, they are shut down or started up simultaneously, and no
+        ordering takes place.  </para></listitem>
       </varlistentry>
 
       <varlistentry>
diff --git a/src/core/job.c b/src/core/job.c
index 00f7d7998f..07f4b74c5c 100644
--- a/src/core/job.c
+++ b/src/core/job.c
@@ -627,6 +627,8 @@ int job_run_and_invalidate(Job *j) {
                         r = job_finish_and_invalidate(j, JOB_ASSERT, true, false);
                 else if (r == -EOPNOTSUPP)
                         r = job_finish_and_invalidate(j, JOB_UNSUPPORTED, true, false);
+                else if (r == -ENOLINK)
+                        r = job_finish_and_invalidate(j, JOB_DEPENDENCY, true, false);
                 else if (r == -EAGAIN)
                         job_set_state(j, JOB_WAITING);
                 else if (r < 0)
diff --git a/src/core/unit.c b/src/core/unit.c
index 5e4b1567d8..0b680e9544 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1527,6 +1527,7 @@ int unit_start_limit_test(Unit *u) {
 }
 
 bool unit_shall_confirm_spawn(Unit *u) {
+        assert(u);
 
         if (manager_is_confirm_spawn_disabled(u->manager))
                 return false;
@@ -1537,6 +1538,31 @@ bool unit_shall_confirm_spawn(Unit *u) {
         return !unit_get_exec_context(u)->same_pgrp;
 }
 
+static bool unit_verify_deps(Unit *u) {
+        Unit *other;
+        Iterator j;
+
+        assert(u);
+
+        /* Checks whether all BindsTo= dependencies of this unit are fulfilled — if they are also combined with
+         * After=. We do not check Requires= or Requisite= here as they only should have an effect on the job
+         * processing, but do not have any effect afterwards. We don't check BindsTo= dependencies that are not used in
+         * conjunction with After= as for them any such check would make things entirely racy. */
+
+        SET_FOREACH(other, u->dependencies[UNIT_BINDS_TO], j) {
+
+                if (!set_contains(u->dependencies[UNIT_AFTER], other))
+                        continue;
+
+                if (!UNIT_IS_ACTIVE_OR_RELOADING(unit_active_state(other))) {
+                        log_unit_notice(u, "Bound to unit %s, but unit isn't active.", other->id);
+                        return false;
+                }
+        }
+
+        return true;
+}
+
 /* Errors:
  *         -EBADR:      This unit type does not support starting.
  *         -EALREADY:   Unit is already started.
@@ -1545,6 +1571,7 @@ bool unit_shall_confirm_spawn(Unit *u) {
  *         -EPROTO:     Assert failed
  *         -EINVAL:     Unit not loaded
  *         -EOPNOTSUPP: Unit type not supported
+ *         -ENOLINK:    The necessary dependencies are not fulfilled.
  */
 int unit_start(Unit *u) {
         UnitActiveState state;
@@ -1590,6 +1617,12 @@ int unit_start(Unit *u) {
         if (!unit_supported(u))
                 return -EOPNOTSUPP;
 
+        /* Let's make sure that the deps really are in order before we start this. Normally the job engine should have
+         * taken care of this already, but let's check this here again. After all, our dependencies might not be in
+         * effect anymore, due to a reload or due to a failed condition. */
+        if (!unit_verify_deps(u))
+                return -ENOLINK;
+
         /* Forward to the main object, if we aren't it. */
         following = unit_following(u);
         if (following) {