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

[JackNewman12]

I believe this might be related to my previous ticket #339 where /enabled is a symlink to a read-writeable directory. Touching .conf files does not seem to cause a reload of the service and I don't see any do_change() debug.

If I touch a file in /etc/finit.d/ I get the excepted do_change() callback (using a modified image that has a r/w rootfs)

I am guessing here the directory iwatches needs to have slightly different flags or realpath() so that it monitors the correct path. https://github.com/troglobit/finit/blob/a47abfdc168a843ce81379805502cdd2f372b230/src/conf.c#L1157-L1162

[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 :)

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