[systemd-commits] 3 commits - man/systemd.service.xml man/systemd.socket.xml man/systemd.swap.xml src/core test/loopy2.service test/loopy3.service test/loopy4.service test/loopy.service test/loopy.service.d

Zbigniew Jędrzejewski-Szmek zbyszek at kemper.freedesktop.org
Thu Aug 7 17:57:04 PDT 2014


 man/systemd.service.xml          |   10 ++--
 man/systemd.socket.xml           |   10 ++--
 man/systemd.swap.xml             |    8 ++-
 src/core/load-dropin.c           |    2 
 src/core/unit.c                  |   83 +++++++++++++++++++++++++++++++++++----
 test/loopy.service               |    2 
 test/loopy.service.d/compat.conf |    5 ++
 test/loopy2.service              |    1 
 test/loopy3.service              |    5 ++
 test/loopy4.service              |    1 
 10 files changed, 107 insertions(+), 20 deletions(-)

New commits:
commit 5e34b37c9fec5da130f6549ddabd8a2af5c9faac
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Aug 7 20:46:34 2014 -0400

    man: correct references to DefaultTimeout*Sec
    
    Noticed by thp on #systemd.

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index 0131250..5c4bd65 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -605,11 +605,12 @@ ExecStart=/bin/echo $ONE $TWO ${TWO}</programlisting>
                                 time span value such as "5min
                                 20s". Pass <literal>0</literal> to
                                 disable the timeout logic. Defaults to
-                                <varname>TimeoutStartSec=</varname> from
+                                <varname>DefaultTimeoutStartSec=</varname> from
                                 the manager configuration file, except
                                 when <varname>Type=oneshot</varname> is
                                 used, in which case the timeout
-                                is disabled by default.
+                                is disabled by default
+                                (see <citerefentry><refentrytitle>systemd-systemd.conf</refentrytitle><manvolnum>5</manvolnum></citerefentry>).
                                 </para></listitem>
                         </varlistentry>
 
@@ -628,8 +629,9 @@ ExecStart=/bin/echo $ONE $TWO ${TWO}</programlisting>
                                 time span value such as "5min
                                 20s". Pass <literal>0</literal> to disable
                                 the timeout logic. Defaults to
-                                <varname>TimeoutStartSec=</varname> from the
-                                manager configuration file.
+                                <varname>DefaultTimeoutStopSec=</varname> from the
+                                manager configuration file
+                                (see <citerefentry><refentrytitle>systemd-systemd.conf</refentrytitle><manvolnum>5</manvolnum></citerefentry>).
                                 </para></listitem>
                         </varlistentry>
 
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 09a7311..9c9af53 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -722,16 +722,18 @@
                                 finish. If a command does not exit
                                 within the configured time, the socket
                                 will be considered failed and be shut
-                                down again. All commands still running,
+                                down again. All commands still running
                                 will be terminated forcibly via
                                 <constant>SIGTERM</constant>, and after another delay of
                                 this time with <constant>SIGKILL</constant>. (See
                                 <option>KillMode=</option> in <citerefentry><refentrytitle>systemd.kill</refentrytitle><manvolnum>5</manvolnum></citerefentry>.)
                                 Takes a unit-less value in seconds, or
                                 a time span value such as "5min
-                                20s". Pass 0 to disable the timeout
-                                logic. Defaults to <varname>TimeoutStartSec=</varname> from the
-                                manager configuration file.</para></listitem>
+                                20s". Pass <literal>0</literal> to disable the timeout
+                                logic. Defaults to <varname>DefaultTimeoutStartSec=</varname> from the
+                                manager configuration file
+                                (see <citerefentry><refentrytitle>systemd-systemd.conf</refentrytitle><manvolnum>5</manvolnum></citerefentry>).
+                                </para></listitem>
                         </varlistentry>
 
                         <varlistentry>
