From d76d6ff0761d47df938f1dab0daeeecac2feb56e Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
Date: Mon, 6 Sep 2021 08:43:28 -0400
Subject: [PATCH 4/5] daemon: Consolidate session-type and
supported-session-types list
There's currently a bug in computing the session-type to use.
The `i > 0` check means wayland will overwrite x11 in the
transient session type list.
Morever, the separation between "session-type" and
"supported-session-types" is a little redundant. Since
supported-session-types is a sorted list, the first item should
always be the same as "session-type".
This commit addresses the bug and the redundant logic, by computing
the supported session types early in the function and indexing into
it to get the session-type.
A future cleanup could probably get rid of session-type entirely.
https://gitlab.gnome.org/GNOME/gdm/-/merge_requests/153
---
daemon/gdm-local-display-factory.c | 193 +++++++++++++++++------------
1 file changed, 116 insertions(+), 77 deletions(-)
diff --git a/daemon/gdm-local-display-factory.c b/daemon/gdm-local-display-factory.c
index 141d64c6..eba38671 100644
--- a/daemon/gdm-local-display-factory.c
+++ b/daemon/gdm-local-display-factory.c
@@ -197,164 +197,226 @@ get_preferred_display_server (GdmLocalDisplayFactory *factory)
}
if (!wayland_enabled && !xorg_enabled) {
return g_strdup ("none");
}
gdm_settings_direct_get_string (GDM_KEY_PREFERRED_DISPLAY_SERVER, &preferred_display_server);
if (g_strcmp0 (preferred_display_server, "wayland") == 0) {
if (wayland_enabled)
return g_strdup (preferred_display_server);
else
return g_strdup ("xorg");
}
if (g_strcmp0 (preferred_display_server, "xorg") == 0) {
if (xorg_enabled)
return g_strdup (preferred_display_server);
else
return g_strdup ("wayland");
}
if (g_strcmp0 (preferred_display_server, "legacy-xorg") == 0) {
if (xorg_enabled)
return g_strdup (preferred_display_server);
}
return g_strdup ("none");
}
+struct GdmDisplayServerConfiguration {
+ const char *display_server;
+ const char *key;
+ const char *binary;
+ const char *session_type;
+} display_server_configuration[] = {
+#ifdef ENABLE_WAYLAND_SUPPORT
+ { "wayland", GDM_KEY_WAYLAND_ENABLE, "/usr/bin/Xwayland", "wayland" },
+#endif
+ { "xorg", GDM_KEY_XORG_ENABLE, "/usr/bin/Xorg", "x11" },
+ { NULL, NULL, NULL },
+};
+
+static gboolean
+display_server_enabled (GdmLocalDisplayFactory *factory,
+ const char *display_server)
+{
+ size_t i;
+
+ for (i = 0; display_server_configuration[i].display_server != NULL; i++) {
+ const char *key = display_server_configuration[i].key;
+ const char *binary = display_server_configuration[i].binary;
+ gboolean enabled = FALSE;
+
+ if (!g_str_equal (display_server_configuration[i].display_server,
+ display_server))
+ continue;
+
+ if (!gdm_settings_direct_get_boolean (key, &enabled) || !enabled)
+ return FALSE;
+
+ if (!g_file_test (binary, G_FILE_TEST_IS_EXECUTABLE))
+ return FALSE;
+
+ return TRUE;
+ }
+
+ return FALSE;
+}
+
static const char *
-gdm_local_display_factory_get_session_type (GdmLocalDisplayFactory *factory,
- gboolean should_fall_back)
+get_session_type_for_display_server (GdmLocalDisplayFactory *factory,
+ const char *display_server)
+{
+ size_t i;
+
+ for (i = 0; display_server_configuration[i].display_server != NULL; i++) {
+ if (!g_str_equal (display_server_configuration[i].display_server,
+ display_server))
+ continue;
+
+ return display_server_configuration[i].session_type;
+ }
+
+ return NULL;
+}
+
+static char **
+gdm_local_display_factory_get_session_types (GdmLocalDisplayFactory *factory,
+ gboolean should_fall_back)
{
- const char *session_types[3] = { NULL };
- gsize i, session_type_index = 0;
g_autofree gchar *preferred_display_server = NULL;
+ const char *fallback_display_server = NULL;
+ gboolean wayland_preferred = FALSE;
+ gboolean xorg_preferred = FALSE;
+ g_autoptr (GPtrArray) session_types_array = NULL;
+ char **session_types;
+
+ session_types_array = g_ptr_array_new ();
preferred_display_server = get_preferred_display_server (factory);
- if (g_strcmp0 (preferred_display_server, "wayland") != 0 &&
- g_strcmp0 (preferred_display_server, "xorg") != 0)
- return NULL;
+ g_debug ("GdmLocalDisplayFactory: Getting session type (prefers %s, falling back: %s)",
+ preferred_display_server, should_fall_back? "yes" : "no");
- for (i = 0; i < G_N_ELEMENTS (session_types) - 1; i++) {
-#ifdef ENABLE_WAYLAND_SUPPORT
- if (i > 0 ||
- g_strcmp0 (preferred_display_server, "wayland") == 0) {
- gboolean wayland_enabled = FALSE;
- if (gdm_settings_direct_get_boolean (GDM_KEY_WAYLAND_ENABLE, &wayland_enabled)) {
- if (wayland_enabled && g_file_test ("/usr/bin/Xwayland", G_FILE_TEST_IS_EXECUTABLE)) {
- session_types[i] = "wayland";
- continue;
- }
- }
- }
-#endif
+ wayland_preferred = g_str_equal (preferred_display_server, "wayland");
+ xorg_preferred = g_str_equal (preferred_display_server, "xorg");
+
+ if (wayland_preferred)
+ fallback_display_server = "xorg";
+ else if (xorg_preferred)
+ fallback_display_server = "wayland";
+ else
+ return NULL;
- if (i > 0 ||
- g_strcmp0 (preferred_display_server, "xorg") == 0) {
- gboolean xorg_enabled = FALSE;
- if (gdm_settings_direct_get_boolean (GDM_KEY_XORG_ENABLE, &xorg_enabled)) {
- if (xorg_enabled && g_file_test ("/usr/bin/Xorg", G_FILE_TEST_IS_EXECUTABLE)) {
- session_types[i] = "x11";
- continue;
- }
- }
- }
+ if (!should_fall_back) {
+ if (display_server_enabled (factory, preferred_display_server))
+ g_ptr_array_add (session_types_array, (gpointer) get_session_type_for_display_server (factory, preferred_display_server));
}
- if (should_fall_back)
- session_type_index++;
+ if (display_server_enabled (factory, fallback_display_server))
+ g_ptr_array_add (session_types_array, (gpointer) get_session_type_for_display_server (factory, fallback_display_server));
- return session_types[session_type_index];
+ if (session_types_array->len == 0)
+ return NULL;
+
+ g_ptr_array_add (session_types_array, NULL);
+
+ session_types = g_strdupv ((char **) session_types_array->pdata);
+
+ return session_types;
}
static void
on_display_disposed (GdmLocalDisplayFactory *factory,
GdmDisplay *display)
{
g_debug ("GdmLocalDisplayFactory: Display %p disposed", display);
}
static void
store_display (GdmLocalDisplayFactory *factory,
GdmDisplay *display)
{
GdmDisplayStore *store;
store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory));
gdm_display_store_add (store, display);
}
/*
Example:
dbus-send --system --dest=org.gnome.DisplayManager \
--type=method_call --print-reply --reply-timeout=2000 \
/org/gnome/DisplayManager/Manager \
org.gnome.DisplayManager.Manager.GetDisplays
*/
gboolean
gdm_local_display_factory_create_transient_display (GdmLocalDisplayFactory *factory,
char **id,
GError **error)
{
gboolean ret;
GdmDisplay *display = NULL;
gboolean is_initial = FALSE;
const char *session_type;
g_autofree gchar *preferred_display_server = NULL;
g_return_val_if_fail (GDM_IS_LOCAL_DISPLAY_FACTORY (factory), FALSE);
ret = FALSE;
g_debug ("GdmLocalDisplayFactory: Creating transient display");
preferred_display_server = get_preferred_display_server (factory);
#ifdef ENABLE_USER_DISPLAY_SERVER
if (g_strcmp0 (preferred_display_server, "wayland") == 0 ||
g_strcmp0 (preferred_display_server, "xorg") == 0) {
- session_type = gdm_local_display_factory_get_session_type (factory, FALSE);
+ g_auto(GStrv) session_types = NULL;
- if (session_type == NULL) {
+ session_types = gdm_local_display_factory_get_session_types (factory, FALSE);
+
+ if (session_types == NULL) {
g_set_error_literal (error,
GDM_DISPLAY_ERROR,
GDM_DISPLAY_ERROR_GENERAL,
"Both Wayland and Xorg are unavailable");
return FALSE;
}
display = gdm_local_display_new ();
- g_object_set (G_OBJECT (display), "session-type", session_type, NULL);
+ g_object_set (G_OBJECT (display),
+ "session-type", session_types[0],
+ "supported-session-types", session_types,
+ NULL);
is_initial = TRUE;
}
#endif
if (g_strcmp0 (preferred_display_server, "legacy-xorg") == 0) {
if (display == NULL) {
guint32 num;
num = take_next_display_number (factory);
display = gdm_legacy_display_new (num);
}
}
if (display == NULL) {
g_set_error_literal (error,
GDM_DISPLAY_ERROR,
GDM_DISPLAY_ERROR_GENERAL,
"Invalid preferred display server configured");
return FALSE;
}
g_object_set (display,
"seat-id", "seat0",
"allow-timed-login", FALSE,
"is-initial", is_initial,
NULL);
store_display (factory, display);
if (! gdm_display_manage (display)) {
@@ -549,243 +611,220 @@ lookup_prepared_display_by_seat_id (const char *id,
if (status != GDM_DISPLAY_PREPARED)
return FALSE;
return lookup_by_seat_id (id, display, user_data);
}
static int
on_seat0_graphics_check_timeout (gpointer user_data)
{
GdmLocalDisplayFactory *factory = user_data;
factory->seat0_graphics_check_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_graphics_check_timed_out = TRUE;
ensure_display_for_seat (factory, "seat0");
return G_SOURCE_REMOVE;
}
static void
ensure_display_for_seat (GdmLocalDisplayFactory *factory,
const char *seat_id)
{
int ret;
gboolean seat_supports_graphics;
gboolean is_seat0;
- const char *session_type = "wayland";
+ g_auto (GStrv) session_types = NULL;
+ const char *legacy_session_types[] = { "x11", NULL };
GdmDisplayStore *store;
GdmDisplay *display = NULL;
g_autofree char *login_session_id = NULL;
gboolean wayland_enabled = FALSE, xorg_enabled = FALSE;
g_autofree gchar *preferred_display_server = NULL;
- gboolean falling_back;
+ gboolean falling_back = FALSE;
gdm_settings_direct_get_boolean (GDM_KEY_WAYLAND_ENABLE, &wayland_enabled);
gdm_settings_direct_get_boolean (GDM_KEY_XORG_ENABLE, &xorg_enabled);
preferred_display_server = get_preferred_display_server (factory);
if (g_strcmp0 (preferred_display_server, "none") == 0) {
g_debug ("GdmLocalDisplayFactory: Preferred display server is none, so not creating display");
return;
}
ret = sd_seat_can_graphical (seat_id);
if (ret < 0) {
g_critical ("Failed to query CanGraphical information for seat %s", seat_id);
return;
}
if (ret == 0) {
g_debug ("GdmLocalDisplayFactory: System doesn't currently support graphics");
seat_supports_graphics = FALSE;
} else {
g_debug ("GdmLocalDisplayFactory: System supports graphics");
seat_supports_graphics = TRUE;
}
if (g_strcmp0 (seat_id, "seat0") == 0) {
is_seat0 = TRUE;
falling_back = factory->num_failures > 0;
- session_type = gdm_local_display_factory_get_session_type (factory, falling_back);
+ session_types = gdm_local_display_factory_get_session_types (factory, falling_back);
g_debug ("GdmLocalDisplayFactory: New displays on seat0 will use %s%s",
- session_type, falling_back? " fallback" : "");
+ session_types[0], falling_back? " fallback" : "");
} else {
is_seat0 = FALSE;
g_debug ("GdmLocalDisplayFactory: New displays on seat %s will use X11 fallback", seat_id);
/* Force legacy X11 for all auxiliary seats */
seat_supports_graphics = TRUE;
- session_type = "x11";
+ session_types = g_strdupv ((char **) legacy_session_types);
}
/* For seat0, we have a fallback logic to still try starting it after
* SEAT0_GRAPHICS_CHECK_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 wayland capable device is present and simply not marked as
* master-of-seat. In these cases, this should likely be fixed in the
* udev rules.
*
* At the moment, systemd always sets CanGraphical for non-seat0 seats.
* This is because non-seat0 seats are defined by having master-of-seat
* set. This means we can avoid the fallback check for non-seat0 seats,
* which simplifies the code.
*/
if (is_seat0) {
if (!seat_supports_graphics) {
if (!factory->seat0_graphics_check_timed_out) {
if (factory->seat0_graphics_check_timeout_id == 0) {
g_debug ("GdmLocalDisplayFactory: seat0 doesn't yet support graphics. Waiting %d seconds to try again.", SEAT0_GRAPHICS_CHECK_TIMEOUT);
factory->seat0_graphics_check_timeout_id = g_timeout_add_seconds (SEAT0_GRAPHICS_CHECK_TIMEOUT,
on_seat0_graphics_check_timeout,
factory);
} else {
/* It is not yet time to force X11 fallback. */
g_debug ("GdmLocalDisplayFactory: seat0 display requested when there is no graphics support before graphics check timeout.");
}
return;
}
g_debug ("GdmLocalDisplayFactory: Assuming we can use seat0 for X11 even though system says it doesn't support graphics!");
g_debug ("GdmLocalDisplayFactory: This might indicate an issue where the framebuffer device is not tagged as master-of-seat in udev.");
seat_supports_graphics = TRUE;
- session_type = "x11";
wayland_enabled = FALSE;
+ g_strfreev (session_types);
+ session_types = g_strdupv ((char **) legacy_session_types);
} else {
g_clear_handle_id (&factory->seat0_graphics_check_timeout_id, g_source_remove);
}
}
if (!seat_supports_graphics)
return;
- if (session_type != NULL)
+ if (session_types != NULL)
g_debug ("GdmLocalDisplayFactory: %s login display for seat %s requested",
- session_type, seat_id);
+ session_types[0], seat_id);
else if (g_strcmp0 (preferred_display_server, "legacy-xorg") == 0)
g_debug ("GdmLocalDisplayFactory: Legacy Xorg login display for seat %s requested",
seat_id);
store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory));
if (is_seat0)
display = gdm_display_store_find (store, lookup_prepared_display_by_seat_id, (gpointer) seat_id);
else
display = gdm_display_store_find (store, lookup_by_seat_id, (gpointer) seat_id);
/* Ensure we don't create the same display more than once */
if (display != NULL) {
g_debug ("GdmLocalDisplayFactory: display already created");
return;
}
/* If we already have a login window, switch to it */
if (gdm_get_login_window_session_id (seat_id, &login_session_id)) {
GdmDisplay *display;
display = gdm_display_store_find (store,
lookup_by_session_id,
(gpointer) login_session_id);
if (display != NULL &&
(gdm_display_get_status (display) == GDM_DISPLAY_MANAGED ||
gdm_display_get_status (display) == GDM_DISPLAY_WAITING_TO_FINISH)) {
g_object_set (G_OBJECT (display), "status", GDM_DISPLAY_MANAGED, NULL);
g_debug ("GdmLocalDisplayFactory: session %s found, activating.",
login_session_id);
gdm_activate_session_by_id (factory->connection, seat_id, login_session_id);
return;
}
}
g_debug ("GdmLocalDisplayFactory: Adding display on seat %s", seat_id);
#ifdef ENABLE_USER_DISPLAY_SERVER
if (g_strcmp0 (preferred_display_server, "wayland") == 0 ||
g_strcmp0 (preferred_display_server, "xorg") == 0) {
if (is_seat0) {
- g_autoptr (GPtrArray) supported_session_types = NULL;
-
- if (session_type == NULL) {
- g_warning ("GdmLocalDisplayFactory: Both Wayland and Xorg sessions are unavailable");
- return;
- }
-
- supported_session_types = g_ptr_array_new ();
-
- if (g_strcmp0 (preferred_display_server, "wayland") == 0) {
- if (wayland_enabled)
- g_ptr_array_add (supported_session_types, "wayland");
- } else {
- if (xorg_enabled)
- g_ptr_array_add (supported_session_types, "x11");
- }
-
- if (!falling_back) {
- if (g_strcmp0 (preferred_display_server, "wayland") == 0) {
- if (xorg_enabled)
- g_ptr_array_add (supported_session_types, "x11");
- } else {
- if (wayland_enabled)
- g_ptr_array_add (supported_session_types, "wayland");
- }
- }
-
- g_ptr_array_add (supported_session_types, NULL);
-
display = gdm_local_display_new ();
- g_object_set (G_OBJECT (display), "session-type", session_type, NULL);
- g_object_set (G_OBJECT (display), "supported-session-types", supported_session_types->pdata, NULL);
+ g_object_set (G_OBJECT (display),
+ "session-type", session_types[0],
+ "supported-session-types", session_types,
+ NULL);
}
}
#endif
if (display == NULL) {
guint32 num;
- const char *supported_session_types[] = { "x11", NULL };
num = take_next_display_number (factory);
display = gdm_legacy_display_new (num);
- g_object_set (G_OBJECT (display), "supported-session-types", supported_session_types, NULL);
+ g_object_set (G_OBJECT (display),
+ "session-type", legacy_session_types[0],
+ "supported-session-types", legacy_session_types,
+ NULL);
}
g_object_set (display, "seat-id", seat_id, NULL);
g_object_set (display, "is-initial", is_seat0, NULL);
store_display (factory, display);
/* let store own the ref */
g_object_unref (display);
if (! gdm_display_manage (display)) {
gdm_display_unmanage (display);
}
return;
}
static void
delete_display (GdmLocalDisplayFactory *factory,
const char *seat_id) {
GdmDisplayStore *store;
g_debug ("GdmLocalDisplayFactory: Removing used_display_numbers on seat %s", seat_id);
store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory));
gdm_display_store_foreach_remove (store, lookup_by_seat_id, (gpointer) seat_id);
}
static gboolean
--
2.34.1