Support for PID files created in sub-directories to /run

[xhebox]

I want some services waiting for dbus, but no luck even with the following patch:

--- finit-3.1/plugins/dbus.c    2018-05-06 22:31:17.640483182 +0800
+++ finit-3.1/plugins/dbus.c    2018-05-06 22:31:17.640483182 +0800
@@ -58,7 +58,7 @@
        erase("/var/run/dbus/pid");
 
        /* Register service with Finit */
-       snprintf(line, sizeof(line), "[S12345] log:prio:daemon.info,tag:dbus %s %s -- %s", cmd, ARGS, DESC);
+       snprintf(line, sizeof(line), "[S12345] pid:!/var/run/dbus/pid log:prio:daemon.info,tag:dbus %s %s -- %s", cmd, ARGS, DESC);
        if (service_register(SVC_TYPE_SERVICE, line, global_rlimit, NULL))
                _pe("Failed registering %s", DAEMON);
        free(cmd);

It just shows:

...
svc/bin/chronyd               on
svc/bin/dhcpcd                on
svc/bin/udevd                 on
...

Seems, only /run/xxx.pid could trigger the cond, any idea?

[troglobit]

Ah, yes that's a limitation of the current pidfile.so plugin. It only monitors /var/run for files with the .pid suffix.

It should be fairly simple to extend it with automatically adding a file watcher for /var/run/$DIR when a new $DIR is created, and for those directories watch for pid and pidfile names as well.

Your patch above may also work if you change it to use pid:/var/run/dbus.pid, that way Finit will create and remove the file dbus.pid when it starts and stops the dbus service. Your other services can then depend on it like this <svc/usr/bin/dbus-daemon> ... I think :)

[xhebox]

Yeah, i could modify the pid path of dbus, too. In my view this should be improved rather than just using workarounds, so i issued this :).

[troglobit]

Thanks, I'll look into it later on then :)

[xhebox]

This is my hack patch. It should be very easy to modify and merge :).

--- finit-3.1/plugins/pidfile.c	2018-01-23 18:29:58.000000000 +0800
+++ finit-3.1/plugins/pidfile.c	2018-01-23 18:29:58.000000000 +0800
@@ -32,11 +32,20 @@
 #include "plugin.h"
 #include "service.h"
 
+#include <stdbool.h>
+struct wd {
+	TAILQ_ENTRY(wd) link;
+	char *path;
+	int wd;
+};
+
 struct context {
 	int fd;
-	int wd;
+	TAILQ_HEAD(tailhead, wd) wd_list;
 };
 
+static struct context pidfile_ctx;
+
 static char *mkcond(char *buf, size_t len, char *nm)
 {
 	snprintf(buf, len, "svc%s%s", nm[0] != '/' ? "/" : "", nm);
@@ -62,7 +70,41 @@
 	for (ev = (void *)ev_buf;
 	     sz > (ssize_t)sizeof(*ev);
 	     len = sizeof(*ev) + ev->len, ev = (void *)ev + len, sz -= len) {
-		if (!ev->mask || !strstr(ev->name, ".pid"))
+		if (!ev->mask)
+			continue;
+
+		if (ev->mask & (IN_CREATE | IN_ATTRIB | IN_MODIFY | IN_ISDIR)) {
+			// find current watcher
+			struct wd *np;
+			TAILQ_FOREACH(np, &pidfile_ctx.wd_list, link)
+				if (np->wd == ev->wd)
+					break;
+
+			// make the path
+			int len = snprintf(NULL, 0, "%s/%s", np->path, ev->name)+1;
+			char *path = malloc(len);
+			snprintf(path, len, "%s/%s", np->path, ev->name);
+
+			// check if we already have a watcher for this dir
+			int wd = inotify_add_watch(pidfile_ctx.fd, path, IN_CREATE | IN_ATTRIB | IN_DELETE | IN_MODIFY);
+			bool new = true;
+			TAILQ_FOREACH(np, &pidfile_ctx.wd_list, link)
+				if (np->wd == wd) {
+					new = false;
+					break;
+				}
+
+			// if no, watch it
+			if (new) {
+				struct wd *new_wd = malloc(sizeof(struct wd));
+				new_wd->path = path;
+				new_wd->wd = wd;
+				TAILQ_INSERT_HEAD(&pidfile_ctx.wd_list, new_wd, link);
+			} else
+				free(path);
+		}
+
+		if (!strstr(ev->name, ".pid"))
 			continue;
 
 		svc = svc_find_by_pidfile(ev->name);
@@ -110,10 +152,9 @@
 	} while (restart);
 }
 
