[gdm/benzea/wait-seat-graphical] local-display-factory: Wait for seats to become graphical

[Benjamin Berg <bberg src gnome org>]

commit 5cb3b1f171091742dd54f3e8fb17168ba02f2c34
Author: Benjamin Berg <bberg redhat com>
Date:   Tue Feb 9 15:43:05 2021 +0100

    local-display-factory: Wait for seats to become graphical
    
    It may happen that seats are not graphical initially because the DRM
    device is not ready yet. In that case, ignore the seat and wait for the
    CanGraphical property notification in order to add it at that point.
    
    However, there are some rare cases where CanGraphical will never turn
    TRUE. To catch these, add a timeout and fall back to simply trying to
    bring seat0 up after 10 seconds. When we do so, go directly for the X11
    fallback as wayland is unlikely to be functional.
    
    Fixes: #662

 daemon/gdm-local-display-factory.c | 124 +++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)
---
diff --git a/daemon/gdm-local-display-factory.c b/daemon/gdm-local-display-factory.c
index 0ac7af13c..80ec341a0 100644
--- a/daemon/gdm-local-display-factory.c
+++ b/daemon/gdm-local-display-factory.c
@@ -48,6 +48,7 @@
 
 #define MAX_DISPLAY_FAILURES 5
 #define WAIT_TO_FINISH_TIMEOUT 10 /* seconds */
