Review Request 112294: Implement multi-seat support in KDM

Sun May 25 10:03:48 BST 2014

On May 24, 2014, 11:46 a.m., Stefan Brüns wrote:
> > regarding DynamicDisplays and your magic backwards compat handling ... i think the seat-encoded config stuff i insisted on makes that redundant, after all. suppose we do it like that:
> > 
> >   StaticServers=:0, at seat1_:4, at seat2_:7
> >   ReserveServers=:1,:2, at seat1_:5, at seat2_:8
> > 
> > local displays without a seat are implicitly on seat0. that's your special case, without much magic.
> > if kdm is started with systemd, *all* static local servers actually go straight into the dynamic state and need to be activated by systemd.
> > if kdm is compiled with systemd support but none is running, displays on seats != 0 are simply eliminated (with a notice to the log).
> > if kdm is compiled without systemd support, seats != 0 are rejected (with an error to the log). if somebody feels like implementing actual support for that, they can.
> > as before, the ReserveServers are only meant for demo purposes. don't worry about them.
> > 
> > would that work?
> 
> Stefan Brüns wrote:
>     Hm, not exactly sure if I understand your proposal correctly - do you mean get rid of DynamicServers and use the StaticServers for either "legacy" static servers or for seats provided by systemd-logind, dependent on logind availability?
>     
>     If yes, I am generally for it. But this would also require that the StaticServers list does not require (but allow) any special syntax for seats. Seats may be totally dynamic, added by e.g. plugging a DisplayLink USB adapter, with a name dependent on the used USB port.
>     
>     So given the following partial config:
>     ---
>     StaticServers=:0,:1,:2_ at seat-foo
>     ReserveServers=:3,:4,:5_ at seat-foo
>     [X-:*_ at seat-foo-Core]
>     ServerArgsLocal=-layout Seat-Foo
>     ---
>     and these seats: 'seat0', 'seat-some-machine-generated-specifier', 'seat-foo' --
>     
>     In case of compiled in support for systemd-logind all StaticServers would be flagged with status dynamic instead of notRunning.
>     [a: logind running]                seat-foo will use DISPLAY=:2, whereas the other two will use one of :0 and :1, which one is unspecified. :2 will use "-layout Seat-Foo" as args, the other two will use the default args (or section [X-:*-Core])
>     [b: logind not running]            print a warning/error in kdm.log. Change the status from dynamic to notRunning for all seats without a seat specifier, this will spawn displays/greeters for :0 and :1, and if there where [X-:{0,1}-Core] config sections, these would be used.
>     
>     [c: No logind support compiled in] print a warning for each display in StaticServers/ReserveServers matching :\d+_ at seat.* and remove from the Static|ReserveServers list, i.e. :0 and :1 will be started.
>     
>     In case ReserveServers support is added later, seat-foo would use :5 and the same [X-:*_ at seat-foo-Core] config section as display :2.

yes, you understood me right.
but as you point out correctly, i didn't consider the case of plugging unknown seats. how about this:

StaticServers=:0,@*:1,:2_ at seat-foo

:0 is a fixed binding, as no seat is implicitly seat0 - exact match.
seat-foo will get :2 - exact match.
seat-some-machine-generated-specifier would get :1 - wildcard match.

for the b case, you slightly modified my proposal, but your version is actually better suited for the wildcard handling. we just define that a legacy-compatible config must have explicit entries without a seat spec (which default to seat0 with systemd). that way we don't need to exclude seat0 from wildcard matching.

note that in the config section headers, :0 and :0_ at seat0 are equivalent.
implementation-wise, that may mean that it is best to null out the seat0 name asap, i.e. when parsing the config and when receiving seat names from systemd. at first sight, that seems to contradict the previous paragraph, but in a way it doesn't ... displays with an explict seat0 spec would just turn into displays with no seat spec. that would have the advantage of needing the fewest conditionals further down the line, which would also mean the fewest #ifdefs.

now comes the next problem: ServerArgs* needs to pass the seat to the server somehow.

[X-:*_@*-Core]
ServerArgsLocal=-seat @SEAT@ -layout @SEAT@

the -layout is actually irrelevant for unknown seats, as they can't have predefined layouts by definition.
i'm not sure yet whether i want to insist on passing -seat via the config. technically, it would be cleaner to take that knowledge about the server command line out of kdm. otoh, it already makes assumptions about how displays and vts are passed, and the seat is quite akin to that. oh, it also knows about auth files. and xdmcp queries. ok, forget the idea - you need no @SEAT@ magic. ^^

- Oswald

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/112294/#review58394
-----------------------------------------------------------

On May 18, 2014, 7:44 p.m., Stefan Brüns wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/112294/
> -----------------------------------------------------------
> 
> (Updated May 18, 2014, 7:44 p.m.)
> 
> 
> Review request for kde-workspace and Oswald Buddenhagen.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> -------
> 
> This patch implements dynamic multiseat in KDM. It follows the description in:
> http://www.freedesktop.org/wiki/Software/systemd/writing-display-managers/
> 
> In case systemd is no found at compile time, nothing changes. If logind is not running, nothing changes. If no additional seats have been configured (some Plugable USB-GPUs are automatically added as additional seats), nothing changes.
> 
> In case there are additional seats beyond seat0, a reserved display is promoted to a local static one (and demoted if the seat is removed) and a new X-Server/greeter is spawned.
> 
> The code has been tested extensively, with a combination of [Radeon dedicated GPU|Intel iGPU], [Intel iGPU|Displaylink USB GPU] and others. For history of this patch, see https://bugzilla.redhat.com/show_bug.cgi?id=884271 and https://bugzilla.redhat.com/show_bug.cgi?id=975079
> 
> 
> Diffs
> -----
> 
>   kdm/backend/dm.h b2f8c61 
>   kdm/backend/dm.c 77a2ef7 
>   kdm/backend/dpylist.c b650c2f 
>   kdm/backend/resource.c 38a8c70 
>   kdm/config.def 751c0ed 
>   kdm/kfrontend/kdm_config.c 368c8d1 
> 
> Diff: https://git.reviewboard.kde.org/r/112294/diff/
> 
> 
> Testing
> -------
> 
> Single seat system, several multiseat systems
> 
> 
> Thanks,
> 
> Stefan Brüns
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20140525/3297c941/attachment.htm>