Review Request 112294: Implement multi-seat support in KDM

[Oswald Buddenhagen]

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



kdm/backend/dm.c
<https://git.reviewboard.kde.org/r/112294/#comment40921>

    it would be more logical to have that one above the empty line, imo.
    
    also, you need a registerCloseOnFork() call.



kdm/backend/dm.c
<https://git.reviewboard.kde.org/r/112294/#comment40922>

    that should go into the if for clarity.
    
    more importantly, you also need an unregisterInput() and a clearCloseOnFork(). well, for formal correctness at least.



kdm/backend/dm.c
<https://git.reviewboard.kde.org/r/112294/#comment40926>

    this doesn't reflect what we discussed ...
    
    you are treating no seat being set as a wildcard. this doesn't match the idea that a legacy spec should automatically be assigned to seat0.
    
    the code here could still work, but you'd need to post-process the config, as i have pondered before. probably not a good idea, though.



kdm/backend/dm.c
<https://git.reviewboard.kde.org/r/112294/#comment40927>

    it just occured to me that early exiting the loop is actually the wrong thing to start with: StaticServers=:0,:1 (both implicitly on seat0) would not work this way.
    
    so you need to act upon all exact hits, and fall back to a single wildcard hit.



kdm/backend/dm.c
<https://git.reviewboard.kde.org/r/112294/#comment40924>

    this is actually tricky ... what if the display is still there but the seat changed? i don't think you are handling the transition (which would involve the 'raiser' state, iirc), so the display would come up again only after the next hup or replug.
    
    it's probably not worth it to handle the complexity, but maybe add a comment somewhere.



kdm/backend/dm.c
<https://git.reviewboard.kde.org/r/112294/#comment40925>

    this belongs into the if



kdm/backend/dm.c
<https://git.reviewboard.kde.org/r/112294/#comment40923>

    this being here makes the continue in line 1461 an infinite loop. move it back into the loop header and add a space and semicolon right after the label (cf. line 1406 in upstream).


the StaticServers doc still needs to be extended to reflect the new capabilities.

i pushed the off-by-one fixes so we have that out of the way.

- Oswald Buddenhagen


On May 29, 2014, 7:03 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 29, 2014, 7:03 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
> -----
> 
>   cmake/modules/CMakeLists.txt 117b3a5 
>   kdm/ConfigureChecks.cmake b61fd90 
>   kdm/backend/CMakeLists.txt 25f383f 
>   kdm/backend/client.c a2d06c2 
>   kdm/backend/dm.h b2f8c61 
>   kdm/backend/dm.c 77a2ef7 
>   kdm/backend/dpylist.c b650c2f 
>   kdm/backend/resource.c 38a8c70 
>   kdm/backend/server.c d8dd6f3 
>   kdm/config-kdm.h.cmake 3e8912d 
>   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/20140531/59ec2046/attachment.htm>