[systemd-devel] [PATCH weston] doc/systemd: system service example

[Matt Hoosier]

Hi Lennart,

On Mon, Dec 4, 2017 at 9:11 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> On Fri, 1 Dec 2017 18:25:35 +0100
> Lennart Poettering <lennart at poettering.net> wrote:
>
>> On Fr, 01.12.17 13:42, Pekka Paalanen (ppaalanen at gmail.com) wrote:
>>
>> > > > > This is racy, as the session ID is not really reliably predictable,
>> > > > > and is synthesized in different contexts in different ways, for
>> > > > > example depnding on whether audit is enabled in the kernel it might be
>> > > > > session-1.scope rather than session-c1.scope.
>> >
>> > > > If we could determine the bug doesn't exist anymore, that would be
>> > > > awesome and I could just drop this.
>> >
>> > Hi Lennart,
>> >
>> > taking a step back, the session-c1.scope directive is definitely not
>> > wanted and we should drop it. We should also use a custom PAM name
>> > setup. If we do that, is the service file otherwise good for
>> > guaranteeing:
>> >
>> > - a full user session setup (I presume we want it), specifically
>> >   XDG_RUNTIME_DIR being set up
>> >
>> > - running exclusively on a VT that is made current
>>
>> This really depends on how weston sets up a VT. I really don't know
>> Weston and what it does.
>
> Weston doesn't set up the VT, we rely on the systemd unit directives to
> set it up.
>
> Weston calls sd_pid_get_session(getpid()), sd_session_get_seat(), and wants
> sd_session_get_vt() to succeed and give a VT number. Then it connects
> to logind, wants TakeControl to succeed, and calls Activate. It uses
> TakeDevice to open the DRM KMS device and input devices. I think that's
> the start-up sequence, it also listens on signals from logind etc.
>
>> > - access to DRM and input devices via logind
>>
>> So, I can't comment on Weston really.
>
> No worries, that was more of a general question about whether the
> example unit file was making any unwarranted assumptions.
>
>> Here are brief (and possibly slightly out-of-date, but probably not)
>> notes on how to write display managers with logind:
>>
>> https://www.freedesktop.org/wiki/Software/systemd/writing-display-managers/
>
> Thanks, I had a quick read through. We expect the systemd unit to also
> set up PAM, Weston itself does not touch PAM.
>
>> > so that all these are already in place by the time the Weston process
>> > is started?
>> >
>> > As you can see from Martyn below, the first issue that prevented Weston
>> > from running was that XDG_RUNTIME_DIR was not set. Furthermore, this
>> > failure did not occur always, sometimes things just worked as we
>> > expected.
>>
>> So, as long as weston runs from a PAM enabled service (and its PAM
>> snippet pulls in pam_systemd) all should be good and race-free: as the
>> PAM session is set up XDG_RUNTIME_DIR will be made available and the
>> systemd --user instance is invoked in the background.
>
> This is exactly what we attempted with the User and PAMName directive,
> and it turned out to be racy somehow.
>
>> What currently is not supported is to run things independently of any
>> session, i.e. run weston as systemd --user service with nothing that
>> creates a session in the first place. In that case XDG_RUNTIME_DIR
>> will not be set up, and no devices are made available to weston...
>
> Weston never was a --user service.
>
> As far as I know, there was also nothing that would manually attempt to
> start user at .service, the only trigger for that were the User and PAMName
> directives in the system weston.service.
>
>> > > > > > +# Set up a full user session for the user, required by Weston.
>> > > > > > +PAMName=login
>> > > > >
>> > > > > Piggy-backing on "login" is a bad idea. "login" is a text tool, and
>> > > > > thus the PAM rules for it usually pull in some TTY specific PAM
>> > > > > modules. YOu shoudl really use your own PAM fragment here, and
>> > > > > configure only the bits you need.
>> > > >
>> > >
>> > > Oh, so could it just be that we needed something other than
>> > > "PAMName=login"?
>> >
>> > What are they key bits in the PAM configuration we must have, and are
>> > there any often used bits we must not have to avoid the race Martyn
>> > describes?
>>
>> well, pam_systemd needs to be pulled in from it, that's the most
>> important thing. A PAM snippet that pulls in pam_systemd means you get
>> a session allcoated in logind, which in turn sets up XDG_RUNTIME_DIR
>> for you.
>
> Yes, it was, but apparently somewhere in the PAM stack or something it
> calls there was a race, which sometimes let Weston to start before
> XDG_RUNTIME_DIR environment variable was set, causing weston to fail.
>
> We all here were quite baffled on what could even be racing, unless it
> is possible that the weston process gets started in parallel with the
> PAM setup done by the User/PAMName in weston.service. We assumed that
> all the setup described in the systemd unit file would be guaranteed to
> complete before the actual process gets started.
>
> It seems our and your expectations are aligned. Maybe we should just
> forget about that race, remove the hacks that tried to work around it,
> and see if anyone ever sees the failure again. Maybe it was something
> very special on that one system alone.
>
>
> Thanks,
> pq

The approach that you and Pekka most recently put on record here:

* User=foo
* PAMName=weston

with a /etc/pam.d/weston that just does minimal stuff (enforce the
account exists and then execute pam_systemd.so for the session phase)
works well for me.

One thing I can't figure out though: using PAMName= causes the service
process's journal entries emitted by regular stdout and stderr not to
be visible with 'journalctl -u weston.service' anymore. Only the
messages coming internally from systemd ("Started Weston." and
similar) show in that journal.

I've tacked in StandardOutput=journal and StandardError=journal to
compensate for the StandardInput=tty-fail. The messages do make it
across to journald; you can view them with 'journalctl
/usr/bin/weston'. But somehow they're not associated with the system
unit weston.service anymore. Does using the PAMName= directive cause
the stdout/stderr messages to be reassigned to a user-session unit or
something?

Thanks,
Matt