dbus plugin failing to make directories with mksubsys

[hongkongkiwi]

I have switched to used the dbus plugin (and some other custom plugins, qmi-proxy & mdevd).

I stopped using the dbus plugin before because it didn't seem to work for me, but now I realise why after some debugging.

I've found that on my system, for some unknown reason mksubsys function doesn't do anything. So it never creates the required dbus directories.... namely /run/dbus & /var/lib/dbus (added in my forked version of the dbus plugin) I've turned on debugging but I cannot see any error here.

Creating the directories manually upon boot, then restarting dbus makes it work just fine, or creating them during fstab mount with a helper.

[hongkongkiwi]

Here is my fork with a few extra dirs: https://github.com/hongkongkiwi/finit/blob/86e6322300fc46163cbf5f7bfd77592788468fa4/plugins/dbus.c#L74

[troglobit]

The mksubsys() function bails out early if it cannot find the user argument in the system. In the case of dbus it defaults to user messagebus:messagebus (user:group). If the group is missing, the function defaults to gid=0.

Do you have the messagebus user in /etc/passwd?

[hongkongkiwi]

Oh, you are totally correct, the user is dbus here rather than messagebus. I'm not sure why I'm not doing anything custom, but maybe just the version I'm using here uses that.

Funny, this was the first thing I checked, but since cat /etc/passwd | grep messagebus returned a result I didn't actually look closely at the user ;)

# cat /etc/passwd | grep messagebus
dbus:x:1001:1001:DBus messagebus user:/run/dbus:/bin/false

Can I suggest a small modification to mksubsys function? How about we make the directory first, then try the user/gid chown? It would be obvious then what failed as it would be owned by root but the directory would still be there. Or, alternatively add some more logging so the error is more obvious in debug mode.

Also, should we add a configure option to allow configuring the user/group/pid of the plugin? I see that they are setup as define's already, but I couldn't see a way to set them in configure.

[troglobit]

Ah, good you found the root cause! :)

Yes, I saw your modification to mksubsys(), that's a good idea. Initially I remember wanting to prevent the daemon from starting up on a half-baked system, but your approach is better I think. If you like, you can submit a pull request for that, including the logging when the user cannot be found.

I'd like to keep plugin options out of configure, the way to change it from the outside is to go:

./configure [... misc options ...] CPPFLAGS="-DDAEMONUSER=dbus"

However, I also noticed your renaming the defines, prefixing them with DBUS_. I liked that a lot, so if you like, that could be part of the same PR (separate commit, preferably).

[hongkongkiwi]

I'm not very familiar with this merging changes in a clean way. I have made other changes as well across multiple commits.... is there an easy way just to bring the changes in this specific file as a change request?

[troglobit]

Nah, it's hard work and heavy lifting still. If you're pressed for time I can look into it later this week?

[hongkongkiwi]

I'll send you a patch based on your code ;)

Actually this issue topic brings up a couple more questions....

So, I'm trying to use DBUS_ARGS with my buildroot config (I'll share my config a bit later in a repo incase you want to use it).

I've set it up so I can configure each plugin's arg's e.g. buildroot.

I'm a bit stuck on how to pass an argument with spaces using CPPFLAGS .... e.g. CPPFLAGS="-DDBUS_ARGS=--nofork --system --syslog-only"

How would you normally quote this? Should it be.... CPPFLAGS="-DDBUS_ARGS=\"--nofork --system --syslog-only\""

This is my only problem with using this style for passing arguments to plugins.

Specifically in my finit.mk file I have:

ifneq ($(BR2_PACKAGE_FINIT_PLUGIN_DBUS_BIN_ARGS),)
PLUGIN_CPPFLAGS += -DDBUS_ARGS="$(BR2_PACKAGE_FINIT_PLUGIN_DBUS_BIN_ARGS)"
endif

FINIT_CONF_OPTS += CPPFLAGS="$(PLUGIN_CPPFLAGS)"

Buildroot looses all the quoting here :/ so I'm a bit stumped on how to handle that special ARGS var.

[troglobit]

Gah, make + shell + escapes ... try with single quotes and \ ... that's my best bet. I can give it a whirl myself, later, but I think you'll eventually find a combo that "works".

