[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