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

Revision history for this message
Richard Hansen (rhansen) wrote :

> > I don't fully understand the interaction between the kernel,
> > Plymouth, and X when it comes to virtual terminals, but setting
> > use-vt=false for all seats would not match the above behavior.
> > I'm not sure what would happen if I did do -sharevts without vtXX
> > on all seats.
>
> Firstly, it seems no-one really understands VTs and more than we
> need to get rid of them as fast as we can :)

I can't wait until kmscon [1] is done. :)

[1] http://www.freedesktop.org/wiki/Software/kmscon/

>
> OK, I think I understand what the X.org code does now.
> - We should always provide a vtXX option to X - if we don't, then X
> will get a new VT or use the active VT if run with -sharevts. It's
> safest for us to explicitly tell X rather than it get it wrong
> (e.g. if you VT switched during creating the second X server).

I think we should always provide vtXX unless -sharevts is used, in
which case we should not provide vtXX. (This is the conclusion you
reached below, but I just wanted to be clear.)

> - You can't have more than one X server on the same VT without using
> -sharevts. This is because each X server would listen for VT
> changes and conflict with each other. In the multi-seat case only
> one of the X servers does this.
> - In multi-seat, only seat0 can switch to another console (e.g. a
> text console). This seat needs to drop the input and output
> devices when switched away from.
> - In multi-seat, seat1-N don't change input or output when a VT
> switch occurs. So they don't need to monitor VT switching and
> have -sharevts passed to them.
> - If -sharevts is set, X.Org basically does nothing with the
> VT. This kind of contradicts my first point that we should set the
> VT because it probably doesn't matter what VT is picked.

X mostly does nothing with the VT, but it does get info about the
current VT. Thus, I think:

- VT switching must happen before instances of X -sharevts are
  started.

And another point:
- All instances of X (and Mir and...) must be on the same VT. If not,
  then you can only use one seat at a time (e.g., to use seat0 you hit
  Ctrl-Alt-F7 and to use seat1 you hit Ctrl-Alt-F8).

>
> In conclusion, I think we might just be able to do this:
>
> [Seat:0]
> xdg-seat=seat0
>
> [Seat:1]
> xdg-seat=seat1
>
> And for seat0 we run X with:
> X vtXX
>
> And for seat1 we run X with:
> X -sharevts
>
> We can determine this with:
> if (strcmp (xdg_seat, "seat0") == 0)
> x_server_local_set_vt (x_server, vt);
> else
> x_server_local_set_sharevts (x_server, TRUE);

Here's what I propose:

  * use-vt=auto (default): If seat0 (or XDG seat name is NULL),
    switch to the automatically chosen VT, pass vtXX to X, and do not
    pass -sharevts to X. Otherwise (not seat0), do not switch VTs, do
    not pass vtXX to X, but do pass -sharevts to X.
  * use-vt=true: Switch to the automatically chosen VT, pass vtXX to
    X, and do not pass -sharevts to X.
  * use-vt=false: Do not switch VTs, do not pass vtXX to X, but do
    pass -sharevts to X.
  * use-vt=<integer>: Switch to the specified VT, pass vt<integer> to
    X, and do not pass -sharevts to X.

This gives the default behavior we want while giving users the ability
to manually override if needed. Thoughts?

>
> Note we should also call seat_set_can_switch(seat, FALSE) for seats
> that are not "seat0" - since they can't VT switch.

I have a patch in the works to call set_seat_can_switch() using the
value of the org.freedesktop.login1.Seat.CanMultiSession dbus
property, but unfortunately there's a problem: If can_switch is true
on a seat, then when you log out LightDM won't respawn a greeter on
that seat. I was going to look into this more after getting this
branch merged.

>
> > I should note that unset xdg-seat and xdg-seat=seat0 are not
> > equivalent. xdg-seat=seat0 causes '-seat seat0' to be passed to
> > X, but unset xdg-seat does not.
> >
> > From browsing the X source code, passing '-seat seat0' to X
> > appears to be a no-op, but:
> > * I'm not 100% certain
> > * the X man page does not guarantee that '-seat seat0' is a
> > no-op
> > * I didn't want to introduce any behavior change from the
> > current defaults (which is also why I don't derive the
> > XDG_SEAT name from the title of the [Seat:foo] section in
> > lightdm.conf)
>
> I'd say we should be optimistic that X will support the -seat option
> if xdg-seat is provided. Since the X command line interface is not
> standard or introspectable lets just assume the best and add an
> option later if required (e.g. xserver-supports-seat=false).

OK, sounds good.

« Back to merge proposal