Re: [systemd-devel] type=notify not working as intended

[Jouke Witteveen]

On Tue, Aug 15, 2017 at 5:59 AM, Kalpa Gunarathna  wrote:
> Hello all,
>
> I have scripted a service unit as follow.
>
> [Unit]
> Description="A test service"
> After=epmd.service epmd.socket
>
> [Service]
> ExecStart=/home/otpuser/bin/start
> Type=notify
> NotifyAccess=all
> WorkingDirectory=~
> User=otpuser
> Group=otp
> Restart=always
> RestartSec=3
>
> [Install]
> WantedBy=multi-user.target
>
>
> ExecStart directs to a shell script which starts a Erlang run_erl daemon.
> This daemon is forking a process which is the BEAM virtual machine. In the
> forked process Im sending "READY=1\nSTATUS=Initialized\nMAINPID="
> <> "\n to systemd via sd_notify. The problem is service
> is getting restarted constantly.
>
> journalctl give this
>
> abc.service: Service hold-off time over, scheduling restart.
> Stopped "A test service.".
>
>
> Is it required to send READY=1 from the daemon it self ? or can systemd
> allow forked process of the daemon to send REDAY=1 notification?
>
> It works when Type=forking and PIDFile is specified, but can we have
> Type=forking and without PIDFIle, notify READY=1 via forked process ?
>
> My systemd version
>
> $ systemd --version
> systemd 229
>

The service fails when the main process terminates before a READY=1
notification is received. Contrary to Reindl Haralds claim, this
notification is allowed to come from whatever process that is allowed
to send notifications, as controlled by NotifyAccess=. In order to not
give up on the service when the main process terminates, but wait for
the remaining processes in the cgroup, you should add
RemainAfterExit=yes to your unit.

Regards,
- Jouke
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Promoting a pull request

[Jouke Witteveen]

On Tue, Nov 22, 2016 at 11:55 AM, Martin Pitt <martin.p...@ubuntu.com> wrote:
> Jouke Witteveen [2016-11-22 11:44 +0100]:
>> I see no way of (trying to) removing the reviewed/needs-rework label
>> in the PR interface.
>
> I click on the cog next to "Labels" on the right, there I can "untick"
> labels. I've done so for #4259.

Thanks, Martin!

> So maybe it's just UI obfuscation, but perhaps label setting/removing
> is limited to project members?

I would guess it is. I can assign labels to my own projects, but not
to e.g. systemd. I would say this makes sense, but it leaves something
to be desired for contributing.

Regards,
- Jouke
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] Promoting a pull request

[Jouke Witteveen]

The contribution guidelines [1] state:

> After you have pushed a new version, try to remove the
> reviewed/needs-rework label. Also add a comment about the new version
> (no notification is sent just for the commits, so it's easy to miss the
> update without an explicit comment).

I see no way of (trying to) removing the reviewed/needs-rework label
in the PR interface. I feel that as a result, my PR (#4259) has fallen
off the radar. Is there anything I can do to sponsor my PR? I am still
interested in seeing it merged.

If it is indeed impossible to do anything with the assigned labels,
maybe the quoted part of the contibution guidelines should be updated.

Regards,
- Jouke

[1] https://github.com/systemd/systemd/blob/master/.github/CONTRIBUTING.md
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] Propagate reload from RELOADING=1 notifications

[Jouke Witteveen]

On Thu, Jan 1, 2015 at 7:15 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Tue, Dec 30, 2014 at 08:22:27PM +0100, Jouke Witteveen wrote:
 ---

 This fixes #87251

 This is actually important information that should be included in the
 commit message (i.e. above not below ---). We usually include the
 full url, since we also use distribution bug trackers and having the full
 url makes things easier to click. In this case:

 https://bugs.freedesktop.org/show_bug.cgi?id=87251

