Suspend only works once when using upower with logind -- s-shim needs to call /lib/systemd/system-sleep/*

Bug #1196752 reported by Joseph Yasi on 2013-07-01
60
This bug affects 12 people
Affects Status Importance Assigned to Milestone
systemd (Ubuntu)
Medium
Unassigned
systemd-shim (Ubuntu)
Medium
Allison Lortie
upower (Debian)
Fix Released
Unknown
upower (Ubuntu)
Medium
Martin Pitt

Bug Description

On saucy with upower 0.9.20-1ubuntu2, suspending via UPower (or the KDE suspend menu) only works once:
dbus-send --print-reply --system --dest=org.freedesktop.UPower /org/freedesktop/UPower org.freedesktop.UPower.Suspend

and resuming works the first time. However, calling it again after resume returns:
"Sleep has already been requested and is pending"

Sleeping directly via logind works every time:
dbus-send --print-reply --system --dest=org.freedesktop.login1 /org/freedesktop/login1 org.freedesktop.login1.Manager.Suspend boolean:true

Returning FALSE for the LOGIND_AVAILABLE() case in gboolean up_backend_emits_resuming (UpBackend *backend) in src/linux/up-backend.c allows suspending multiple times via UPower (or the KDE suspend menu). UPower is somehow not getting the resume signal from logind.

Related branches

Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in upower (Ubuntu):
status: New → Confirmed
Philip Muškovac (yofel) wrote :

From what I see upower does the correct thing and calls logind in the right way when it sees that it's running. But then logind doesn't actually complete the suspend operation.

gdbus call --system --dest org.freedesktop.login1 --object-path /org/freedesktop/login1 --method org.freedesktop.login1.Manager.Suspend "true"

Try #1 succeeds, takes down services like network-manager etc. but doesn't acutally suspend.
Trying again results in:
Error: GDBus.Error:org.freedesktop.DBus.Error.ServiceUnknown: The name org.freedesktop.systemd1 was not provided by any .service files
(According to introspection data, you need to pass 'b')

so it seems to depend on systemd running to actually suspend?

affects: upower (Ubuntu) → systemd (Ubuntu)
tags: added: saucy
Martin Pitt (pitti) wrote :

Do you have systemd-shim installed? It's needed for proper suspending. It's recommended by systemd-services and also a strict dependency of indicator-datetime.

Changed in systemd (Ubuntu):
status: Confirmed → Incomplete
Joseph Yasi (joe-yasi) wrote :

Yes, systemd-shim is installed. The problem is UPower is not getting the resume signal from logind, so suspend gets blocked the 2nd time.

Philip Muškovac (yofel) wrote :

Ok, ignore my systemd1 issue, that was indeed systemd-shim missing which I'll file a seperate bug for.
Suspend still only works once so the initial issue remains

Changed in systemd (Ubuntu):
status: Incomplete → Confirmed
Martin Pitt (pitti) wrote :

The first org.freedesktop.UPower.Suspend call indeed never finishes (you get a D-BUS timeout), apparently because it never gets a "resuming" signal.

Changed in systemd (Ubuntu):
status: Confirmed → Triaged
Jonathan Riddell (jr) on 2013-07-15
tags: added: kubuntu
Changed in systemd (Ubuntu):
milestone: none → ubuntu-13.10
Joseph Yasi (joe-yasi) wrote :

Interesting. The D-BUS timeout for the org.freedesktop.UPower.Suspend is new with the latest update to systemd-logind. I did not get a D-BUS timeout before.

Joseph Yasi (joe-yasi) wrote :

So Powerdevil in KDE is supposed to call systemd-logind now instead of UPower if it finds it. However, it is checking for the systemd version with a call to org.freedesktop.systemd1.Manager.VErsion and not geting a valid response. For some reason, that method doesn't exist on my system.
gdbus introspect --system --dest org.freedesktop.systemd1 --object-path /org/freedesktop/systemd1
node /org/freedesktop/systemd1 {
  interface org.freedesktop.DBus.Properties {
    methods:
      Get(in s interface_name,
          in s property_name,
          out v value);
      GetAll(in s interface_name,
             out a{sv} properties);
      Set(in s interface_name,
          in s property_name,
          in v value);
    signals:
      PropertiesChanged(s interface_name,
                        a{sv} changed_properties,
                        as invalidated_properties);
    properties:
  };
  interface org.freedesktop.DBus.Introspectable {
    methods:
      Introspect(out s xml_data);
    signals:
    properties:
  };
  interface org.freedesktop.DBus.Peer {
    methods:
      Ping();
      GetMachineId(out s machine_uuid);
    signals:
    properties:
  };
  interface org.freedesktop.systemd1.Manager {
    methods:
      GetUnitFileState(in s file,
                       out s state);
      DisableUnitFiles(in as files,
                       in b runtime,
                       out a(sss) changes);
      EnableUnitFiles(in as files,
                      in b runtime,
                      in b force,
                      out b carries_install_info,
                      out a(sss) changes);
      Reload();
      StartUnit(in s name,
                in s mode,
                out o job);
      StopUnit(in s name,
               in s mode,
               out o job);
      Reload();
    signals:
    properties:
      readonly s Virtualization = '';
  };
};

Martin Pitt (pitti) wrote :

> However, it is checking for the systemd version with a call to org.freedesktop.systemd1.Manager.VErsion and not geting a valid response. For some reason, that method doesn't exist on my system.

That would be because that's not systemd at pid1, but our systemd-shim which just doesn't have this method. Perhaps we should add it, perhaps we shouldn't because it might lure programs into thinking that systemd would actually be pid 1.

Albert Astals Cid (aacid) wrote :

That's a weird answer, if you don't want people thinking we have systemd, should we just simply not exposing org.freedesktop.systemd1 ?

Albert Astals Cid [2013-07-23 7:29 -0000]:
> That's a weird answer, if you don't want people thinking we have
> systemd, should we just simply not exposing org.freedesktop.systemd1 ?

Admittedly it's a weird answer, because we have a weird setup -- we
need some parts of org.freedesktop.systemd1 for e. g. NTP or suspend
to work. We can certainly add the version API, if logind consumers
call this.

Changed in systemd-shim (Ubuntu):
assignee: nobody → Ryan Lortie (desrt)

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in systemd-shim (Ubuntu):
status: New → Confirmed
Martin Pitt (pitti) wrote :

So it seems part of the problem is that upower forgets to install /lib/systemd/system-sleep/notify-upower.sh. I just fixed that in the packaging git. However, systemd-shim does not call these scripts, this needs to happen until this can work a second time.

Changed in upower (Ubuntu):
assignee: nobody → Martin Pitt (pitti)
status: New → Fix Committed
summary: - Suspend only works once when using upower with logind
+ Suspend only works once when using upower with logind -- s-shim needs to
+ call /lib/systemd/system-sleep/*
Changed in upower (Debian):
status: Unknown → New
Changed in upower (Debian):
status: New → Fix Released
Martin Pitt (pitti) wrote :

Let's fix that in upower.

Changed in systemd (Ubuntu):
milestone: ubuntu-13.10 → none
status: Triaged → Invalid
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package upower - 0.9.21-3

---------------
upower (0.9.21-3) unstable; urgency=low

  * Add 00git_updates.patch to update to today's upstream git:
    - Fix batteries which report capacity, but not energy/charge
    - Add temperature property for batteries
    - Fix warning in up_device_supply_get_design_voltage
    - Make GetHistory() array order consistent

upower (0.9.21-2) unstable; urgency=low

  [ Martin Pitt ]
  * debian/rules: Run dh_install with --fail-missing to avoid forgetting about
    some scripts.
  * debian/rules: Explicitly set systemd util dir, to avoid build-depending on
    systemd.
  * debian/upower.install: Install notify-upower.sh. (LP: #1196752)

  [ Michael Biebl ]
  * debian/patches/always_use_pm-utils_backend.patch: Always use the pm-utils
    backend, even if logind is running. The availability of logind does not
    necessarily imply, that systemd is running (in a recent enough version),
    which is required for a successful suspend/hibernate.
    (Closes: #718458, LP: #1196752)
 -- Martin Pitt <email address hidden> Thu, 29 Aug 2013 11:34:28 +0200

Changed in upower (Ubuntu):
status: Fix Committed → Fix Released
Joseph Yasi (joe-yasi) wrote :

Are there still plans to add the version API to systemd-shim?

Allison Lortie (desrt) wrote :

No. I talked about this with Martin today and we both agree that it's not really worth it for supporting a deprecated API that we will remove soon anyway.

Joseph Yasi (joe-yasi) wrote :

Which deprecated API? upower suspend? The version API is needed for KDE to correctly use the systemd logind suspend system instead of upower.

Joseph Yasi (joe-yasi) wrote :

I have a hack of a patch which implements reading the systemd version string in systemd-shim. It just calls systemd-udevd --version and reports that.

The attachment "Implements version API in systemd-shim" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

tags: added: patch

Joseph Yasi [2013-08-29 19:54 -0000]:
> Which deprecated API? upower suspend?

Yes.

> The version API is needed for KDE to correctly use the systemd
> logind suspend system instead of upower.

Ah thanks for pointing out. Then yes, let's add the version API to
-shim then.

Michael Biebl (mbiebl) wrote :

Do you think it's a good idea if systemd-shim "lies" about the version number? It's not like it is implementing the full systemd (D-Bus) API.
Somehow I have the feeling it would be better/cleaner if systemd-shim would export its own version number (maybe with distinct prefix) so clients can find out if they are talking to systemd proper or systemd-shim.

Joseph Yasi (joe-yasi) wrote :

I guess it depends on how user programs are currently using the systemd version API to check things. KDE PowerDevil strips everything before the space and just compares against the number, so in principle, the shim could use the prefix string to say something. I don't know how others use the API.

KDE PowerDevil usage is here:
https://projects.kde.org/projects/kde/kde-workspace/repository/revisions/master/entry/powerdevil/daemon/backends/upower/powerdevilupowerbackend.cpp#L40

A cleaner approach to handling this would be an API for advertising implemented features so programs aren't checking the version to see if something is supported. Instead, they check if a feature they require is supported. It's the best way if you're just going to implement part of systemd, but you'd have to convince upstream to implement such an API.

Martin Pitt (pitti) wrote :

Joseph Yasi [2013-08-30 18:50 -0000]:
> A cleaner approach to handling this would be an API for advertising
> implemented features so programs aren't checking the version to see if
> something is supported. Instead, they check if a feature they require is
> supported. It's the best way if you're just going to implement part of
> systemd, but you'd have to convince upstream to implement such an API.

I'm 99% sure that won't happen upstream. Our current cannibalization
of logind is scary enough for them :-)

Ubuntu QA Website (ubuntuqa) wrote :

This bug has been reported on the Ubuntu ISO testing tracker.

A list of all reports related to this bug can be found here:
http://iso.qa.ubuntu.com/qatracker/reports/bugs/1196752

tags: added: iso-testing
Changed in systemd-shim (Ubuntu):
importance: Undecided → Medium
Changed in systemd (Ubuntu):
importance: Undecided → Medium
Changed in upower (Ubuntu):
importance: Undecided → Medium
Changed in systemd-shim (Ubuntu):
status: Confirmed → In Progress
status: In Progress → Triaged
Joseph Yasi (joe-yasi) wrote :

KDE PowerDevil now checks for Upstart if the systemd version check fails. The version API implementation is no longer needed. The call to the sleep scripts still needs to happen.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.