ubuntu-multiseat team mailing list archive

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

Laércio de Sousa has proposed merging lp:~ubuntu-multiseat/lightdm/new-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/new-automatic-multiseat/+merge/231903

This new branch implements automatic multiseat support in LightDM, following steps 1 & 2 of "Complete porting" section in systemd-logind documentation [1]. This is a rework of old branch lp:~ubuntu-multiseat/lightdm/automatic-multiseat after introduction of login1 service object in upstream revision 2028.

This new implementation has some additional features, added after discussion with Robert Ancell:

 * If logind is running, LightDM won't try to load seats statically as before. Instead, any [Seat:seat*] config sections in lightdm.conf will be loaded on demand, as new seats are added from logind. Since logind returns at least one seat (namely, seat0), start-default-seat condition should never be reached in this case.

 * Seat properties "seat-name" and "xdg-seat" will be both automatically set to return value of function login1_seat_get_id(). It opens the possibility of merging both properties, or deprecating "xdg-seat", in the near future.

At the moment, this branch has some issues:

1. Test scripts have not been added yet.

2. What should it do if a logind seat has property CanGraphical=no?

3. It still doesn't watch PropertyChanged events on logind seats. What should it do if logind seat property CanGraphical changes?

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

=== modified file 'src/lightdm.c'
--- src/lightdm.c	2014-08-22 09:01:20 +0000
+++ src/lightdm.c	2014-08-22 14:40:39 +0000
@@ -979,6 +979,80 @@
     exit (EXIT_FAILURE);
 }
 
