Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 677062 - sys-auth/elogind-239.3 (with LVM) breaks hibernation - elogind-daemon: Failed to write hibernation disk offset: Invalid argument
Summary: sys-auth/elogind-239.3 (with LVM) breaks hibernation - elogind-daemon: Failed...
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Stabilization (show other bugs)
Hardware: All Linux
: Normal normal
Assignee: Andreas Sturmlechner
URL: https://github.com/elogind/elogind/is...
Whiteboard:
Keywords: STABLEREQ
Depends on: 682158
Blocks:
  Show dependency tree
 
Reported: 2019-02-01 18:29 UTC by Herbert Wantesh
Modified: 2019-04-01 21:45 UTC (History)
4 users (show)

See Also:
Package list:
sys-auth/elogind-239.4
Runtime testing required: ---
stable-bot: sanity-check+


Attachments
this patch fixes the bug (fix.patch,1.23 KB, patch)
2019-02-02 20:37 UTC, Herbert Wantesh
no flags Details | Diff
codestyle++ (as,1.59 KB, patch)
2019-02-04 21:36 UTC, wranz
no flags Details | Diff
fix typo (as,1.59 KB, patch)
2019-02-04 21:51 UTC, wranz
no flags Details | Diff
use snprintf (hibernate.patch,1.62 KB, patch)
2019-02-10 21:15 UTC, wranz
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Herbert Wantesh 2019-02-01 18:29:18 UTC
hibernating from plasma works with elogind 238.3 works.

But when i try to hibernate with elogind 239.3 i get:

elogind-daemon[3500]: Failed to write hibernation disk offset: Invalid argument
elogind-daemon[3500]: Error during inhibitor-delayed operation (already returned success to client): Invalid argument

btw. "echo disk > /sys/power/state" also works
Comment 1 Andreas Sturmlechner gentoo-dev 2019-02-01 19:15:54 UTC
Works fine for me fwiw.
Comment 2 Herbert Wantesh 2019-02-01 19:17:18 UTC
i currently use lvm inside a luks container and the swap partition is also a lvm partitio inside the luks container
Comment 3 Herbert Wantesh 2019-02-02 19:43:43 UTC
the problem is lvm

in the src/sleep/sleep.c

        /* if it's a swap partition, we just write the disk to /sys/power/resume */
        if (streq(type, "partition"))
                return write_string_file("/sys/power/resume", device, 0);

the code tries to write the lvm swap name to /sys/power/resume

in my case device holds /dev/mapper/lm-swap

trying to the same in a shell outputs

# echo /dev/mapper/lm-swap > /sys/power/resume
bash: echo: write error: Invalid argument

the correct way would be to get the major and minor id of the device and write this into the file, the bash command would be

printf '%u:%u\n' $(busybox stat -L -c '0x%t 0x%T' "/dev/mapper/lm-swap")

which doesn't through the error.
Comment 4 Herbert Wantesh 2019-02-02 20:37:30 UTC
Created attachment 563544 [details, diff]
this patch fixes the bug
Comment 5 wranz 2019-02-04 21:36:54 UTC
Created attachment 563760 [details, diff]
codestyle++

codestyle++
Comment 6 wranz 2019-02-04 21:51:26 UTC
Created attachment 563762 [details, diff]
fix typo
Comment 7 wranz 2019-02-10 21:15:32 UTC
Created attachment 564618 [details, diff]
use snprintf
Comment 8 Sven Eden 2019-02-13 21:03:11 UTC
I have just finished migrating toward systemd-240 (over 1000 commits!) and will have time to test this patch tomorrow. If it does not have any impact on "normal" operation, I do not see any reason for not to include it in both 239.4 and 240.1.
Comment 9 Sven Eden 2019-02-14 16:55:11 UTC
I have adapted the patch a bit and pushed it to the elogind v239-stable branch. It does not effect my setup, hibernation still works.

Can you please try it out whether it fixes the issue for you?

You can install the live version of the v239-stable branch using my life ebuild for sys-auth/elogind-239.9999 from my overlay "seden", available via layman.

Thanks in advance!
Comment 10 Herbert Wantesh 2019-02-14 23:24:22 UTC
but why do you try to write the device first and if a failure happens the major:minor value?

this code is used the convert the value https://github.com/torvalds/linux/blob/master/init/do_mounts.c#L221 and the code natively handles https://github.com/torvalds/linux/blob/master/init/do_mounts.c#L253 major minor, so there is no need to write both values
Comment 11 Sven Eden 2019-02-15 07:24:43 UTC
Because it works for everybody who does need hibernation and does not have swap in LVM. Which is the vast majority of people.

I know it is completely okay to write major:minor to that file. But can you guarantee for all setups that stat() will always be fast (and successful) for every single setup out there? And thus justifying to add another system call for the vast majority of setups which simply do not need it?

Yes, it should be harmless. stat() doesn't even need any privileges on the file/device that is checked. But still it is more code for everyone, solving an issue for a tiny fraction.
Comment 12 Sven Eden 2019-02-15 07:25:14 UTC
(and I deliberately ignored the additional snprintf() here...)
Comment 13 Herbert Wantesh 2019-02-16 12:37:56 UTC
i still think it's not necessary to write both values but i tested it and i can confirm it works
Comment 14 Sven Eden 2019-02-16 13:32:52 UTC
(In reply to Herbert Wantesh from comment #13)
> i still think it's not necessary to write both values but i tested it and i
> can confirm it works

Thank you very much!

Basically I concur. I will do some more research, you said 238 worked without issues, and will adapt if necessary. For now this change will do, but nothing is carved in stone. ;-)

@Andreas: Fixed in https://github.com/elogind/elogind/commit/29fc24ebd6513f9aa14b8292e73f5217f91a99b1
Comment 15 Andreas Sturmlechner gentoo-dev 2019-02-16 13:39:34 UTC
thanks Sven, this will be in 239.4?
Comment 16 Herbert Wantesh 2019-02-20 19:09:14 UTC
< Basically I concur. I will do some more research

I thought about it, /sys/power/resume will only take devices from /dev and every device in /dev has a major:minor id so there is no reason to paste the /dev name and if it doesn't work paste the major:minor.
Comment 17 Andreas Sturmlechner gentoo-dev 2019-03-26 15:01:38 UTC
Arches, please stabilise.
Comment 18 Mikle Kolyada (RETIRED) archtester Gentoo Infrastructure gentoo-dev Security 2019-03-27 08:21:52 UTC
amd64 stable
Comment 19 Andreas Sturmlechner gentoo-dev 2019-03-31 18:48:35 UTC
Stabilisation superseded by bug 682158.
Comment 20 Larry the Git Cow gentoo-dev 2019-04-01 21:45:29 UTC
The bug has been closed via the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=29a3948ef6f55a500b4549a276fe5ca7c28f1173

commit 29a3948ef6f55a500b4549a276fe5ca7c28f1173
Author:     Andreas Sturmlechner <asturm@gentoo.org>
AuthorDate: 2019-04-01 21:36:41 +0000
Commit:     Andreas Sturmlechner <asturm@gentoo.org>
CommitDate: 2019-04-01 21:45:09 +0000

    sys-auth/elogind: Drop 239.3
    
    Closes: https://bugs.gentoo.org/677062
    Package-Manager: Portage-2.3.62, Repoman-2.3.12
    Signed-off-by: Andreas Sturmlechner <asturm@gentoo.org>

 sys-auth/elogind/Manifest             |   1 -
 sys-auth/elogind/elogind-239.3.ebuild | 129 ----------------------------------
 2 files changed, 130 deletions(-)