Reload service on touched .conf file not working in symlinked /available

[JackNewman12]

Reload service on touched .conf file not working in symlinked /available

[troglobit]

This change caused a regression in Finit:

From commit 414a49d:

When the inotify callback is called for a file that has been removed,
the call to realpath() obviously fails (ENOENT). This fix only takes
the regular case into account, files removed from a symlinked enabled
directory is not handled -- needs more work for that.

As it says, I've not made any attempts at fixing the code for your use-case, @JackNewman12, but I thought it worth mentioning here as a heads-up for you.

[troglobit]

Yup, correct.

I fail to recall why it looks like it does rn ... does it work better if you drop the IN_DONT_FOLLOW?

[JackNewman12]

I fiddled around with the flags for a bit but couldn't get it working. I
I think the issue is the symlink destination doesn't exist that early in the boot so the inotify probably fails. (At least trying to do an inotifyd command on a bad symlink seems to fail).

I think perhaps I might need to have an event on the symlink update to add an additional watch.
'IN_DONT_FOLLOW' should be correct I believe.

[JackNewman12]

Yeah. If I hack in a mkdir very early on so that the symlink is valid then it seems to work.

[troglobit]

Aha, of course! Yeah, that's a bit tricky, but you have the early hook points perhaps?

[JackNewman12]

I was thinking about that just before I left work but in my (very brief) look it wasn't obvious to me when finit has setup those conf watches vs the hooks.
I think the script would have to be part of HOOK_MOUNT_POST, but according to the bootstrap finit has already loaded all the *.conf files.

Edit: Actually a few more seconds of looking and it seems like this should work.

[troglobit]

Yeah, a hook script on HOOK_BASEFS_UP should do the trick.

[JackNewman12]

Thanks! HOOK_BASEFS_UP works perfectly.

Although it looks like conf_changed() converts the path to a realpath before doing a conf_find(), but do_change() would end up adding the non-real path to the change list. So touching files would never cause an HUP.

I just quickly added this minimal patch to get it working for me.

diff --git a/src/conf.c b/src/conf.c
--- a/src/conf.c
+++ b/src/conf.c
@@ -1040,30 +1040,40 @@ static void drop_changes(void)
 static int do_change(char *dir, char *name, uint32_t mask)
 {
 	char fn[strlen(dir) + strlen(name) + 2];
+	char *rp;
 	struct conf_change *node;
 
 	paste(fn, sizeof(fn), dir, name);
 	dbg("path: %s mask: %08x", fn, mask);
 
-	node = conf_find(fn);
+	rp = realpath(fn, NULL);
+	if (!rp)
+		return 0;
+
+	node = conf_find(rp);
 	if (node) {
 		dbg("Event already registered for %s ...", name);
+		free(rp);
 		return 0;
 	}
 
 	node = malloc(sizeof(*node));
-	if (!node)
+	if (!node) {
+		free(rp);
 		return 1;
+	}
 
-	node->name = strdup(fn);
+	node->name = strdup(rp);
 	if (!node->name) {
 		free(node);
+		free(rp);
 		return 1;
 	}
 
-	dbg("Event registered for %s, mask 0x%x", fn, mask);
+	dbg("Event registered for %s, mask 0x%x", rp, mask);
 	TAILQ_INSERT_HEAD(&conf_change_list, node, link);
 
+	free(rp);
 	return 0;
 }

[troglobit]

Nice catch, would you like to make PR for that? Because that patch looks good to go for me :)