Suspend via logind over UPower if available. #4168

Closed
wants to merge 20 commits into
from

Projects

None yet

9 participants

Contributor
yasij commented Feb 7, 2014

The suspend feature of UPower is deprecated. XBMC supports suspending using systemd-logind, however it defaults to a few other UPower methods first before trying Logind. This patch checks for logind first, and uses it if available.

Contributor
yasij commented Feb 9, 2014

@Anyone the UPower suspend methods will be removed from distros pretty soon. Logind should be the default choice for suspending on Linux when it exists. If the default order isn't changed, there should be a way to configure and force XBMC to use logind for suspend.

Member
fritsch commented Feb 9, 2014

@yasij: thanks for doing that PR. The dev in charge @topfs2 will be available next week again to review.

@MartijnKaijser MartijnKaijser added this to the Pending for inclusion milestone Feb 9, 2014
Contributor

@topfs2 I think I saw you active on the forums this morning. Any comments?

Member
topfs2 commented Feb 11, 2014

I haven't really kept myself up to date if logind is superseeding upower and such but the patch looks correct if thats true.
The one thing which strikes me is that isn't it possible to have logind without suspend capabilities but upower with? Have a small recollection that one ubuntu (latest?) behaved like that?

Contributor
yasij commented Feb 11, 2014

Ubuntu supports suspend with logind. It uses upstart for init instead of systemd, and implements the part it needs through some extra scripts in the systemd-shim package. The problem is that UPower suspend can act weird in this case because the shim doesn't properly signal UPower on wakeup when UPower is used to suspend.

Actually, I think Debian with sysvinit does not have the shim scripts implemented, and thus suspend wouldn't work. It also won't work with systemd-logind versions before 183 because suspend/resume was added in that version. Maybe the suspend method should be configurable?

Contributor
yasij commented Feb 11, 2014

The way KDE checks for it is here: https://projects.kde.org/projects/kde/kde-workspace/repository/revisions/master/entry/powerdevil/daemon/backends/upower/powerdevilupowerbackend.cpp#L40

It checks the systemd version, and if the version API isn't implemented, it checks for upstart.

Contributor

What's the potential for this affecting older distros? I'm more concerned about breaking people's current installs than new ones, for Gotham.

Contributor
yasij commented Feb 11, 2014

The potential is certainly there for affecting older distros that are using systemd with logind before suspend/resume support was added. It looks like Fedora 17 and OpenSUSE 12.2 use systemd version 44 which doesn't support suspend. Ubuntu added systemd-logind in raring 13.04 and has the shim scripts for suspend. Before that, Ubuntu didn't have any piece of systemd, so the check for logind will fail.

There is a CanSuspend() method in the logind dbus API (as well as CanPowerOff(), CanReboot(), CanHibernate(), CanHybridSleep()). These might need to be checked with a transition over to the UPower method if they return no.

Contributor

@topfs2 @yasij If you guys can work something out to minimize the potential effect on current installs, I'll consider this for Gotham.

Member
topfs2 commented Feb 16, 2014

Sorry for slow response from me, I've been sick so haven't touched the computer in a while :)
Anyways, so from what I understand the simplest solution would be to check for systemd and if version is > 183?

Contributor
yasij commented Feb 16, 2014

Unfortunately, that's not the entire solution because of systems without
systemd as PID 1. Ubuntu doesn't implement the systemd version API but does
support suspend via logind. KDE checks for the systemd version. I'd the
dbus call fails, it checks the upstart version since Ubuntu has promised to
support the logind suspend API with Upstart 1.11.
On Feb 16, 2014 5:44 AM, "Tobias Arrskog" notifications@github.com wrote:

Sorry for slow response from me, I've been sick so haven't touched the
computer in a while :)
Anyways, so from what I understand the simplest solution would be to check
for systemd and if version is > 183?


Reply to this email directly or view it on GitHubhttps://github.com/xbmc/xbmc/pull/4168#issuecomment-35182028
.

Member
topfs2 commented Feb 16, 2014

Isch I just wished they'd stop altering the APIs every 6 months :) so I
guess that's the way then? Check if systemd has version or if upstart is
available and is version 11?
Den 16 feb 2014 14:30 skrev "Joseph A. Yasi" notifications@github.com:

Unfortunately, that's not the entire solution because of systems without
systemd as PID 1. Ubuntu doesn't implement the systemd version API but
does
support suspend via logind. KDE checks for the systemd version. I'd the
dbus call fails, it checks the upstart version since Ubuntu has promised
to
support the logind suspend API with Upstart 1.11.
On Feb 16, 2014 5:44 AM, "Tobias Arrskog" notifications@github.com
wrote:

