ubuntu-multiseat team mailing list archive

[Laércio de Sousa <lbsousajr@xxxxxxxxx>]

Laércio de Sousa has proposed merging lp:~ubuntu-multiseat/lightdm/automatic-multiseat into lp:lightdm.

Requested reviews:
  LightDM Development Team (lightdm-team)
Related bugs:
  Bug #1190581 in Light Display Manager: "Support logind's automatic multiseat feature"
  https://bugs.launchpad.net/lightdm/+bug/1190581

For more details, see:
https://code.launchpad.net/~ubuntu-multiseat/lightdm/automatic-multiseat/+merge/229682

This branch implements automatic multiseat support in LightDM, as reported in bug #1190581. Basically, it should implement steps 1 & 2 of "Complete porting" section in systemd documentation [1]. It's largely based on GDM implementation of the feature.

There are some pieces missing in revision 2041, but support for adding new seats is finished. I'm proposing this merge right now, so you can review it and suggest improvements.

At the moment, I can list the following issues:

* It doesn't watch PropertyChanged events on seats.

* In order to keep login1.{h,c} simple (namely, avoid #include'ing seat.h and display-manager.h), callback functions for login1 SeatNew()/SeatRemoved() signals are implemented directly in lightdm.c. Is this OK, or should I move these callback functions elsewhere?

* It still lacks a proper implementation of a callback function for SeatRemoved() signal. Maybe a display_manager_remove_seat() function (counterpart for display_manager_add_seat() one) is required.

* When loading initial seat list from logind (by calling login1_sync_seats()), we always get at least one seat (namely, seat0). It may conflict with LightDM option "start-default-seat", ending up with two seat0 seats registered in LightDM (one triggered by start-default-seat, and another one triggered by login1_sync_seats()). Which is the preferred way to prevent this conflict?

* Should we set property "seat-name" for automatically added seats? Maybe we can even implement support for special sections in lightdm.conf for these ones, which are loaded whenever a seat matching this name is added. Example:

[AutoSeat:foo]
autologin-user=user1

sets autologin user whenever a seat "seatfoo" is added.

* Test scripts are missing.

[1] http://www.freedesktop.org/wiki/Software/systemd/writing-display-managers/
-- 
https://code.launchpad.net/~ubuntu-multiseat/lightdm/automatic-multiseat/+merge/229682
Your team Ubuntu Multiseat is subscribed to branch lp:~ubuntu-multiseat/lightdm/automatic-multiseat.

=== modified file 'src/lightdm.c'
--- src/lightdm.c	2014-07-31 04:59:55 +0000
+++ src/lightdm.c	2014-08-05 19:59:46 +0000
@@ -22,6 +22,7 @@
 #include <errno.h>
 
 #include "configuration.h"
+#include "login1.h"
 #include "display-manager.h"
 #include "xdmcp-server.h"
 #include "vnc-server.h"
@@ -980,6 +981,62 @@
     exit (EXIT_FAILURE);
 }
 
