Code review comment for lp:~rhansen/lightdm/multiseat

Nice work!

The xserver-vt option seems a bit odd - you don't want the configuration to define a particular VT, but instead if LightDM should assign them.

I think it would work better with:

[SeatDefaults]
use-vt=false

Then seat-xlocal.c and seat-unity.c wouldn't call xserver_local_set_vt () if this was false.

The logic in xserver-local.c using vt == 0 as share is a bit overly complicated - just make a new x_server_local_set_share_vt () and set that from seat-xlocal.c and seat-unity.c.

This needs a test case - the test cases are a bit hard to add and multi-seat have never been tested so I'm flexible here. Take a look at tests/scripts/autologin.conf and modify that to have two seats. We should also test more cases but that's a start. Lean on me if you have any questions and if it's too hard I can write the first tests. Ongoing features will all need tests.

Minor details (not blockers):

Seems an excessive use of parenthesis!
+ if (((vt < 0) && (active_vt >= vt_get_min ())) || (vt == active_vt))
why not?
+ if ((vt < 0 && active_vt >= vt_get_min ()) || vt == active_vt)

The sample configuration comments are very wordy, i.e.

8 +# xdg-seat=<name>: If specified, XDG_SEAT will be set to <name> (for
9 +# pam_systemd) and, for X sessions, '-seat <name>' will be passed
10 +# to X. By default, XDG_SEAT is set to 'seat0' and '-seat' is not
11 +# passed to X.

I'd do something more like:

xdg-seat=<name>: Set the XDG name for this seat

and lower down

xdg-seat=seat0 (implying that seat0 is the default)

Remember that this can be used by other display servers than just X. (This is also why xserver-vt is a bad config name)