+static void
+login1_service_seat_added_cb (Login1Service *service, Login1Seat *login1_seat)
+{
+    const gchar *seat_name = login1_seat_get_id (login1_seat);
+    gchar **groups, **i;
+    Seat *seat;
+
+    g_debug ("Add seat for login1 seat id %s", seat_name);
+    seat = seat_new ("xlocal");
+
+    if (seat)
+    {
+        groups = config_get_groups (config_get_instance ());
+        set_seat_properties (seat, NULL);
+
+        if (!login1_seat_get_can_multi_session (login1_seat))
+        {
+            g_debug ("Seat %s has property CanMultiSession=no", seat_name);
+            seat_set_property (seat, "allow-user-switching", "false");
+        }
+
+        for (i = groups; *i; i++)
+        {
+            gchar *config_section = *i;
+
+            if (!g_str_has_prefix (config_section, "Seat:") ||
+                !g_str_has_suffix (config_section, seat_name))
+                continue;
+
+            g_debug ("Loading properties from config section %s", config_section);
+            set_seat_properties (seat, config_section);
+        }
+
+        seat_set_property (seat, "seat-name", seat_name);
+        seat_set_property (seat, "xdg-seat", seat_name);
+        g_strfreev (groups);
+    }
+    else
+    {
+        // FIXME: Need to make proper error
+        g_warning ("Unable to create seat for login1 seat id %s", seat_name);
+        return;
+    }
+
+    if (!display_manager_add_seat (display_manager, seat)) // FIXME: Need to make proper error
+        g_warning ("Failed to start seat for login1 seat id %s", seat_name);
+
+    g_object_unref (seat);
+}
+
+static void
+login1_service_seat_removed_cb (Login1Service *service, Login1Seat *login1_seat)
+{
+    GList *seats, *link;
+    Seat *seat;
+
+    /* Stop all seats matching given xdg-seat property value.
+     * Copy the list as it might be modified if a seat stops during this loop */
+    seats = g_list_copy (display_manager_get_seats (display_manager));
+
+    /* FIXME: This loop should be uneeded, provided we can ensure
+     *        there's only one Seat object in DisplayManager list
+     *        matching given Login1Seat object id. */
+    for (link = seats; link; link = link->next)
+    {
+        seat = link->data;
+
+        if (g_strcmp0 (seat_get_name (seat), login1_seat_get_id (login1_seat)) == 0)
+            seat_stop (seat);
+    }
+
+    g_list_free (seats);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -997,6 +1071,7 @@
     gchar *default_cache_dir = g_strdup (CACHE_DIR);
     gboolean show_config = FALSE, show_version = FALSE;
     GList *link, *messages = NULL;
+    Login1Service *login1_service;
     GOptionEntry options[] =
     {
         { "config", 'c', 0, G_OPTION_ARG_STRING, &config_path,
@@ -1311,48 +1386,62 @@
     shared_data_manager_start (shared_data_manager_get_instance ());
 
     /* Connect to logind */
-    login1_service_connect (login1_service_get_instance ());
-
-    /* Load the static display entries */
-    groups = config_get_groups (config_get_instance ());
-    for (i = groups; *i; i++)
+    login1_service = login1_service_get_instance ();
+    if (login1_service_connect (login1_service))
     {
-        gchar *config_section = *i;
-        gchar **types;
-        gchar **type;
-        Seat *seat = NULL;
-        const gchar *const seatpfx = "Seat:";
-
-        if (!g_str_has_prefix (config_section, seatpfx))
-            continue;
-
-        g_debug ("Loading seat %s", config_section);
-        types = config_get_string_list (config_get_instance (), config_section, "type");
-        if (!types)
-            types = config_get_string_list (config_get_instance (), "SeatDefaults", "type");
-        for (type = types; type && *type; type++)
-        {
-            seat = seat_new (*type);
-            if (seat)
-                break;
-        }
-        g_strfreev (types);
-        if (seat)
-        {
-            const gsize seatpfxlen = strlen(seatpfx);
-            gchar *seatname = config_section + seatpfxlen;
-
-            seat_set_property (seat, "seat-name", seatname);
-
-            set_seat_properties (seat, config_section);
-            display_manager_add_seat (display_manager, seat);
-            g_object_unref (seat);
+        /* Load dynamic seats from logind */
+        g_signal_connect (login1_service, "seat-added", G_CALLBACK (login1_service_seat_added_cb), NULL);
+        g_signal_connect (login1_service, "seat-removed", G_CALLBACK (login1_service_seat_removed_cb), NULL);
+
+        for (link = login1_service_get_seats (login1_service); link; link = link->next)
+        {
+            login1_service_seat_added_cb (login1_service, (Login1Seat *) link->data);
             n_seats++;
         }
-        else
-            g_warning ("Failed to create seat %s", config_section);
-    }
-    g_strfreev (groups);
+    }
+    else
+    {
+        /* Load the static display entries */
+        groups = config_get_groups (config_get_instance ());
+        for (i = groups; *i; i++)
+        {
+            gchar *config_section = *i;
+            gchar **types;
+            gchar **type;
+            Seat *seat = NULL;
+            const gchar *const seatpfx = "Seat:";
+
+            if (!g_str_has_prefix (config_section, seatpfx))
+                continue;
+
+            g_debug ("Loading seat %s", config_section);
+            types = config_get_string_list (config_get_instance (), config_section, "type");
+            if (!types)
+                types = config_get_string_list (config_get_instance (), "SeatDefaults", "type");
+            for (type = types; type && *type; type++)
+            {
+                seat = seat_new (*type);
+                if (seat)
+                    break;
+            }
+            g_strfreev (types);
+            if (seat)
+            {
+                const gsize seatpfxlen = strlen(seatpfx);
+                gchar *seatname = config_section + seatpfxlen;
+
+                seat_set_property (seat, "seat-name", seatname);
+
+                set_seat_properties (seat, config_section);
+                display_manager_add_seat (display_manager, seat);
+                g_object_unref (seat);
+                n_seats++;
+            }
+            else
+                g_warning ("Failed to create seat %s", config_section);
+        }
+        g_strfreev (groups);
+    }
 
     /* If no seats start a default one */
     if (n_seats == 0 && config_get_boolean (config_get_instance (), "LightDM", "start-default-seat"))

=== modified file 'src/login1.c'
--- src/login1.c	2014-08-22 01:56:45 +0000
+++ src/login1.c	2014-08-22 14:40:39 +0000
@@ -290,33 +290,22 @@
            gpointer user_data)
 {
     Login1Service *service = user_data;
-
-    if (strcmp (signal_name, "SeatNew") == 0)
+    Login1Seat *seat;
+    const gchar *id, *path;
+
+    g_variant_get (parameters, "(&s&o)", &id, &path);
+    seat = login1_service_get_seat (service, id);
+
+    if (strcmp (signal_name, "SeatNew") == 0 && !seat)
     {
-        const gchar *id, *path;
-        Login1Seat *seat;
-
-        g_variant_get (parameters, "(&s&o)", &id, &path);
-        seat = login1_service_get_seat (service, id);
-        if (!seat)
-        {
-            seat = add_seat (service, id, path);
-            g_signal_emit (service, signals[SEAT_ADDED], 0, seat);
-        }
+        seat = add_seat (service, id, path);
+        g_signal_emit (service, signals[SEAT_ADDED], 0, seat);
     }
-    else if (strcmp (signal_name, "SeatRemoved") == 0)
+    else if (strcmp (signal_name, "SeatRemoved") == 0 && seat)
     {
-        const gchar *id, *path;
-        Login1Seat *seat;
-
-        g_variant_get (parameters, "(&s&o)", &id, &path);
-        seat = login1_service_get_seat (service, id);
-        if (seat)
-        {
-            service->priv->seats = g_list_remove (service->priv->seats, seat);            
-            g_signal_emit (service, signals[SEAT_REMOVED], 0, seat);
-            g_object_unref (seat);
-        }                        
+        service->priv->seats = g_list_remove (service->priv->seats, seat);            
+        g_signal_emit (service, signals[SEAT_REMOVED], 0, seat);
+        g_object_unref (seat);
     } 
 }