Re: [systemd-devel] [PATCH] path: conditionally depend on the triggered unit

[Lennart Poettering]

On Mon, 29.12.14 14:07, Jouke Witteveen (j.wittev...@gmail.com) wrote:

heya,

sorry for the late review, still busy catching up with all the queued
mail.

 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.

Hmm, I see how this could be useful, but the patch like this is not
OK. Dependencies in systemd are strictly additive. The only way to get
rid of dependencies is by doing a full reload of the configuration. We
coalesce dependencies from a variety of sources, and if we really
wanted to allow dynamically dropping deps again we'd couldn't do that
anymore and would have to track precisely the reason for each dep that
is added.

I wonder if we can find a better way to handle this...

So far the issue never came up since we expected that services would
exit-in-idle on their own, so that we never need an external trigger
to automatically shut something down, but the daemons would do so on
their own.

Can you elaborate on your precise usecase for this?

I mean, even if we added the facility you propose this would require
the services to operate in a certain way to be fully safe and
race-free. But if that's the case, then they might as well shut down
on their own at the right time?

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] path: conditionally depend on the triggered unit

[Jouke Witteveen]

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.
---

This fixes #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 varnamePathChanged=/varname and
 varnamePathModified=/varname./para
 
+paraIf the triggered unit has
+varnameStopWhenUnneeded=true/varname
+then it is not stopped as long as a path
+exists (in case of
+varnamePathExists=/varname and
+varnamePathExistsGlob=/varname) or
+a directory is not empty (in case of
+varnameDirectoryNotEmpty=/varname).
+/para
+
 paraIf the path itself or any of the
 containing directories are not
 accessible, commandsystemd/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,