<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/112294/">https://git.reviewboard.kde.org/r/112294/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 28th, 2014, 10:59 a.m. UTC, <b>Oswald Buddenhagen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/112294/diff/2/?file=186612#file186612line1397" style="color: black; font-weight: bold; text-decoration: underline;">kdm/backend/dm.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1397</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">link</span><span class="o">-></span><span class="n">displayType</span> <span class="o">=</span> <span class="n">dLocal</span> <span class="o">|</span> <span class="n">dPermanent</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On March 29th, 2014, 7:54 p.m. UTC, <b>Stefan Brüns</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On March 30th, 2014, 8:52 a.m. UTC, <b>Oswald Buddenhagen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On March 30th, 2014, 6:23 p.m. UTC, <b>Stefan Brüns</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On March 31st, 2014, 9:54 a.m. UTC, <b>Oswald Buddenhagen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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 ...) </pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 28th, 2014, 10:59 a.m. UTC, <b>Oswald Buddenhagen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/112294/diff/2/?file=186612#file186612line1409" style="color: black; font-weight: bold; text-decoration: underline;">kdm/backend/dm.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1409</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">strcmp</span><span class="p">(</span><span class="o">*</span><span class="n">iter_name</span><span class="p">,</span> <span class="s">"seat0"</span><span class="p">)</span> <span class="o">==</span> <span class="mi">0</span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 28th, 2014, 10:59 a.m. UTC, <b>Oswald Buddenhagen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/112294/diff/2/?file=186613#file186613line79" style="color: black; font-weight: bold; text-decoration: underline;">kdm/backend/server.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">prepareServerArgv(struct display *d, const char *args)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">79</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="p">(</span><span class="n">argv</span> <span class="o">=</span> <span class="n">parseArgs</span><span class="p">(</span><span class="n">argv</span><span class="p">,</span> <span class="s">"-layout"</span><span class="p">)))</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">why the redundant supply of the seat as a layout?</pre>
</blockquote>
<p>On March 29th, 2014, 6:08 p.m. UTC, <b>Stefan Brüns</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">There are no config matches taking the "seat" into account, so the only possibility to apply a specific config is to use "layout".</pre>
</blockquote>
<p>On March 30th, 2014, 8:52 a.m. UTC, <b>Oswald Buddenhagen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On March 30th, 2014, 6:23 p.m. UTC, <b>Stefan Brüns</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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).</pre>
</blockquote>
<p>On March 31st, 2014, 9:54 a.m. UTC, <b>Oswald Buddenhagen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">IMHO this is material for a second patchset. Secondary seats with reserve displays will require much more integration with logind, and this will be major surgery. Auto-login on secondary seats would be nice to have (and you can use it as long as you only have one secondary seat in addition to seat0), but I prefer to postpone this.</pre>
<br />
<p>- Stefan</p>
<br />
<p>On April 6th, 2014, 10:01 p.m. UTC, Stefan Brüns wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kde-workspace and Oswald Buddenhagen.</div>
<div>By Stefan Brüns.</div>
<p style="color: grey;"><i>Updated April 6, 2014, 10:01 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kde-workspace
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Single seat system, several multiseat systems</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>cmake/modules/CMakeLists.txt <span style="color: grey">(117b3a5)</span></li>
<li>cmake/modules/FindSystemd.cmake <span style="color: grey">(PRE-CREATION)</span></li>
<li>kdm/ConfigureChecks.cmake <span style="color: grey">(b61fd90)</span></li>
<li>kdm/backend/CMakeLists.txt <span style="color: grey">(25f383f)</span></li>
<li>kdm/backend/client.c <span style="color: grey">(26bb0b4)</span></li>
<li>kdm/backend/dm.h <span style="color: grey">(b2f8c61)</span></li>
<li>kdm/backend/dm.c <span style="color: grey">(77a2ef7)</span></li>
<li>kdm/backend/server.c <span style="color: grey">(d8dd6f3)</span></li>
<li>kdm/backend/session.c <span style="color: grey">(0e7901c)</span></li>
<li>kdm/config-kdm.h.cmake <span style="color: grey">(3e8912d)</span></li>
<li>plasma/desktop/applets/windowlist/plasma-applet-windowlist.desktop <span style="color: grey">(50c9f66)</span></li>
<li>plasma/desktop/containments/desktop/plasma-containment-desktop.desktop <span style="color: grey">(ce9fe38)</span></li>
<li>plasma/generic/wallpapers/image/plasma-wallpaper-image.desktop <span style="color: grey">(6484545)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/112294/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>