Will do. For consistency it is also better to not abort propagation if
one unit fails to reload, so I have a second version ready already.

  static void service_enter_reload_by_notify(Service *s) {
 +_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
 +int r;
 +
  assert(s);

  if (s-timeout_start_usec  0)
  service_arm_timer(s, s-timeout_start_usec);

 +r = manager_propagate_reload(UNIT(s)-manager, UNIT(s), 
 JOB_REPLACE, false, error);
 +if(r  0)
 +log_unit_warning(UNIT(s)-id, %s failed to schedule 
 propagation of reload: %s, UNIT(s)-id, bus_error_message(error, -r));
 +

 Let's say that a.service has PropagateReloadsTo=b.service, and a.service 
 provides
 the RELOADING=1 notification during a reload.
 What happens if a reload is requested with 'systemctl reload a', and systemd
 schedules a reload of a and b. Is it possible for b to be reloaded a second 
 time
 as a result of notification of a? This should not happen, have you verified 
 that
 this will not happen?

Isn't that just bad behavior? Sending a RELOADING=1 notification after
a reload is initiated? I guess if both service_enter_reload() and
service_enter_reload_by_notify() are called it is justified to
propagate two reloads. Before testing it might be nice to know what we
want :-).

Regards,
- Jouke
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] Propagate reload from RELOADING=1 notifications

[Jouke Witteveen]

---

This fixes #87251

 src/core/manager.c | 42 ++
 src/core/manager.h |  1 +
 src/core/service.c |  7 +++
 3 files changed, 50 insertions(+)

diff --git a/src/core/manager.c b/src/core/manager.c
index 9705e64..11cca17 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1226,6 +1226,48 @@ int manager_add_job_by_name(Manager *m, JobType type, 
const char *name, JobMode
 return manager_add_job(m, type, unit, mode, override, e, _ret);
 }
 
+int manager_propagate_reload(Manager *m, Unit *unit, JobMode mode, bool 
override, sd_bus_error *e) {
+int r;
+Transaction *tr;
+Iterator i;
+Unit *dep;
+
+
+assert(m);
+assert(unit);
+assert(mode  _JOB_MODE_MAX);
+
+tr = transaction_new(mode == JOB_REPLACE_IRREVERSIBLY);
+if (!tr)
+return -ENOMEM;
+
+SET_FOREACH(dep, unit-dependencies[UNIT_PROPAGATES_RELOAD_TO], i) {
+r = transaction_add_job_and_dependencies(tr, JOB_RELOAD, dep, 
NULL, false, override, false, false, mode == JOB_IGNORE_DEPENDENCIES, e);
+if (r  0) {
+log_unit_warning(dep-id,
+ Cannot add dependency reload job for 
unit %s, ignoring: %s,
+ dep-id, bus_error_message(e, r));
+
+if (e)
+sd_bus_error_free(e);
+
+goto tr_abort;
+}
+}
+
+r = transaction_activate(tr, m, mode, e);
+if (r  0)
+goto tr_abort;
+
+transaction_free(tr);
+return 0;
+
+tr_abort:
+transaction_abort(tr);
+transaction_free(tr);
+return r;
+}
+
 Job *manager_get_job(Manager *m, uint32_t id) {
 assert(m);
 
diff --git a/src/core/manager.h b/src/core/manager.h
index ab75f90..bc11f87 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -316,6 +316,7 @@ int manager_load_unit_from_dbus_path(Manager *m, const char 
*s, sd_bus_error *e,
 
 int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, bool 
force, sd_bus_error *e, Job **_ret);
 int manager_add_job_by_name(Manager *m, JobType type, const char *name, 
JobMode mode, bool force, sd_bus_error *e, Job **_ret);
+int manager_propagate_reload(Manager *m, Unit *unit, JobMode mode, bool force, 
sd_bus_error *e);
 
 void manager_dump_units(Manager *s, FILE *f, const char *prefix);
 void manager_dump_jobs(Manager *s, FILE *f, const char *prefix);
diff --git a/src/core/service.c b/src/core/service.c
index bfbe959..71c7bf6 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1496,11 +1496,18 @@ fail:
 }
 
 static void service_enter_reload_by_notify(Service *s) {
+_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
+int r;
+
 assert(s);
 
 if (s-timeout_start_usec  0)
 service_arm_timer(s, s-timeout_start_usec);
 
+r = manager_propagate_reload(UNIT(s)-manager, UNIT(s), JOB_REPLACE, 
false, error);
+if(r  0)
+log_unit_warning(UNIT(s)-id, %s failed to schedule 
propagation of reload: %s, UNIT(s)-id, bus_error_message(error, -r));
+
 service_set_state(s, SERVICE_RELOAD);
 }
 
-- 
2.2.1

___
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,