Discussion:
[systemd-devel] [PATCH v2 01/10] logind: listen actively for session devices
David Herrmann
2013-09-17 15:39:54 UTC
Permalink
Session compositors need access to fbdev, DRM and evdev devices if they
control a session. To make logind pass them to sessions, we need to
listen for them actively.

However, we avoid creating new seats for non master-of-seat devices. Only
once a seat is created, we start remembering all other session devices. If
the last master-device is removed (even if there are other non-master
devices still available), we destroy the seat. This is the current
behavior, but we need to explicitly implement it now as there may be
non-master devices in the seat->devices list.

Unlike master devices, we don't care whether our list of non-master
devices is complete. We don't export this list but use it only as cache if
sessions request these devices. Hence, if a session requests a device that
is not in the list, we will simply look it up. However, once a session
requested a device, we must be notified of "remove" udev events. So we
must link the devices somehow into the device-list.

Regarding the implementation, we now sort the device list by the "master"
flag. This guarantees that master devices are at the front and non-master
devices at the tail of the list. Thus, we can easily test whether a seat
has a master device attached.
---
src/login/logind-device.c | 35 ++++++++++++++++++---
src/login/logind-device.h | 3 +-
src/login/logind-seat.c | 11 +++++--
src/login/logind-seat.h | 1 +
src/login/logind.c | 79 +++++++++++++++++++++++++++++++++++++++++------
src/login/logind.h | 6 ++--
6 files changed, 116 insertions(+), 19 deletions(-)

diff --git a/src/login/logind-device.c b/src/login/logind-device.c
index 51b1535..a9a9633 100644
--- a/src/login/logind-device.c
+++ b/src/login/logind-device.c
@@ -25,7 +25,7 @@
#include "logind-device.h"
#include "util.h"