+#define SEAT0_CAN_GRAPHICAL_TIMEOUT 10 /* seconds */
 
 struct _GdmLocalDisplayFactory
 {
@@ -62,6 +63,10 @@ struct _GdmLocalDisplayFactory
 
         guint            seat_new_id;
         guint            seat_removed_id;
+        guint            seat_properties_changed_id;
+
+        gboolean         seat0_supports_graphical;
+        guint            seat0_can_graphical_timeout_id;
 
 #if defined(ENABLE_USER_DISPLAY_SERVER)
         unsigned int     active_vt;
@@ -449,16 +454,39 @@ lookup_prepared_display_by_seat_id (const char *id,
         return lookup_by_seat_id (id, display, user_data);
 }
 
+static int
+seat0_can_graphical_timeout_cb (gpointer user_data)
+{
+        GdmLocalDisplayFactory *factory = user_data;
+
+        factory->seat0_can_graphical_timeout_id = 0;
+
+        /* Simply try to re-add seat0. If it is there already (i.e. CanGraphical
+         * turned TRUE, then we'll find it and it will not be created again.
+         */
+        factory->seat0_supports_graphical = FALSE;
+        ensure_display_for_seat (factory, "seat0");
+
+        return G_SOURCE_REMOVE;
+}
+
 static void
 ensure_display_for_seat (GdmLocalDisplayFactory *factory,
                          const char             *seat_id)
 {
         gboolean is_initial;
+        int can_graphical;
         const char *session_type = NULL;
         GdmDisplayStore *store;
         GdmDisplay      *display = NULL;
         g_autofree char *login_session_id = NULL;
 
+        can_graphical = sd_seat_can_graphical (seat_id);
+        if (can_graphical < 0) {
+                g_critical ("Failed to query CanGraphical information for seat %s", seat_id);
+                return;
+        }
+
         if (g_strcmp0 (seat_id, "seat0") == 0) {
                 is_initial = TRUE;
                 if (factory->num_failures == 0 && gdm_local_display_factory_use_wayland ())
@@ -467,6 +495,37 @@ ensure_display_for_seat (GdmLocalDisplayFactory *factory,
                 is_initial = FALSE;
         }
 
+        /* For seat0, we have a fallback logic to still try starting it after
+         * WAIT_GRAPHICAL_TIMEOUT seconds. i.e. we simply continue even if
+         * CanGraphical is unset.
+         * This is ugly, but it means we'll come up eventually in some
+         * scenarios where no master device is present.
+         * Note that we'll force an X11 fallback even though there might be
+         * cases where an FB device is present and simply not marked as
+         * master-of-seat. In these cases, this should likely be fixed in
+         * systemd.
+         */
+        if (is_initial && !can_graphical) {
+                if (!factory->seat0_supports_graphical && !factory->seat0_can_graphical_timeout_id)
+                        factory->seat0_can_graphical_timeout_id = g_timeout_add_seconds 
(SEAT0_CAN_GRAPHICAL_TIMEOUT,
+                                                                                         
seat0_can_graphical_timeout_cb,
+                                                                                         factory);
+
+                /* It is not yet time to force graphical mode. */
+                if (!factory->seat0_supports_graphical)
+                        return;
+
+                g_debug ("GdmLocalDisplayFactory: Assuming we can use seat0 for X11 even though it is not 
marked as graphical!");
+                g_debug ("GdmLocalDisplayFactory: This might indicate an issue where the framebuffer device 
is not tagged as master-of-seat in udev.");
+                can_graphical = TRUE;
+                session_type = NULL;
+        } else if (is_initial && can_graphical) {
+                g_clear_handle_id (&factory->seat0_can_graphical_timeout_id, g_source_remove);
+        }
+
+        if (!can_graphical)
+                return;
+
         g_debug ("GdmLocalDisplayFactory: %s login display for seat %s requested",
                  session_type? : "X11", seat_id);
         store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory));
@@ -615,6 +674,53 @@ on_seat_removed (GDBusConnection *connection,
         delete_display (GDM_LOCAL_DISPLAY_FACTORY (user_data), seat);
 }
 
+static void
+on_seat_properties_changed (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 *seat = NULL;
+        g_autoptr(GVariant) changed_props = NULL;
+        g_autoptr(GVariant) changed_prop = NULL;
+        g_autofree const gchar * const *invalidated_props = NULL;
+        gboolean changed = FALSE;
+        int res;
+
+        /* Extract seat id, i.e. the last element of the object path. */
+        seat = strrchr (object_path, '/');
+        if (seat == NULL)
+                return;
+        seat += 1;
+
+        /* Valid seat IDs must start with seat, i.e. ignore "auto" */
+        if (!g_str_has_prefix (seat, "seat"))
+                return;
+
+        g_variant_get (parameters, "(s@a{sv}^a&s)", NULL, &changed_props, &invalidated_props);
+
+        changed_prop = g_variant_lookup_value (changed_props, "CanGraphical", NULL);
+        if (changed_prop)
+                changed = TRUE;
+        if (!changed && g_strv_contains (invalidated_props, "CanGraphical"))
+                changed = TRUE;
+
+        if (!changed)
+                return;
+
+        res = sd_seat_can_graphical (seat);
+        if (res < 0)
+                return;
+
+        if (res)
+                ensure_display_for_seat (GDM_LOCAL_DISPLAY_FACTORY (user_data), seat);
+        else
+                delete_display (GDM_LOCAL_DISPLAY_FACTORY (user_data), seat);
+}
+
 static gboolean
 lookup_by_session_id (const char *id,
                       GdmDisplay *display,
@@ -849,6 +955,16 @@ gdm_local_display_factory_start_monitor (GdmLocalDisplayFactory *factory)
                                                                              on_seat_removed,
                                                                              g_object_ref (factory),
                                                                              g_object_unref);
+        factory->seat_properties_changed_id = g_dbus_connection_signal_subscribe (factory->connection,
+                                                                                  "org.freedesktop.login1",
+                                                                                  
"org.freedesktop.DBus.Properties",
+                                                                                  "PropertiesChanged",
+                                                                                  NULL,
+                                                                                  
"org.freedesktop.login1.Seat",
+                                                                                  
G_DBUS_SIGNAL_FLAGS_MATCH_ARG0_NAMESPACE,
+                                                                                  on_seat_properties_changed,
+                                                                                  g_object_ref (factory),
+                                                                                  g_object_unref);
 
 #if defined(ENABLE_USER_DISPLAY_SERVER)
         io_channel = g_io_channel_new_file ("/sys/class/tty/tty0/active", "r", NULL);
@@ -877,6 +993,11 @@ gdm_local_display_factory_stop_monitor (GdmLocalDisplayFactory *factory)
                                                       factory->seat_removed_id);
                 factory->seat_removed_id = 0;
         }
+        if (factory->seat_properties_changed_id) {
+                g_dbus_connection_signal_unsubscribe (factory->connection,
+                                                      factory->seat_properties_changed_id);
+                factory->seat_properties_changed_id = 0;
+        }
 #if defined(ENABLE_USER_DISPLAY_SERVER)
         if (factory->active_vt_watch_id) {
                 g_source_remove (factory->active_vt_watch_id);
@@ -962,6 +1083,8 @@ gdm_local_display_factory_stop (GdmDisplayFactory *base_factory)
                                               G_CALLBACK (on_display_removed),
                                               factory);
 
+        g_clear_handle_id (&factory->seat0_can_graphical_timeout_id, g_source_remove);
+
         return TRUE;
 }
 
@@ -1084,6 +1207,7 @@ static void
 gdm_local_display_factory_init (GdmLocalDisplayFactory *factory)
 {
         factory->used_display_numbers = g_hash_table_new (NULL, NULL);
+        factory->seat0_supports_graphical = TRUE;
 }
 
 static void