Attachment 112339 Details for Bug 87287 – implementation of the proposed logic (now with documentation)

[patch] implementation of the proposed logic (now with documentation)

conditionally-depend-on-the-triggered-unit.patch (text/plain), 9.68 KB, created by j.witteveen on 2015-01-16 12:23:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: j.witteveen

Created: 2015-01-16 12:23:10 UTC

Size: 9.68 KB

patch

obsolete

>From 2b856947e690c63b2e928a4a3da6155a150b82cf Mon Sep 17 00:00:00 2001
>From: Jouke Witteveen <j.witteveen@gmail.com>
>Date: Sun, 21 Dec 2014 16:22:44 +0100
>Subject: [PATCH] path: conditionally depend on the triggered unit
>
>Path units having either PathExists=, PathExistsGlob=, or
>DirectoryNotEmpty= want the service they trigger when the condition is
>met. This way it becomes meaningful to include StopWhenUnneeded=true in
>the triggered service.
>
>https://bugs.freedesktop.org/show_bug.cgi?id=87287
>---
> man/systemd.path.xml | 10 ++++++
> src/core/path.c      | 42 +++++++++++++++----------
> src/core/path.h      |  1 +
> src/core/unit.c      | 86 +++++++++++++++++++++++++++++++++++-----------------
> src/core/unit.h      |  2 ++
> 5 files changed, 98 insertions(+), 43 deletions(-)
>
>diff --git a/man/systemd.path.xml b/man/systemd.path.xml
>index c6d61cf..9b78d45 100644
>--- a/man/systemd.path.xml
>+++ b/man/systemd.path.xml
>@@ -170,6 +170,16 @@
>                                 to <varname>PathChanged=</varname> and
>                                 <varname>PathModified=</varname>.</para>
> 
>+                                <para>If the triggered unit has
>+                                <varname>StopWhenUnneeded=true</varname>
>+                                then it is not stopped as long as a path
>+                                exists (in case of
>+                                <varname>PathExists=</varname> and
>+                                <varname>PathExistsGlob=</varname>) or
>+                                a directory is not empty (in case of
>+                                <varname>DirectoryNotEmpty=</varname>).
>+                                </para>
>+
>                                 <para>If the path itself or any of the
>                                 containing directories are not
>                                 accessible, <command>systemd</command>
>diff --git a/src/core/path.c b/src/core/path.c
>index 0fdf483..5b1bccd 100644
>--- a/src/core/path.c
>+++ b/src/core/path.c
>@@ -467,6 +467,22 @@ static void path_enter_dead(Path *p, PathResult f) {
>         path_set_state(p, p->result != PATH_SUCCESS ? PATH_FAILED : PATH_DEAD);
> }
> 
>+static bool path_check_good(Path *p, bool initial) {
>+        PathSpec *s;
>+        bool good = false;
>+
>+        assert(p);
>+
>+        LIST_FOREACH(spec, s, p->specs) {
>+                good = path_spec_check_good(s, initial);
>+
>+                if (good)
>+                        break;
>+        }
>+
>+        return good;
>+}
>+
> static void path_enter_running(Path *p) {
>         _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
>         int r;
>@@ -477,6 +493,11 @@ static void path_enter_running(Path *p) {
>         if (unit_stop_pending(UNIT(p)))
>                 return;
> 
>+        if (!p->has_dependency && path_check_good(p, true)) {
>+                if (unit_add_dependency(UNIT(p), UNIT_WANTS, UNIT_TRIGGER(UNIT(p)), false) >= 0)
>+                        p->has_dependency = true;
>+        }
>+
>         r = manager_add_job(UNIT(p)->manager, JOB_START, UNIT_TRIGGER(UNIT(p)),
>                             JOB_REPLACE, true, &error, NULL);
>         if (r < 0)
>@@ -497,22 +518,6 @@ fail:
>         path_enter_dead(p, PATH_FAILURE_RESOURCES);
> }
> 
>-static bool path_check_good(Path *p, bool initial) {
>-        PathSpec *s;
>-        bool good = false;
>-
>-        assert(p);
>-
>-        LIST_FOREACH(spec, s, p->specs) {
>-                good = path_spec_check_good(s, initial);
>-
>-                if (good)
>-                        break;
>-        }
>-
>-        return good;
>-}
>-
> static void path_enter_waiting(Path *p, bool initial, bool recheck) {
>         int r;
> 
>@@ -538,6 +543,11 @@ static void path_enter_waiting(Path *p, bool initial, bool recheck) {
>                         return;
>                 }
> 
>+        if (p->has_dependency) {
>+                unit_delete_dependency(UNIT(p), UNIT_WANTS, UNIT_TRIGGER(UNIT(p)), false);
>+                p->has_dependency = false;
>+        }
>+
>         path_set_state(p, PATH_WAITING);
>         return;
> 
>diff --git a/src/core/path.h b/src/core/path.h
>index d2e91d7..0a9b1cf 100644
>--- a/src/core/path.h
>+++ b/src/core/path.h
>@@ -85,6 +85,7 @@ struct Path {
>         PathState state, deserialized_state;
> 
>         bool inotify_triggered;
>+        bool has_dependency;
> 
>         bool make_directory;
>         mode_t directory_mode;
>diff --git a/src/core/unit.c b/src/core/unit.c
>index a2f3728..e06b41c 100644
>--- a/src/core/unit.c
>+++ b/src/core/unit.c
>@@ -69,6 +69,33 @@ const UnitVTable * const unit_vtable[_UNIT_TYPE_MAX] = {
>         [UNIT_SCOPE] = &scope_vtable
> };
> 
>+const UnitDependency inverse_table[_UNIT_DEPENDENCY_MAX] = {
>+        [UNIT_REQUIRES] = UNIT_REQUIRED_BY,
>+        [UNIT_REQUIRES_OVERRIDABLE] = UNIT_REQUIRED_BY_OVERRIDABLE,
>+        [UNIT_WANTS] = UNIT_WANTED_BY,
>+        [UNIT_REQUISITE] = UNIT_REQUIRED_BY,
>+        [UNIT_REQUISITE_OVERRIDABLE] = UNIT_REQUIRED_BY_OVERRIDABLE,
>+        [UNIT_BINDS_TO] = UNIT_BOUND_BY,
>+        [UNIT_PART_OF] = UNIT_CONSISTS_OF,
>+        [UNIT_REQUIRED_BY] = _UNIT_DEPENDENCY_INVALID,
>+        [UNIT_REQUIRED_BY_OVERRIDABLE] = _UNIT_DEPENDENCY_INVALID,
>+        [UNIT_WANTED_BY] = _UNIT_DEPENDENCY_INVALID,
>+        [UNIT_BOUND_BY] = UNIT_BINDS_TO,
>+        [UNIT_CONSISTS_OF] = UNIT_PART_OF,
>+        [UNIT_CONFLICTS] = UNIT_CONFLICTED_BY,
>+        [UNIT_CONFLICTED_BY] = UNIT_CONFLICTS,
>+        [UNIT_BEFORE] = UNIT_AFTER,
>+        [UNIT_AFTER] = UNIT_BEFORE,
>+        [UNIT_ON_FAILURE] = _UNIT_DEPENDENCY_INVALID,
>+        [UNIT_REFERENCES] = UNIT_REFERENCED_BY,
>+        [UNIT_REFERENCED_BY] = UNIT_REFERENCES,
>+        [UNIT_TRIGGERS] = UNIT_TRIGGERED_BY,
>+        [UNIT_TRIGGERED_BY] = UNIT_TRIGGERS,
>+        [UNIT_PROPAGATES_RELOAD_TO] = UNIT_RELOAD_PROPAGATED_FROM,
>+        [UNIT_RELOAD_PROPAGATED_FROM] = UNIT_PROPAGATES_RELOAD_TO,
>+        [UNIT_JOINS_NAMESPACE_OF] = UNIT_JOINS_NAMESPACE_OF,
>+};
>+
> static int maybe_warn_about_dependency(const char *id, const char *other, UnitDependency dependency);
> 
> Unit *unit_new(Manager *m, size_t size) {
>@@ -2171,33 +2198,6 @@ static int maybe_warn_about_dependency(const char *id, const char *other, UnitDe
> }
> 
> int unit_add_dependency(Unit *u, UnitDependency d, Unit *other, bool add_reference) {
>-
>-        static const UnitDependency inverse_table[_UNIT_DEPENDENCY_MAX] = {
>-                [UNIT_REQUIRES] = UNIT_REQUIRED_BY,
>-                [UNIT_REQUIRES_OVERRIDABLE] = UNIT_REQUIRED_BY_OVERRIDABLE,
>-                [UNIT_WANTS] = UNIT_WANTED_BY,
>-                [UNIT_REQUISITE] = UNIT_REQUIRED_BY,
>-                [UNIT_REQUISITE_OVERRIDABLE] = UNIT_REQUIRED_BY_OVERRIDABLE,
>-                [UNIT_BINDS_TO] = UNIT_BOUND_BY,
>-                [UNIT_PART_OF] = UNIT_CONSISTS_OF,
>-                [UNIT_REQUIRED_BY] = _UNIT_DEPENDENCY_INVALID,
>-                [UNIT_REQUIRED_BY_OVERRIDABLE] = _UNIT_DEPENDENCY_INVALID,
>-                [UNIT_WANTED_BY] = _UNIT_DEPENDENCY_INVALID,
>-                [UNIT_BOUND_BY] = UNIT_BINDS_TO,
>-                [UNIT_CONSISTS_OF] = UNIT_PART_OF,
>-                [UNIT_CONFLICTS] = UNIT_CONFLICTED_BY,
>-                [UNIT_CONFLICTED_BY] = UNIT_CONFLICTS,
>-                [UNIT_BEFORE] = UNIT_AFTER,
>-                [UNIT_AFTER] = UNIT_BEFORE,
>-                [UNIT_ON_FAILURE] = _UNIT_DEPENDENCY_INVALID,
>-                [UNIT_REFERENCES] = UNIT_REFERENCED_BY,
>-                [UNIT_REFERENCED_BY] = UNIT_REFERENCES,
>-                [UNIT_TRIGGERS] = UNIT_TRIGGERED_BY,
>-                [UNIT_TRIGGERED_BY] = UNIT_TRIGGERS,
>-                [UNIT_PROPAGATES_RELOAD_TO] = UNIT_RELOAD_PROPAGATED_FROM,
>-                [UNIT_RELOAD_PROPAGATED_FROM] = UNIT_PROPAGATES_RELOAD_TO,
>-                [UNIT_JOINS_NAMESPACE_OF] = UNIT_JOINS_NAMESPACE_OF,
>-        };
>         int r, q = 0, v = 0, w = 0;
>         Unit *orig_u = u, *orig_other = other;
> 
>@@ -2405,6 +2405,38 @@ int unit_add_two_dependencies_by_name_inverse(Unit *u, UnitDependency d, UnitDep
>         return r;
> }
> 
>+void unit_delete_dependency(Unit *u, UnitDependency d, Unit *other, bool remove_reference) {
>+        Unit *orig_u = u, *orig_other = other;
>+
>+        assert(u);
>+        assert(d >= 0 && d < _UNIT_DEPENDENCY_MAX);
>+        assert(other);
>+
>+        u = unit_follow_merge(u);
>+        other = unit_follow_merge(other);
>+
>+        /* We won't allow dependencies on ourselves. We will not
>+         * consider them an error however. */
>+        if (u == other) {
>+                maybe_warn_about_dependency(orig_u->id, orig_other->id, d);
>+                return;
>+        }
>+
>+        set_remove(u->dependencies[d], other);
>+
>+        if (inverse_table[d] != _UNIT_DEPENDENCY_INVALID && inverse_table[d] != d)
>+                set_remove(other->dependencies[inverse_table[d]], u);
>+
>+        if (remove_reference) {
>+                set_remove(u->dependencies[UNIT_REFERENCES], other);
>+                set_remove(other->dependencies[UNIT_REFERENCED_BY], u);
>+        }
>+
>+        unit_check_unneeded(other);
>+        unit_add_to_dbus_queue(u);
>+        return;
>+}
>+
> int set_unit_path(const char *p) {
>         /* This is mostly for debug purposes */
>         if (setenv("SYSTEMD_UNIT_PATH", p, 0) < 0)
>diff --git a/src/core/unit.h b/src/core/unit.h
>index 19fa2f0..6026303 100644
>--- a/src/core/unit.h
>+++ b/src/core/unit.h
>@@ -468,6 +468,8 @@ int unit_add_two_dependencies_by_name(Unit *u, UnitDependency d, UnitDependency
> int unit_add_dependency_by_name_inverse(Unit *u, UnitDependency d, const char *name, const char *filename, bool add_reference);
> int unit_add_two_dependencies_by_name_inverse(Unit *u, UnitDependency d, UnitDependency e, const char *name, const char *path, bool add_reference);
> 
>+void unit_delete_dependency(Unit *u, UnitDependency d, Unit *other, bool remove_reference);
>+
> int unit_add_exec_dependencies(Unit *u, ExecContext *c);
> 
> int unit_choose_id(Unit *u, const char *name);
>-- 
>2.2.2
>

Actions: View | Splinter Review

Attachments on bug 87287: 111110 | 112339

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.