Fix check for systemd in modules that talk to logind



Hello fellow GNOMErs,

I recently analyzed the usage of logind in GNOME, Debian, and Ubuntu,
and noticed a problem with checks for both systemd init and logind:

Many modules do something like this:

| #include <systemd/sd-daemon.h>
| 
| if (sd_booted())
|     <talk to logind>

There is other software (not yet in GNOME, however) which actually
wants to talk to systemd init or systemd journal, which also uses
sd_booted().

sd_booted() by and large checks whether /sys/fs/cgroup/systemd/
exists, so modules like gnome-shell just replicate this test in e. g.
JavaScript. But this is imprecise:

 * Upstream systemd supports building without logind
   (--disable-logind), for embedded use cases.

   In this case, /sys/fs/cgroup/systemd/ exists, but logind doesn't.

 * At least Debian and Ubuntu will soon use logind without necessarily
   the systemd init parts, as they support other init systems [1]. For
   those, /sys/fs/cgroup/systemd/ exists as logind uses these cgroups
   as well. So for the majority of cases that we have today,
   sd_booted() actually happens to work and succeeds if logind is
   available, but not systemd init.

However, sd_booted() was never really meant to succeed for the second
case, i. e. where systemd is not actually the init system. While the
majority of software that uses this actually talks to logind [2], a
discussion between Lennart Poettering, Kay Sievers (both systemd
upstream), Michael Biebl, Tollef Fog Heen (systemd Debian
maintainers), and me led to the consensus that sd_booted() should be
fixed to do what it actually was meant to be, i. e. check whether
systemd is being used as init. This happened upstream in [3], thus
this behaviour will change in systemd >= 199.

That means that we need to fix software which wants to talk to logind.
In short, above sd_booted() check should be replaced with

| if (access("/run/systemd/seats/", F_OK) >= 0)
|     <talk to logind>

If you use that multiple times, you might want to define a macro:

| #define LOGIND_AVAILABLE() (access("/run/systemd/seats/", F_OK) >= 0)

(See [4] for the full details)

If your project carries a copy of sd_daemon.[hc], you should update
this to the current systemd git master version, or link to
libsystemd-daemon. Also, if your code already calls something like
sd_uid_get_*(), or builds a D-BUS connection to
org.freedesktop.login1, you can just use the success/failure of this
of course, instead of doing an additional LOGIND_AVAILABLE() check.

This affects the following GNOME modules:

  gnome-shell gnome-session gnome-screensaver gdm gnome-system-monitor

and the following software around/beneath GNOME:

  pulseaudio accountsservice upower udisks dbus

I will file bugs with patches for those in the next days. IMHO they
are unintrusive enough to be included into GNOME 3.8.1, but if you
want to wait until 3.9.x we can just distro-patch packages until then.

Thank you for your consideration!

Martin



[1] Pretty let's not discuss the details of this all over again. We
need to do the systemd transition in phases, and this way lies blood,
sweat, tears, and politics.

[2] http://people.canonical.com/~pitti/tmp/cgroup-sd_booted.txt
[3] http://cgit.freedesktop.org/systemd/systemd/commit/?id=66e411811b8090

[4] Lennart's original reply about the logind check:
After discussing this with Kay we propose the following:

As check whether systemd was actually booted:
 access("/run/systemd/system/", F_OK) >= 0

As check whether logind is in use:
 access("/run/systemd/seats/", F_OK) >= 0

This should work on all current and older systemd versions. (During the
phone call you already had suggested to check for /run/systemd, and I
was conservative, since /run might not actually be on tmpfs and thus
left from a previous boot -- however, that is actually irrelevant since
when systemd started to make use of /run it unconditionally enforced
tmpfs on it, hence /run/systemd was only ever available with /run as
tmpfs.).

I'll change sd_booted() to the first check, which should also clean
things up for us in the container case, since /run is always a fresh
instance, unlike the cgroup tree which is shared between the containers
and the host.

We won't provide you with a high-level API for the logind check though,
as that is out of scope for us upstream. However, I'd propose you to
simply define some macro or so, in the individual programs:

#define LOGIND_AVAILABLE() (access("/run/systemd/seats/", F_OK) >= 0)

And then you can replace this:

        if (sd_booted() > 0) {
                ...
        }

by this:

        if (LOGIND_AVAILABLE()) {
                ...
        }

These two checks are nicely symmetric and can easily be implemented in
shell too.

-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)

Attachment: signature.asc
Description: Digital signature



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]