Sorry for slow response from me, I've been sick so haven't touched the
computer in a while :)
Anyways, so from what I understand the simplest solution would be to
check
for systemd and if version is > 183?

Reply to this email directly or view it on GitHub<
https://github.com/xbmc/xbmc/pull/4168#issuecomment-35182028>
.

Reply to this email directly or view it on GitHubhttps://github.com/xbmc/xbmc/pull/4168#issuecomment-35196685
.

Contributor
yasij commented Feb 16, 2014

Yes, it's a real mess with the partial implementation of different APIs
going on. Now that Debian and Ubuntu are going to switch over to systemd,
it should get a lot cleaner.

Checking the systemd version, then checking the upstart version is the
correct thing to do. The dbus calls are in the KDE function I mentioned
upthread for that check. KDE's comment says as of Upstart 1.11, but is
actually checking Upstart >= 1.1. There is probably a bug there.

On Sun, Feb 16, 2014 at 9:29 AM, Tobias Arrskog notifications@github.comwrote:

Isch I just wished they'd stop altering the APIs every 6 months :) so I
guess that's the way then? Check if systemd has version or if upstart is
available and is version 11?
Den 16 feb 2014 14:30 skrev "Joseph A. Yasi" notifications@github.com:

Unfortunately, that's not the entire solution because of systems without
systemd as PID 1. Ubuntu doesn't implement the systemd version API but
does
support suspend via logind. KDE checks for the systemd version. I'd the
dbus call fails, it checks the upstart version since Ubuntu has promised
to
support the logind suspend API with Upstart 1.11.
On Feb 16, 2014 5:44 AM, "Tobias Arrskog" notifications@github.com
wrote:

Sorry for slow response from me, I've been sick so haven't touched the
computer in a while :)
Anyways, so from what I understand the simplest solution would be to
check
for systemd and if version is > 183?

Reply to this email directly or view it on GitHub<
https://github.com/xbmc/xbmc/pull/4168#issuecomment-35182028>
.

Reply to this email directly or view it on GitHub<
https://github.com/xbmc/xbmc/pull/4168#issuecomment-35196685>

.


Reply to this email directly or view it on GitHubhttps://github.com/xbmc/xbmc/pull/4168#issuecomment-35197977
.

Shine and others added some commits Feb 18, 2014
Shine Prevent unnecessary colorspace conversions 0369b8c
Jonathan Marshall [settings] implement OnSettingsUnloaded, fixes various advancedsettin…
…gs leaking from profile to profile (e.g. mysql database config)
20fcc00
Jonathan Marshall [settings] switch zeroconf setting to standard 2e6fe26
@jmarshallnz jmarshallnz Merge pull request #4254 from jmarshallnz/onsettingsunloaded
Implements OnSettingsUnloaded
2547096
@jmarshallnz jmarshallnz Merge pull request #4192 from Shine-/dxvarenderer
Prevent unnecessary colorspace conversions (regression in #4163)
62d6cff
@jmarshallnz jmarshallnz Merge pull request #4057 from Karlson2k/win32_fix_nonutf8
[win32] fix ANSI used as UTF-8
99461eb
@jmarshallnz jmarshallnz Merge pull request #4259 from phate89/master
Allow xbmc to handle cue tags without quotes
090710f
Jonathan Marshall [gui] scrollbar didn't scroll properly with the mouse. fixes #14873 f3530bd
Jonathan Marshall [addons] ensure we reload the current skin if it's updated via a .zip…
… file. fixes #14890
7f2dda3
@MartijnKaijser MartijnKaijser [language] fix inconsistent add-on state description. fixes #14851 d688682
@Memphiz Memphiz Merge pull request #4261 from jmarshallnz/zeroconf_level
[settings] switch zeroconf setting to standard level
d4c0cd9
@t-nelson t-nelson Merge pull request #4007 from wsnipex/crashlog
[linux] allow override of crashlog directory
7926079
@t-nelson t-nelson Merge pull request #3640 from wsnipex/joystick-fix
Input: fix Accelerometer being detected as joystick
7035048
ronie [Confluence] don't crop 'local subs..' label 3c0d901
Member

24023 is also used as an error message (triggered from content dialog) on disabled scrapers. This now doesn't make as much sense as all you get is "Disabled" "Name of scraper". A better solution would be to add a new label with the same context string you've used here.

hmm thought it was only used in the manager

@yasij yasij closed this Feb 24, 2014
@yasij yasij deleted the yasij:logind_default_suspend branch Feb 24, 2014
Contributor
yasij commented Feb 24, 2014

Superceded by pull request #4273.

@yasij yasij reopened this Feb 24, 2014
@yasij yasij closed this Feb 24, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment