GNOME Bugzilla – Bug 511881
support for ConsoleKit reboot and shutdown
Last modified: 2008-06-12 23:41:47 UTC
Hi, We have moved the reboot and shutdown functionality from GDM to ConsoleKit. This provides a number of benefits including: cross desktop support, ability to use methods when GDM isn't available or running, knowledge of system users and sessions without going over a bus, and ability to update ConsoleKit history files for restart events, etc. Here is a patch to add this support to the panel. The ConsoleKit integration requires PolicyKit-gnome. But if gnome-panel isn't built with PK support it will continue to use the old GDM socket interface (for GDM 2.20 and before).
Created attachment 103676 [details] [review] patch
Will try to look at the patch soon, but I'm away all next week...
Created attachment 104240 [details] [review] updated patch Only changes: centers the dialog on screen and remove c++ comments
Thanks for the patch. Some comments: + the panel_action_shutdown_reboot_is_disabled() change seems wrong to me. We surely want to add the ConsoleKit check here, instead of removing the gdm tests? + in do_request_reboot() and do_request_shutdown(), with consolekit, we don't call panel_session_request_logout(). Is this right? Shouldn't we tell the session manager we're stopping the session (so that it can saves it, if necessary)? + nitpick: "if (! ret) {" => "if (!ret) {" + in panel_consolekit_on_name_owner_changed(): are we 100% sure that name is always not NULL? If yes, fine. If not, we'll crash in the strcmp. + in panel_consolekit_finalize(), it looks like we should unref the dbus connection, the bus proxy, etc. + in system_restart_auth_cb/system_stop_auth_cb: what's the point of the first g_warning()? Doesn't it only happen when the user can't get the privilege? If yes, we don't need the warning. Else we want a dialog. + system_stop_auth_cb() calls try_system_restart() instead of try_system_stop() (I guess) + if try_system_restart()/stop() fails, we get an error. Shouldn't we display a dialog thanks to the request-completed signal? + in get_action_from_error(): isn't there a real way to get the action? Parsing an error message looks really hacky. + in request_restart_priv/request_stop_priv: if we don't have policykit, we just emit_restart_complete() without an error. This is wrong, we should have an error there. + in panel_consolekit_can_restart/stop: we should call panel_consolekit_ensure_ck_connection() instead of returning TRUE.
Created attachment 105268 [details] [review] updated patch Thanks for the review! This should address all the comments except that for now there is no better way to get the action name other than parsing the error. David is aware of this and will add this in a future version of policykit.
Created attachment 107371 [details] [review] updated patch
According to http://live.gnome.org/SessionManagement/Todo gnome-panel will use the gnome-session API for logout and shutdown. Should this patch ported to gnome-session and checked in there?
Marco: yes, that's the plan.
Re-assigning to gnome-session.
Porting the patch now...
The patch is ported to gnome-session already but I'm getting the following error when trying to stop the system with: ** (gnome-session:12567): WARNING **: Unable to stop system: Method "Stop" with signature "" on interface "org.freedesktop.ConsoleKit.Manager" doesn't exist Any clue?
I was using an old release of ConsoleKit. Commited in trunk. 2008-06-13 Lucas Rocha <lucasr@gnome.org> Add support for ConsoleKit reboot and shutdown. #511881, William Jon McCann. Patch by William Jon McCann and Matthias Clasen. * configure.in: check for optional dependency on polkit-gnome. * gnome-session/Makefile.am: use polkit-gnome flags and libs on session manager build. * gnome-session/consolekit.[ch]: interface to abstract the communication with ConsoleKit and PolicyKit. * gnome-session/logout-dialog.c (gsm_logout_supports_reboot, gsm_logout_supports_shutdown): first check if ConsoleKit can shutdown/restart system. If not, try GDM directly. * gnome-session/session.c (do_request_reboot, do_request_shutdown, session_shutdown): use ConsoleKit to shutdown/restart system after logout. If not possible, use GDM directly.