-Device* device_new(Manager *m, const char *sysfs) {
+Device* device_new(Manager *m, const char *sysfs, bool master) {
Device *d;

assert(m);
@@ -48,6 +48,7 @@ Device* device_new(Manager *m, const char *sysfs) {
}

d->manager = m;
+ d->master = master;
dual_timestamp_get(&d->timestamp);

return d;
@@ -75,11 +76,16 @@ void device_detach(Device *d) {
LIST_REMOVE(Device, devices, d->seat->devices, d);
d->seat = NULL;

- seat_add_to_gc_queue(s);
- seat_send_changed(s, "CanGraphical\0");
+ if (!seat_has_master_device(s)) {
+ seat_add_to_gc_queue(s);
+ seat_send_changed(s, "CanGraphical\0");
+ }
}

void device_attach(Device *d, Seat *s) {
+ Device *i;
+ bool had_master;
+
assert(d);
assert(s);

@@ -90,7 +96,26 @@ void device_attach(Device *d, Seat *s) {
device_detach(d);

d->seat = s;
- LIST_PREPEND(Device, devices, s->devices, d);
+ had_master = seat_has_master_device(s);
+
+ /* We keep the device list sorted by the "master" flag. That is, master
+ * devices are at the front, other devices at the tail. As there is no
+ * way to easily add devices at the list-tail, we need to iterate the
+ * list to find the first non-master device when adding non-master
+ * devices. We assume there is only a few (normally 1) master devices
+ * per seat, so we iterate only a few times. */
+
+ if (d->master || !s->devices) {
+ LIST_PREPEND(Device, devices, s->devices, d);
+ } else {
+ LIST_FOREACH(devices, i, s->devices) {
+ if (!i->devices_next || !i->master) {
+ LIST_INSERT_AFTER(Device, devices, s->devices, i, d);
+ break;
+ }
+ }
+ }

- seat_send_changed(s, "CanGraphical\0");
+ if (!had_master && d->master)
+ seat_send_changed(s, "CanGraphical\0");
}
diff --git a/src/login/logind-device.h b/src/login/logind-device.h
index 3b15356..315f0e6 100644
--- a/src/login/logind-device.h
+++ b/src/login/logind-device.h
@@ -33,13 +33,14 @@ struct Device {

char *sysfs;
Seat *seat;
+ bool master;

dual_timestamp timestamp;

LIST_FIELDS(struct Device, devices);
};

-Device* device_new(Manager *m, const char *sysfs);
+Device* device_new(Manager *m, const char *sysfs, bool master);
void device_free(Device *d);
void device_attach(Device *d, Seat *s);
void device_detach(Device *d);
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 470d08b..2c60b8a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -448,10 +448,17 @@ bool seat_can_tty(Seat *s) {
return seat_is_vtconsole(s);
}

+bool seat_has_master_device(Seat *s) {
+ assert(s);
+
+ /* device list is ordered by "master" flag */
+ return !!s->devices && s->devices->master;
+}
+
bool seat_can_graphical(Seat *s) {
assert(s);

- return !!s->devices;
+ return seat_has_master_device(s);
}

int seat_get_idle_hint(Seat *s, dual_timestamp *t) {
@@ -499,7 +506,7 @@ int seat_check_gc(Seat *s, bool drop_not_started) {
if (seat_is_vtconsole(s))
return 1;

- return !!s->devices;
+ return seat_has_master_device(s);
}

void seat_add_to_gc_queue(Seat *s) {
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index c8ab17f..bd5390f 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -63,6 +63,7 @@ int seat_attach_session(Seat *s, Session *session);
bool seat_is_vtconsole(Seat *s);
bool seat_can_multi_session(Seat *s);
bool seat_can_tty(Seat *s);
+bool seat_has_master_device(Seat *s);
bool seat_can_graphical(Seat *s);

int seat_get_idle_hint(Seat *s, dual_timestamp *t);
diff --git a/src/login/logind.c b/src/login/logind.c
index 4ef92b8..29019c2 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -151,6 +151,8 @@ void manager_free(Manager *m) {

if (m->udev_seat_monitor)
udev_monitor_unref(m->udev_seat_monitor);
+ if (m->udev_device_monitor)
+ udev_monitor_unref(m->udev_device_monitor);
if (m->udev_vcsa_monitor)
udev_monitor_unref(m->udev_vcsa_monitor);
if (m->udev_button_monitor)
@@ -184,7 +186,7 @@ void manager_free(Manager *m) {
free(m);
}

-int manager_add_device(Manager *m, const char *sysfs, Device **_device) {
+int manager_add_device(Manager *m, const char *sysfs, bool master, Device **_device) {
Device *d;

assert(m);
@@ -195,10 +197,13 @@ int manager_add_device(Manager *m, const char *sysfs, Device **_device) {
if (_device)
*_device = d;

+ /* we support adding master-flags, but not removing them */
+ d->master = d->master || master;
+
return 0;
}

- d = device_new(m, sysfs);
+ d = device_new(m, sysfs, master);
if (!d)
return -ENOMEM;

@@ -373,7 +378,8 @@ int manager_process_seat_device(Manager *m, struct udev_device *d) {

} else {
const char *sn;
- Seat *seat;
+ Seat *seat = NULL;
+ bool master;

sn = udev_device_get_property_value(d, "ID_SEAT");
if (isempty(sn))
@@ -384,16 +390,23 @@ int manager_process_seat_device(Manager *m, struct udev_device *d) {
return 0;
}

- r = manager_add_device(m, udev_device_get_syspath(d), &device);
+ /* ignore non-master devices for unknown seats */
+ master = udev_device_has_tag(d, "master-of-seat");
+ if (!master && !(seat = hashmap_get(m->seats, sn)))
+ return 0;
+
+ r = manager_add_device(m, udev_device_get_syspath(d), master, &device);
if (r < 0)
return r;

- r = manager_add_seat(m, sn, &seat);
- if (r < 0) {
- if (!device->seat)
- device_free(device);
+ if (!seat) {
+ r = manager_add_seat(m, sn, &seat);
+ if (r < 0) {
+ if (!device->seat)
+ device_free(device);

- return r;
+ return r;
+ }
}

device_attach(device, seat);
@@ -762,6 +775,22 @@ int manager_dispatch_seat_udev(Manager *m) {
return r;
}

+static int manager_dispatch_device_udev(Manager *m) {
+ struct udev_device *d;
+ int r;
+
+ assert(m);
+
+ d = udev_monitor_receive_device(m->udev_device_monitor);
+ if (!d)
+ return -ENOMEM;
+
+ r = manager_process_seat_device(m, d);
+ udev_device_unref(d);
+
+ return r;
+}
+
int manager_dispatch_vcsa_udev(Manager *m) {
struct udev_device *d;
int r = 0;
@@ -1149,6 +1178,7 @@ static int manager_connect_udev(Manager *m) {

assert(m);
assert(!m->udev_seat_monitor);
+ assert(!m->udev_device_monitor);
assert(!m->udev_vcsa_monitor);
assert(!m->udev_button_monitor);

@@ -1169,6 +1199,33 @@ static int manager_connect_udev(Manager *m) {
if (epoll_ctl(m->epoll_fd, EPOLL_CTL_ADD, m->udev_seat_fd, &ev) < 0)
return -errno;

+ m->udev_device_monitor = udev_monitor_new_from_netlink(m->udev, "udev");
+ if (!m->udev_device_monitor)
+ return -ENOMEM;
+
+ r = udev_monitor_filter_add_match_subsystem_devtype(m->udev_device_monitor, "input", NULL);
+ if (r < 0)
+ return r;
+
+ r = udev_monitor_filter_add_match_subsystem_devtype(m->udev_device_monitor, "graphics", NULL);
+ if (r < 0)
+ return r;
+
+ r = udev_monitor_filter_add_match_subsystem_devtype(m->udev_device_monitor, "drm", NULL);
+ if (r < 0)
+ return r;
+
+ r = udev_monitor_enable_receiving(m->udev_device_monitor);
+ if (r < 0)
+ return r;
+
+ m->udev_device_fd = udev_monitor_get_fd(m->udev_device_monitor);
+ zero(ev);
+ ev.events = EPOLLIN;
+ ev.data.u32 = FD_DEVICE_UDEV;
+ if (epoll_ctl(m->epoll_fd, EPOLL_CTL_ADD, m->udev_device_fd, &ev) < 0)
+ return -errno;
+
/* Don't watch keys if nobody cares */
if (m->handle_power_key != HANDLE_IGNORE ||
m->handle_suspend_key != HANDLE_IGNORE ||
@@ -1545,6 +1602,10 @@ int manager_run(Manager *m) {
manager_dispatch_seat_udev(m);
break;

+ case FD_DEVICE_UDEV:
+ manager_dispatch_device_udev(m);
+ break;
+
case FD_VCSA_UDEV:
manager_dispatch_vcsa_udev(m);
break;
diff --git a/src/login/logind.h b/src/login/logind.h
index e9838a8..1a97351 100644
--- a/src/login/logind.h
+++ b/src/login/logind.h
@@ -57,9 +57,10 @@ struct Manager {
LIST_HEAD(User, user_gc_queue);

struct udev *udev;
- struct udev_monitor *udev_seat_monitor, *udev_vcsa_monitor, *udev_button_monitor;
+ struct udev_monitor *udev_seat_monitor, *udev_device_monitor, *udev_vcsa_monitor, *udev_button_monitor;

int udev_seat_fd;
+ int udev_device_fd;
int udev_vcsa_fd;
int udev_button_fd;

@@ -121,6 +122,7 @@ struct Manager {

enum {
FD_SEAT_UDEV,
+ FD_DEVICE_UDEV,
FD_VCSA_UDEV,
FD_BUTTON_UDEV,
FD_CONSOLE,
@@ -132,7 +134,7 @@ enum {
Manager *manager_new(void);
void manager_free(Manager *m);

-int manager_add_device(Manager *m, const char *sysfs, Device **_device);
+int manager_add_device(Manager *m, const char *sysfs, bool master, Device **_device);
int manager_add_button(Manager *m, const char *name, Button **_button);
int manager_add_seat(Manager *m, const char *id, Seat **_seat);
int manager_add_session(Manager *m, const char *id, Session **_session);
--
1.8.4
David Herrmann
2013-09-17 15:39:55 UTC
Permalink
If we want to track bus-names to allow exclusive resource-access, we need
a way to get notified when a bus-name is gone. We make logind watch for
NameOwnerChanged dbus events and check whether the name is currently
watched. If it is, we remove it from the watch-list (notification for
other objects can be added in follow-up patches).
---
src/login/logind-dbus.c | 17 ++++++++++++++++
src/login/logind.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++-
src/login/logind.h | 4 ++++
3 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index d052e74..5decf81 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -2459,6 +2459,23 @@ DBusHandlerResult bus_message_filter(
HASHMAP_FOREACH(session, m->sessions, i)
session_add_to_gc_queue(session);
}
+
+ } else if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS, "NameOwnerChanged")) {
+ const char *name, *old, *new;
+ char *key;
+
+ if (!dbus_message_get_args(message, &error,
+ DBUS_TYPE_STRING, &name,
+ DBUS_TYPE_STRING, &old,
+ DBUS_TYPE_STRING, &new,
+ DBUS_TYPE_INVALID)) {
+ log_error("Failed to parse NameOwnerChanged message: %s", bus_error_message(&error));
+ goto finish;
+ }
+
+ if (*old && !*new && (key = hashmap_remove(m->busnames, old))) {
+ free(key);
+ }
}

finish:
diff --git a/src/login/logind.c b/src/login/logind.c
index 29019c2..89e4eee 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -74,6 +74,7 @@ Manager *manager_new(void) {
m->users = hashmap_new(trivial_hash_func, trivial_compare_func);
m->inhibitors = hashmap_new(string_hash_func, string_compare_func);
m->buttons = hashmap_new(string_hash_func, string_compare_func);
+ m->busnames = hashmap_new(string_hash_func, string_compare_func);

m->user_units = hashmap_new(string_hash_func, string_compare_func);
m->session_units = hashmap_new(string_hash_func, string_compare_func);
@@ -82,7 +83,7 @@ Manager *manager_new(void) {
m->inhibitor_fds = hashmap_new(trivial_hash_func, trivial_compare_func);
m->button_fds = hashmap_new(trivial_hash_func, trivial_compare_func);

- if (!m->devices || !m->seats || !m->sessions || !m->users || !m->inhibitors || !m->buttons ||
+ if (!m->devices || !m->seats || !m->sessions || !m->users || !m->inhibitors || !m->buttons || !m->busnames ||
!m->user_units || !m->session_units ||
!m->session_fds || !m->inhibitor_fds || !m->button_fds) {
manager_free(m);
@@ -111,6 +112,7 @@ void manager_free(Manager *m) {
Seat *s;
Inhibitor *i;
Button *b;
+ char *n;

assert(m);

@@ -132,12 +134,16 @@ void manager_free(Manager *m) {
while ((b = hashmap_first(m->buttons)))
button_free(b);

+ while ((n = hashmap_first(m->busnames)))
+ free(hashmap_remove(m->busnames, n));
+
hashmap_free(m->devices);
hashmap_free(m->seats);
hashmap_free(m->sessions);
hashmap_free(m->users);
hashmap_free(m->inhibitors);
hashmap_free(m->buttons);
+ hashmap_free(m->busnames);

hashmap_free(m->user_units);
hashmap_free(m->session_units);
@@ -361,6 +367,40 @@ int manager_add_button(Manager *m, const char *name, Button **_button) {
return 0;
}

+int manager_watch_busname(Manager *m, const char *name) {
+ char *n;
+ int r;
+
+ assert(m);
+ assert(name);
+
+ if (hashmap_get(m->busnames, name))
+ return 0;
+
+ n = strdup(name);
+ if (!n)
+ return -ENOMEM;
+
+ r = hashmap_put(m->busnames, n, n);
+ if (r < 0) {
+ free(n);
+ return r;
+ }
+
+ return 0;
+}
+
+void manager_drop_busname(Manager *m, const char *name) {
+ char *key;
+
+ assert(m);
+ assert(name);
+
+ key = hashmap_remove(m->busnames, name);
+ if (key)
+ free(key);
+}
+
int manager_process_seat_device(Manager *m, struct udev_device *d) {
Device *device;
int r;
@@ -1045,6 +1085,18 @@ static int manager_connect_bus(Manager *m) {

dbus_bus_add_match(m->bus,
"type='signal',"
+ "sender='"DBUS_SERVICE_DBUS"',"
+ "interface='"DBUS_INTERFACE_DBUS"',"
+ "member='NameOwnerChanged',"
+ "path='"DBUS_PATH_DBUS"'",
+ &error);
+ if (dbus_error_is_set(&error)) {
+ log_error("Failed to add match for NameOwnerChanged: %s", bus_error_message(&error));
+ dbus_error_free(&error);
+ }
+
+ dbus_bus_add_match(m->bus,
+ "type='signal',"
"sender='org.freedesktop.systemd1',"
"interface='org.freedesktop.systemd1.Manager',"
"member='JobRemoved',"
diff --git a/src/login/logind.h b/src/login/logind.h
index 1a97351..a76936d 100644
--- a/src/login/logind.h
+++ b/src/login/logind.h
@@ -51,6 +51,7 @@ struct Manager {
Hashmap *users;
Hashmap *inhibitors;
Hashmap *buttons;
+ Hashmap *busnames;

LIST_HEAD(Seat, seat_gc_queue);
LIST_HEAD(Session, session_gc_queue);
@@ -190,3 +191,6 @@ int manager_unit_is_active(Manager *manager, const char *unit);

/* gperf lookup function */
const struct ConfigPerfItem* logind_gperf_lookup(const char *key, unsigned length);
+
+int manager_watch_busname(Manager *manager, const char *name);
+void manager_drop_busname(Manager *manager, const char *name);
--
1.8.4
David Herrmann
2013-09-17 15:39:56 UTC
Permalink
A session usually has only a single compositor or other application that
controls graphics and input devices on it. To avoid multiple applications
from hijacking each other's devices or even using the devices in parallel,
we add session controllers.

A session controller is an application that manages a session. Specific
API calls may be limited to controllers to avoid others from getting
unprivileged access to restricted resources. A session becomes a
controller by calling the RequestControl() dbus API call. It can drop it
via ReleaseControl().

logind tracks bus-names to release the controller once an application
closes the bus. We use the new bus-name tracking to do that. Note that
during ReleaseControl() we need to check whether some other session also
tracks the name before we remove it from the bus-name tracking list.

Currently, we only allow one controller at a time. However, the public API
does not enforce this restriction. So if it makes sense, we can allow
multiple controllers in parallel later. Or we can add a "scope" parameter,
which allows a different controller for graphics-devices, sound-devices
and whatever you want.
Note that currently you get -EBUSY if there is already a controller. You
can force the RequestControl() call (root-only) to drop the current
controller and recover the session during an emergency. To recover a seat,
this is not needed, though. You can simply create a new session or
force-activate it.

To become a session controller, a dbus caller must either be root or the
same user as the user of the session. This allows us to run a session
compositor as user and we no longer need any CAP_SYS_ADMIN.
---
src/login/logind-dbus.c | 8 +++++++
src/login/logind-session-dbus.c | 42 ++++++++++++++++++++++++++++++++++++
src/login/logind-session.c | 48 +++++++++++++++++++++++++++++++++++++++++
src/login/logind-session.h | 6 ++++++
src/login/logind.c | 10 +++++++++
5 files changed, 114 insertions(+)

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 5decf81..113a2b7 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -2473,8 +2473,16 @@ DBusHandlerResult bus_message_filter(
goto finish;
}

+ /* drop all controllers owned by this name */
if (*old && !*new && (key = hashmap_remove(m->busnames, old))) {
+ Session *session;
+ Iterator i;
+
free(key);
+
+ HASHMAP_FOREACH(session, m->sessions, i)
+ if (session_is_controller(session, old))
+ session_drop_controller(session);
}
}

diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c
index 2cc4d85..b8b32cd 100644
--- a/src/login/logind-session-dbus.c
+++ b/src/login/logind-session-dbus.c
@@ -40,6 +40,10 @@
" <arg name=\"who\" type=\"s\"/>\n" \
" <arg name=\"signal\" type=\"s\"/>\n" \
" </method>\n" \
+ " <method name=\"RequestControl\"/>\n" \
+ " <arg name=\"force\" type=\"b\"/>\n" \
+ " </method>\n" \
+ " <method name=\"DropControl\"/>\n" \
" <signal name=\"Lock\"/>\n" \
" <signal name=\"Unlock\"/>\n" \
" <property name=\"Id\" type=\"s\" access=\"read\"/>\n" \
@@ -366,6 +370,44 @@ static DBusHandlerResult session_message_dispatch(
if (!reply)
goto oom;

+ } else if (dbus_message_is_method_call(message, "org.freedesktop.login1.Session", "RequestControl")) {
+ dbus_bool_t force;
+ unsigned long ul;
+
+ if (!dbus_message_get_args(
+ message,
+ &error,
+ DBUS_TYPE_BOOLEAN, &force,
+ DBUS_TYPE_INVALID))
+ return bus_send_error_reply(connection, message, &error, -EINVAL);
+
+ ul = dbus_bus_get_unix_user(connection, dbus_message_get_sender(message), &error);
+ if (ul == (unsigned long) -1)
+ return bus_send_error_reply(connection, message, &error, -EIO);
+
+ if (ul != 0 && (force || ul != s->user->uid))
+ return bus_send_error_reply(connection, message, NULL, -EPERM);
+
+ r = session_set_controller(s, bus_message_get_sender_with_fallback(message), force);
+ if (r < 0)
+ return bus_send_error_reply(connection, message, NULL, r);
+
+ reply = dbus_message_new_method_return(message);
+ if (!reply)
+ goto oom;
+
+ } else if (dbus_message_is_method_call(message, "org.freedesktop.login1.Session", "DropControl")) {
+ const char *sender = bus_message_get_sender_with_fallback(message);
+
+ if (!session_is_controller(s, sender))
+ return bus_send_error_reply(connection, message, NULL, -EPERM);
+
+ session_drop_controller(s);
+
+ reply = dbus_message_new_method_return(message);
+ if (!reply)
+ goto oom;
+
} else {
const BusBoundProperties bps[] = {
{ "org.freedesktop.login1.Session", bus_login_session_properties, s },
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 2d22a68..fe5fa27 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -73,6 +73,8 @@ void session_free(Session *s) {
if (s->in_gc_queue)
LIST_REMOVE(Session, gc_queue, s->manager->session_gc_queue, s);

+ session_drop_controller(s);
+
if (s->user) {
LIST_REMOVE(Session, sessions_by_user, s->user->sessions, s);

@@ -918,6 +920,52 @@ int session_kill(Session *s, KillWho who, int signo) {
return manager_kill_unit(s->manager, s->scope, who, signo, NULL);
}

+bool session_is_controller(Session *s, const char *sender)
+{
+ assert(s);
+
+ return streq_ptr(s->controller, sender);
+}
+
+int session_set_controller(Session *s, const char *sender, bool force) {
+ char *t;
+ int r;
+
+ assert(s);
+ assert(sender);
+
+ if (session_is_controller(s, sender))
+ return 0;
+ if (s->controller && !force)
+ return -EBUSY;
+
+ t = strdup(sender);
+ if (!t)
+ return -ENOMEM;
+
+ r = manager_watch_busname(s->manager, sender);
+ if (r) {
+ free(t);
+ return r;
+ }
+
+ session_drop_controller(s);
+
+ s->controller = t;
+ return 0;
+}
+
+void session_drop_controller(Session *s) {
+ assert(s);
+
+ if (!s->controller)
+ return;
+
+ manager_drop_busname(s->manager, s->controller);
+ free(s->controller);
+ s->controller = NULL;
+}
+
static const char* const session_state_table[_SESSION_STATE_MAX] = {
[SESSION_OPENING] = "opening",
[SESSION_ONLINE] = "online",
diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index 9cf6485..839fb23 100644
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -106,6 +106,8 @@ struct Session {

DBusMessage *create_message;

+ char *controller;
+
LIST_FIELDS(Session, sessions_by_user);
LIST_FIELDS(Session, sessions_by_seat);

@@ -154,3 +156,7 @@ SessionClass session_class_from_string(const char *s) _pure_;

const char *kill_who_to_string(KillWho k) _const_;
KillWho kill_who_from_string(const char *s) _pure_;
+
+bool session_is_controller(Session *s, const char *sender);
+int session_set_controller(Session *s, const char *sender, bool force);
+void session_drop_controller(Session *s);
diff --git a/src/login/logind.c b/src/login/logind.c
index 89e4eee..c99c284 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -391,11 +391,21 @@ int manager_watch_busname(Manager *m, const char *name) {
}

void manager_drop_busname(Manager *m, const char *name) {
+ Session *session;
+ Iterator i;
char *key;

assert(m);
assert(name);

+ if (!hashmap_get(m->busnames, name))
+ return;
+
+ /* keep it if the name still owns a controller */
+ HASHMAP_FOREACH(session, m->sessions, i)
+ if (session_is_controller(session, name))
+ return;
+
key = hashmap_remove(m->busnames, name);
if (key)
free(key);
--
1.8.4
Lennart Poettering
2013-09-17 16:29:00 UTC
Permalink
On Tue, 17.09.13 17:39, David Herrmann (***@gmail.com) wrote:

Please rename the bus calls to TakeControl()/ReleaseControl() (as
discussed at the hackfest). Otherwise patch #2 and #3 look good to merge!
Post by David Herrmann
A session usually has only a single compositor or other application that
controls graphics and input devices on it. To avoid multiple applications
from hijacking each other's devices or even using the devices in parallel,
we add session controllers.
A session controller is an application that manages a session. Specific
API calls may be limited to controllers to avoid others from getting
unprivileged access to restricted resources. A session becomes a
controller by calling the RequestControl() dbus API call. It can drop it
via ReleaseControl().
logind tracks bus-names to release the controller once an application
closes the bus. We use the new bus-name tracking to do that. Note that
during ReleaseControl() we need to check whether some other session also
tracks the name before we remove it from the bus-name tracking list.
Currently, we only allow one controller at a time. However, the public API
does not enforce this restriction. So if it makes sense, we can allow
multiple controllers in parallel later. Or we can add a "scope" parameter,
which allows a different controller for graphics-devices, sound-devices
and whatever you want.
Note that currently you get -EBUSY if there is already a controller. You
can force the RequestControl() call (root-only) to drop the current
controller and recover the session during an emergency. To recover a seat,
this is not needed, though. You can simply create a new session or
force-activate it.
To become a session controller, a dbus caller must either be root or the
same user as the user of the session. This allows us to run a session
compositor as user and we no longer need any CAP_SYS_ADMIN.
---
src/login/logind-dbus.c | 8 +++++++
src/login/logind-session-dbus.c | 42 ++++++++++++++++++++++++++++++++++++
src/login/logind-session.c | 48 +++++++++++++++++++++++++++++++++++++++++
src/login/logind-session.h | 6 ++++++
src/login/logind.c | 10 +++++++++
5 files changed, 114 insertions(+)
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 5decf81..113a2b7 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -2473,8 +2473,16 @@ DBusHandlerResult bus_message_filter(
goto finish;
}
+ /* drop all controllers owned by this name */
if (*old && !*new && (key = hashmap_remove(m->busnames, old))) {
+ Session *session;
+ Iterator i;
+
free(key);
+
+ HASHMAP_FOREACH(session, m->sessions, i)
+ if (session_is_controller(session, old))
+ session_drop_controller(session);
}
}
diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c
index 2cc4d85..b8b32cd 100644
--- a/src/login/logind-session-dbus.c
+++ b/src/login/logind-session-dbus.c
@@ -40,6 +40,10 @@
" <arg name=\"who\" type=\"s\"/>\n" \
" <arg name=\"signal\" type=\"s\"/>\n" \
" </method>\n" \
+ " <method name=\"RequestControl\"/>\n" \
+ " <arg name=\"force\" type=\"b\"/>\n" \
+ " </method>\n" \
+ " <method name=\"DropControl\"/>\n" \
" <signal name=\"Lock\"/>\n" \
" <signal name=\"Unlock\"/>\n" \
" <property name=\"Id\" type=\"s\" access=\"read\"/>\n" \
@@ -366,6 +370,44 @@ static DBusHandlerResult session_message_dispatch(
if (!reply)
goto oom;
+ } else if (dbus_message_is_method_call(message, "org.freedesktop.login1.Session", "RequestControl")) {
+ dbus_bool_t force;
+ unsigned long ul;
+
+ if (!dbus_message_get_args(
+ message,
+ &error,
+ DBUS_TYPE_BOOLEAN, &force,
+ DBUS_TYPE_INVALID))
+ return bus_send_error_reply(connection, message, &error, -EINVAL);
+
+ ul = dbus_bus_get_unix_user(connection, dbus_message_get_sender(message), &error);
+ if (ul == (unsigned long) -1)
+ return bus_send_error_reply(connection, message, &error, -EIO);
+
+ if (ul != 0 && (force || ul != s->user->uid))
+ return bus_send_error_reply(connection, message, NULL, -EPERM);
+
+ r = session_set_controller(s, bus_message_get_sender_with_fallback(message), force);
+ if (r < 0)
+ return bus_send_error_reply(connection, message, NULL, r);
+
+ reply = dbus_message_new_method_return(message);
+ if (!reply)
+ goto oom;
+
+ } else if (dbus_message_is_method_call(message, "org.freedesktop.login1.Session", "DropControl")) {
+ const char *sender = bus_message_get_sender_with_fallback(message);
+
+ if (!session_is_controller(s, sender))
+ return bus_send_error_reply(connection, message, NULL, -EPERM);
+
+ session_drop_controller(s);
+
+ reply = dbus_message_new_method_return(message);
+ if (!reply)
+ goto oom;
+
} else {
const BusBoundProperties bps[] = {
{ "org.freedesktop.login1.Session", bus_login_session_properties, s },
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 2d22a68..fe5fa27 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -73,6 +73,8 @@ void session_free(Session *s) {
if (s->in_gc_queue)
LIST_REMOVE(Session, gc_queue, s->manager->session_gc_queue, s);
+ session_drop_controller(s);
+
if (s->user) {
LIST_REMOVE(Session, sessions_by_user, s->user->sessions, s);
@@ -918,6 +920,52 @@ int session_kill(Session *s, KillWho who, int signo) {
return manager_kill_unit(s->manager, s->scope, who, signo, NULL);
}
+bool session_is_controller(Session *s, const char *sender)
+{
+ assert(s);
+
+ return streq_ptr(s->controller, sender);
+}
+
+int session_set_controller(Session *s, const char *sender, bool force) {
+ char *t;
+ int r;
+
+ assert(s);
+ assert(sender);
+
+ if (session_is_controller(s, sender))
+ return 0;
+ if (s->controller && !force)
+ return -EBUSY;
+
+ t = strdup(sender);
+ if (!t)
+ return -ENOMEM;
+
+ r = manager_watch_busname(s->manager, sender);
+ if (r) {
+ free(t);
+ return r;
+ }
+
+ session_drop_controller(s);
+
+ s->controller = t;
+ return 0;
+}
+
+void session_drop_controller(Session *s) {
+ assert(s);
+
+ if (!s->controller)
+ return;
+
+ manager_drop_busname(s->manager, s->controller);
+ free(s->controller);
+ s->controller = NULL;
+}
+
static const char* const session_state_table[_SESSION_STATE_MAX] = {
[SESSION_OPENING] = "opening",
[SESSION_ONLINE] = "online",
diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index 9cf6485..839fb23 100644
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -106,6 +106,8 @@ struct Session {
DBusMessage *create_message;
+ char *controller;
+
LIST_FIELDS(Session, sessions_by_user);
LIST_FIELDS(Session, sessions_by_seat);
@@ -154,3 +156,7 @@ SessionClass session_class_from_string(const char *s) _pure_;
const char *kill_who_to_string(KillWho k) _const_;
KillWho kill_who_from_string(const char *s) _pure_;
+
+bool session_is_controller(Session *s, const char *sender);
+int session_set_controller(Session *s, const char *sender, bool force);
+void session_drop_controller(Session *s);
diff --git a/src/login/logind.c b/src/login/logind.c
index 89e4eee..c99c284 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -391,11 +391,21 @@ int manager_watch_busname(Manager *m, const char *name) {
}
void manager_drop_busname(Manager *m, const char *name) {
+ Session *session;
+ Iterator i;
char *key;
assert(m);
assert(name);
+ if (!hashmap_get(m->busnames, name))
+ return;
+
+ /* keep it if the name still owns a controller */
+ HASHMAP_FOREACH(session, m->sessions, i)
+ if (session_is_controller(session, name))
+ return;
+
key = hashmap_remove(m->busnames, name);
if (key)
free(key);
Lennart
--
Lennart Poettering - Red Hat, Inc.
Lennart Poettering
2013-09-17 16:35:41 UTC
Permalink
Post by Lennart Poettering
Please rename the bus calls to TakeControl()/ReleaseControl() (as
discussed at the hackfest). Otherwise patch #2 and #3 look good to merge!
Oh well, did the change myself now and commited! Thanks!

Also commited #2, and #4.

Lennart
--
Lennart Poettering - Red Hat, Inc.
David Herrmann
2013-09-17 15:39:57 UTC
Permalink
Currently, Activate() calls chvt(), which does an ioctl(VT_ACTIVATE) and
immediately calls seat_set_active(). However, VTs are allowed to prevent
being deactivated. Therefore, logind cannot be sure the VT_ACTIVATE call
was actually successful.

Furthermore, compositors often need to clean up their devices before they
acknowledge the VT switch. The immediate call to seat_set_active() may
modify underlying ACLs, though. Thus, some compositors may fail cleaning
up their stuff. Moreover, the compositor being switched to (if listening
to logind instead of VTs) will not be able to activate its devices if the
old VT still has them active.

We could simply add an VT_WAITACTIVE call, which blocks until the given VT
is active. However, this can block forever if the compositor hangs.

So to fix this, we make Activate() lazy. That is, it only schedules a
session-switch but does not wait for it to complete. The caller can no
longer rely on it being immediate. Instead, a caller is required to wait
for the PropertiesChanged signal and read the "Active" field.

We could make Activate() wait asynchronously for the session-switch to
complete and then send the return-message afterwards. However, this would
add a lot of state-tracking with no real gain:
1) Sessions normally don't care whether Activate() was actually
successful as they currently _must_ wait for the VT activation to do
anything for real.
2) Error messages for failed session switches can be printed by logind
instead of the session issuing Activate().
3) Sessions that require synchronous Activate() calls can simply issue
the call and then wait for "Active" properties to change. This also
allows them to implement their own timeout.

This change prepares for multi-session on seats without VTs. Forced VT
switches are always bad as compositors cannot perform any cleanup. This
isn't strictly required, but may lead to loss of information and ambiguous
error messages.
So for multi-session on seats without VTs, we must wait for the current
session to clean-up before finalizing the session-switch. This requires
Activate() to be lazy as we cannot block here.

Note that we can always implement a timeout which allows us to guarantee
the session switch to happen. Nevertheless, this calls for a lazy
Activate().
---
src/login/logind-session.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index fe5fa27..fa8b515 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -360,8 +360,6 @@ int session_load(Session *s) {
}

int session_activate(Session *s) {
- int r;
-
assert(s);
assert(s->user);

@@ -376,11 +374,7 @@ int session_activate(Session *s) {

assert(seat_is_vtconsole(s->seat));

- r = chvt(s->vtnr);
- if (r < 0)
- return r;
-
- return seat_set_active(s->seat, s);
+ return chvt(s->vtnr);
}

static int session_link_x11_socket(Session *s) {
--
1.8.4
David Herrmann
2013-09-17 15:39:58 UTC
Permalink
This post might be inappropriate. Click to display it.
Lennart Poettering
2013-09-17 17:34:15 UTC
Permalink
Post by David Herrmann
" <arg name=\"force\" type=\"b\"/>\n" \
" </method>\n" \
" <method name=\"DropControl\"/>\n" \
+ " <method name=\"RequestDevice\">\n" \
+ " <arg name=\"node\" type=\"s\" direction=\"in\"/>\n" \
+ " <arg name=\"fd\" type=\"h\" direction=\"out\"/>\n" \
+ " <arg name=\"paused\" type=\"b\" direction=\"out\"/>\n" \
+ " </method>\n" \
+ " <method name=\"ReleaseDevice\">\n" \
+ " <arg name=\"node\" type=\"s\"/>\n" \
+ " </method>\n"
\
Please rename this pair to TakeDevice() and ReleaseDevice().
Post by David Herrmann
index 0000000..659f161
--- /dev/null
+++ b/src/login/logind-session-device.c
@@ -0,0 +1,489 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2013 David Herrmann
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <assert.h>
+#include <fcntl.h>
+#include <libudev.h>
+#include <linux/input.h>
+#include <linux/ioctl.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "dbus-common.h"
+#include "logind-session-device.h"
+#include "util.h"
+
+enum SessionDeviceNotifications {
+ SESSION_DEVICE_RESUME,
+ SESSION_DEVICE_TRY_PAUSE,
+ SESSION_DEVICE_PAUSE,
+ SESSION_DEVICE_RELEASE,
+};
+
+static void session_device_notify(SessionDevice *sd, enum SessionDeviceNotifications type) {
+ _cleanup_dbus_message_unref_ DBusMessage *m = NULL;
+ _cleanup_free_ char *path = NULL;
+ const char *t = NULL;
+
+ assert(sd);
+
+ if (!sd->session->controller)
+ return;
+
+ path = session_bus_path(sd->session);
+ if (!path)
+ return;
+
+ m = dbus_message_new_signal(path,
+ "org.freedesktop.login1.Session",
+ (type == SESSION_DEVICE_RESUME) ? "ResumeDevice" : "PauseDevice");
+ if (!m)
+ return;
+
+ if (!dbus_message_set_destination(m, sd->session->controller))
+ return;
+
+ switch (type) {
+ if (!dbus_message_append_args(m,
+ DBUS_TYPE_STRING, &sd->node,
+ DBUS_TYPE_UNIX_FD, &sd->fd,
+ DBUS_TYPE_INVALID))
+ return;
+ break;
+ t = "pause";
+ break;
+ t = "force";
+ break;
+ t = "gone";
+ break;
+ return;
+ }
+
+ if (t && !dbus_message_append_args(m,
+ DBUS_TYPE_STRING, &sd->node,
+ DBUS_TYPE_STRING, &t,
+ DBUS_TYPE_INVALID))
+ return;
+
+ dbus_connection_send(sd->session->manager->bus, m, NULL);
+}
+
+static int sd_eviocrevoke(int fd)
+{
"{" please on the same line as the function name.
Post by David Herrmann
+ static bool warned;
+ int r;
+
+#ifndef EVIOCREVOKE
+# define EVIOCREVOKE _IOW('E', 0x91, int)
+#endif
Please move this to missing.h.
Post by David Herrmann
+
+ assert(fd >= 0);
+
+ r = ioctl(fd, EVIOCREVOKE, 1);
+ if (r < 0) {
+ r = -errno;
+ if (r == -ENOSYS && !warned) {
+ warned = true;
+ log_warning("kernel does not support evdev-revocation");
+ }
+ }
+
+ return 0;
+}
+
+static int sd_drmsetmaster(int fd)
+{
"{"...
Post by David Herrmann
+ int r;
+
+#ifndef DRM_IOCTL_SET_MASTER
+# define DRM_IOCTL_SET_MASTER _IO('d', 0x1e)
+#endif
→ missing.h!
Post by David Herrmann
+
+ assert(fd >= 0);
+
+ r = ioctl(fd, DRM_IOCTL_SET_MASTER, 0);
+ if (r < 0)
+ return -errno;
+
+ return 0;
+}
+
+static int sd_drmdropmaster(int fd)
+{
"{"....
Post by David Herrmann
+ int r;
+
+#ifndef DRM_IOCTL_DROP_MASTER
+# define DRM_IOCTL_DROP_MASTER _IO('d', 0x1f)
+#endif
→ missing.h
Post by David Herrmann
+ switch (sd->type) {
+ if (active) {
+ sd_drmsetmaster(fd);
+ } else {
Please remove "{}".
Post by David Herrmann
+ dnum = udev_device_get_devnum(dev);
+ sysname = udev_device_get_sysname(dev);
+ type = DEVICE_TYPE_UNKNOWN;
+
+ if (major(dnum) == 29) {
+ if (startswith(sysname, "fb"))
+ type = DEVICE_TYPE_FBDEV;
+ } else if (major(dnum) == 226) {
+ if (startswith(sysname, "card"))
+ type = DEVICE_TYPE_DRM;
+ } else if (major(dnum) == 13) {
+ if (startswith(sysname, "event"))
+ type = DEVICE_TYPE_EVDEV;
Please use udev_device_get_subsystem() for this! Comparing majors is evil!
Post by David Herrmann
+ dev = udev_device_new_from_devnum(sd->session->manager->udev, 'c', st.st_rdev);
+ if (!dev)
+ return -ENODEV;
+ sp = udev_device_get_syspath(dev);
+
+ /* Only allow access to "uaccess" tagged devices! */
+ if (!udev_device_has_tag(dev, "uaccess")) {
+ r = -EPERM;
+ goto err_dev;
+ }
+
This sounds wrong, after all the suer could access the device directly
anyway if uaccess is set.
Post by David Herrmann
+ /* Only allow opening the _real_ device node. This is the node provided
+ * by devtmpfs! Issue a readlink() if you are not sure. We cannot allow
+ * any path here as user-space could otherwise trigger OOM by requesting
+ * duplicates. */
+ real_node = udev_device_get_devnode(dev);
+ if (!streq_ptr(real_node, node)) {
+ r = -EINVAL;
+ goto err_dev;
+ }
I think it would make sense to drop this. Instead verify with
path_startswith() that the specified path is in /dev. Then do an lstat()
on the specified device path, and verify the backing major/minor matches
the one of /dev, so that people cannot use /dev/shm to confuse us.
Post by David Herrmann
+typedef struct SessionDevice SessionDevice;
+
+#include "list.h"
+#include "util.h"
+#include "logind.h"
+#include "logind-device.h"
+#include "logind-seat.h"
+#include "logind-session.h"
+
+enum DeviceType {
+ DEVICE_TYPE_UNKNOWN,
+ DEVICE_TYPE_FBDEV,
+ DEVICE_TYPE_DRM,
+ DEVICE_TYPE_EVDEV,
+};
exported enums plese with a typdef!

Lennart
--
Lennart Poettering - Red Hat, Inc.
David Herrmann
2013-09-17 21:39:04 UTC
Permalink
This post might be inappropriate. Click to display it.
David Herrmann
2013-09-17 15:39:59 UTC
Permalink
The seat->vtconsole member always points to the default seat seat0. Even
if VTs are disabled, it's used as default seat. Therefore, rename it to
seat0 to correctly state what it is.

This also changes the seat files in /run from IS_VTCONSOLE to IS_SEAT0. It
wasn't used by any code, yet, so this seems fine.

While we are at it, we also remove every "if (s->vtconsole)" as this
pointer is always valid!
---
src/login/logind-dbus.c | 8 ++++----
src/login/logind-seat.c | 14 +++++++-------
src/login/logind-seat.h | 2 +-
src/login/logind-session.c | 2 +-
src/login/logind.c | 8 ++++----
src/login/logind.h | 2 +-
6 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 113a2b7..4a23c93 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -404,8 +404,8 @@ static int bus_manager_create_session(Manager *m, DBusMessage *message) {
int v;

if (!seat)
- seat = m->vtconsole;
- else if (seat != m->vtconsole)
+ seat = m->seat0;
+ else if (seat != m->seat0)
return -EINVAL;

v = vtnr_from_tty(tty);
@@ -420,8 +420,8 @@ static int bus_manager_create_session(Manager *m, DBusMessage *message) {
} else if (tty_is_console(tty)) {

if (!seat)
- seat = m->vtconsole;
- else if (seat != m->vtconsole)
+ seat = m->seat0;
+ else if (seat != m->seat0)
return -EINVAL;

if (vtnr != 0)
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index dcaf0ac..88fd724 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -105,11 +105,11 @@ int seat_save(Seat *s) {

fprintf(f,
"# This is private data. Do not parse.\n"
- "IS_VTCONSOLE=%i\n"
+ "IS_SEAT0=%i\n"
"CAN_MULTI_SESSION=%i\n"
"CAN_TTY=%i\n"
"CAN_GRAPHICAL=%i\n",
- seat_is_vtconsole(s),
+ seat_is_seat0(s),
seat_can_multi_session(s),
seat_can_tty(s),
seat_can_graphical(s));
@@ -429,16 +429,16 @@ int seat_attach_session(Seat *s, Session *session) {
return 0;
}

-bool seat_is_vtconsole(Seat *s) {
+bool seat_is_seat0(Seat *s) {
assert(s);

- return s->manager->vtconsole == s;
+ return s->manager->seat0 == s;
}

bool seat_can_multi_session(Seat *s) {
assert(s);

- if (!seat_is_vtconsole(s))
+ if (!seat_is_seat0(s))
return false;

/* If we can't watch which VT is in the foreground, we don't
@@ -450,7 +450,7 @@ bool seat_can_multi_session(Seat *s) {
bool seat_can_tty(Seat *s) {
assert(s);

- return seat_is_vtconsole(s);
+ return seat_is_seat0(s);
}

bool seat_has_master_device(Seat *s) {
@@ -508,7 +508,7 @@ int seat_check_gc(Seat *s, bool drop_not_started) {
if (drop_not_started && !s->started)
return 0;

- if (seat_is_vtconsole(s))
+ if (seat_is_seat0(s))
return 1;

return seat_has_master_device(s);
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index bd5390f..47fe89a 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -60,7 +60,7 @@ int seat_preallocate_vts(Seat *s);

int seat_attach_session(Seat *s, Session *session);

-bool seat_is_vtconsole(Seat *s);
+bool seat_is_seat0(Seat *s);
bool seat_can_multi_session(Seat *s);
bool seat_can_tty(Seat *s);
bool seat_has_master_device(Seat *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 5a1e621..77eeb9c 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -387,7 +387,7 @@ int session_activate(Session *s) {
if (s->seat->active == s)
return 0;

- assert(seat_is_vtconsole(s->seat));
+ assert(seat_is_seat0(s->seat));

return chvt(s->vtnr);
}
diff --git a/src/login/logind.c b/src/login/logind.c
index c99c284..702382a 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -858,7 +858,7 @@ int manager_dispatch_vcsa_udev(Manager *m) {
* VTs, to make sure our auto VTs never go away. */

if (name && startswith(name, "vcsa") && streq_ptr(udev_device_get_action(d), "remove"))
- r = seat_preallocate_vts(m->vtconsole);
+ r = seat_preallocate_vts(m->seat0);

udev_device_unref(d);

@@ -883,9 +883,9 @@ int manager_dispatch_button_udev(Manager *m) {

int manager_dispatch_console(Manager *m) {
assert(m);
+ assert(m->seat0);

- if (m->vtconsole)
- seat_read_active_vt(m->vtconsole);
+ seat_read_active_vt(m->seat0);

return 0;
}
@@ -1543,7 +1543,7 @@ int manager_startup(Manager *m) {
return r;

/* Instantiate magic seat 0 */
- r = manager_add_seat(m, "seat0", &m->vtconsole);
+ r = manager_add_seat(m, "seat0", &m->seat0);
if (r < 0)
return r;

diff --git a/src/login/logind.h b/src/login/logind.h
index a76936d..9e6296c 100644
--- a/src/login/logind.h
+++ b/src/login/logind.h
@@ -74,7 +74,7 @@ struct Manager {
unsigned reserve_vt;
int reserve_vt_fd;

- Seat *vtconsole;
+ Seat *seat0;

char **kill_only_users, **kill_exclude_users;
bool kill_user_processes;
--
1.8.4
David Herrmann
2013-09-17 15:40:00 UTC
Permalink
A seat provides text-logins if it has VTs. This is always limited to seat0
so the seat_is_seat0() check is correct. However, if VTs are disabled, no
seat provides text-logins so we also need to check for the console-fd.

This was previously:
return seat_is_vtconsole();
It looked right, but was functionally equivalent to seat_is_seat0(). The
rename of this helper made it more obvious that it is missing the VT test.
---
src/login/logind-seat.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 88fd724..4c2c424 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -450,7 +450,7 @@ bool seat_can_multi_session(Seat *s) {
bool seat_can_tty(Seat *s) {
assert(s);

- return seat_is_seat0(s);
+ return seat_is_seat0(s) && s->manager->console_active_fd >= 0;
}

bool seat_has_master_device(Seat *s) {
--
1.8.4
David Herrmann
2013-09-17 15:40:01 UTC
Permalink
VT numbers start with 1. If a session has vtnr == 0, we must not assume it
is running on a VT.
Note that this could trigger the assert() below as CreateSession() sets
vtnr to 0, not <0.
---
src/login/logind-session.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 77eeb9c..1df2a06 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -378,7 +378,7 @@ int session_activate(Session *s) {
assert(s);
assert(s->user);

- if (s->vtnr < 0)
+ if (s->vtnr <= 0)
return -ENOTSUP;

if (!s->seat)
--
1.8.4
David Herrmann
2013-09-17 15:40:02 UTC
Permalink
We currently use seat_can_multi_session() to test for two things:
* whether the seat can handle session-switching
* whether the seat has VTs

As both are currently logically equivalent, we didn't care. However, we
want to allow session-switching on seats without VTs, so split this helper
into:
* seat_can_multi_session(): whether session-switching is supported
* seat_has_vts(): whether the seat has VTs

Note that only one seat on a system can have VTs. There is only one set of
them. We automatically assign them to seat0 as usual.

With this patch in place, we can easily add new session-switching/tracking
methods without breaking any VT code as it is now protected by has_vts(),
no longer by can_multi_session().
---
src/login/logind-dbus.c | 2 +-
src/login/logind-seat.c | 32 ++++++++++++++------------------
src/login/logind-seat.h | 1 +
src/login/logind-session.c | 6 +++---
4 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 4a23c93..fd8ee1b 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -429,7 +429,7 @@ static int bus_manager_create_session(Manager *m, DBusMessage *message) {
}

if (seat) {
- if (seat_can_multi_session(seat)) {
+ if (seat_has_vts(seat)) {
if (vtnr > 63)
return -EINVAL;
} else {
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 4c2c424..f88738a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -201,7 +201,7 @@ int seat_preallocate_vts(Seat *s) {
if (s->manager->n_autovts <= 0)
return 0;

- if (!seat_can_multi_session(s))
+ if (!seat_has_vts(s))
return 0;

for (i = 1; i <= s->manager->n_autovts; i++) {
@@ -282,7 +282,7 @@ int seat_active_vt_changed(Seat *s, int vtnr) {
assert(s);
assert(vtnr >= 1);

- if (!seat_can_multi_session(s))
+ if (!seat_has_vts(s))
return -EINVAL;

log_debug("VT changed to %i", vtnr);
@@ -306,7 +306,7 @@ int seat_read_active_vt(Seat *s) {

assert(s);

- if (!seat_can_multi_session(s))
+ if (!seat_has_vts(s))
return 0;

lseek(s->manager->console_active_fd, SEEK_SET, 0);
@@ -417,18 +417,20 @@ int seat_attach_session(Seat *s, Session *session) {

seat_send_changed(s, "Sessions\0");

- /* Note that even if a seat is not multi-session capable it
- * still might have multiple sessions on it since old, dead
- * sessions might continue to be tracked until all their
- * processes are gone. The most recently added session
- * (i.e. the first in s->sessions) is the one that matters. */
-
- if (!seat_can_multi_session(s))
+ /* On seats with VTs, the VT logic defines which session is active. On
+ * seats without VTs, we automatically activate the first session. */
+ if (!seat_has_vts(s) && !s->active)
seat_set_active(s, session);

return 0;
}

+bool seat_has_vts(Seat *s) {
+ assert(s);
+
+ return seat_is_seat0(s) && s->manager->console_active_fd >= 0;
+}
+
bool seat_is_seat0(Seat *s) {
assert(s);

@@ -438,19 +440,13 @@ bool seat_is_seat0(Seat *s) {
bool seat_can_multi_session(Seat *s) {
assert(s);

- if (!seat_is_seat0(s))
- return false;
-
- /* If we can't watch which VT is in the foreground, we don't
- * support VT switching */
-
- return s->manager->console_active_fd >= 0;
+ return seat_has_vts(s);
}

bool seat_can_tty(Seat *s) {
assert(s);

- return seat_is_seat0(s) && s->manager->console_active_fd >= 0;
+ return seat_has_vts(s);
}

bool seat_has_master_device(Seat *s) {
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index 47fe89a..d3438b8 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -60,6 +60,7 @@ int seat_preallocate_vts(Seat *s);

int seat_attach_session(Seat *s, Session *session);

+bool seat_has_vts(Seat *s);
bool seat_is_seat0(Seat *s);
bool seat_can_multi_session(Seat *s);
bool seat_can_tty(Seat *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 1df2a06..906115a 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -206,7 +206,7 @@ int session_save(Session *s) {
if (s->service)
fprintf(f, "SERVICE=%s\n", s->service);

- if (s->seat && seat_can_multi_session(s->seat))
+ if (s->seat && seat_has_vts(s->seat))
fprintf(f, "VTNR=%i\n", s->vtnr);

if (s->leader > 0)
@@ -316,7 +316,7 @@ int session_load(Session *s) {
seat_attach_session(o, s);
}

- if (vtnr && s->seat && seat_can_multi_session(s->seat)) {
+ if (vtnr && s->seat && seat_has_vts(s->seat)) {
int v;

k = safe_atoi(vtnr, &v);
@@ -387,7 +387,7 @@ int session_activate(Session *s) {
if (s->seat->active == s)
return 0;

- assert(seat_is_seat0(s->seat));
+ assert(seat_has_vts(s->seat));

return chvt(s->vtnr);
}
--
1.8.4
Lennart Poettering
2013-09-17 18:52:57 UTC
Permalink
On Tue, 17.09.13 17:40, David Herrmann (***@gmail.com) wrote:

Applied #6 to #9, please update/rebase #5 and #10.

Thanks a lot!
Post by David Herrmann
* whether the seat can handle session-switching
* whether the seat has VTs
As both are currently logically equivalent, we didn't care. However, we
want to allow session-switching on seats without VTs, so split this helper
* seat_can_multi_session(): whether session-switching is supported
* seat_has_vts(): whether the seat has VTs
Note that only one seat on a system can have VTs. There is only one set of
them. We automatically assign them to seat0 as usual.
With this patch in place, we can easily add new session-switching/tracking
methods without breaking any VT code as it is now protected by has_vts(),
no longer by can_multi_session().
---
src/login/logind-dbus.c | 2 +-
src/login/logind-seat.c | 32 ++++++++++++++------------------
src/login/logind-seat.h | 1 +
src/login/logind-session.c | 6 +++---
4 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 4a23c93..fd8ee1b 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -429,7 +429,7 @@ static int bus_manager_create_session(Manager *m, DBusMessage *message) {
}
if (seat) {
- if (seat_can_multi_session(seat)) {
+ if (seat_has_vts(seat)) {
if (vtnr > 63)
return -EINVAL;
} else {
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 4c2c424..f88738a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -201,7 +201,7 @@ int seat_preallocate_vts(Seat *s) {
if (s->manager->n_autovts <= 0)
return 0;
- if (!seat_can_multi_session(s))
+ if (!seat_has_vts(s))
return 0;
for (i = 1; i <= s->manager->n_autovts; i++) {
@@ -282,7 +282,7 @@ int seat_active_vt_changed(Seat *s, int vtnr) {
assert(s);
assert(vtnr >= 1);
- if (!seat_can_multi_session(s))
+ if (!seat_has_vts(s))
return -EINVAL;
log_debug("VT changed to %i", vtnr);
@@ -306,7 +306,7 @@ int seat_read_active_vt(Seat *s) {
assert(s);
- if (!seat_can_multi_session(s))
+ if (!seat_has_vts(s))
return 0;
lseek(s->manager->console_active_fd, SEEK_SET, 0);
@@ -417,18 +417,20 @@ int seat_attach_session(Seat *s, Session *session) {
seat_send_changed(s, "Sessions\0");
- /* Note that even if a seat is not multi-session capable it
- * still might have multiple sessions on it since old, dead
- * sessions might continue to be tracked until all their
- * processes are gone. The most recently added session
- * (i.e. the first in s->sessions) is the one that matters. */
-
- if (!seat_can_multi_session(s))
+ /* On seats with VTs, the VT logic defines which session is active. On
+ * seats without VTs, we automatically activate the first session. */
+ if (!seat_has_vts(s) && !s->active)
seat_set_active(s, session);
return 0;
}
+bool seat_has_vts(Seat *s) {
+ assert(s);
+
+ return seat_is_seat0(s) && s->manager->console_active_fd >= 0;
+}
+
bool seat_is_seat0(Seat *s) {
assert(s);
@@ -438,19 +440,13 @@ bool seat_is_seat0(Seat *s) {
bool seat_can_multi_session(Seat *s) {
assert(s);
- if (!seat_is_seat0(s))
- return false;
-
- /* If we can't watch which VT is in the foreground, we don't
- * support VT switching */
-
- return s->manager->console_active_fd >= 0;
+ return seat_has_vts(s);
}
bool seat_can_tty(Seat *s) {
assert(s);
- return seat_is_seat0(s) && s->manager->console_active_fd >= 0;
+ return seat_has_vts(s);
}
bool seat_has_master_device(Seat *s) {
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index 47fe89a..d3438b8 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -60,6 +60,7 @@ int seat_preallocate_vts(Seat *s);
int seat_attach_session(Seat *s, Session *session);
+bool seat_has_vts(Seat *s);
bool seat_is_seat0(Seat *s);
bool seat_can_multi_session(Seat *s);
bool seat_can_tty(Seat *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 1df2a06..906115a 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -206,7 +206,7 @@ int session_save(Session *s) {
if (s->service)
fprintf(f, "SERVICE=%s\n", s->service);
- if (s->seat && seat_can_multi_session(s->seat))
+ if (s->seat && seat_has_vts(s->seat))
fprintf(f, "VTNR=%i\n", s->vtnr);
if (s->leader > 0)
@@ -316,7 +316,7 @@ int session_load(Session *s) {
seat_attach_session(o, s);
}
- if (vtnr && s->seat && seat_can_multi_session(s->seat)) {
+ if (vtnr && s->seat && seat_has_vts(s->seat)) {
int v;
k = safe_atoi(vtnr, &v);
@@ -387,7 +387,7 @@ int session_activate(Session *s) {
if (s->seat->active == s)
return 0;
- assert(seat_is_seat0(s->seat));
+ assert(seat_has_vts(s->seat));
return chvt(s->vtnr);
}
Lennart
--
Lennart Poettering - Red Hat, Inc.
David Herrmann
2013-09-17 15:40:03 UTC
Permalink
This enables the multi-session capability for seats that don't have VTs.
For legacy seats with VTs, everything stays the same. However, all other
seats now also get the multi-session capability.

The only feature that was missing was session-switching. As logind can
force a session-switch and signal that via the "Active" property, we only
need a way to allow synchronized/delayed session switches. Compositors
need to cleanup some devices before acknowledging the session switch.
Therefore, we use the session-devices to give compositors a chance to
block a session-switch until they cleaned everything up.

If you activate a session on a seat without VTs, we send a PauseDevice
signal to the active session for every active device. Only once the
session acknowledged all these with a PauseDeviceComplete() call, we
perform the final session switch.

One important note is that delayed session-switching is meant for
backwards compatibility. New compositors or other sessions should really
try to deal correctly with forced session switches! They only need to
handle EACCES/EPERM from syscalls and treat them as "PauseDevice" signal.

Following logind patches will add a timeout to session-switches which
forces the switch if the active session does not react in a timely
fashion. Moreover, explicit ForceActivate() calls might also be supported.
Hence, sessions must not crash if their devices get paused.
---
src/login/logind-seat.c | 15 +++++++++++++++
src/login/logind-seat.h | 2 ++
src/login/logind-session-device.c | 28 ++++++++++++++++++++++++++++
src/login/logind-session-device.h | 1 +
src/login/logind-session.c | 31 ++++++++++++++++++++++++++-----
5 files changed, 72 insertions(+), 5 deletions(-)

diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index f88738a..4a4d40a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -425,6 +425,21 @@ int seat_attach_session(Seat *s, Session *session) {
return 0;
}

+void seat_complete_switch(Seat *s) {
+ Session *session;
+
+ assert(s);
+
+ /* if no session-switch is pending or if it got canceled, do nothing */
+ if (!s->pending_switch)
+ return;
+
+ session = s->pending_switch;
+ s->pending_switch = NULL;
+
+ seat_set_active(s, session);
+}
+
bool seat_has_vts(Seat *s) {
assert(s);

diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index d3438b8..be6db6e 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -38,6 +38,7 @@ struct Seat {
LIST_HEAD(Device, devices);

Session *active;
+ Session *pending_switch;
LIST_HEAD(Session, sessions);

bool in_gc_queue:1;
@@ -59,6 +60,7 @@ 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);

bool seat_has_vts(Seat *s);
bool seat_is_seat0(Seat *s);
diff --git a/src/login/logind-session-device.c b/src/login/logind-session-device.c
index 659f161..af9d5a1 100644
--- a/src/login/logind-session-device.c
+++ b/src/login/logind-session-device.c
@@ -452,10 +452,21 @@ void session_device_free(SessionDevice *sd) {
}

void session_device_complete_pause(SessionDevice *sd) {
+ SessionDevice *iter;
+ Iterator i;
+
if (!sd->active)
return;

session_device_stop(sd);
+
+ /* if not all devices are paused, wait for further completion events */
+ HASHMAP_FOREACH(iter, sd->session->devices, i)
+ if (iter->active)
+ return;
+
+ /* complete any pending session switch */
+ seat_complete_switch(sd->session->seat);
}

void session_device_resume_all(Session *s) {
@@ -487,3 +498,20 @@ void session_device_pause_all(Session *s) {
}
}
}
+
+unsigned int session_device_try_pause_all(Session *s) {
+ SessionDevice *sd;
+ Iterator i;
+ unsigned int num_pending = 0;
+
+ assert(s);
+
+ HASHMAP_FOREACH(sd, s->devices, i) {
+ if (sd->active) {
+ session_device_notify(sd, SESSION_DEVICE_TRY_PAUSE);
+ ++num_pending;
+ }
+ }
+
+ return num_pending;
+}
diff --git a/src/login/logind-session-device.h b/src/login/logind-session-device.h
index 98e61e6..61732bc 100644
--- a/src/login/logind-session-device.h
+++ b/src/login/logind-session-device.h
@@ -55,3 +55,4 @@ void session_device_complete_pause(SessionDevice *sd);

void session_device_resume_all(Session *s);
void session_device_pause_all(Session *s);
+unsigned int session_device_try_pause_all(Session *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 906115a..722e3d3 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -100,6 +100,8 @@ void session_free(Session *s) {
if (s->seat) {
if (s->seat->active == s)
s->seat->active = NULL;
+ if (s->seat->pending_switch == s)
+ s->seat->pending_switch = NULL;

LIST_REMOVE(Session, sessions_by_seat, s->seat->sessions, s);
}
@@ -375,21 +377,40 @@ int session_load(Session *s) {
}

int session_activate(Session *s) {
+ unsigned int num_pending;
+
assert(s);
assert(s->user);

- if (s->vtnr <= 0)
- return -ENOTSUP;
-
if (!s->seat)
return -ENOTSUP;

if (s->seat->active == s)
return 0;

- assert(seat_has_vts(s->seat));
+ /* on seats with VTs, we let VTs manage session-switching */
+ if (seat_has_vts(s->seat)) {
+ if (s->vtnr <= 0)
+ return -ENOTSUP;
+
+ return chvt(s->vtnr);
+ }

- return chvt(s->vtnr);
+ /* On seats without VTs, we implement session-switching in logind. We
+ * try to pause all session-devices and wait until the session
+ * controller acknowledged them. Once all devices are asleep, we simply
+ * switch the active session and be done.
+ * We save the session we want to switch to in seat->pending_switch and
+ * seat_complete_switch() will perform the final switch. */
+
+ s->seat->pending_switch = s;
+
+ /* if no devices are running, immediately perform the session switch */
+ num_pending = session_device_try_pause_all(s);
+ if (!num_pending)
+ seat_complete_switch(s->seat);
+
+ return 0;
}

static int session_link_x11_socket(Session *s) {
--
1.8.4
David Herrmann
2013-09-17 21:40:19 UTC
Permalink
This enables the multi-session capability for seats that don't have VTs.
For legacy seats with VTs, everything stays the same. However, all other
seats now also get the multi-session capability.

The only feature that was missing was session-switching. As logind can
force a session-switch and signal that via the "Active" property, we only
need a way to allow synchronized/delayed session switches. Compositors
need to cleanup some devices before acknowledging the session switch.
Therefore, we use the session-devices to give compositors a chance to
block a session-switch until they cleaned everything up.

If you activate a session on a seat without VTs, we send a PauseDevice
signal to the active session for every active device. Only once the
session acknowledged all these with a PauseDeviceComplete() call, we
perform the final session switch.

One important note is that delayed session-switching is meant for
backwards compatibility. New compositors or other sessions should really
try to deal correctly with forced session switches! They only need to
handle EACCES/EPERM from syscalls and treat them as "PauseDevice" signal.

Following logind patches will add a timeout to session-switches which
forces the switch if the active session does not react in a timely
fashion. Moreover, explicit ForceActivate() calls might also be supported.
Hence, sessions must not crash if their devices get paused.
---
src/login/logind-seat.c | 15 +++++++++++++++
src/login/logind-seat.h | 2 ++
src/login/logind-session-device.c | 28 ++++++++++++++++++++++++++++
src/login/logind-session-device.h | 1 +
src/login/logind-session.c | 31 ++++++++++++++++++++++++++-----
5 files changed, 72 insertions(+), 5 deletions(-)

diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index f88738a..4a4d40a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -425,6 +425,21 @@ int seat_attach_session(Seat *s, Session *session) {
return 0;
}

+void seat_complete_switch(Seat *s) {
+ Session *session;
+
+ assert(s);
+
+ /* if no session-switch is pending or if it got canceled, do nothing */
+ if (!s->pending_switch)
+ return;
+
+ session = s->pending_switch;
+ s->pending_switch = NULL;
+
+ seat_set_active(s, session);
+}
+
bool seat_has_vts(Seat *s) {
assert(s);

diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index d3438b8..be6db6e 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -38,6 +38,7 @@ struct Seat {
LIST_HEAD(Device, devices);

Session *active;
+ Session *pending_switch;
LIST_HEAD(Session, sessions);

bool in_gc_queue:1;
@@ -59,6 +60,7 @@ 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);

bool seat_has_vts(Seat *s);
bool seat_is_seat0(Seat *s);
diff --git a/src/login/logind-session-device.c b/src/login/logind-session-device.c
index 80fd364..e92bb54 100644
--- a/src/login/logind-session-device.c
+++ b/src/login/logind-session-device.c
@@ -414,10 +414,21 @@ void session_device_free(SessionDevice *sd) {
}

void session_device_complete_pause(SessionDevice *sd) {
+ SessionDevice *iter;
+ Iterator i;
+
if (!sd->active)
return;

session_device_stop(sd);
+
+ /* if not all devices are paused, wait for further completion events */
+ HASHMAP_FOREACH(iter, sd->session->devices, i)
+ if (iter->active)
+ return;
+
+ /* complete any pending session switch */
+ seat_complete_switch(sd->session->seat);
}

void session_device_resume_all(Session *s) {
@@ -449,3 +460,20 @@ void session_device_pause_all(Session *s) {
}
}
}
+
+unsigned int session_device_try_pause_all(Session *s) {
+ SessionDevice *sd;
+ Iterator i;
+ unsigned int num_pending = 0;
+
+ assert(s);
+
+ HASHMAP_FOREACH(sd, s->devices, i) {
+ if (sd->active) {
+ session_device_notify(sd, SESSION_DEVICE_TRY_PAUSE);
+ ++num_pending;
+ }
+ }
+
+ return num_pending;
+}
diff --git a/src/login/logind-session-device.h b/src/login/logind-session-device.h
index 511fce0..eac7ca7 100644
--- a/src/login/logind-session-device.h
+++ b/src/login/logind-session-device.h
@@ -57,3 +57,4 @@ void session_device_complete_pause(SessionDevice *sd);

void session_device_resume_all(Session *s);
void session_device_pause_all(Session *s);
+unsigned int session_device_try_pause_all(Session *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index fcc1901..eea0bfb 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -100,6 +100,8 @@ void session_free(Session *s) {
if (s->seat) {
if (s->seat->active == s)
s->seat->active = NULL;
+ if (s->seat->pending_switch == s)
+ s->seat->pending_switch = NULL;

LIST_REMOVE(Session, sessions_by_seat, s->seat->sessions, s);
}
@@ -375,21 +377,40 @@ int session_load(Session *s) {
}

int session_activate(Session *s) {
+ unsigned int num_pending;
+
assert(s);
assert(s->user);

- if (s->vtnr <= 0)
- return -ENOTSUP;
-
if (!s->seat)
return -ENOTSUP;

if (s->seat->active == s)
return 0;

- assert(seat_has_vts(s->seat));
+ /* on seats with VTs, we let VTs manage session-switching */
+ if (seat_has_vts(s->seat)) {
+ if (s->vtnr <= 0)
+ return -ENOTSUP;
+
+ return chvt(s->vtnr);
+ }

- return chvt(s->vtnr);
+ /* On seats without VTs, we implement session-switching in logind. We
+ * try to pause all session-devices and wait until the session
+ * controller acknowledged them. Once all devices are asleep, we simply
+ * switch the active session and be done.
+ * We save the session we want to switch to in seat->pending_switch and
+ * seat_complete_switch() will perform the final switch. */
+
+ s->seat->pending_switch = s;
+
+ /* if no devices are running, immediately perform the session switch */
+ num_pending = session_device_try_pause_all(s);
+ if (!num_pending)
+ seat_complete_switch(s->seat);
+
+ return 0;
}

static int session_link_x11_socket(Session *s) {
--
1.8.4
Lennart Poettering
2013-09-17 22:16:03 UTC
Permalink
On Tue, 17.09.13 23:40, David Herrmann (***@gmail.com) wrote:

Applied both! Thanks a lot!
Post by David Herrmann
This enables the multi-session capability for seats that don't have VTs.
For legacy seats with VTs, everything stays the same. However, all other
seats now also get the multi-session capability.
The only feature that was missing was session-switching. As logind can
force a session-switch and signal that via the "Active" property, we only
need a way to allow synchronized/delayed session switches. Compositors
need to cleanup some devices before acknowledging the session switch.
Therefore, we use the session-devices to give compositors a chance to
block a session-switch until they cleaned everything up.
If you activate a session on a seat without VTs, we send a PauseDevice
signal to the active session for every active device. Only once the
session acknowledged all these with a PauseDeviceComplete() call, we
perform the final session switch.
One important note is that delayed session-switching is meant for
backwards compatibility. New compositors or other sessions should really
try to deal correctly with forced session switches! They only need to
handle EACCES/EPERM from syscalls and treat them as "PauseDevice" signal.
Following logind patches will add a timeout to session-switches which
forces the switch if the active session does not react in a timely
fashion. Moreover, explicit ForceActivate() calls might also be supported.
Hence, sessions must not crash if their devices get paused.
---
src/login/logind-seat.c | 15 +++++++++++++++
src/login/logind-seat.h | 2 ++
src/login/logind-session-device.c | 28 ++++++++++++++++++++++++++++
src/login/logind-session-device.h | 1 +
src/login/logind-session.c | 31 ++++++++++++++++++++++++++-----
5 files changed, 72 insertions(+), 5 deletions(-)
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index f88738a..4a4d40a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -425,6 +425,21 @@ int seat_attach_session(Seat *s, Session *session) {
return 0;
}
+void seat_complete_switch(Seat *s) {
+ Session *session;
+
+ assert(s);
+
+ /* if no session-switch is pending or if it got canceled, do nothing */
+ if (!s->pending_switch)
+ return;
+
+ session = s->pending_switch;
+ s->pending_switch = NULL;
+
+ seat_set_active(s, session);
+}
+
bool seat_has_vts(Seat *s) {
assert(s);
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index d3438b8..be6db6e 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -38,6 +38,7 @@ struct Seat {
LIST_HEAD(Device, devices);
Session *active;
+ Session *pending_switch;
LIST_HEAD(Session, sessions);
bool in_gc_queue:1;
@@ -59,6 +60,7 @@ 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);
bool seat_has_vts(Seat *s);
bool seat_is_seat0(Seat *s);
diff --git a/src/login/logind-session-device.c b/src/login/logind-session-device.c
index 80fd364..e92bb54 100644
--- a/src/login/logind-session-device.c
+++ b/src/login/logind-session-device.c
@@ -414,10 +414,21 @@ void session_device_free(SessionDevice *sd) {
}
void session_device_complete_pause(SessionDevice *sd) {
+ SessionDevice *iter;
+ Iterator i;
+
if (!sd->active)
return;
session_device_stop(sd);
+
+ /* if not all devices are paused, wait for further completion events */
+ HASHMAP_FOREACH(iter, sd->session->devices, i)
+ if (iter->active)
+ return;
+
+ /* complete any pending session switch */
+ seat_complete_switch(sd->session->seat);
}
void session_device_resume_all(Session *s) {
@@ -449,3 +460,20 @@ void session_device_pause_all(Session *s) {
}
}
}
+
+unsigned int session_device_try_pause_all(Session *s) {
+ SessionDevice *sd;
+ Iterator i;
+ unsigned int num_pending = 0;
+
+ assert(s);
+
+ HASHMAP_FOREACH(sd, s->devices, i) {
+ if (sd->active) {
+ session_device_notify(sd, SESSION_DEVICE_TRY_PAUSE);
+ ++num_pending;
+ }
+ }
+
+ return num_pending;
+}
diff --git a/src/login/logind-session-device.h b/src/login/logind-session-device.h
index 511fce0..eac7ca7 100644
--- a/src/login/logind-session-device.h
+++ b/src/login/logind-session-device.h
@@ -57,3 +57,4 @@ void session_device_complete_pause(SessionDevice *sd);
void session_device_resume_all(Session *s);
void session_device_pause_all(Session *s);
+unsigned int session_device_try_pause_all(Session *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index fcc1901..eea0bfb 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -100,6 +100,8 @@ void session_free(Session *s) {
if (s->seat) {
if (s->seat->active == s)
s->seat->active = NULL;
+ if (s->seat->pending_switch == s)
+ s->seat->pending_switch = NULL;
LIST_REMOVE(Session, sessions_by_seat, s->seat->sessions, s);
}
@@ -375,21 +377,40 @@ int session_load(Session *s) {
}
int session_activate(Session *s) {
+ unsigned int num_pending;
+
assert(s);
assert(s->user);
- if (s->vtnr <= 0)
- return -ENOTSUP;
-
if (!s->seat)
return -ENOTSUP;
if (s->seat->active == s)
return 0;
- assert(seat_has_vts(s->seat));
+ /* on seats with VTs, we let VTs manage session-switching */
+ if (seat_has_vts(s->seat)) {
+ if (s->vtnr <= 0)
+ return -ENOTSUP;
+
+ return chvt(s->vtnr);
+ }
- return chvt(s->vtnr);
+ /* On seats without VTs, we implement session-switching in logind. We
+ * try to pause all session-devices and wait until the session
+ * controller acknowledged them. Once all devices are asleep, we simply
+ * switch the active session and be done.
+ * We save the session we want to switch to in seat->pending_switch and
+ * seat_complete_switch() will perform the final switch. */
+
+ s->seat->pending_switch = s;
+
+ /* if no devices are running, immediately perform the session switch */
+ num_pending = session_device_try_pause_all(s);
+ if (!num_pending)
+ seat_complete_switch(s->seat);
+
+ return 0;
}
static int session_link_x11_socket(Session *s) {
Lennart
--
Lennart Poettering - Red Hat, Inc.
Lennart Poettering
2013-09-17 16:16:12 UTC
Permalink
On Tue, 17.09.13 17:39, David Herrmann (***@gmail.com) wrote:

Applied! Thanks!
Post by David Herrmann
Session compositors need access to fbdev, DRM and evdev devices if they
control a session. To make logind pass them to sessions, we need to
listen for them actively.
However, we avoid creating new seats for non master-of-seat devices. Only
once a seat is created, we start remembering all other session devices. If
the last master-device is removed (even if there are other non-master
devices still available), we destroy the seat. This is the current
behavior, but we need to explicitly implement it now as there may be
non-master devices in the seat->devices list.
Unlike master devices, we don't care whether our list of non-master
devices is complete. We don't export this list but use it only as cache if
sessions request these devices. Hence, if a session requests a device that
is not in the list, we will simply look it up. However, once a session
requested a device, we must be notified of "remove" udev events. So we
must link the devices somehow into the device-list.
Regarding the implementation, we now sort the device list by the "master"
flag. This guarantees that master devices are at the front and non-master
devices at the tail of the list. Thus, we can easily test whether a seat
has a master device attached.
---
src/login/logind-device.c | 35 ++++++++++++++++++---
src/login/logind-device.h | 3 +-
src/login/logind-seat.c | 11 +++++--
src/login/logind-seat.h | 1 +
src/login/logind.c | 79 +++++++++++++++++++++++++++++++++++++++++------
src/login/logind.h | 6 ++--
6 files changed, 116 insertions(+), 19 deletions(-)
diff --git a/src/login/logind-device.c b/src/login/logind-device.c
index 51b1535..a9a9633 100644
--- a/src/login/logind-device.c
+++ b/src/login/logind-device.c
@@ -25,7 +25,7 @@
#include "logind-device.h"
#include "util.h"
-Device* device_new(Manager *m, const char *sysfs) {
+Device* device_new(Manager *m, const char *sysfs, bool master) {
Device *d;
assert(m);
@@ -48,6 +48,7 @@ Device* device_new(Manager *m, const char *sysfs) {
}
d->manager = m;
+ d->master = master;
dual_timestamp_get(&d->timestamp);
return d;
@@ -75,11 +76,16 @@ void device_detach(Device *d) {
LIST_REMOVE(Device, devices, d->seat->devices, d);
d->seat = NULL;
- seat_add_to_gc_queue(s);
- seat_send_changed(s, "CanGraphical\0");
+ if (!seat_has_master_device(s)) {
+ seat_add_to_gc_queue(s);
+ seat_send_changed(s, "CanGraphical\0");
+ }
}
void device_attach(Device *d, Seat *s) {
+ Device *i;
+ bool had_master;
+
assert(d);
assert(s);
@@ -90,7 +96,26 @@ void device_attach(Device *d, Seat *s) {
device_detach(d);
d->seat = s;
- LIST_PREPEND(Device, devices, s->devices, d);
+ had_master = seat_has_master_device(s);
+
+ /* We keep the device list sorted by the "master" flag. That is, master
+ * devices are at the front, other devices at the tail. As there is no
+ * way to easily add devices at the list-tail, we need to iterate the
+ * list to find the first non-master device when adding non-master
+ * devices. We assume there is only a few (normally 1) master devices
+ * per seat, so we iterate only a few times. */
+
+ if (d->master || !s->devices) {
+ LIST_PREPEND(Device, devices, s->devices, d);
+ } else {
+ LIST_FOREACH(devices, i, s->devices) {
+ if (!i->devices_next || !i->master) {
+ LIST_INSERT_AFTER(Device, devices, s->devices, i, d);
+ break;
+ }
+ }
+ }
- seat_send_changed(s, "CanGraphical\0");
+ if (!had_master && d->master)
+ seat_send_changed(s, "CanGraphical\0");
}
diff --git a/src/login/logind-device.h b/src/login/logind-device.h
index 3b15356..315f0e6 100644
--- a/src/login/logind-device.h
+++ b/src/login/logind-device.h
@@ -33,13 +33,14 @@ struct Device {
char *sysfs;
Seat *seat;
+ bool master;
dual_timestamp timestamp;
LIST_FIELDS(struct Device, devices);
};
-Device* device_new(Manager *m, const char *sysfs);
+Device* device_new(Manager *m, const char *sysfs, bool master);
void device_free(Device *d);
void device_attach(Device *d, Seat *s);
void device_detach(Device *d);
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 470d08b..2c60b8a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -448,10 +448,17 @@ bool seat_can_tty(Seat *s) {
return seat_is_vtconsole(s);
}
+bool seat_has_master_device(Seat *s) {
+ assert(s);
+
+ /* device list is ordered by "master" flag */
+ return !!s->devices && s->devices->master;
+}
+
bool seat_can_graphical(Seat *s) {
assert(s);
- return !!s->devices;
+ return seat_has_master_device(s);
}
int seat_get_idle_hint(Seat *s, dual_timestamp *t) {
@@ -499,7 +506,7 @@ int seat_check_gc(Seat *s, bool drop_not_started) {
if (seat_is_vtconsole(s))
return 1;
- return !!s->devices;
+ return seat_has_master_device(s);
}
void seat_add_to_gc_queue(Seat *s) {
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index c8ab17f..bd5390f 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -63,6 +63,7 @@ int seat_attach_session(Seat *s, Session *session);
bool seat_is_vtconsole(Seat *s);
bool seat_can_multi_session(Seat *s);
bool seat_can_tty(Seat *s);
+bool seat_has_master_device(Seat *s);
bool seat_can_graphical(Seat *s);
int seat_get_idle_hint(Seat *s, dual_timestamp *t);
diff --git a/src/login/logind.c b/src/login/logind.c
index 4ef92b8..29019c2 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -151,6 +151,8 @@ void manager_free(Manager *m) {
if (m->udev_seat_monitor)
udev_monitor_unref(m->udev_seat_monitor);
+ if (m->udev_device_monitor)
+ udev_monitor_unref(m->udev_device_monitor);
if (m->udev_vcsa_monitor)
udev_monitor_unref(m->udev_vcsa_monitor);
if (m->udev_button_monitor)
@@ -184,7 +186,7 @@ void manager_free(Manager *m) {
free(m);
}
-int manager_add_device(Manager *m, const char *sysfs, Device **_device) {
+int manager_add_device(Manager *m, const char *sysfs, bool master, Device **_device) {
Device *d;
assert(m);
@@ -195,10 +197,13 @@ int manager_add_device(Manager *m, const char *sysfs, Device **_device) {
if (_device)
*_device = d;
+ /* we support adding master-flags, but not removing them */
+ d->master = d->master || master;
+
return 0;
}
- d = device_new(m, sysfs);
+ d = device_new(m, sysfs, master);
if (!d)
return -ENOMEM;
@@ -373,7 +378,8 @@ int manager_process_seat_device(Manager *m, struct udev_device *d) {
} else {
const char *sn;
- Seat *seat;
+ Seat *seat = NULL;
+ bool master;
sn = udev_device_get_property_value(d, "ID_SEAT");
if (isempty(sn))
@@ -384,16 +390,23 @@ int manager_process_seat_device(Manager *m, struct udev_device *d) {
return 0;
}
- r = manager_add_device(m, udev_device_get_syspath(d), &device);
+ /* ignore non-master devices for unknown seats */
+ master = udev_device_has_tag(d, "master-of-seat");
+ if (!master && !(seat = hashmap_get(m->seats, sn)))
+ return 0;
+
+ r = manager_add_device(m, udev_device_get_syspath(d), master, &device);
if (r < 0)
return r;
- r = manager_add_seat(m, sn, &seat);
- if (r < 0) {
- if (!device->seat)
- device_free(device);
+ if (!seat) {
+ r = manager_add_seat(m, sn, &seat);
+ if (r < 0) {
+ if (!device->seat)
+ device_free(device);
- return r;
+ return r;
+ }
}
device_attach(device, seat);
@@ -762,6 +775,22 @@ int manager_dispatch_seat_udev(Manager *m) {
return r;
}
+static int manager_dispatch_device_udev(Manager *m) {
+ struct udev_device *d;
+ int r;
+
+ assert(m);
+
+ d = udev_monitor_receive_device(m->udev_device_monitor);
+ if (!d)
+ return -ENOMEM;
+
+ r = manager_process_seat_device(m, d);
+ udev_device_unref(d);
+
+ return r;
+}
+
int manager_dispatch_vcsa_udev(Manager *m) {
struct udev_device *d;
int r = 0;
@@ -1149,6 +1178,7 @@ static int manager_connect_udev(Manager *m) {
assert(m);
assert(!m->udev_seat_monitor);
+ assert(!m->udev_device_monitor);
assert(!m->udev_vcsa_monitor);
assert(!m->udev_button_monitor);
@@ -1169,6 +1199,33 @@ static int manager_connect_udev(Manager *m) {
if (epoll_ctl(m->epoll_fd, EPOLL_CTL_ADD, m->udev_seat_fd, &ev) < 0)
return -errno;
+ m->udev_device_monitor = udev_monitor_new_from_netlink(m->udev, "udev");
+ if (!m->udev_device_monitor)
+ return -ENOMEM;
+
+ r = udev_monitor_filter_add_match_subsystem_devtype(m->udev_device_monitor, "input", NULL);
+ if (r < 0)
+ return r;
+
+ r = udev_monitor_filter_add_match_subsystem_devtype(m->udev_device_monitor, "graphics", NULL);
+ if (r < 0)
+ return r;
+
+ r = udev_monitor_filter_add_match_subsystem_devtype(m->udev_device_monitor, "drm", NULL);
+ if (r < 0)
+ return r;
+
+ r = udev_monitor_enable_receiving(m->udev_device_monitor);
+ if (r < 0)
+ return r;
+
+ m->udev_device_fd = udev_monitor_get_fd(m->udev_device_monitor);
+ zero(ev);
+ ev.events = EPOLLIN;
+ ev.data.u32 = FD_DEVICE_UDEV;
+ if (epoll_ctl(m->epoll_fd, EPOLL_CTL_ADD, m->udev_device_fd, &ev) < 0)
+ return -errno;
+
/* Don't watch keys if nobody cares */
if (m->handle_power_key != HANDLE_IGNORE ||
m->handle_suspend_key != HANDLE_IGNORE ||
@@ -1545,6 +1602,10 @@ int manager_run(Manager *m) {
manager_dispatch_seat_udev(m);
break;
+ manager_dispatch_device_udev(m);
+ break;
+
manager_dispatch_vcsa_udev(m);
break;
diff --git a/src/login/logind.h b/src/login/logind.h
index e9838a8..1a97351 100644
--- a/src/login/logind.h
+++ b/src/login/logind.h
@@ -57,9 +57,10 @@ struct Manager {
LIST_HEAD(User, user_gc_queue);
struct udev *udev;
- struct udev_monitor *udev_seat_monitor, *udev_vcsa_monitor, *udev_button_monitor;
+ struct udev_monitor *udev_seat_monitor, *udev_device_monitor, *udev_vcsa_monitor, *udev_button_monitor;
int udev_seat_fd;
+ int udev_device_fd;
int udev_vcsa_fd;
int udev_button_fd;
@@ -121,6 +122,7 @@ struct Manager {
enum {
FD_SEAT_UDEV,
+ FD_DEVICE_UDEV,
FD_VCSA_UDEV,
FD_BUTTON_UDEV,
FD_CONSOLE,
@@ -132,7 +134,7 @@ enum {
Manager *manager_new(void);
void manager_free(Manager *m);
-int manager_add_device(Manager *m, const char *sysfs, Device **_device);
+int manager_add_device(Manager *m, const char *sysfs, bool master, Device **_device);
int manager_add_button(Manager *m, const char *name, Button **_button);
int manager_add_seat(Manager *m, const char *id, Seat **_seat);
int manager_add_session(Manager *m, const char *id, Session **_session);
Lennart
--
Lennart Poettering - Red Hat, Inc.
Loading...