diff --git a/man/systemd.swap.xml b/man/systemd.swap.xml
index 492309e..61901d2 100644
--- a/man/systemd.swap.xml
+++ b/man/systemd.swap.xml
@@ -185,9 +185,11 @@
                                 <citerefentry><refentrytitle>systemd.kill</refentrytitle><manvolnum>5</manvolnum></citerefentry>.)
                                 Takes a unit-less value in seconds, or
                                 a time span value such as "5min
-                                20s". Pass 0 to disable the timeout
-                                logic. Defaults to <varname>TimeoutStartSec=</varname> from the
-                                manager configuration file.</para></listitem>
+                                20s". Pass <literal>0</literal> to disable the timeout
+                                logic. Defaults to <varname>DefaultTimeoutStartSec=</varname> from the
+                                manager configuration file
+                                (see <citerefentry><refentrytitle>systemd-systemd.conf</refentrytitle><manvolnum>5</manvolnum></citerefentry>).
+                                </para></listitem>
                         </varlistentry>
                 </variablelist>
 

commit d1fab3fe88ae873b26b50359758776ad9e31968e
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Aug 7 20:46:49 2014 -0400

    core: warn when merged units have conflicting dependencies
    
    A unit should not Conflict with itself. It also does not make
    much sense for a unit to be After or Before itself, or to
    trigger itself in some way.
    
    If one of those dependency types is encountered, warn, instead
    of dropping it silently like other dependency types.
    
    % build/systemd-analyze verify test/loopy3.service
    ...
    Dependency Conflicts dropped when merging unit loopy4.service into loopy3.service
    Dependency ConflictedBy dropped when merging unit loopy4.service into loopy3.service

diff --git a/src/core/unit.c b/src/core/unit.c
index 0926732..a5f6b2e 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -69,6 +69,8 @@ const UnitVTable * const unit_vtable[_UNIT_TYPE_MAX] = {
         [UNIT_SCOPE] = &scope_vtable
 };
 
