Bug 307412 - PowerDevil should tell systemd not to handle events like «lid closed» or «power button pressed» in a KDE session.
Summary: PowerDevil should tell systemd not to handle events like «lid closed» or «pow...
Status: RESOLVED FIXED
Alias: None
Product: solid
Classification: Frameworks and Libraries
Component: powermanagement-daemon (show other bugs)
Version: 4.9.2
Platform: Arch Linux Linux
: NOR normal
Target Milestone: ---
Assignee: Lukáš Tinkl
URL: http://lists.freedesktop.org/archives...
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-25 22:27 UTC by Nikita Skovoroda
Modified: 2013-04-05 17:06 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.9.3


Attachments
tentative patch to store the fd (2.30 KB, patch)
2012-10-03 12:03 UTC, Lukáš Tinkl
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Skovoroda 2012-09-25 22:27:20 UTC
In sysemd 191, the handling of «Lid closed», «Power button pressed», «Suspend button pressed», «Hibernate button pressed» has changed. Now, they are handled by default even when a graphical DE is running: http://lists.freedesktop.org/archives/systemd-devel/2012-September/006604.html

PowerDevil should create inhibitor lock on "handle-power-key:handle-sleep-key:handle-lid-switch" via dbus (as stated here: http://www.freedesktop.org/wiki/Software/systemd/inhibit ) for systemd-logind not to handle those events.

Reproducible: Always

Steps to Reproduce:
1. Install systemd 191 or later (available in ArchLinux testing) with default logind.conf.
2. Start a KDE session.
3. Configure PowerDevil to suspend on lid close and to show a shutdown dialog on power button press.
A1. Close the lid.
A2. Open the lid (and resume the computer).
A3. Wait a moment and see what happens.
B1. Press the power button and see what happens.
Actual Results:  
A. The computer is suspended two times: first by systemd, next (when it is resumed) — it is suspended again by powerdevil.
B. The computer is powered off by systemd when the power button is pressed.

Expected Results:  
A. The computer should be suspended only one time upon a lid close event.
B. The shutdown dialog should be shown when the power button is pressed, no immediate poweroff should happen.

This can cause data loss. For example when a user configured powerdevil to suspend on a power key press, and pressing the power key unexpectedly shutdowns the computer completely, closing all open documents and loosing all unsaved changes. But that's a corner case.
Comment 1 Nikita Skovoroda 2012-09-25 22:40:40 UTC
> "handle-power-key:handle-sleep-key:handle-lid-switch" 

I'm not sure if it should be "handle-power-key:handle-sleep-key:handle-lid-switch" , "handle-power-key:handle-suspend-key:handle-hibernate-key:handle-lid-switch" or even "handle-power-key:handle-sleep-key:handle-suspend-key:handle-hibernate-key:handle-lid-switch".

 http://www.freedesktop.org/wiki/Software/systemd/inhibit is unclear to me in that part.
Comment 2 Nikita Skovoroda 2012-09-25 22:56:57 UTC
Ok, the page got updated: http://www.freedesktop.org/wiki/Software/systemd/inhibit?action=diff&rev1=29&rev2=30

The correct string is "handle-power-key:handle-suspend-key:handle-hibernate-key:handle-lid-switch".
Comment 3 Lukáš Tinkl 2012-09-26 09:06:54 UTC
https://git.reviewboard.kde.org/r/106533/
Comment 4 Lukáš Tinkl 2012-09-26 18:59:54 UTC
Fixed today, see http://commits.kde.org/kde-workspace/03a27e496a178a6aeeca23e53b52f4d87cd0fbef
Comment 5 Nikita Skovoroda 2012-09-26 21:34:44 UTC
Thanks!
Will this fix be available in 4.9.2?
Comment 6 Nikita Skovoroda 2012-09-26 21:39:25 UTC
> Version Fixed In :	 4.9.2
Ah, missed that.
Thanks!
Comment 7 Nikita Skovoroda 2012-10-03 02:01:31 UTC
Does not work in 4.9.2.

Logs say «kded(1045) PowerDevil::PolicyAgent::onSessionHandlerRegistered: systemd powersave events handling inhibited», but no effect.

Still double suspending, «systemd-inhibit --list» is empty.

Running systemd-inhibit manually works as expected (and populates «systemd-inhibit --list»).
Comment 8 Nikita Skovoroda 2012-10-03 02:07:49 UTC
…
PowerDevil::BackendLoader::loadBackend: Loading UPower backend...
PowerDevil::BackendLoader::loadBackend: Success!
KDEDPowerDevil::init: Backend loaded, loading core
PowerDevil::Core::loadCore: Core loaded, initializing backend
PowerDevilUPowerBackend::brightness: Screen brightness:  100
PowerDevilUPowerBackend::init: current screen brightness:  100
PowerDevilUPowerBackend::init: Can suspend
PowerDevilUPowerBackend::init: Can hibernate
PowerDevil::Core::onBackendReady: Backend is ready, KDE Power Management system initialized
PowerDevil::Core::onDeviceAdded: A new battery was detected
PowerDevil::PolicyAgent::onSessionHandlerRegistered: Session path: "/org/freedesktop/login1/session/c1"
PowerDevil::PolicyAgent::onSessionHandlerRegistered: ACTIVE SESSION PATH: "/org/freedesktop/login1/session/c1"
PowerDevil::PolicyAgent::onActiveSessionChanged: Current session is now active
PowerDevil::PolicyAgent::onSessionHandlerRegistered: systemd powersave events handling inhibited
PowerDevil::PolicyAgent::onSessionHandlerRegistered: systemd support initialized
PowerDevil::PolicyAgent::onActiveSessionChanged: Current session is now active
PowerDevil::PolicyAgent::onSessionHandlerRegistered: ConsoleKit support initialized
PowerDevil::ActionPool::init: Got a valid offer for  "DPMSControl"
…
Comment 9 Nikita Skovoroda 2012-10-03 02:22:37 UTC
Wait a moment. I just looked at the code.

Where is the file descriptor that is meant to be stored?
Comment 10 Nikita Skovoroda 2012-10-03 02:27:21 UTC
> http://commits.kde.org/kde-workspace/03a27e496a178a6aeeca23e53b52f4d87cd0fbef

This commit does not store anything!

It was fine when it was on the reviewboard.

/Ok, seems like we'll have to wait a month until 4.9.3.
Comment 11 Nikita Skovoroda 2012-10-03 10:53:43 UTC
You can put «systemd-inhibit --what=handle-lid-switch:handle-power-key:handle-suspend-key:handle-hibernate-key sleep 9999d» in KDE autostart (not bash autostart!) as a workaround.
Comment 12 Lukáš Tinkl 2012-10-03 12:03:53 UTC
Created attachment 74314 [details]
tentative patch to store the fd

Attached is a patch to store the fd, hopefully it fixes the inhibition
Comment 13 Lukáš Tinkl 2012-10-03 14:35:38 UTC
[ltinkl@goblin ~]$ systemd-inhibit --what=handle-lid-switch:handle-power-key:handle-suspend-key:handle-hibernate-key sleep 9999d
Failed to inhibit: Invalid argument

The workaround doesn't seem to work with older systemd versions, although the command is present
Comment 14 Nikita Skovoroda 2012-10-03 20:08:28 UTC
Workaround is intended to newer systemd versions (191 or later).

On older systemd versions it's just not needed, there is no issue to work-around.
Comment 15 Nikita Skovoroda 2012-10-03 20:13:49 UTC
This whole thing is about something that was introduced in systemd 191.

KDE + systemd 190 or lower (with out-of-the-box configurations) works ok with no issues.
KDE + systemd 191 or higher (with out-of-the-box configurations) has an issue described in this bug, that can be worked-around with #c11 until it is fixed in newer KDE version (hope it'll be 4.9.3).

So, it's perfectly ok if the work-around doesn't work with older KDE versions — it's just not needed.
Comment 16 Nikita Skovoroda 2012-10-03 21:32:07 UTC
Btw, https://bugzilla.redhat.com/show_bug.cgi?id=859227 should be reopened too.
Comment 17 Lukáš Tinkl 2012-10-04 09:18:43 UTC
I'm more interested in knowing whether the attached patch works for you :)
Comment 18 Nikita Skovoroda 2012-10-04 11:13:09 UTC
I'll test it today (in my timezone), when i'll get home.
Thanks.
Comment 19 Lukáš Tinkl 2012-10-05 09:58:53 UTC
Git commit a18b78d7da8cb8d627ad2e85f666bfcf1a2721e1 by Lukas Tinkl.
Committed on 05/10/2012 at 11:57.
Pushed by lukas into branch 'KDE/4.9'.

store the filedescriptor in a member variable

make systemd-inhibit work as intended, PowerDevil now handles
power/sleep/lid buttons as intended

M  +6    -3    powerdevil/daemon/powerdevilpolicyagent.cpp
M  +2    -0    powerdevil/daemon/powerdevilpolicyagent.h

http://commits.kde.org/kde-workspace/a18b78d7da8cb8d627ad2e85f666bfcf1a2721e1
Comment 20 Lukáš Tinkl 2012-10-05 09:59:48 UTC
Git commit 29a65c6f984f3bcb5792b0b89061e45c925bbe7a by Lukas Tinkl.
Committed on 05/10/2012 at 11:57.
Pushed by lukas into branch 'master'.

store the filedescriptor in a member variable

make systemd-inhibit work as intended, PowerDevil now handles
power/sleep/lid buttons as intended

M  +6    -3    powerdevil/daemon/powerdevilpolicyagent.cpp
M  +2    -0    powerdevil/daemon/powerdevilpolicyagent.h

http://commits.kde.org/kde-workspace/29a65c6f984f3bcb5792b0b89061e45c925bbe7a
Comment 21 Nikita Skovoroda 2012-10-05 23:30:07 UTC
Sorry for the late comment. Yes, the patched version works fine, thanks!
Comment 22 Bastian Beischer 2012-10-28 17:18:16 UTC
Hey,

Indeed the patch helps, but when I'm still running into the same issue _sometimes_. I think it has to do with disconnecting the AC cord or swichting activities (and the associated power devil configuration)... Because it works fine when I first boot into my system, login and then suspend/resume.

But after I worked for a while I took my laptop out of the docking station so that the battery was not charging anymore, of course power devil switches the power saving configuration then and when I suspend then it still immediately resuspends after waking up...
Comment 23 Lukáš Tinkl 2012-10-29 11:03:10 UTC
Interesting... so it looks like systemd cookie (stored in a file descriptor) goes away after the resume, so we need to recreate it
Comment 24 Lukáš Tinkl 2012-10-30 10:34:09 UTC
Git commit 80e9e6e48ff5b84962f3a8543ee06bcd4f122623 by Lukáš Tinkl.
Committed on 30/10/2012 at 11:32.
Pushed by lukas into branch 'KDE/4.9'.

move systemd inhibition initialization to a slot

and call it on resume. It looks like the filedescriptor
goes away when you suspend so we need to recreate it.

M  +4    -2    powerdevil/daemon/powerdevilcore.cpp
M  +27   -17   powerdevil/daemon/powerdevilpolicyagent.cpp
M  +2    -0    powerdevil/daemon/powerdevilpolicyagent.h

http://commits.kde.org/kde-workspace/80e9e6e48ff5b84962f3a8543ee06bcd4f122623
Comment 25 Lukáš Tinkl 2012-10-30 10:48:24 UTC
Git commit c9e66006e6b2bc4176e9b475b2c7d5e40c180133 by Lukáš Tinkl.
Committed on 30/10/2012 at 11:32.
Pushed by lukas into branch 'master'.

move systemd inhibition initialization to a slot

and call it on resume. It looks like the filedescriptor
goes away when you suspend so we need to recreate it.

M  +4    -2    powerdevil/daemon/powerdevilcore.cpp
M  +27   -17   powerdevil/daemon/powerdevilpolicyagent.cpp
M  +2    -0    powerdevil/daemon/powerdevilpolicyagent.h

http://commits.kde.org/kde-workspace/c9e66006e6b2bc4176e9b475b2c7d5e40c180133
Comment 26 Bastian Beischer 2012-10-31 10:24:09 UTC
Hey Lukas,

I still have problems. Maybe my logind config file is not set correctly? Here's what I have (systemd version is 195):

#HandlePowerKey=poweroff
#HandleSuspendKey=suspend
#HandleHibernateKey=hibernate
#HandleLidSwitch=suspend
#PowerKeyIgnoreInhibited=no
#SuspendKeyIgnoreInhibited=no
#HibernateKeyIgnoreInhibited=no
LidSwitchIgnoreInhibited=no

Note that the default for LidSwitchIgnoreInhibited is yes. To my understanding that would mean that systemd will not accept the inhibit issued by KDE for lid switch events?

My problems appear when I have the AC cord attached and then close the lid. System will suspend. Then I remove the cord and open the lid. System will immediately resuspend. After I wake up from that second suspend powerdevil has set the profile to battery mode and then once I close and open the lid again it will again resuspend immediately.

Am I doing something wrong?
Comment 27 Lukáš Tinkl 2012-10-31 11:08:55 UTC
Try to set the option to the default (yes), otherwise I'm out of ideas as to what could cause this
Comment 28 Nikita Skovoroda 2012-10-31 12:54:04 UTC
>
> Note that the default for LidSwitchIgnoreInhibited is yes. To my
> understanding
> that would mean that systemd will not accept the inhibit issued by KDE for
> lid
> switch events?
>

Wrong.

The LidSwitchIgnoreInhibited parameter is not related to inhibiting the
«lid close» event, but, instead, it tells systemd to ignore suspend (not
key events) inhibits when the lid close event is not inhibited and handeled
by systemd itself.
For example, when HandleLidSwitch={action} and
LidSwitchIgnoreInhibited=yes, when {action} is inhibited, the lid switch is
not inhibited and the lid is closed, the {action} inhibitor is ignored, and
the {action} is called.

You better leave default values there.
Comment 29 Arkadiusz Miskiewicz 2013-04-05 16:28:37 UTC
With this patch I'm unable to get my systemd manage lid key (instead of kde).

Unchecking "handle button events" doesn't help. systemd still gets information that PowerDevil manages lid events.

Could this also be fixed, so there is a choice for a user?
Comment 30 Kevin Kofler 2013-04-05 16:37:57 UTC
No, this is exactly as designed. Unchecking "handle button events" means, you want button events to not get handled at all. It has always meant that.

We do not let systemd handle the event because it does not honor KDE settings.
Comment 31 Arkadiusz Miskiewicz 2013-04-05 16:48:20 UTC
And now KDE doesn't honor user desires. I really want my systemd to handle events (like lid closing) and not KDE. I have custom systemd scripts that do many things on suspend and resume.

There should be a way for user to get desired behaviour. Could some setting for that be added? Like "Allow systemd to handle button events"
Comment 32 Kevin Kofler 2013-04-05 17:06:56 UTC
Those custom scripts should still be firing, because PowerDevil uses systemd to perform the actual suspend/resume, IF suspending is what you're asking PowerDevil to do.