Review Request 112294: Implement multi-seat support in KDM
Stefan Brüns
stefan.bruens at rwth-aachen.de
Sun May 18 21:32:59 BST 2014
> On March 28, 2014, 10:59 a.m., Oswald Buddenhagen wrote:
> > kdm/backend/dm.c, line 1397
> > <https://git.reviewboard.kde.org/r/112294/diff/2/?file=186612#file186612line1397>
> >
> > that seems questionable to me. why are you re-defining the display to be permanent? when the seat goes away, kdm won't know what to do with it.
>
> Stefan Brüns wrote:
> When a seat goes away, rStopDisplay(d, DS_RESERVE) is called - maybe a "d->displayType = dLocal | dReserve" is missing for these cases.
> But as long as the seat exists, you want it to behave like a static display (restart server after session exit), so dPermanent is IMHO correct.
>
> Oswald Buddenhagen wrote:
> i see what you mean ...
> still, it seems wrong to change the display type just so. this is supposed to be a constant. this is of course a consequence of abusing reserve displays in the first place - technically, there should be a dDynamic display type. if having reserve displays on dynamic seats is technically possible at all, this would also need some thought.
>
> Stefan Brüns wrote:
> So, what approach to take here:
> 1. Its somewhat ugly, but it works, so use it.
> 2. Introduce a dDynamic display type, and make it behave like dPermanent.
> a) add a DynamicServers option for this
> b) create list of displays on the fly.
>
> Reserve displays on dynamic seats is not possible currently, you cannot change the VT. For this to work you need support for systemd-login, but this is more serious surgery.
>
> Oswald Buddenhagen wrote:
> i would prefer 2.a. it shouldn't be much work.
> i had shortly pondered 2.b myself, but then you'd need a new factory concept, and the configuration would be different, so i discarded it.
>
> Stefan Brüns wrote:
> Currently I have not implemented (2a), but still use (1). I still can do that, although I see no apparent gain here until there is a possibility to configure individual seats. If this is really required I will do that, but this will take another week (I can only do this in my spare time ...)
>
> Oswald Buddenhagen wrote:
> the gain is that it makes the system cleaner and more predictable.
> it should be really pretty much trivial to implement it (just follow the pattern for ReserveDisplays).
Added an option DynamicServers, displayType dDynamic and DisplayStatus dynamicReserve
> On March 28, 2014, 10:59 a.m., Oswald Buddenhagen wrote:
> > kdm/backend/dm.c, line 1409
> > <https://git.reviewboard.kde.org/r/112294/diff/2/?file=186612#file186612line1409>
> >
> > i prefer !strcmp(). repeats several times.
> >
> > also, this seems like a hack. it should be possible to properly query whether kdm should manage this display.
>
> Stefan Brüns wrote:
> 1. changed to !strcmp()
>
> 2. with the current patch all seats but seat0 which also have the CanGraphical property are picked up by KDM. Although this is not ideal, it is still a major improvement to the current situation. Currently I can not see any use case for a graphical seat not managed by KDM, but I see a use case for multiseat in general.
> In general this can be achieved if one extends the patch set with the extended configuration mentioned below (using display classes).
>
> I would prefer to keep the current patch set as is.
>
> Oswald Buddenhagen wrote:
> the question is why you are handling seat0 differently to start with.
>
> the obvious solution would seem having an empty StaticServers and pulling :0 from DynamicServers as well.
> now the question is what to do if systemd isn't actually there. does kdm have to do anything useful when it is configured with the incorrect assumption of systemd being there? i don't think so, given how central systemd is.
>
> and given the config stuff discussed below, you can have both:
> StaticServers=:0_ at seat0
> DynamicServers=:3_ at seat1
> i.e., kdm manages :0 as belonging to a seat, and if systemd *also* reports it, then kdm can ignore it without any hard-coded special cases.
> not sure about automatically passing the -seat option to static servers declared this way - what does a server do in such a case if there is no systemd running?
For backwards compatibility the display for seat0 will only be taken from the DynamicServers list is specified explicitly, e.g. DynamicServers=:0_ at seat0. This is consistent with how seat0 is handled specially by the X server.
- Stefan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/112294/#review54418
-----------------------------------------------------------
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/20140518/d4bd73cb/attachment.htm>
More information about the kde-core-devel
mailing list