[systemd-devel] [RFC] logind: introduce session "positions"
Shawn Landden
shawn at churchofgit.com
Sun Dec 1 09:02:47 PST 2013
On Sun, Dec 1, 2013 at 3:43 AM, David Herrmann <dh.herrmann at gmail.com> wrote:
> logind has no concept of session ordering. Sessions have a unique name,
> some attributes about the capabilities and that's already it. There is
> currently no stable+total order on sessions. If we use the logind API to
> switch between sessions, we are faced with an unordered list of sessions
> we have no clue of.
>
> This used to be no problem on seats with VTs or on seats with only a
> single active session. However, with the introduction of multi-session
> capability for seats without VTs, we need to find a way to order sessions
> in a stable way.
>
> This patch introduces session "positions". A position is a simple integer
> assigned to a session which is never changed implicitly (currently, we
> also don't change it explicitly, but that may be changed someday). For
> seats with VTs, we force the position to be the same as the VTnr. Without
> VTs, we simply find the lowest unassigned number and use it as position.
> If position-assignment fails or if, for any reason, we decide to not
> assign a position to a session, the position is set to 0 (which is treated
> as invalid position).
> During session_load() or if two sessions have the same VTnr, we may end up
> with two sessions with the same position (this shouldn't happen, but lets
> be fail-safe in case some other part of the stack fails). This case is
> dealt with gracefully by ignoring any session but the first session
> assigned to the position. Thus, session->pos is a hint, seat->positions[i]
> is the definite position-assignment. Always verify both match in case you
> need to modify them!
>
> Additionally, we introduce SwitchTo(unsigned int) on the seat-dbus-API.
> You can call it with any integer value != 0 and logind will try to switch
> to the request position. If you implement a compositor or any other
> session-controller, you simply watch for ctrl+alt+F1 to F12 and call
> SwitchTo(Fx). logind will figure a way out deal with this number.
> For convenience, we also introduce SwitchToNext/Previous(). It should be
> called on ctrl+alt+Left/Right (like the kernel-console used to support).
This has some conflict with workspaces, but not in gnome-shell (w/o
gnome-tweak-tool)
as there workspaces are all vertical. I personally like the idea.
>
> Note that the public API (SwitchTo*()) is *not* bound to the underlying
> logic that is implemented now. We don't export "session-positions" on the
> dbus/C API! They are an implementation detail. Instead, the SwitchTo*()
> API is supposed to be a hint to let logind choose the session-switching
> logic. Any foreground session-controller is free to enumerate/order
> existing sessions according to their needs and call Session.Activate()
> manually. But the SwitchTo*() API provides a uniform behavior across
> session-controllers.
>
> Background: Session-switching keys depend on the active keymap. The XKB
> specification provides the XKB_KEY_XF86Switch_VT_1-12 key-symbols which
> have to be mapped by all keymaps to allow session-switching. It is usually
> bound to ctrl+alt+Fx but may be set differently. A compositor passes any
> keyboard input to XKB before passing it to clients. In case a key-press
> invokes the XKB_KEY_XF86Switch_VT_x action, the keypress is *not*
> forwarded to clients, but instead a session-switch is scheduled.
>
> This actually prevents us from handling these keys outside of the session.
> If an active compositor has a keymap with a different mapping of these
> keys, and logind itself tries to catch these combinations, we end up with
> the key-press sent to the compositor's clients *and* handled by logind.
> This is *bad* and we must avoid this. The only situation where a
> background process is allowed to handle key-presses is debugging and
> emergency-keys. In these cases, we don't care for keymap mismatches and
> accept the double-event. Another exception is unmapped keys like
> PowerOff/Suspend (even though this one is controversial).
>
> Future ideas: As this commit-msg isn't long enough, yet, some notes on
> future ideas. The current position-assignment is compatible with the
> legacy VT numbers. However, it is a rather outdated way of addressing
> sessions. Instead, we can make use of session-classes of logind. We
> already tag session with one of the classes "greeter", "user",
> "background", "lock-screen". So one of my ideas is to make
> "position-assignment" a "per-class" thing. And instead of mapping F1-F12
> directly to the positions, we map it as follows:
> - F1: Activate the last-spawned session in the "greeter" class. Usually,
> only a single greeter should be active, but in case
> systemd-welcomed runs and gdm is spawned later, this will switch to
> gdm (actually gdm.service should stop systemd-welcomed.service but
> lets be overly pedantic here).
> - F2: Activate the session from the "user" class which has been active
> last. So if you switch to F1 and back to F2, you're guaranteed to
> get back to your last active user-session.
> - F3-F11: Direct mapping to "user" sessions. So F3 maps to the
> user-session with position 3, F11 to position 11 (or apply a
> "-2" offset, so F3=>1 and F11=>9..)
> - F12: Switch to the last-spawned session in the "emergency" class. This
> doesn't exist, yet, but we could start sessions showing the
> kernel-log or an emergency-console here.
>
> Inside of each group, you can use SwitchToNext/Previous() to switch to the
> next/previous session. As only the "user" group has a way of directly
> addressing sessions, this is needed for the "greeter" and "emergency"
> groups. Note that both groups are expected to only have one session, so
> this is really just a fallback for odd cases. However, for the "user"
> group the Next/Previous can come in quite handy.
Emergency sessions are light weight enough that we could auto-spawn them like
we currently do with gettys on VTs.
>
> Comments?
> ---
> src/login/logind-seat-dbus.c | 56 ++++++++++++++++++++++++++
> src/login/logind-seat.c | 96 ++++++++++++++++++++++++++++++++++++++++++++
> src/login/logind-seat.h | 8 ++++
> src/login/logind-session.c | 13 ++++++
> src/login/logind-session.h | 1 +
> 5 files changed, 174 insertions(+)
>
> diff --git a/src/login/logind-seat-dbus.c b/src/login/logind-seat-dbus.c
> index 23f975b..ff9e2e4 100644
> --- a/src/login/logind-seat-dbus.c
> +++ b/src/login/logind-seat-dbus.c
> @@ -235,6 +235,59 @@ static int method_activate_session(sd_bus *bus, sd_bus_message *message, void *u
> return sd_bus_reply_method_return(message, NULL);
> }
>
> +static int method_switch_to(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) {
> + Seat *s = userdata;
> + unsigned int to;
> + int r;
> +
> + assert(bus);
> + assert(message);
> + assert(s);
> +
> + r = sd_bus_message_read(message, "u", &to);
> + if (r < 0)
> + return r;
> +
> + if (!to)
> + return -EINVAL;
> +
> + r = seat_switch_to(s, to);
> + if (r < 0)
> + return r;
> +
> + return sd_bus_reply_method_return(message, NULL);
> +}
> +
> +static int method_switch_to_next(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) {
> + Seat *s = userdata;
> + int r;
> +
> + assert(bus);
> + assert(message);
> + assert(s);
> +
> + r = seat_switch_to_next(s);
> + if (r < 0)
> + return r;
> +
> + return sd_bus_reply_method_return(message, NULL);
> +}
> +
> +static int method_switch_to_previous(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) {
> + Seat *s = userdata;
> + int r;
> +
> + assert(bus);
> + assert(message);
> + assert(s);
> +
> + r = seat_switch_to_previous(s);
> + if (r < 0)
> + return r;
> +
> + return sd_bus_reply_method_return(message, NULL);
> +}
> +
> const sd_bus_vtable seat_vtable[] = {
> SD_BUS_VTABLE_START(0),
>
> @@ -250,6 +303,9 @@ const sd_bus_vtable seat_vtable[] = {
>
> SD_BUS_METHOD("Terminate", NULL, NULL, method_terminate, 0),
> SD_BUS_METHOD("ActivateSession", "s", NULL, method_activate_session, 0),
> + SD_BUS_METHOD("SwitchTo", "u", NULL, method_switch_to, 0),
> + SD_BUS_METHOD("SwitchToNext", NULL, NULL, method_switch_to_next, 0),
> + SD_BUS_METHOD("SwitchToPrevious", NULL, NULL, method_switch_to_previous, 0),
>
> SD_BUS_VTABLE_END
> };
> diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
> index b1a5ec3..928e2c5 100644
> --- a/src/login/logind-seat.c
> +++ b/src/login/logind-seat.c
> @@ -79,6 +79,7 @@ void seat_free(Seat *s) {
>
> hashmap_remove(s->manager->seats, s->id);
>
> + free(s->positions);
> free(s->state_file);
> free(s);
> }
> @@ -270,6 +271,60 @@ int seat_set_active(Seat *s, Session *session) {
> return 0;
> }
>
> +int seat_switch_to(Seat *s, unsigned int num) {
> + /* Public session positions skip 0 (there is only F1-F12). Maybe it
> + * will get reassigned in the future, so return error for now. */
> + if (!num)
> + return -EINVAL;
> +
> + if (num >= s->position_count || !s->positions[num])
> + return -EINVAL;
> +
> + return session_activate(s->positions[num]);
> +}
> +
> +int seat_switch_to_next(Seat *s) {
> + unsigned int start, i;
> +
> + if (!s->position_count)
> + return -EINVAL;
> +
> + start = 1;
> + if (s->active && s->active->pos > 0)
> + start = s->active->pos;
> +
> + for (i = start + 1; i < s->position_count; ++i)
> + if (s->positions[i])
> + return session_activate(s->positions[i]);
> +
> + for (i = 1; i < start; ++i)
> + if (s->positions[i])
> + return session_activate(s->positions[i]);
> +
> + return -EINVAL;
> +}
> +
> +int seat_switch_to_previous(Seat *s) {
> + unsigned int start, i;
> +
> + if (!s->position_count)
> + return -EINVAL;
> +
> + start = 1;
> + if (s->active && s->active->pos > 0)
> + start = s->active->pos;
> +
> + for (i = start - 1; i > 0; --i)
> + if (s->positions[i])
> + return session_activate(s->positions[i]);
> +
> + for (i = s->position_count - 1; i > start; --i)
> + if (s->positions[i])
> + return session_activate(s->positions[i]);
> +
> + return -EINVAL;
> +}
> +
> int seat_active_vt_changed(Seat *s, unsigned int vtnr) {
> Session *i, *new_active = NULL;
> int r;
> @@ -403,6 +458,46 @@ int seat_stop_sessions(Seat *s) {
> return r;
> }
>
> +void seat_evict_position(Seat *s, Session *session) {
> + unsigned int pos = session->pos;
> +
> + session->pos = 0;
> +
> + if (!pos)
> + return;
> +
> + if (pos < s->position_count && s->positions[pos] == session)
> + s->positions[pos] = NULL;
> +}
> +
> +void seat_claim_position(Seat *s, Session *session, unsigned int pos) {
> + /* with VTs, the position is always the same as the VTnr */
> + if (seat_has_vts(s))
> + pos = session->vtnr;
> +
> + if (!GREEDY_REALLOC0(s->positions, s->position_count, pos + 1))
> + return;
> +
> + seat_evict_position(s, session);
> +
> + session->pos = pos;
> + if (pos > 0 && !s->positions[pos])
> + s->positions[pos] = session;
> +}
> +
> +static void seat_assign_position(Seat *s, Session *session) {
> + unsigned int pos;
> +
> + if (session->pos > 0)
> + return;
> +
> + for (pos = 1; pos < s->position_count; ++pos)
> + if (!s->positions[pos])
> + break;
> +
> + seat_claim_position(s, session, pos);
> +}
> +
> int seat_attach_session(Seat *s, Session *session) {
> assert(s);
> assert(session);
> @@ -413,6 +508,7 @@ int seat_attach_session(Seat *s, Session *session) {
>
> session->seat = s;
> LIST_PREPEND(sessions_by_seat, s->sessions, session);
> + seat_assign_position(s, session);
>
> seat_send_changed(s, "Sessions", NULL);
>
> diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
> index 1254268..9e21e3a 100644
> --- a/src/login/logind-seat.h
> +++ b/src/login/logind-seat.h
> @@ -41,6 +41,9 @@ struct Seat {
> Session *pending_switch;
> LIST_HEAD(Session, sessions);
>
> + Session **positions;
> + size_t position_count;
> +
> bool in_gc_queue:1;
> bool started:1;
>
> @@ -55,12 +58,17 @@ int seat_load(Seat *s);
>
> int seat_apply_acls(Seat *s, Session *old_active);
> int seat_set_active(Seat *s, Session *session);
> +int seat_switch_to(Seat *s, unsigned int num);
> +int seat_switch_to_next(Seat *s);
> +int seat_switch_to_previous(Seat *s);
> int seat_active_vt_changed(Seat *s, unsigned int vtnr);
> int seat_read_active_vt(Seat *s);
> int seat_preallocate_vts(Seat *s);
>
> int seat_attach_session(Seat *s, Session *session);
> void seat_complete_switch(Seat *s);
> +void seat_evict_position(Seat *s, Session *session);
> +void seat_claim_position(Seat *s, Session *session, unsigned int pos);
>
> bool seat_has_vts(Seat *s);
> bool seat_is_seat0(Seat *s);
> diff --git a/src/login/logind-session.c b/src/login/logind-session.c
> index beaa601..c24e573 100644
> --- a/src/login/logind-session.c
> +++ b/src/login/logind-session.c
> @@ -125,6 +125,7 @@ void session_free(Session *s) {
> if (s->seat->pending_switch == s)
> s->seat->pending_switch = NULL;
>
> + seat_evict_position(s->seat, s);
> LIST_REMOVE(sessions_by_seat, s->seat->sessions, s);
> }
>
> @@ -231,6 +232,9 @@ int session_save(Session *s) {
> if (s->seat && seat_has_vts(s->seat))
> fprintf(f, "VTNR=%u\n", s->vtnr);
>
> + if (!s->vtnr)
> + fprintf(f, "POS=%u\n", s->pos);
> +
> if (s->leader > 0)
> fprintf(f, "LEADER=%lu\n", (unsigned long) s->leader);
>
> @@ -266,6 +270,7 @@ int session_load(Session *s) {
> _cleanup_free_ char *remote = NULL,
> *seat = NULL,
> *vtnr = NULL,
> + *pos = NULL,
> *leader = NULL,
> *type = NULL,
> *class = NULL,
> @@ -290,6 +295,7 @@ int session_load(Session *s) {
> "REMOTE_USER", &s->remote_user,
> "SERVICE", &s->service,
> "VTNR", &vtnr,
> + "POS", &pos,
> "LEADER", &leader,
> "TYPE", &type,
> "CLASS", &class,
> @@ -350,6 +356,13 @@ int session_load(Session *s) {
> if (!s->seat || !seat_has_vts(s->seat))
> s->vtnr = 0;
>
> + if (pos && s->seat) {
> + unsigned int npos;
> +
> + safe_atou(pos, &npos);
> + seat_claim_position(s->seat, s, npos);
> + }
> +
> if (leader) {
> k = parse_pid(leader, &s->leader);
> if (k >= 0)
> diff --git a/src/login/logind-session.h b/src/login/logind-session.h
> index ee93101..d724e20 100644
> --- a/src/login/logind-session.h
> +++ b/src/login/logind-session.h
> @@ -69,6 +69,7 @@ struct Session {
> Manager *manager;
>
> char *id;
> + unsigned int pos;
> SessionType type;
> SessionClass class;
>
> --
> 1.8.4.2
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
More information about the systemd-devel
mailing list