+static int maybe_warn_about_dependency(const char *id, const char *other, UnitDependency dependency);
+
 Unit *unit_new(Manager *m, size_t size) {
         Unit *u;
 
@@ -583,7 +585,7 @@ static void merge_names(Unit *u, Unit *other) {
                 assert_se(hashmap_replace(u->manager->units, t, u) == 0);
 }
 
-static void merge_dependencies(Unit *u, Unit *other, UnitDependency d) {
+static void merge_dependencies(Unit *u, Unit *other, const char *other_id, UnitDependency d) {
         Iterator i;
         Unit *back;
         int r;
@@ -599,7 +601,8 @@ static void merge_dependencies(Unit *u, Unit *other, UnitDependency d) {
                 for (k = 0; k < _UNIT_DEPENDENCY_MAX; k++) {
                         /* Do not add dependencies between u and itself */
                         if (back == u) {
-                                set_remove(back->dependencies[k], other);
+                                if (set_remove(back->dependencies[k], other))
+                                        maybe_warn_about_dependency(u->id, other_id, k);
                         } else {
                                 r = set_remove_and_put(back->dependencies[k], other, u);
                                 if (r == -EEXIST)
@@ -611,7 +614,9 @@ static void merge_dependencies(Unit *u, Unit *other, UnitDependency d) {
         }
 
         /* Also do not move dependencies on u to itself */
-        set_remove(other->dependencies[d], u);
+        back = set_remove(other->dependencies[d], u);
+        if (back)
+                maybe_warn_about_dependency(u->id, other_id, d);
 
         complete_move(&u->dependencies[d], &other->dependencies[d]);
 
@@ -621,6 +626,7 @@ static void merge_dependencies(Unit *u, Unit *other, UnitDependency d) {
 
 int unit_merge(Unit *u, Unit *other) {
         UnitDependency d;
+        const char *other_id = NULL;
 
         assert(u);
         assert(other);
@@ -651,6 +657,9 @@ int unit_merge(Unit *u, Unit *other) {
         if (!UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(other)))
                 return -EEXIST;
 
+        if (other->id)
+                other_id = strdupa(other->id);
+
         /* Merge names */
         merge_names(u, other);
 
@@ -660,7 +669,7 @@ int unit_merge(Unit *u, Unit *other) {
 
         /* Merge dependencies */
         for (d = 0; d < _UNIT_DEPENDENCY_MAX; d++)
-                merge_dependencies(u, other, d);
+                merge_dependencies(u, other, other_id, d);
 
         other->load_state = UNIT_MERGED;
         other->merged_into = u;
@@ -1039,6 +1048,9 @@ static int unit_add_slice_dependencies(Unit *u) {
         if (UNIT_ISSET(u->slice))
                 return unit_add_two_dependencies(u, UNIT_AFTER, UNIT_WANTS, UNIT_DEREF(u->slice), true);
 
+        if (streq(u->id, SPECIAL_ROOT_SLICE))
+                return 0;
+
         return unit_add_two_dependencies_by_name(u, UNIT_AFTER, UNIT_WANTS, SPECIAL_ROOT_SLICE, NULL, true);
 }
 
@@ -1945,6 +1957,50 @@ bool unit_job_is_applicable(Unit *u, JobType j) {
         }
 }
 
+static int maybe_warn_about_dependency(const char *id, const char *other, UnitDependency dependency) {
+        switch (dependency) {
+        case UNIT_REQUIRES:
+        case UNIT_REQUIRES_OVERRIDABLE:
+        case UNIT_WANTS:
+        case UNIT_REQUISITE:
+        case UNIT_REQUISITE_OVERRIDABLE:
+        case UNIT_BINDS_TO:
+        case UNIT_PART_OF:
+        case UNIT_REQUIRED_BY:
+        case UNIT_REQUIRED_BY_OVERRIDABLE:
+        case UNIT_WANTED_BY:
+        case UNIT_BOUND_BY:
+        case UNIT_CONSISTS_OF:
+        case UNIT_REFERENCES:
+        case UNIT_REFERENCED_BY:
+        case UNIT_PROPAGATES_RELOAD_TO:
+        case UNIT_RELOAD_PROPAGATED_FROM:
+        case UNIT_JOINS_NAMESPACE_OF:
+                return 0;
+
+        case UNIT_CONFLICTS:
+        case UNIT_CONFLICTED_BY:
+        case UNIT_BEFORE:
+        case UNIT_AFTER:
+        case UNIT_ON_FAILURE:
+        case UNIT_TRIGGERS:
+        case UNIT_TRIGGERED_BY:
+                if (streq_ptr(id, other))
+                        log_warning_unit(id, "Dependency %s=%s dropped from unit %s",
+                                         unit_dependency_to_string(dependency), id, other);
+                else
+                        log_warning_unit(id, "Dependency %s=%s dropped from unit %s merged into %s",
+                                         unit_dependency_to_string(dependency), id,
+                                         strna(other), id);
+                return -EINVAL;
+
+        case _UNIT_DEPENDENCY_MAX:
+        case _UNIT_DEPENDENCY_INVALID:
+                break;
+        }
+        assert_not_reached("Invalid dependency type");
+}
+
 int unit_add_dependency(Unit *u, UnitDependency d, Unit *other, bool add_reference) {
 
         static const UnitDependency inverse_table[_UNIT_DEPENDENCY_MAX] = {
@@ -1974,6 +2030,7 @@ int unit_add_dependency(Unit *u, UnitDependency d, Unit *other, bool add_referen
                 [UNIT_JOINS_NAMESPACE_OF] = UNIT_JOINS_NAMESPACE_OF,
         };
         int r, q = 0, v = 0, w = 0;
+        Unit *orig_u = u, *orig_other = other;
 
         assert(u);
         assert(d >= 0 && d < _UNIT_DEPENDENCY_MAX);
@@ -1984,8 +2041,10 @@ int unit_add_dependency(Unit *u, UnitDependency d, Unit *other, bool add_referen
 
         /* We won't allow dependencies on ourselves. We will not
          * consider them an error however. */
-        if (u == other)
+        if (u == other) {
+                maybe_warn_about_dependency(orig_u->id, orig_other->id, d);
                 return 0;
+        }
 
         r = set_ensure_allocated(&u->dependencies[d], trivial_hash_func, trivial_compare_func);
         if (r < 0)
diff --git a/test/loopy3.service b/test/loopy3.service
new file mode 100644
index 0000000..606e26b
--- /dev/null
+++ b/test/loopy3.service
@@ -0,0 +1,5 @@
+[Service]
+ExecStart=/bin/true
+
+[Unit]
+Conflicts=loopy4.service
diff --git a/test/loopy4.service b/test/loopy4.service
new file mode 120000
index 0000000..43e5658
--- /dev/null
+++ b/test/loopy4.service
@@ -0,0 +1 @@
+loopy3.service
\ No newline at end of file

commit e66047ff62c971eefa32b42373420d61e3f2a9c1
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Aug 7 20:42:58 2014 -0400

    core: do not add dependencies to self
    
    Adds a pair of files which cause a segfault (also with
    systemd-analyze verify).
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1124843

diff --git a/src/core/load-dropin.c b/src/core/load-dropin.c
index 21c9915..ffc68b4 100644
--- a/src/core/load-dropin.c
+++ b/src/core/load-dropin.c
@@ -181,7 +181,7 @@ int unit_load_dropin(Unit *u) {
         }
 
         u->dropin_paths = unit_find_dropin_paths(u);
-        if (! u->dropin_paths)
+        if (!u->dropin_paths)
                 return 0;
 
         STRV_FOREACH(f, u->dropin_paths) {
diff --git a/src/core/unit.c b/src/core/unit.c
index b68796a..0926732 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -597,14 +597,22 @@ static void merge_dependencies(Unit *u, Unit *other, UnitDependency d) {
                 UnitDependency k;
 
                 for (k = 0; k < _UNIT_DEPENDENCY_MAX; k++) {
-                        r = set_remove_and_put(back->dependencies[k], other, u);
-                        if (r == -EEXIST)
+                        /* Do not add dependencies between u and itself */
+                        if (back == u) {
                                 set_remove(back->dependencies[k], other);
-                        else
-                                assert(r >= 0 || r == -ENOENT);
+                        } else {
+                                r = set_remove_and_put(back->dependencies[k], other, u);
+                                if (r == -EEXIST)
+                                        set_remove(back->dependencies[k], other);
+                                else
+                                        assert(r >= 0 || r == -ENOENT);
+                        }
                 }
         }
 
+        /* Also do not move dependencies on u to itself */
+        set_remove(other->dependencies[d], u);
+
         complete_move(&u->dependencies[d], &other->dependencies[d]);
 
         set_free(other->dependencies[d]);
diff --git a/test/loopy.service b/test/loopy.service
new file mode 100644
index 0000000..9eb6457
--- /dev/null
+++ b/test/loopy.service
@@ -0,0 +1,2 @@
+[Service]
+ExecStart=/bin/true
diff --git a/test/loopy.service.d/compat.conf b/test/loopy.service.d/compat.conf
new file mode 100644
index 0000000..51b84b8
--- /dev/null
+++ b/test/loopy.service.d/compat.conf
@@ -0,0 +1,5 @@
+[Unit]
+BindsTo=loopy2.service
+
+[Install]
+Also=loopy2.service
diff --git a/test/loopy2.service b/test/loopy2.service
new file mode 120000
index 0000000..961b1fe
--- /dev/null
+++ b/test/loopy2.service
@@ -0,0 +1 @@
+loopy.service
\ No newline at end of file



More information about the systemd-commits mailing list