Thanks btw, I'd appreciate an extracted patch on a side channel :-)

[troglobit]

There's an example of this escape madness in Buildroot package/mdadm/mdadm.mk:

MDADM_BUILD_OPTS = \
        CC=$(TARGET_CC) \
        COROSYNC=-DNO_COROSYNC \
        DLM=-DNO_DLM \
        CWFLAGS="" \
        CXFLAGS="$(MDADM_CXFLAGS)" \
        CPPFLAGS="$(TARGET_CPPFLAGS) -DBINDIR=\\\"/sbin\\\"" \
        CHECK_RUN_DIR=0

Maybe that helps ...

[hongkongkiwi]

Here's the dbus patch

Index: b/plugins/dbus.c
===================================================================
--- a/plugins/dbus.c
+++ b/plugins/dbus.c
@@ -35,16 +35,20 @@
 #include "service.h"
 #include "conf.h"

-#define DAEMON "dbus-daemon"
-#define ARGS   "--nofork --system --syslog-only"
-#define DESC   "D-Bus message bus daemon"
+#define DBUS_DAEMON "dbus-daemon"
+#define DBUS_ARGS   "--nofork --system --syslog-only"
+#define DBUS_DESC   "D-Bus message bus daemon"

-#ifndef DAEMONUSER
-#define DAEMONUSER "messagebus"
+#ifndef DBUS_DAEMONUSER
+#define DBUS_DAEMONUSER "messagebus"
 #endif

-#ifndef DAEMONPIDFILE
-#define DAEMONPIDFILE "/var/run/dbus/pid"
+#ifndef DBUS_DAEMONGROUP
+#define DBUS_DAEMONGROUP "messagebus"
+#endif
+
+#ifndef DBUS_DAEMONPIDFILE
+#define DBUS_DAEMONPIDFILE "/var/run/dbus/pid"
 #endif

 static void setup(void *arg)
@@ -58,26 +62,32 @@ static void setup(void *arg)
        return;
    }

-   cmd = which(DAEMON);
+   cmd = which(DBUS_DAEMON);
    if (!cmd) {
-       _d("Skipping plugin, %s is not installed.", DAEMON);
+       _d("Skipping plugin, %s is not installed.", DBUS_DAEMON);
        return;
    }

    prev =umask(0);

-   mksubsys("/var/run/dbus", 0755, DAEMONUSER, DAEMONUSER);
+  _d("Creating D-Bus Required Directories ...");
+  mksubsys("/var/run/dbus", 0755, DBUS_DAEMONUSER, DBUS_DAEMONGROUP);
+  mksubsys("/var/run/lock/subsys", 0755, DBUS_DAEMONUSER, DBUS_DAEMONGROUP);
+  mksubsys("/var/lib/dbus", 0755, DBUS_DAEMONUSER, DBUS_DAEMONGROUP);
+  mksubsys("/tmp/dbus", 0755, DBUS_DAEMONUSER, DBUS_DAEMONGROUP);
+
+  /* Generate machine id for dbus */
    if (whichp("dbus-uuidgen"))
        run_interactive("dbus-uuidgen --ensure", "Creating machine UUID for D-Bus");

    /* Clean up from any previous pre-bootstrap run */
-   remove(DAEMONPIDFILE);
+   remove(DBUS_DAEMONPIDFILE);

    /* Register service with Finit */
    snprintf(line, sizeof(line), "[S12345789] cgroup.system pid:!%s @%s:%s %s %s -- %s",
-        DAEMONPIDFILE, DAEMONUSER, DAEMONUSER, cmd, ARGS, DESC);
+        DBUS_DAEMONPIDFILE, DBUS_DAEMONUSER, DBUS_DAEMONGROUP, cmd, DBUS_ARGS, DBUS_DESC);
    if (service_register(SVC_TYPE_SERVICE, line, global_rlimit, NULL))
-       _pe("Failed registering %s", DAEMON);
+       _pe("Failed registering %s", DBUS_DAEMON);
    free(cmd);

    umask(prev);

[troglobit]

Thank you! <3

[troglobit]

There, finally merged in this :)