GNOME Bugzilla – Bug 594833
Enhancement: Bring back failsafe session support to GDM
Last modified: 2009-11-24 07:27:11 UTC
Created attachment 142953 [details] [review] Add's failsafe session support to GDM 2.27.90 Prior to the GDM 2.27.x branch, GDM had a built in failsafe session option. This was a useful feature in a number of scenarios where the normal desktop session may have been broken requiring intervention by the user or sys-admin. Examples include not having virtual terminal or virtual console access, non-console access such as over XDMCP or terminal server environments. When selected, the failsafe option would allow the user to attempt to fix potential problems such as errors in .profile or scripts like PreSession by directly launching an xterm window and bypassing or ignoring erros from these scripts. I wish to reintroduce support for a failsafe session option into GDM 2.27.x I've a attached a patch that introduces the notion of a failsafe session in a generic and flexible manner as opposed to the hardcoded xterm failsafe session in GDM prior to the 2.27.x branch. It introduces an optional boolean key to a session .desktop file: "FailSafe". If this key is not present then it's value is presumed to be false. If the value is false, then the behaviour of the session is unchanged. If true then the session gets launched directly rather than passing it as an argument to the Xsession script. This is the way GDM previously dealt with it's internal failsafe session. The patch adds a new API in the daemon session direct code: gdm_session_direct_is_failsafe() This function is not currently used anywhere outside of gdm-session-direct.c however that will presumably change once gdm-simple-slave.c implements checking of the return code from invocation of the PreSession script. In such cases the old GDM would ignore erros from this script for failsafe sessions. I've commented this FIXME appropriately so that is does not get overlooked later. On the gui side I have adjusted the simple-greeter and test-sessions.c to append " (Failsafe)" to the user visible name of any session whose .desktop file is tagged with the key/value: FailSafe=true. Vendors who wish to provide a failsafe session option can easily to so by supplying a .desktop xsession file with the key/value: FailSafe=true This leaves them free to choose the most appropriate failsafe session command for their environment such as launching xterm or a failsafe gnome-session etc.
Comment on attachment 142953 [details] [review] Add's failsafe session support to GDM 2.27.90 Hey, I think this is a pretty good idea. A couple of comments: 1) We shouldn't us FailSafe, but instead something prefixed with X-GDM- (since the format of desktop files are standardized). I'd propose: X-GDM-FailSafe or X-GDM-SkipXsession I also wouldn't bother adding the (Failsafe) automatically. If admins want that added, they can add it in their description.
Created attachment 143259 [details] [review] Second draft of patch to add failsafe session support.
Hi Ray, I agree with your suggestions. I am using X-GDM-FailSafe as the key name in the desktop file now. I also removed the diffs corresponding to the greeter side of my initial patch so (Failsafe) is no longer appended to the session name in the greeter gui. Revised patch is attached.
Is there any update on this? Would be really nice to have this feature again.
Ping. I haven't received any feedback on the revised patch I submitted. Is it OK to go upstream?
Yes, this patch looks good. It will probably require some thought to determine if this will go into a 2.28.1 release or 2.29.0. But I anticipate this patch will go upstream once a decision is made.
At the Boston Summit, Ray Strode and I discussed this with Jon McCann. Jon seemed to feel that the term "Failsafe" is not a very good term for this feature since it really does not add any special "failsafe" features. All we are really doing is adding a boolean to control whether or not the Xsession script is run. I think the main reason the term "failsafe" was used for this feature was just because that is the term the old GDM used. However, Jon seemed to feel that it would be best to avoid that terminology. So, should we change the new key to something else? Ray suggested X-GDM-SkipXsession. Ray, what do you think? It would be best to get this right before we integrate. If we change this, then we should probably also change the variable names in the code to also avoid using the term "failsafe".
The tag and it's handling by GDM doesn't automatically make a given session "failsafe", that's for sure, but it's not the intention to. To me, the tag should be the means for the vendor or sys-admin providing the .desktop file to indicate that the session in question is a failsafe/(when all else fails) type of session and that GDM should not try to apply any processing or do anything fancy with it that might break the login. To me, calling it X-GDM-SkipXsession expresses how you are going to do it, rather than what you actually want to do, and is somewhat less meaningful.
Alternatively, how about "X-GDM-SessionDirect"? I think it captures the idea that the session will be executed directly by GDM with no internal scripts or preprocessing applied, yet is a bit more meaningful than "X-GDM-SkipXSession" if Xsession doesn't mean anything special to you.
I wouldn't have a problem with X-GDM-SessionDirect other than the name of the file being modified is gdm-session-direct.c. That file uses the word "direct" to mean something different than your proposed usage which leaves room for confusion. I'd recommend sticking with X-GDM-SkipXSession or if you really oppose it coming up with something else... X-GDM-Bypass-Internal-Scripts maybe? Not sure.
I don't want to get hung up on the name of this. X-GDM-SkipXSession is fine with me. I'll update the patch.
I don't mean to be a pain, but I actually prefer X-GDM-BypassXsession. This seems to more accurately describe what the feature does, more than Skip.
Niall, do you have an updated patch?
Created attachment 147631 [details] [review] Patch to skip XSession invocation when X-GDM-SkipXSesssion=true
Hi Brian, The revised patch is attached.
Comment on attachment 147631 [details] [review] Patch to skip XSession invocation when X-GDM-SkipXSesssion=true I've commited this with minor changes: http://git.gnome.org/cgit/gdm/commit/?id=b01a179507aa08d65a57109e5418a7c240ff324f Thanks for working on this.
I added this paragraph to the end of the "GDM Session Configuration" section, so the feature is documented. + <para> + Desktop files support a GDM specific key + "X-GDM-BypassXsession". If the key is not specified in a + desktop file, the value defaults to "false". If this key is + specified to be "true" in a desktop file, then GDM will + launch the program specified by the desktop file "Exec" key + directly when starting the user session. It will not run the program + via the <filename><etc>/gdm/Xsession</filename> script, which is + the normal behavior. Since bypassing the + <filename><etc>/gdm/Xsession</filename> script avoids setting up + the user session with the normal system and user settings, sessions + started this way can be useful for debugging problems in the system or + user scripts that might be preventing a user from being able to start + a session. + </para>
Hi Ray, thanks for integrating the patch upstream. I have found a bug in the changes you committed: + res = g_key_file_has_key (key_file, G_KEY_FILE_DESKTOP_GROUP, "X-GDM-BypassXsession", NULL); + if (!res) { + goto out; + } else { + bypasses_xsession = g_key_file_get_boolean (key_file, G_KEY_FILE_DESKTOP_GROUP, "X-GDM-BypassXSession", &error); The wrong case is used in the second reference to the key file key "X-GDM-BypassXsession". In the second reference, it uses "X-GDM-BypassXSession" (upper-cased Session), which causes the function to always return FALSE. Cheers, Niall.
Fixed in master. Now only X-GDM-BypassXsession is used.