[RFC] Declare that non-seat0 seats can support multisession.

[n3rdopolis]

As talked about on the mailing list, multisession support does
work on nont-seat0 seats, but logind undersells this ability
and onlt sets CanMultiSession on seat0 with TTYs. This is because
seat_can_tty acts the same way as seat_can_multi_session() as and
only reports that the seat has TTYs, which hasn't been needed to
support multi session switching on non-seat0 seats since I tested
it first in at least 2017

[poettering]

Hmm, what about seat0 there? Maybe we should return false for it, in case the VT subsystem is turned off in the kernel?

@dvdhrm any idea why you claimed that CanMultiSession was false for all non-seat0 sessions back when you added multi-session support without VT to that?

[dvdhrm]

Hmm, what about seat0 there? Maybe we should return false for it, in case the VT subsystem is turned off in the kernel?

@dvdhrm any idea why you claimed that CanMultiSession was false for all non-seat0 sessions back when you added multi-session support without VT to that?

I never changed the logic. This is still from the original commit. I merely did some refactoring. I think this originates in:

commit addedec48ba0ffc4472ef6d3b5a45c9d4239f1cd
Author: Lennart Poettering <lennart@poettering.net>
Date:   Tue Jan 3 21:47:54 2012 +0100

    logind: if we can't open /dev/tty0, assume there is no VT subsystem and don't pretend we could do VT switching

Anyway, I did definitely introduce multi-session support for all seats. No idea why I never adjusted can_multi_session(). But this function is also not used by anything, right? So I think it would mostly be a cosmetic change.

I do agree, the correct thing would be to return true unconditionally. We should probably still write CAN_MULTI_SESSION into the logind state file, to make sure sd-login can read it correctly. But the seat_can_multi_session() function should just be dropped entirely.

[n3rdopolis]

Are you sure it can be dropped? Because I see
src/login/logind-seat-dbus.c:static BUS_DEFINE_PROPERTY_GET(property_get_can_multi_session, "b", Seat, seat_can_multi_session);
unless I am overlooking something... ...or you want that to just be changed to true too.

[dvdhrm]

Are you sure it can be dropped? Because I see
src/login/logind-seat-dbus.c:static BUS_DEFINE_PROPERTY_GET(property_get_can_multi_session, "b", Seat, seat_can_multi_session);
unless I am overlooking something... ...or you want that to just be changed to true too.

I mostly meant changing seat_can_multi_session() to return true unconditionally, and then taking it from there and simplifying the callers. But feel free to leave it around. I am not sure whether sd-bus allows returning constants from BUS_DEFINE_PROPERTY. And I definitely did not check all the call sites.

The only thing we need to be careful about is sd-login, since we don't want it to break during live-updates. So I would keep writing the CAN_MULTI_SESSION property in logind and I would keep sd-login as it is.

[n3rdopolis]

ah. makes sense. I have it changing seat_can_multi_session() now, to return true, so is this what you want? Or should I change something else like the commit message or something?

[dvdhrm]

ah. makes sense. I have it changing seat_can_multi_session() now, to return true, so is this what you want? Or should I change something else like the commit message or something?

That is up to the systemd maintainers to decide. I recommend dropping the [RFC] prefix, describing the discussion-outcome in your commit-message and simplifying the call-sites if possible.

[n3rdopolis]

Should I close this, and open a new one with the change message? not sure how in github to change the commit message...

[poettering]

just force push the new version of your commit over the existing branch

[n3rdopolis]

Ok, I think I figured it out

[poettering]

Could you add a comment saying "All our seats can do multi-session these days" or so to the function, to indicate this is mostly a historical artifact?

[n3rdopolis]

Let me know if that comment is good enough

[dvdhrm]

Looks good! Thanks a lot!

[poettering]

@dvdhrm thanks for the review!