+static void
+seat_new_login1_cb (GDBusConnection *connection,
+                    const gchar *sender_name,
+                    const gchar *object_path,
+                    const gchar *interface_name,
+                    const gchar *signal_name,
+                    GVariant *parameters,
+                    gpointer user_data)
+{
+    const gchar *xdg_seat;
+    const gchar *login1_seat_path;
+    Seat *seat;
+
+    g_variant_get (parameters, "(&s&o)", &xdg_seat, &login1_seat_path);
+
+    if (g_strcmp0 (signal_name, "SeatNew") == 0)
+        g_debug ("Got signal SeatNew() from logind: %s", xdg_seat);
+
+    g_debug ("Add seat for login1 seat path %s", login1_seat_path);
+    seat = seat_new ("xlocal");
+
+    if (seat)
+    {
+        set_seat_properties (seat, NULL);
+        seat_set_property (seat, "xdg-seat", xdg_seat);
+    }
+    else
+    {
+        // FIXME: Need to make proper error
+        g_warning ("Unable to create seat for login1 seat path %s", login1_seat_path);
+        return;
+    }
+
+    if (!display_manager_add_seat (display_manager, seat)) // FIXME: Need to make proper error
+        g_warning ("Failed to start seat for login1 seat path %s", login1_seat_path);
+
+    g_object_unref (seat);
+}
+
+static void
+seat_removed_login1_cb (GDBusConnection *connection,
+                        const gchar *sender_name,
+                        const gchar *object_path,
+                        const gchar *interface_name,
+                        const gchar *signal_name,
+                        GVariant *parameters,
+                        gpointer user_data)
+{
+    const gchar *xdg_seat;
+    const gchar *login1_seat_path;
+
+    g_variant_get (parameters, "(&s&o)", &xdg_seat, &login1_seat_path);
+    g_debug ("Got signal SeatRemoved() from logind: %s", xdg_seat);
+    // FIXME: Implement algorithm to remove the seat
+}
+
 int
 main (int argc, char **argv)
 {
@@ -987,6 +1044,7 @@
     GOptionContext *option_context;
     gboolean result;
     gchar **groups, **i, *dir;
+    guint login1_seat_new_id = 0, login1_seat_removed_id = 0;
     gint n_seats = 0;
     gboolean test_mode = FALSE;
     gchar *pid_path = "/var/run/lightdm.pid";
@@ -1232,6 +1290,22 @@
 
     shared_data_manager_start (shared_data_manager_get_instance ());
 
+    /* Load dynamic seats from logind
+     * NOTE: since it always return at least one seat (namely, seat0),
+     *       it may conflict with start-default-seat option,
+     *       so we'll disable it here. */
+    if (login1_is_running ())
+    {
+        g_debug ("Start monitoring logind for new seats");
+        login1_start_monitor (&login1_seat_new_id, &login1_seat_removed_id,
+                              seat_new_login1_cb,
+                              seat_removed_login1_cb);
+
+        g_debug ("Load initial seat list from logind");
+        login1_sync_seats (seat_new_login1_cb);
+        config_set_boolean (config_get_instance (), "LightDM", "start-default-seat", FALSE);
+    }
+
     /* Load the static display entries */
     groups = config_get_groups (config_get_instance ());
     for (i = groups; *i; i++)
@@ -1307,6 +1381,13 @@
 
     g_main_loop_run (loop);
 
+    /* Clean up logind connections */
+    if (login1_is_running ())
+    {
+        g_debug ("Stop monitoring logind for new seats");
+        login1_stop_monitor (&login1_seat_new_id, &login1_seat_removed_id);
+    }
+
     /* Clean up shared data manager */
     shared_data_manager_cleanup ();
 

=== modified file 'src/login1.c'
--- src/login1.c	2014-05-14 20:19:00 +0000
+++ src/login1.c	2014-08-05 19:59:46 +0000
@@ -10,8 +10,6 @@
  * license.
  */
 
-#include <gio/gio.h>
-
 #include "login1.h"
 
 static gboolean
@@ -218,3 +216,119 @@
     }
     g_object_unref (bus);
 }
