From 5741239219d0f6a6bdd5c3d21cf3ee80963b4fdb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Jun 23 2014 12:29:28 +0000 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 Backported by Ray Strode https://bugzilla.gnome.org/show_bug.cgi?id=511168 --- diff --git a/configure.ac b/configure.ac index 282217c..53825e3 100644 --- a/configure.ac +++ b/configure.ac @@ -1387,6 +1387,8 @@ else XSESSION_SHELL=/bin/sh fi +AC_DEFINE_UNQUOTED(GDM_INITIAL_VT, "1", [Initial Virtual Terminal]) + # Set configuration choices. # AC_SUBST(XSESSION_SHELL) diff --git a/daemon/gdm-display.c b/daemon/gdm-display.c index 77877b4..0eacbce 100644 --- a/daemon/gdm-display.c +++ b/daemon/gdm-display.c @@ -66,8 +66,7 @@ struct GdmDisplayPrivate GdmDisplayAccessFile *access_file; guint is_local : 1; - guint force_active_vt : 1; - + guint is_initial : 1; guint finish_idle_id; GdmSlaveProxy *slave_proxy; @@ -86,8 +85,8 @@ enum { PROP_X11_COOKIE, PROP_X11_AUTHORITY_FILE, PROP_IS_LOCAL, - PROP_FORCE_ACTIVE_VT, PROP_SLAVE_COMMAND, + PROP_IS_INITIAL }; static void gdm_display_class_init (GdmDisplayClass *klass); @@ -492,6 +491,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) { @@ -577,10 +590,9 @@ gdm_display_real_prepare (GdmDisplay *display) gdm_slave_proxy_set_log_path (display->priv->slave_proxy, log_path); g_free (log_path); - command = g_strdup_printf ("%s --display-id %s %s", + command = g_strdup_printf ("%s --display-id %s", display->priv->slave_command, - display->priv->id, - display->priv->force_active_vt? "--force-active-vt" : ""); + display->priv->id); gdm_slave_proxy_set_command (display->priv->slave_proxy, command); g_free (command); @@ -828,13 +840,6 @@ _gdm_display_set_is_local (GdmDisplay *display, } static void -_gdm_display_set_force_active_vt (GdmDisplay *display, - gboolean force_active_vt) -{ - display->priv->force_active_vt = force_active_vt; -} - -static void _gdm_display_set_slave_command (GdmDisplay *display, const char *command) { @@ -843,6 +848,13 @@ _gdm_display_set_slave_command (GdmDisplay *display, } 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, const GValue *value, @@ -877,12 +889,12 @@ gdm_display_set_property (GObject *object, case PROP_IS_LOCAL: _gdm_display_set_is_local (self, g_value_get_boolean (value)); break; - case PROP_FORCE_ACTIVE_VT: - _gdm_display_set_force_active_vt (self, g_value_get_boolean (value)); - break; 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; @@ -928,12 +940,12 @@ gdm_display_get_property (GObject *object, case PROP_IS_LOCAL: g_value_set_boolean (value, self->priv->is_local); break; - case PROP_FORCE_ACTIVE_VT: - g_value_set_boolean (value, self->priv->force_active_vt); - break; 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; @@ -1101,14 +1113,13 @@ gdm_display_class_init (GdmDisplayClass *klass) NULL, TRUE, G_PARAM_READWRITE | G_PARAM_CONSTRUCT)); - g_object_class_install_property (object_class, - PROP_FORCE_ACTIVE_VT, - g_param_spec_boolean ("force-active-vt", - NULL, - NULL, - FALSE, - 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)); g_object_class_install_property (object_class, PROP_SLAVE_COMMAND, g_param_spec_string ("slave-command", diff --git a/daemon/gdm-display.h b/daemon/gdm-display.h index 607ea1d..89cd975 100644 --- a/daemon/gdm-display.h +++ b/daemon/gdm-display.h @@ -122,6 +122,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, diff --git a/daemon/gdm-display.xml b/daemon/gdm-display.xml index a92e37f..96b09f5 100644 --- a/daemon/gdm-display.xml +++ b/daemon/gdm-display.xml @@ -25,6 +25,9 @@ + + + diff --git a/daemon/gdm-factory-slave.c b/daemon/gdm-factory-slave.c index e0be0ea..4163717 100644 --- a/daemon/gdm-factory-slave.c +++ b/daemon/gdm-factory-slave.c @@ -665,7 +665,7 @@ gdm_factory_slave_run (GdmFactorySlave *slave) if (display_is_local) { gboolean res; - slave->priv->server = gdm_server_new (display_name, auth_file); + slave->priv->server = gdm_server_new (display_name, auth_file, FALSE); g_signal_connect (slave->priv->server, "exited", G_CALLBACK (on_server_exited), diff --git a/daemon/gdm-local-display-factory.c b/daemon/gdm-local-display-factory.c index 5881c05..e33008a 100644 --- a/daemon/gdm-local-display-factory.c +++ b/daemon/gdm-local-display-factory.c @@ -65,7 +65,8 @@ static void gdm_local_display_factory_class_init (GdmLocalDisplayFactoryC static void gdm_local_display_factory_init (GdmLocalDisplayFactory *factory); static void gdm_local_display_factory_finalize (GObject *object); -static GdmDisplay *create_display (GdmLocalDisplayFactory *factory); +static GdmDisplay *create_display (GdmLocalDisplayFactory *factory, + gboolean is_initial); static gpointer local_display_factory_object = NULL; @@ -283,11 +284,14 @@ on_static_display_status_changed (GdmDisplay *display, int status; GdmDisplayStore *store; int num; + gboolean is_initial = TRUE; num = -1; gdm_display_get_x11_display_number (display, &num, NULL); g_assert (num != -1); + g_object_get (display, "is-initial", &is_initial, NULL); + store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory)); status = gdm_display_get_status (display); @@ -301,7 +305,7 @@ on_static_display_status_changed (GdmDisplay *display, gdm_display_store_remove (store, display); /* reset num failures */ factory->priv->num_failures = 0; - create_display (factory); + create_display (factory, is_initial); break; case GDM_DISPLAY_FAILED: /* leave the display number in factory->priv->displays @@ -314,7 +318,7 @@ on_static_display_status_changed (GdmDisplay *display, /* FIXME: should monitor hardware changes to try again when seats change */ } else { - create_display (factory); + create_display (factory, is_initial); } break; case GDM_DISPLAY_UNMANAGED: @@ -330,7 +334,8 @@ on_static_display_status_changed (GdmDisplay *display, } static GdmDisplay * -create_display (GdmLocalDisplayFactory *factory) +create_display (GdmLocalDisplayFactory *factory, + gboolean initial) { GdmDisplay *display; guint32 num; @@ -349,6 +354,7 @@ create_display (GdmLocalDisplayFactory *factory) /* FIXME: don't hardcode seat1? */ g_object_set (display, "seat-id", CK_SEAT1_PATH, NULL); + g_object_set (display, "is-initial", initial, NULL); g_signal_connect (display, "notify::status", @@ -386,7 +392,7 @@ gdm_local_display_factory_start (GdmDisplayFactory *base_factory) } /* FIXME: use seat configuration */ - display = create_display (factory); + display = create_display (factory, TRUE); if (display == NULL) { ret = FALSE; } diff --git a/daemon/gdm-product-slave.c b/daemon/gdm-product-slave.c index d4611a9..539c3bc 100644 --- a/daemon/gdm-product-slave.c +++ b/daemon/gdm-product-slave.c @@ -430,7 +430,7 @@ gdm_product_slave_create_server (GdmProductSlave *slave) if (display_is_local) { gboolean res; - slave->priv->server = gdm_server_new (display_name, auth_file); + slave->priv->server = gdm_server_new (display_name, auth_file, FALSE); g_signal_connect (slave->priv->server, "exited", G_CALLBACK (on_server_exited), diff --git a/daemon/gdm-server.c b/daemon/gdm-server.c index 295cb71..0bd62c6 100644 --- a/daemon/gdm-server.c +++ b/daemon/gdm-server.c @@ -84,7 +84,8 @@ struct GdmServerPrivate char *display_device; char *auth_file; - gboolean is_parented; + guint is_parented : 1; + guint is_initial : 1; char *parent_display_name; char *parent_auth_file; char *chosen_hostname; @@ -107,6 +108,7 @@ enum { PROP_SESSION_ARGS, PROP_LOG_DIR, PROP_DISABLE_TCP, + PROP_IS_INITIAL, }; enum { @@ -678,44 +680,6 @@ gdm_server_spawn (GdmServer *server, return ret; } -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); -} - /** * gdm_server_start: * @disp: Pointer to a GdmDisplay structure @@ -727,34 +691,15 @@ gboolean gdm_server_start (GdmServer *server) { gboolean res; + const char *vtarg = NULL; - /* fork X server process */ - res = gdm_server_spawn (server, NULL); - - return res; -} - -gboolean -gdm_server_start_on_active_vt (GdmServer *server) -{ - gboolean res; - gboolean debug = FALSE; - const char *logverbose; - char *vt; - - gdm_settings_direct_get_boolean (GDM_KEY_DEBUG, &debug); - - if (debug) { - logverbose = " -logverbose 7"; - } else { - logverbose = ""; + /* Hardcode the VT for the initial X server, but nothing else */ + if (server->priv->is_initial) { + vtarg = "vt" GDM_INITIAL_VT; } - g_free (server->priv->command); - server->priv->command = g_strdup_printf (X_SERVER " -nr -verbose -audit 4 %s", logverbose); - vt = get_active_vt_as_string (); - res = gdm_server_spawn (server, vt); - g_free (vt); + /* fork X server process */ + res = gdm_server_spawn (server, vtarg); return res; } @@ -844,6 +789,13 @@ _gdm_server_set_disable_tcp (GdmServer *server, } 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, const GValue *value, @@ -866,6 +818,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; @@ -899,6 +854,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; @@ -999,6 +957,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)); } @@ -1007,7 +972,6 @@ gdm_server_init (GdmServer *server) { gboolean debug = FALSE; const char *logverbose; - char *vt; server->priv = GDM_SERVER_GET_PRIVATE (server); @@ -1058,13 +1022,15 @@ gdm_server_finalize (GObject *object) GdmServer * gdm_server_new (const char *display_name, - const char *auth_file) + const char *auth_file, + gboolean initial) { GObject *object; object = g_object_new (GDM_TYPE_SERVER, "display-name", display_name, "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 bd6c60a..ce6d454 100644 --- a/daemon/gdm-server.h +++ b/daemon/gdm-server.h @@ -54,9 +54,9 @@ typedef struct GType gdm_server_get_type (void); GdmServer * gdm_server_new (const char *display_id, - const char *auth_file); + const char *auth_file, + gboolean initial); gboolean gdm_server_start (GdmServer *server); -gboolean gdm_server_start_on_active_vt (GdmServer *server); 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 8681415..8187546 100644 --- a/daemon/gdm-simple-slave.c +++ b/daemon/gdm-simple-slave.c @@ -1278,13 +1278,13 @@ gdm_simple_slave_run (GdmSimpleSlave *slave) char *display_name; char *auth_file; gboolean display_is_local; - gboolean force_active_vt; + gboolean display_is_initial; g_object_get (slave, "display-is-local", &display_is_local, "display-name", &display_name, "display-x11-authority-file", &auth_file, - "force-active-vt", &force_active_vt, + "display-is-initial", &display_is_initial, NULL); /* if this is local display start a server if one doesn't @@ -1293,7 +1293,7 @@ gdm_simple_slave_run (GdmSimpleSlave *slave) gboolean res; gboolean disable_tcp; - slave->priv->server = gdm_server_new (display_name, auth_file); + slave->priv->server = gdm_server_new (display_name, auth_file, display_is_initial); disable_tcp = TRUE; if (gdm_settings_client_get_boolean (GDM_KEY_DISALLOW_TCP, @@ -1320,13 +1320,10 @@ 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 { - if (force_active_vt) - res = gdm_server_start_on_active_vt (slave->priv->server); - else - res = gdm_server_start (slave->priv->server); } + + res = gdm_server_start (slave->priv->server); + if (! res) { g_warning (_("Could not start the X " "server (your graphical environment) " @@ -1477,14 +1474,12 @@ gdm_simple_slave_finalize (GObject *object) } GdmSlave * -gdm_simple_slave_new (const char *id, - gboolean force_active_vt) +gdm_simple_slave_new (const char *id) { GObject *object; object = g_object_new (GDM_TYPE_SIMPLE_SLAVE, "display-id", id, - "force-active-vt", force_active_vt, NULL); return GDM_SLAVE (object); diff --git a/daemon/gdm-simple-slave.h b/daemon/gdm-simple-slave.h index e9aa624..1f1aa1d 100644 --- a/daemon/gdm-simple-slave.h +++ b/daemon/gdm-simple-slave.h @@ -48,8 +48,7 @@ typedef struct } GdmSimpleSlaveClass; GType gdm_simple_slave_get_type (void); -GdmSlave * gdm_simple_slave_new (const char *id, - gboolean force_active_vt); +GdmSlave * gdm_simple_slave_new (const char *id); G_END_DECLS diff --git a/daemon/gdm-slave.c b/daemon/gdm-slave.c index 4a394f3..869448a 100644 --- a/daemon/gdm-slave.c +++ b/daemon/gdm-slave.c @@ -96,6 +96,7 @@ struct GdmSlavePrivate char *windowpath; GArray *display_x11_cookie; + gboolean display_is_initial; DBusGProxy *display_proxy; DBusGConnection *connection; @@ -110,7 +111,8 @@ enum { PROP_DISPLAY_IS_LOCAL, PROP_FORCE_ACTIVE_VT, PROP_DISPLAY_SEAT_ID, - PROP_DISPLAY_X11_AUTHORITY_FILE + PROP_DISPLAY_X11_AUTHORITY_FILE, + PROP_DISPLAY_IS_INITIAL, }; enum { @@ -912,6 +914,24 @@ gdm_slave_real_start (GdmSlave *slave) return FALSE; } + error = NULL; + res = dbus_g_proxy_call (slave->priv->display_proxy, + "IsInitial", + &error, + G_TYPE_INVALID, + G_TYPE_BOOLEAN, &slave->priv->display_is_initial, + G_TYPE_INVALID); + if (! res) { + if (error != NULL) { + g_warning ("Failed to get value: %s", error->message); + g_error_free (error); + } else { + g_warning ("Failed to get value"); + } + + return FALSE; + } + return TRUE; } @@ -1605,10 +1625,10 @@ _gdm_slave_set_display_is_local (GdmSlave *slave, } static void -_gdm_slave_set_force_active_vt (GdmSlave *slave, - gboolean force_active_vt) +_gdm_slave_set_display_is_initial (GdmSlave *slave, + gboolean is) { - slave->priv->force_active_vt = force_active_vt; + slave->priv->display_is_initial = is; } static void @@ -1643,8 +1663,8 @@ 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_FORCE_ACTIVE_VT: - _gdm_slave_set_force_active_vt (self, g_value_get_boolean (value)); + 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); @@ -1684,8 +1704,8 @@ gdm_slave_get_property (GObject *object, case PROP_DISPLAY_IS_LOCAL: g_value_set_boolean (value, self->priv->display_is_local); break; - case PROP_FORCE_ACTIVE_VT: - g_value_set_boolean (value, self->priv->force_active_vt); + 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); @@ -1811,13 +1831,12 @@ 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_FORCE_ACTIVE_VT, - g_param_spec_boolean ("force-active-vt", - "Force Active VT", - "Force display to active VT", - TRUE, + PROP_DISPLAY_IS_INITIAL, + g_param_spec_boolean ("display-is-initial", + NULL, + NULL, + FALSE, G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY)); signals [STOPPED] = diff --git a/daemon/gdm-static-display.c b/daemon/gdm-static-display.c index 8a96b42..a747ee3 100644 --- a/daemon/gdm-static-display.c +++ b/daemon/gdm-static-display.c @@ -86,27 +86,10 @@ gdm_static_display_remove_user_authorization (GdmDisplay *display, } static gboolean -triggered_to_force_display_on_active_vt (void) -{ - gboolean should_force_display_on_active_vt; - - should_force_display_on_active_vt = g_file_test (GDM_SPOOL_DIR "/force-display-on-active-vt", - G_FILE_TEST_EXISTS); - g_unlink (GDM_SPOOL_DIR "/force-display-on-active-vt"); - - return should_force_display_on_active_vt; -} - -static gboolean gdm_static_display_manage (GdmDisplay *display) { g_return_val_if_fail (GDM_IS_DISPLAY (display), FALSE); - if (triggered_to_force_display_on_active_vt ()) { - g_object_set (display, "force-active-vt", TRUE, NULL); - } else { - g_object_set (display, "force-active-vt", FALSE, NULL); - } GDM_DISPLAY_CLASS (gdm_static_display_parent_class)->manage (display); return TRUE; diff --git a/daemon/simple-slave-main.c b/daemon/simple-slave-main.c index ec94341..eed1742 100644 --- a/daemon/simple-slave-main.c +++ b/daemon/simple-slave-main.c @@ -177,11 +177,9 @@ main (int argc, DBusGConnection *connection; GdmSlave *slave; static char *display_id = NULL; - static gboolean force_active_vt = FALSE; GdmSignalHandler *signal_handler; static GOptionEntry entries [] = { { "display-id", 0, 0, G_OPTION_ARG_STRING, &display_id, N_("Display ID"), N_("ID") }, - { "force-active-vt", 0, 0, G_OPTION_ARG_NONE, &force_active_vt, N_("Force X to start on active vt"), NULL }, { NULL } }; @@ -250,7 +248,7 @@ main (int argc, gdm_signal_handler_add (signal_handler, SIGUSR1, signal_cb, NULL); gdm_signal_handler_add (signal_handler, SIGUSR2, signal_cb, NULL); - slave = gdm_simple_slave_new (display_id, force_active_vt); + slave = gdm_simple_slave_new (display_id); if (slave == NULL) { goto out; }