> 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.
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. > On March 28, 2014, 10:59 a.m., Oswald Buddenhagen wrote: > > kdm/backend/server.c, line 79 > > <https://git.reviewboard.kde.org/r/112294/diff/2/?file=186613#file186613line79> > > > > why the redundant supply of the seat as a layout? > > Stefan Brüns wrote: > There are no config matches taking the "seat" into account, so the only > possibility to apply a specific config is to use "layout". > > Oswald Buddenhagen wrote: > something is wrong about this. > what does the -seat option do in the first place? if it is sufficient to > actually launch the server correctly, then that's all that should be done - > the user can manually specifiy ServerArgs if he needs to configure the layout > for some reason. otherwise every user is forced to actually have a "seatN" > layout in his config. > also, cannot the layout determine the seat? seems a bit stupid that these > are entirely orthogonal. > > Stefan Brüns wrote: > The seat option is used for matching devices from udev, i.e. input > devices, GPUs, ... It specifies *which* devices to use, but now *how to > configure* these. > > Unfortunately the config files lack a "MatchSeat" option, so the only > possible options are either specifying a config file (or directory) or a > layout. There is a patch from Oleg Samarin floating around which adds > MatchSeat, but this is not upstream so currently no option. > > If a layout is specified but not available, the default layout is chosen, > so no problem here. > > ServerArgs is no alternative as it is specific to a display, but the > seats will use a more or less random display, dependent on when they become > available (much like Reserve servers, which take the first display available). hrmpf. again somebody did half a job, and others need to make workarounds to make it usable. it may be actually possible to integrate this nicely with kdm's super-complex config system. it already supports a display-identifying mechanism, the so-called display class. you can have groups that look like [X-*:*_foos-Core]. you can even have StaticServers=:0_foos,:1_foos,:3_bars. one way to make use of this would be simply replacing the display class with @seat1 or something like that. the actual display class is pretty much meaningless for local displays anyway (it was introduced to group remote (predominantly xdmcp) displays, which could have different physical properties for example). another way would be making local displays look like @seat1:1. that makes a lot of sense - the seat kinda is the "host", even if of a different kind. this approach might be a bit more complex, though (it would need special handling of the host part, while just replacing the display class would be practically zero effort). that way one could even have static secondary seats (something people had hacked a decade ago already). and it would have potential for supporting reserve displays on secondary seats (ReserveServers=:1,:2,:3,@seat1:5,@seat1:6). DynamicServers could use both static (@seat1:4,@seat2:7) and dynamic (:4,:7) seat to display assignment. if the display class based approach was chosen, this would be :4_@seat1 and so on. this seems quite complex from the user perspective, but the default set would be pretty static, so one can put it into config.def to have genkdmconf produce the common skeleton. at some point we'd remove the template, and people could remove it from their old configs (as opposed to leaving it in forever or making a hard cutoff, which would be the options if -layout was hard-coded in kdm itself). note that integration with the config system is sort of necessary anyway - for example to make auto-login configurable on secondary seats. - Oswald ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112294/#review54418 ----------------------------------------------------------- On March 29, 2014, 5:38 p.m., Stefan Brüns wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/112294/ > ----------------------------------------------------------- > > (Updated March 29, 2014, 5:38 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 > ----- > > CMakeLists.txt d5c76d8 > cmake/modules/CMakeLists.txt 117b3a5 > cmake/modules/FindSystemd.cmake PRE-CREATION > kdm/backend/CMakeLists.txt 25f383f > kdm/backend/client.c 26bb0b4 > kdm/backend/dm.h b2f8c61 > kdm/backend/dm.c 77a2ef7 > kdm/backend/server.c d8dd6f3 > kdm/backend/session.c 0e7901c > > Diff: https://git.reviewboard.kde.org/r/112294/diff/ > > > Testing > ------- > > Single seat system, several multiseat systems > > > Thanks, > > Stefan Brüns > >