+
+void
+login1_sync_seats (GDBusSignalCallback seat_new_cb)
+{
+    GDBusConnection *bus;
+    GVariant *xdg_seat_path;
+    GVariant *result;
+    GVariant *array;
+    GVariantIter iter;
+    GError *error = NULL;
+    const gchar *xdg_seat;
+
+    bus = g_bus_get_sync (G_BUS_TYPE_SYSTEM, NULL, &error);
+    if (error)
+        g_warning ("Failed to get system bus: %s", error->message);
+    g_clear_error (&error);
+    if (!bus)
+        return;
+    result = g_dbus_connection_call_sync (bus,
+                                          "org.freedesktop.login1",
+                                          "/org/freedesktop/login1",
+                                          "org.freedesktop.login1.Manager",
+                                          "ListSeats",
+                                          NULL,
+                                          G_VARIANT_TYPE ("(a(so))"),
+                                          G_DBUS_CALL_FLAGS_NONE,
+                                          -1,
+                                          NULL,
+                                          &error);
+    g_object_unref (bus);
+
+    if (error)
+        g_warning ("Failed to sync login1 seats: %s", error->message);
+    g_clear_error (&error);
+    if (!result)
+        return;
+
+    array = g_variant_get_child_value (result, 0);
+    g_variant_iter_init (&iter, array);
+
+    while (g_variant_iter_loop (&iter, "(&s&o)", &xdg_seat, &xdg_seat_path))
+    {
+        g_debug ("Add login1 seat: %s", xdg_seat);
+        seat_new_cb (bus,
+                     "org.freedesktop.login1",
+                     "org/freedesktop/login1",
+                     "org.freedesktop.login1.Manager",
+                     "ListSeats",
+                     g_variant_new ("(so)", xdg_seat, xdg_seat_path),
+                     NULL);
+    }
+
+    g_variant_unref (result);
+    g_variant_unref (array);
+}
+
+void
+login1_start_monitor (guint *seat_new_id, guint *seat_removed_id,
+                      GDBusSignalCallback seat_new_cb,
+                      GDBusSignalCallback seat_removed_cb)
+{
+    GDBusConnection *bus;
+    GError *error = NULL;
+
+    bus = g_bus_get_sync (G_BUS_TYPE_SYSTEM, NULL, &error);
+    if (error)
+        g_warning ("Failed to get system bus: %s", error->message);
+    g_clear_error (&error);
+    if (!bus)
+        return;
+    *seat_new_id = g_dbus_connection_signal_subscribe (bus,
+                                                       "org.freedesktop.login1",
+                                                       "org.freedesktop.login1.Manager",
+                                                       "SeatNew",
+                                                       "/org/freedesktop/login1",
+                                                       NULL,
+                                                       G_DBUS_SIGNAL_FLAGS_NONE,
+                                                       seat_new_cb,
+                                                       NULL,
+                                                       NULL);
+    *seat_removed_id = g_dbus_connection_signal_subscribe (bus,
+                                                           "org.freedesktop.login1",
+                                                           "org.freedesktop.login1.Manager",
+                                                           "SeatRemoved",
+                                                           "/org/freedesktop/login1",
+                                                           NULL,
+                                                           G_DBUS_SIGNAL_FLAGS_NONE,
+                                                           seat_removed_cb,
+                                                           NULL,
+                                                           NULL);
+}
+
+void
+login1_stop_monitor (guint *seat_new_id, guint *seat_removed_id)
+{
+    GDBusConnection *bus;
+    GError *error = NULL;
+
+    bus = g_bus_get_sync (G_BUS_TYPE_SYSTEM, NULL, &error);
+    if (error)
+        g_warning ("Failed to get system bus: %s", error->message);
+    g_clear_error (&error);
+    if (!bus)
+        return;
+    if (*seat_new_id)
+    {
+       g_dbus_connection_signal_unsubscribe (bus, *seat_new_id);
+       *seat_new_id = 0;
+    }
+
+    if (*seat_removed_id)
+    {
+       g_dbus_connection_signal_unsubscribe (bus, *seat_removed_id);
+       *seat_removed_id = 0;
+    }
+}

=== modified file 'src/login1.h'
--- src/login1.h	2014-03-17 16:02:32 +0000
+++ src/login1.h	2014-08-05 19:59:46 +0000
@@ -13,6 +13,7 @@
 #define _LOGIN1_H_
 
 #include <glib-object.h>
+#include <gio/gio.h>
 
 G_BEGIN_DECLS
 
@@ -26,6 +27,14 @@
 
 void login1_activate_session (const gchar *session_path);
 
+void login1_sync_seats (GDBusSignalCallback seat_new_cb);
+
+void login1_start_monitor (guint *seat_new_id, guint *seat_removed_id,
+                           GDBusSignalCallback seat_new_cb,
+                           GDBusSignalCallback seat_removed_cb);
+
+void login1_stop_monitor (guint *seat_new_id, guint *seat_removed_id);
+
 G_END_DECLS
 
 #endif /* _LOGIN1_H_ */