After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 511881 - support for ConsoleKit reboot and shutdown
support for ConsoleKit reboot and shutdown
Status: RESOLVED FIXED
Product: gnome-session
Classification: Core
Component: general
2.23.x
Other Linux
: Normal normal
: ---
Assigned To: Lucas Rocha
Session Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-01-24 20:32 UTC by William Jon McCann
Modified: 2008-06-12 23:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (24.44 KB, patch)
2008-01-24 20:46 UTC, William Jon McCann
none Details | Review
updated patch (25.63 KB, patch)
2008-02-02 02:48 UTC, William Jon McCann
needs-work Details | Review
updated patch (31.21 KB, patch)
2008-02-14 18:51 UTC, William Jon McCann
none Details | Review
updated patch (31.32 KB, patch)
2008-03-16 03:51 UTC, Matthias Clasen
committed Details | Review

Description William Jon McCann 2008-01-24 20:32:15 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).
Comment 1 William Jon McCann 2008-01-24 20:46:32 UTC
Created attachment 103676 [details] [review]
patch
Comment 2 Vincent Untz 2008-01-24 20:54:48 UTC
Will try to look at the patch soon, but I'm away all next week...
Comment 3 William Jon McCann 2008-02-02 02:48:14 UTC
Created attachment 104240 [details] [review]
updated patch

Only changes: centers the dialog on screen and remove c++ comments
Comment 4 Vincent Untz 2008-02-11 13:16:35 UTC
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.
Comment 5 William Jon McCann 2008-02-14 18:51:29 UTC
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.
Comment 6 Matthias Clasen 2008-03-16 03:51:50 UTC
Created attachment 107371 [details] [review]
updated patch
Comment 7 Marco Pesenti Gritti 2008-05-01 12:24:31 UTC
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?

Comment 8 Vincent Untz 2008-05-01 16:35:43 UTC
Marco: yes, that's the plan.
Comment 9 Lucas Rocha 2008-06-03 15:16:22 UTC
Re-assigning to gnome-session.
Comment 10 Lucas Rocha 2008-06-05 23:09:23 UTC
Porting the patch now...
Comment 11 Lucas Rocha 2008-06-10 23:49:48 UTC
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?
Comment 12 Lucas Rocha 2008-06-12 23:15:30 UTC
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.