From 42ef43e603d65390c2c86e276489c8a08c7808a0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2012 21:41:16 +0200 Subject: gdm: always run gdm on VT1 This drops automatic VT allocation schemes for the initial display in favour of a compile time hardcoded VT assignment. The automatic allocation schemes are inherently racy since a simpler output to the console might already influence it and result in gdm running on another than the intended VT. This patch adds a --with-initial-vt= switch to configure which may be used to set the VT gdm will run the initial server on. It defaults to 1. https://fedoraproject.org/wiki/Features/DisplayManagerRework https://bugzilla.gnome.org/show_bug.cgi?id=511168 --- configure.ac | 15 ++++++ daemon/gdm-display.c | 53 +++++++++++++++++++ daemon/gdm-display.h | 4 +- daemon/gdm-display.xml | 3 ++ daemon/gdm-local-display-factory.c | 19 ++++--- daemon/gdm-server.c | 101 +++++++++++++------------------------ daemon/gdm-server.h | 6 +-- daemon/gdm-simple-slave.c | 11 ++-- daemon/gdm-slave.c | 35 ++++++++++++- 9 files changed, 161 insertions(+), 86 deletions(-) diff --git a/configure.ac b/configure.ac index 7e962a69..26f7826d 100644 --- a/configure.ac +++ b/configure.ac @@ -1439,6 +1439,20 @@ else XSESSION_SHELL=/bin/sh fi +# +# Set VT to use for initial server +# +AC_ARG_WITH(initial-vt, + AS_HELP_STRING([--with-initial-vt=], + [Initial virtual terminal to use])) +if ! test -z "$with_initial_vt"; then + GDM_INITIAL_VT="$with_initial_vt" +else + GDM_INITIAL_VT="1" +fi +AC_SUBST(GDM_INITIAL_VT) +AC_DEFINE_UNQUOTED(GDM_INITIAL_VT, "$GDM_INITIAL_VT", [Initial Virtual Terminal]) + # Set configuration choices. # AC_SUBST(XSESSION_SHELL) @@ -1558,4 +1572,5 @@ echo \ plymouth support: ${use_plymouth} UPower support: ${have_upower} Build with RBAC: ${msg_rbac_shutdown} + Initial VT: ${GDM_INITIAL_VT} " diff --git a/daemon/gdm-display.c b/daemon/gdm-display.c index 25fff698..6e010ff0 100644 --- a/daemon/gdm-display.c +++ b/daemon/gdm-display.c @@ -79,6 +79,8 @@ struct GdmDisplayPrivate GdmDBusDisplay *display_skeleton; GDBusObjectSkeleton *object_skeleton; + + gboolean is_initial; }; enum { @@ -94,6 +96,7 @@ enum { PROP_X11_AUTHORITY_FILE, PROP_IS_LOCAL, PROP_SLAVE_COMMAND, + PROP_IS_INITIAL }; static void gdm_display_class_init (GdmDisplayClass *klass); @@ -535,6 +538,20 @@ gdm_display_get_seat_id (GdmDisplay *display, return TRUE; } +gboolean +gdm_display_is_initial (GdmDisplay *display, + gboolean *is_initial, + GError **error) +{ + g_return_val_if_fail (GDM_IS_DISPLAY (display), FALSE); + + if (is_initial != NULL) { + *is_initial = display->priv->is_initial; + } + + return TRUE; +} + static gboolean finish_idle (GdmDisplay *display) { @@ -885,6 +902,13 @@ _gdm_display_set_slave_command (GdmDisplay *display, display->priv->slave_command = g_strdup (command); } +static void +_gdm_display_set_is_initial (GdmDisplay *display, + gboolean initial) +{ + display->priv->is_initial = initial; +} + static void gdm_display_set_property (GObject *object, guint prop_id, @@ -926,6 +950,9 @@ gdm_display_set_property (GObject *object, case PROP_SLAVE_COMMAND: _gdm_display_set_slave_command (self, g_value_get_string (value)); break; + case PROP_IS_INITIAL: + _gdm_display_set_is_initial (self, g_value_get_boolean (value)); + break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -977,6 +1004,9 @@ gdm_display_get_property (GObject *object, case PROP_SLAVE_COMMAND: g_value_set_string (value, self->priv->slave_command); break; + case PROP_IS_INITIAL: + g_value_set_boolean (value, self->priv->is_initial); + break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -1128,6 +1158,20 @@ handle_is_local (GdmDBusDisplay *skeleton, return TRUE; } +static gboolean +handle_is_initial (GdmDBusDisplay *skeleton, + GDBusMethodInvocation *invocation, + GdmDisplay *display) +{ + gboolean is_initial = FALSE; + + gdm_display_is_initial (display, &is_initial, NULL); + + gdm_dbus_display_complete_is_initial (skeleton, invocation, is_initial); + + return TRUE; +} + static gboolean handle_get_slave_bus_name (GdmDBusDisplay *skeleton, GDBusMethodInvocation *invocation, @@ -1237,6 +1281,8 @@ register_display (GdmDisplay *display) G_CALLBACK (handle_get_x11_display_number), display); g_signal_connect (display->priv->display_skeleton, "handle-is-local", G_CALLBACK (handle_is_local), display); + g_signal_connect (display->priv->display_skeleton, "handle-is-initial", + G_CALLBACK (handle_is_initial), display); g_signal_connect (display->priv->display_skeleton, "handle-get-slave-bus-name", G_CALLBACK (handle_get_slave_bus_name), display); g_signal_connect (display->priv->display_skeleton, "handle-set-slave-bus-name", @@ -1460,6 +1506,13 @@ gdm_display_class_init (GdmDisplayClass *klass) "session id", NULL, G_PARAM_READWRITE)); + g_object_class_install_property (object_class, + PROP_IS_INITIAL, + g_param_spec_boolean ("is-initial", + NULL, + NULL, + FALSE, + G_PARAM_READWRITE | G_PARAM_CONSTRUCT)); g_object_class_install_property (object_class, PROP_X11_COOKIE, g_param_spec_string ("x11-cookie", diff --git a/daemon/gdm-display.h b/daemon/gdm-display.h index 5050eecd..e750ffa6 100644 --- a/daemon/gdm-display.h +++ b/daemon/gdm-display.h @@ -136,6 +136,9 @@ gboolean gdm_display_get_timed_login_details (GdmDisplay *disp char **username, int *delay, GError **error); +gboolean gdm_display_is_initial (GdmDisplay *display, + gboolean *initial, + GError **error); /* exported but protected */ gboolean gdm_display_get_x11_cookie (GdmDisplay *display, @@ -155,7 +158,6 @@ gboolean gdm_display_set_slave_bus_name (GdmDisplay *disp const char *name, GError **error); - G_END_DECLS #endif /* __GDM_DISPLAY_H */ diff --git a/daemon/gdm-display.xml b/daemon/gdm-display.xml index 13c734d2..904e0aed 100644 --- a/daemon/gdm-display.xml +++ b/daemon/gdm-display.xml @@ -19,6 +19,9 @@ + + + diff --git a/daemon/gdm-local-display-factory.c b/daemon/gdm-local-display-factory.c index bef22093..571c9fd0 100644 --- a/daemon/gdm-local-display-factory.c +++ b/daemon/gdm-local-display-factory.c @@ -76,7 +76,8 @@ static void gdm_local_display_factory_init (GdmLocalDisplayFactory static void gdm_local_display_factory_finalize (GObject *object); static GdmDisplay *create_display (GdmLocalDisplayFactory *factory, - const char *seat_id); + const char *seat_id, + gboolean initial_display); static void on_display_status_changed (GdmDisplay *display, GParamSpec *arg1, @@ -271,6 +272,7 @@ on_display_status_changed (GdmDisplay *display, GdmDisplayStore *store; int num; char *seat_id = NULL; + gboolean is_initial = TRUE; num = -1; gdm_display_get_x11_display_number (display, &num, NULL); @@ -279,6 +281,7 @@ on_display_status_changed (GdmDisplay *display, store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory)); g_object_get (display, "seat-id", &seat_id, NULL); + g_object_get (display, "is-initial", &is_initial, NULL); status = gdm_display_get_status (display); @@ -295,7 +298,7 @@ on_display_status_changed (GdmDisplay *display, /* reset num failures */ factory->priv->num_failures = 0; - create_display (factory, seat_id); + create_display (factory, seat_id, is_initial); } break; case GDM_DISPLAY_FAILED: @@ -314,7 +317,7 @@ on_display_status_changed (GdmDisplay *display, /* FIXME: should monitor hardware changes to try again when seats change */ } else { - create_display (factory, seat_id); + create_display (factory, seat_id, is_initial); } } break; @@ -352,7 +355,8 @@ lookup_by_seat_id (const char *id, static GdmDisplay * create_display (GdmLocalDisplayFactory *factory, - const char *seat_id) + const char *seat_id, + gboolean initial) { GdmDisplayStore *store; GdmDisplay *display; @@ -372,6 +376,7 @@ create_display (GdmLocalDisplayFactory *factory, display = gdm_static_display_new (num); g_object_set (display, "seat-id", seat_id, NULL); + g_object_set (display, "is-initial", initial, NULL); store_display (factory, num, display); @@ -428,7 +433,7 @@ static gboolean gdm_local_display_factory_sync_seats (GdmLocalDisplayFactory *fa g_variant_iter_init (&iter, array); while (g_variant_iter_loop (&iter, "(&so)", &seat, NULL)) - create_display (factory, seat); + create_display (factory, seat, TRUE); g_variant_unref (result); g_variant_unref (array); @@ -447,7 +452,7 @@ on_seat_new (GDBusConnection *connection, const char *seat; g_variant_get (parameters, "(&s)", &seat); - create_display (GDM_LOCAL_DISPLAY_FACTORY (user_data), seat); + create_display (GDM_LOCAL_DISPLAY_FACTORY (user_data), seat, FALSE); } static void @@ -523,7 +528,7 @@ gdm_local_display_factory_start (GdmDisplayFactory *base_factory) #endif /* On ConsoleKit just create Seat1, and that's it. */ - display = create_display (factory, CK_SEAT1_PATH); + display = create_display (factory, CK_SEAT1_PATH, TRUE); return display != NULL; } diff --git a/daemon/gdm-server.c b/daemon/gdm-server.c index edbb3d42..673c4013 100644 --- a/daemon/gdm-server.c +++ b/daemon/gdm-server.c @@ -97,6 +97,8 @@ struct GdmServerPrivate char *chosen_hostname; guint child_watch_id; + + gboolean is_initial; }; enum { @@ -115,6 +117,7 @@ enum { PROP_SESSION_ARGS, PROP_LOG_DIR, PROP_DISABLE_TCP, + PROP_IS_INITIAL, }; enum { @@ -757,70 +760,6 @@ gdm_server_spawn (GdmServer *server, return ret; } -#ifdef WITH_PLYMOUTH -static int -get_active_vt (void) -{ - int console_fd; - struct vt_stat console_state = { 0 }; - - console_fd = open ("/dev/tty0", O_RDONLY | O_NOCTTY); - - if (console_fd < 0) { - goto out; - } - - if (ioctl (console_fd, VT_GETSTATE, &console_state) < 0) { - goto out; - } - -out: - if (console_fd >= 0) { - close (console_fd); - } - - return console_state.v_active; -} - -static char * -get_active_vt_as_string (void) -{ - int vt; - - vt = get_active_vt (); - - if (vt <= 0) { - return NULL; - } - - return g_strdup_printf ("vt%d", vt); -} - -gboolean -gdm_server_start_on_active_vt (GdmServer *server) -{ - gboolean res; - gboolean debug; - char *vt; - const char *debug_options; - - gdm_settings_direct_get_boolean (GDM_KEY_DEBUG, &debug); - if (debug) { - debug_options = " -logverbose 7 -core "; - } else { - debug_options = ""; - } - - g_free (server->priv->command); - server->priv->command = g_strdup_printf (X_SERVER " -background none -verbose%s", debug_options); - vt = get_active_vt_as_string (); - res = gdm_server_spawn (server, vt); - g_free (vt); - - return res; -} -#endif - /** * gdm_server_start: * @disp: Pointer to a GdmDisplay structure @@ -832,9 +771,16 @@ gboolean gdm_server_start (GdmServer *server) { gboolean res; + const char *vtarg = NULL; + + /* Hardcode the VT for the initial X server, but nothing else */ + if (server->priv->is_initial + && g_strcmp0 (server->priv->display_seat_id, "seat0") == 0) { + vtarg = "vt" GDM_INITIAL_VT; + } /* fork X server process */ - res = gdm_server_spawn (server, NULL); + res = gdm_server_spawn (server, vtarg); return res; } @@ -931,6 +877,13 @@ _gdm_server_set_disable_tcp (GdmServer *server, server->priv->disable_tcp = disabled; } +static void +_gdm_server_set_is_initial (GdmServer *server, + gboolean initial) +{ + server->priv->is_initial = initial; +} + static void gdm_server_set_property (GObject *object, guint prop_id, @@ -957,6 +910,9 @@ gdm_server_set_property (GObject *object, case PROP_DISABLE_TCP: _gdm_server_set_disable_tcp (self, g_value_get_boolean (value)); break; + case PROP_IS_INITIAL: + _gdm_server_set_is_initial (self, g_value_get_boolean (value)); + break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -993,6 +949,9 @@ gdm_server_get_property (GObject *object, case PROP_DISABLE_TCP: g_value_set_boolean (value, self->priv->disable_tcp); break; + case PROP_IS_INITIAL: + g_value_set_boolean (value, self->priv->is_initial); + break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -1086,7 +1045,13 @@ gdm_server_class_init (GdmServerClass *klass) NULL, TRUE, G_PARAM_READWRITE | G_PARAM_CONSTRUCT)); - + g_object_class_install_property (object_class, + PROP_IS_INITIAL, + g_param_spec_boolean ("is-initial", + NULL, + NULL, + FALSE, + G_PARAM_READWRITE | G_PARAM_CONSTRUCT)); } static void @@ -1136,7 +1101,8 @@ gdm_server_finalize (GObject *object) GdmServer * gdm_server_new (const char *display_name, const char *seat_id, - const char *auth_file) + const char *auth_file, + gboolean initial) { GObject *object; @@ -1144,6 +1110,7 @@ gdm_server_new (const char *display_name, "display-name", display_name, "display-seat-id", seat_id, "auth-file", auth_file, + "is-initial", initial, NULL); return GDM_SERVER (object); diff --git a/daemon/gdm-server.h b/daemon/gdm-server.h index 56e20254..e427f823 100644 --- a/daemon/gdm-server.h +++ b/daemon/gdm-server.h @@ -55,11 +55,9 @@ typedef struct GType gdm_server_get_type (void); GdmServer * gdm_server_new (const char *display_id, const char *seat_id, - const char *auth_file); + const char *auth_file, + gboolean initial); gboolean gdm_server_start (GdmServer *server); -#ifdef WITH_PLYMOUTH -gboolean gdm_server_start_on_active_vt (GdmServer *server); -#endif gboolean gdm_server_stop (GdmServer *server); char * gdm_server_get_display_device (GdmServer *server); diff --git a/daemon/gdm-simple-slave.c b/daemon/gdm-simple-slave.c index f0f27b47..137d238f 100644 --- a/daemon/gdm-simple-slave.c +++ b/daemon/gdm-simple-slave.c @@ -1311,12 +1311,14 @@ gdm_simple_slave_run (GdmSimpleSlave *slave) char *auth_file; char *seat_id; gboolean display_is_local; + gboolean display_is_initial; g_object_get (slave, "display-is-local", &display_is_local, "display-name", &display_name, "display-seat-id", &seat_id, "display-x11-authority-file", &auth_file, + "display-is-initial", &display_is_initial, NULL); /* if this is local display start a server if one doesn't @@ -1325,7 +1327,7 @@ gdm_simple_slave_run (GdmSimpleSlave *slave) gboolean res; gboolean disable_tcp; - slave->priv->server = gdm_server_new (display_name, seat_id, auth_file); + slave->priv->server = gdm_server_new (display_name, seat_id, auth_file, display_is_initial); disable_tcp = TRUE; if (gdm_settings_client_get_boolean (GDM_KEY_DISALLOW_TCP, @@ -1353,12 +1355,9 @@ gdm_simple_slave_run (GdmSimpleSlave *slave) if (slave->priv->plymouth_is_running) { plymouth_prepare_for_transition (slave); - res = gdm_server_start_on_active_vt (slave->priv->server); - } else -#endif - { - res = gdm_server_start (slave->priv->server); } +#endif + res = gdm_server_start (slave->priv->server); if (! res) { g_warning (_("Could not start the X " "server (your graphical environment) " diff --git a/daemon/gdm-slave.c b/daemon/gdm-slave.c index 94c369c5..1dd73948 100644 --- a/daemon/gdm-slave.c +++ b/daemon/gdm-slave.c @@ -103,6 +103,7 @@ struct GdmSlavePrivate char *parent_display_x11_authority_file; char *windowpath; char *display_x11_cookie; + gboolean display_is_initial; GdmDBusDisplay *display_proxy; GDBusConnection *connection; @@ -118,7 +119,8 @@ enum { PROP_DISPLAY_HOSTNAME, PROP_DISPLAY_IS_LOCAL, PROP_DISPLAY_SEAT_ID, - PROP_DISPLAY_X11_AUTHORITY_FILE + PROP_DISPLAY_X11_AUTHORITY_FILE, + PROP_DISPLAY_IS_INITIAL, }; enum { @@ -895,6 +897,17 @@ gdm_slave_real_start (GdmSlave *slave) return FALSE; } + error = NULL; + res = gdm_dbus_display_call_is_initial_sync (slave->priv->display_proxy, + &slave->priv->display_is_initial, + NULL, + &error); + if (! res) { + g_warning ("Failed to get value: %s", error->message); + g_error_free (error); + return FALSE; + } + return TRUE; } @@ -1786,6 +1799,13 @@ _gdm_slave_set_display_is_local (GdmSlave *slave, slave->priv->display_is_local = is; } +static void +_gdm_slave_set_display_is_initial (GdmSlave *slave, + gboolean is) +{ + slave->priv->display_is_initial = is; +} + static void gdm_slave_set_property (GObject *object, guint prop_id, @@ -1821,6 +1841,9 @@ gdm_slave_set_property (GObject *object, case PROP_DISPLAY_IS_LOCAL: _gdm_slave_set_display_is_local (self, g_value_get_boolean (value)); break; + case PROP_DISPLAY_IS_INITIAL: + _gdm_slave_set_display_is_initial (self, g_value_get_boolean (value)); + break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -1862,6 +1885,9 @@ gdm_slave_get_property (GObject *object, case PROP_DISPLAY_IS_LOCAL: g_value_set_boolean (value, self->priv->display_is_local); break; + case PROP_DISPLAY_IS_INITIAL: + g_value_set_boolean (value, self->priv->display_is_initial); + break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -2094,6 +2120,13 @@ gdm_slave_class_init (GdmSlaveClass *klass) "display is local", TRUE, G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY)); + g_object_class_install_property (object_class, + PROP_DISPLAY_IS_INITIAL, + g_param_spec_boolean ("display-is-initial", + NULL, + NULL, + FALSE, + G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY)); signals [STOPPED] = g_signal_new ("stopped", -- cgit v1.2.1