-static void pidfile_init(void *arg)
+static void pidfile_init()
 {
 	char *path;
-	struct context *ctx = arg;
 
 	/*
 	 * The bootmisc plugin is responsible for setting up /var/run.
@@ -126,27 +167,29 @@
 		return;
 	}
 
-	ctx->wd = inotify_add_watch(ctx->fd, path, IN_CREATE | IN_ATTRIB | IN_DELETE | IN_MODIFY);
-	free(path);
+	TAILQ_INIT(&pidfile_ctx.wd_list);
+	struct wd *wd = malloc(sizeof(struct wd));
+	wd->path = path;
+	wd->wd = inotify_add_watch(pidfile_ctx.fd, path, IN_CREATE | IN_ATTRIB | IN_DELETE | IN_MODIFY);
 
-	if (ctx->wd < 0) {
+	if (wd->wd < 0) {
 		_pe("inotify_add_watch()");
-		close(ctx->fd);
+		close(pidfile_ctx.fd);
+		free(path);
 		return;
 	}
 
+	TAILQ_INSERT_HEAD(&pidfile_ctx.wd_list, wd, link);
 	_d("pidfile monitor active");
 }
 
-static struct context pidfile_ctx;
-
 /*
  * We require /var/run to be set up before calling pidfile_init(),
  * so the bootmisc plugin must run first.
  */
 static plugin_t plugin = {
 	.name = __FILE__,
-	.hook[HOOK_BASEFS_UP]  = { .arg = &pidfile_ctx, .cb = pidfile_init },
+	.hook[HOOK_BASEFS_UP]  = { .cb = pidfile_init },
 	.hook[HOOK_SVC_RECONF] = { .cb = pidfile_reconf },
 	.io = {
 		.cb    = pidfile_callback,
@@ -169,7 +212,14 @@
 
 PLUGIN_EXIT(plugin_exit)
 {
-	inotify_rm_watch(pidfile_ctx.fd, pidfile_ctx.wd);
+	struct wd *np = TAILQ_FIRST(&pidfile_ctx.wd_list), *npn = NULL;
+	while (np != NULL) {
+		npn = TAILQ_NEXT(np, link);
+		free(np->path);
+		inotify_rm_watch(pidfile_ctx.fd, np->wd);
+		free(np);
+		np = npn;
+	}
 	close(pidfile_ctx.fd);
 
 	plugin_unregister(&plugin);

[troglobit]

Cool, nicely done! :-)

I really have to get back to hacking in userspace again, a bit stuck atm. in kernel land because of work. Only doing some minor artifact restoration for recreational purposes. Cannot promise any timeline, but I suspect I can merge this in a month or so.

Cheers!

[xhebox]

Update: finit do not watch rename() event. This led to a missing of a.pid.tmp -> a.pid. So, we need to add IN_MOVED_TO flag for all calls:

+       wd->wd = inotify_add_watch(pidfile_ctx.fd, path, IN_CREATE | IN_ATTRIB | IN_DELETE | IN_MODIFY | IN_MOVED_TO);

There's also another problem, i'll stick it there when i am available.

Update: OK, i find it, we also need a IN_MOVED_TO flag there other than those inotify_add_watch:

 if (ev->mask & (IN_CREATE | IN_ATTRIB | IN_MODIFY | IN_MOVED_TO)) {

[troglobit]

This bug/feature took a little too long for me to fix, sorry about that! We finally needed the same feature at $DAYJOB, so I took the opportunity to fix the general case. Still haven't tested the actual case of dbus-daemon, but several others with a similar setup.

What's still needed is adding 'pid:!/run/dbus/pid' to the service stanza.