Bug 1332266

Summary: systemd-logind does not verify if system has resume device defined when checking if can.hibernate or can.hybridsleep
Product: [Fedora] Fedora Reporter: James Hogarth <james.hogarth>
Component: systemdAssignee: Zbigniew Jędrzejewski-Szmek <zbyszek>
Status: CLOSED EOL QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 25CC: bugzilla, gmarr, james.hogarth, johannbg, kparal, lnykryn, mcatanzaro+wrong-account-do-not-cc, msekleta, muadda, rhughes, robatino, s, systemd-maint, thib, zbyszek
Target Milestone: ---Keywords: Reopened
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: RejectedBlocker AcceptedFreezeException
Fixed In Version: systemd-229-8.fc24 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-12-12 10:47:31 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1277290    
Attachments:
Description Flags
System will hibernate soon critical battery notification
none
Generic low battery message
none
System will shutdown soon critical battery notification
none
F23 check resume= patch
none
F24/25 check resume= patch
none
Upstream check resume= patch
none
Upstream check resume= patch
none
F24/25 check resume= patch
none
F23 check resume= patch
none
simplified version of the F24/rawhide patch none

Description James Hogarth 2016-05-02 17:24:14 UTC
Description of problem:
Due to bz1206936 no resume from hibernate is presently attempted in a standard Fedora install. Consequently though the hibernation successfully occurs, with the resume not even attempted any application that is open effectively had the power cord pulled on it, albeit after a sync, so any TERM cleanup (such as autosaving) that may happen does not get called. Worse than this the user is notified of a hibernation event about to happen which would imply data would be suspended to disk and resumed, rather than highlighting the impending data loss if anything open is not saved.

How reproducible:
Always

Steps to Reproduce:
1. Have documents open
2. Wait for a battery critcal event (or use systemctl to trigger a hibernation)
3. Turn back on the system to witness a fresh new session

Actual results:
User is notified of impending hibernation. Hibernation action suspends to disk correctly. No resume is attempted. A new session to log into with no restoration of data is witnessed.

Expected results:
User gets notified of impending shutdown so had a chance to save documents. Normal shutdown gets called on critical battery level.

Additional info:
Adjusting the critical action in /etc/Upower/Upower.conf from the known "broken" HybridSleep to Shutdown is sufficient to solve the issue to the scope of this bug.

Comment 1 Fedora Blocker Bugs Application 2016-05-02 17:29:34 UTC
Proposed as a Blocker for 24-final by Fedora user jhogarth using the blocker tracking app because:

 Since 1206936 was rejected the present configuration in Fedora results in data loss, which would match the final blocker criteria. 

As referenced in the meeting logs from today this bug is scoped specifically to the critical battery action in Upower to avoid the user being notified of a hibernation event, and then applications not having any resume attempt so effectively having the power cord pulled (albeit after a sync) without being sent a TERM to handle any cleanup, and without notifying the user of the impending data loss event.

Comment 2 Geoffrey Marr 2016-05-09 19:36:05 UTC
Discussed during the 2016-05-09 blocker review meeting: [1]

Decision was made to delay the classification of this as a bug until the exact message that is displayed when this occurs is known; this will be discussed at next week's blocker review meeting. Additional testing will also occur between now and then.


[1] https://meetbot-raw.fedoraproject.org/fedora-blocker-review/2016-05-09/f24-blocker-review.2016-05-09-16.00.txt

Comment 3 Kamil Páral 2016-05-10 08:26:31 UTC
James, can you please attach screenshots of the messages that are displayed to the user when battery gets critical and "the user is notified of impending hibernation"? Thanks.

Comment 4 James Hogarth 2016-05-10 09:26:53 UTC
I'll endeavour to do so and clear the needinfo when I have.

I'm sure you appreciate it's 2-3 hours from fully charged to the critical message, and a short window of opportunity to screenshot it.

Plus I have realwork™ that needs doing whilst I drain my battery multiple times ;)

Comment 5 James Hogarth 2016-05-10 10:52:14 UTC
Whilst waiting on my battery to drain here's a little bit of investigation into the message...

gnome-settings-daemon monitors dbus via the power plugin for events from Upower

when the low battery situation event gets put on dbus the low battery notification gets raised with a rough estimate of battery remaining:

https://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/power/gsd-power-manager.c#n437

when the critical battery event gets put on dbus then the critical notification gets raised with the action to be carried out defining the message:

https://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/power/gsd-power-manager.c#n711

The decision of shutdown or hibernate comes from upower via https://cgit.freedesktop.org/upower/tree/libupower-glib/up-client.c#n153

If the function returns PowerOff then shutdown will be selected, otherwise the policy used is GSD_POWER_ACTION_HIBERNATE when determining the message to be issued.

https://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/power/gsd-power-manager.c#n568

In order to determine this upower is asked what the critical action is.

This is determined by up_backend_get_critical_action:

https://cgit.freedesktop.org/upower/tree/src/linux/up-backend.c#n386

The configured action is checked and via dbus logind is queried.

This is ultimately decided by logind with method_can_shutdown_or_sleep

https://github.com/systemd/systemd/blob/master/src/login/logind-dbus.c#L2118

Which works it out via can_sleep:
https://github.com/systemd/systemd/blob/master/src/shared/sleep-config.c#L262

This queries the kernel via /sys/power/state and /sys/power/disk to check what is permitted from there and does a few additional checks (swap partition present for hibernation use and memory in use versus swap size etc).

==========================
SUMMARY OF FINDINGS ABOVE:

  * gnome-settings-daemon listens to upower updates on dbus
  * on critical battery gnome-settings-daemon asks upower what to do (default we ship is HybridSleep)
  * upower asks logind what's permitted out of HybridSleep, Hibernate and PowerOff
  * logind asks the kernel (via /sys/power/*) and does some additonal sanity checks
  * if the response is *not* PowerOff as the critical action then the notification explicitly says it'll hibernate, if it's PowerOff then the notification will say shutdown
  * after a bit of time without power being supplied the critical action is called via dbus to logind

Based on this investigation this specific blocker proposal could be fixed either:

1) with the Upower.conf change, even though it says this should be configured by the distribution and not the user so this would effectively result in the Fedora Project deciding it's better to shutdown than attempt to hibernate on critical battery... which is possibly a valid position to take

2) or systemd adding in to the can_sleep() heuristics a check for resume= on /proc/cmdline so that given the present situation hibernate would not be permitted if the system didn't define where to get the hibernation image form on next start. In this event the upowerd would be notified that the default option of HybridSleep was not available and instead fall back to PowerOff with the appropriate notification. 

As for the needinfo ... I managed to get screenshots so they will be incoming shortly.

Comment 6 James Hogarth 2016-05-10 11:01:27 UTC
Created attachment 1155678 [details]
System will hibernate soon critical battery notification

This is with the default shipped Upower.conf configuration in place

Comment 7 James Hogarth 2016-05-10 11:02:36 UTC
Created attachment 1155679 [details]
Generic low battery message

On low battery the estimated time remaining is shown, but no action is mentioned about what will happen (this is at ~20% remaining and before the critical warning).

Comment 8 James Hogarth 2016-05-10 11:05:56 UTC
Created attachment 1155680 [details]
System will shutdown soon critical battery notification

With Upower.conf changed to have the CriticalPowerAction set to PowerOff the notification states that a shutdown will occur, not a hibernation.

As such the user would expect data to be lost unless saved (or SIGTERM activities carrying out autosave stuff in a cleanup function), contrary to hibernation where if successful the user would expect the session to be resumed with any application open (and pending data) being safe.

Comment 9 James Hogarth 2016-05-11 22:05:35 UTC
After spending the past day pondering this in the back of my mind I think I don't like option 1.

When a solution is found for bz1206936 (hopefully anaconda agreeing to add the argument defined in the kernel docs) that would necessitate either the user modifying a file that explicitly states "do not modify" at the top or UPower reverting the config change in order to enable hibernate on critical again.

With that in mind I have patches for systemd I've tested on my laptop (F23) which checks if resume= is in the kernel cmdline and if it points to a valid block device. This seems to make sense to do the check here given it's systemd that uses the hibernate generator to write the resume image to the kernel on start, and these are the criteria that need to match for that to work.

If these criteria are not met then the request to logind to see if hibernate/hybridsleep is permitted will result in 'na' and a shutdown will be notified and actioned on a critical battery event instead.

Would the systemd maintainers mind reviewing the attached patches?

The functions were moved between headers between F23 and upstream which results in the different #include needed and the slightly different patches.

I've tested a valid resume block device, a non existent device and no resume=

On the first hibernate and resume works on my laptop fine.

On the latter two logind reports hibernate and hybrid sleep na and on draining the battery the shutdown, rather than hibernate, notification was displayed.

The check of logond below was made by:
gdbus call --system --dest org.freedesktop.login1 --object-path /org/freedesktop/login1 --method org.freedesktop.login1.Manager.CanHibernate
gdbus call --system --dest org.freedesktop.login1 --object-path /org/freedesktop/login1 --method org.freedesktop.login1.Manager.CanHybridSleep
gdbus call --system --dest org.freedesktop.login1 --object-path /org/freedesktop/login1 --method org.freedesktop.login1.Manager.CanSuspend


==========
no resume=
can hibernate?
('na',)
can hybridsleep?
('na',)
can suspend?
('yes',)
==========
invalid device in resume=
can hibernate?
('na',)
can hybridsleep?
('na',)
can suspend?
('yes',)
==========
valid device in resume=
can hibernate?
('yes',)
can hybridsleep?
('yes',)
can suspend?
('yes',)

Comment 10 James Hogarth 2016-05-11 22:06:24 UTC
Created attachment 1156318 [details]
F23 check resume= patch

Comment 11 James Hogarth 2016-05-11 22:06:59 UTC
Created attachment 1156319 [details]
F24/25 check resume= patch

Comment 12 James Hogarth 2016-05-11 22:07:29 UTC
Created attachment 1156320 [details]
Upstream check resume= patch

Comment 13 Geoffrey Marr 2016-05-16 18:11:03 UTC
Discussed during the 2016-05-16 blocker review meeting: [1]

The decision was made to discuss this bug with other teams before issuing this bug a classification (accepted/rejected).


[1] https://meetbot-raw.fedoraproject.org/fedora-blocker-review/2016-05-16/f24-blocker-review.2016-05-16-16.00.txt

Comment 15 Michael Catanzaro 2016-05-17 11:30:24 UTC
+1 blocker. Thanks for providing patches, James.

So, this has come up recently but I don't remember seeing answers: why is resume= not in the kernel command line?

Comment 16 Kamil Páral 2016-05-17 13:23:01 UTC
(In reply to Michael Catanzaro from comment #15)
> So, this has come up recently but I don't remember seeing answers: why is
> resume= not in the kernel command line?

Michael, see bug 1206936. There seems to be a disagreement how to handle it and no group wants to push this forward to get it resolved.

Comment 17 James Hogarth 2016-05-17 13:35:26 UTC
Pull request opened upstream[1] with the patch amended to include an configuration option to disable the resume= validation check for distros not using the systemd hibernate generator.

I'll update the patches here in a moment to match the pull request for consistency too.

@Michel and Kamil:

Indeed and whilst the debate is held on who should provide resume= (dracut somehow, systemd do some probing or anaconda - my thoughts being the last the correct place given kernel docs) it seems sane to either have Upower provide a config for the better SIGTERM -> poweroff behaviour or to have systemd do a sanity check of the kernel commandline when deciding if a system is permitted to hibernate.

[1] https://github.com/systemd/systemd/pull/3278

Comment 18 James Hogarth 2016-05-17 14:10:43 UTC
Created attachment 1158310 [details]
Upstream check resume= patch

This the same as in the pull request

Comment 19 James Hogarth 2016-05-17 14:11:38 UTC
Created attachment 1158311 [details]
F24/25 check resume= patch

This is the patch adapted for F24, rawhide builds are presently failing so I couldn't test the patch on that.

Comment 20 James Hogarth 2016-05-17 14:29:07 UTC
Created attachment 1158328 [details]
F23 check resume= patch

The upstream PR adapted to the systemd build in F23

Comment 21 James Hogarth 2016-05-17 14:35:04 UTC
Reading through the blocker meeting log this stands out to me:

> <cmurf> 1. is this a feature request or a bug? I think for systemd as suggested in the bug report, it's a feature request and thus not a blocker.

I'd consider not carrying out a sanity check for the hibernation method shipped (systemd hibernate generator) and declaring a system CanHibernate when there's 0% probability of resume working since the generator isn't triggered to be a bug, rather than this being a feature request.

Still hopefully a systemd maintainer can weigh in upstream and on the prosposed patches here.

Comment 22 Chris Murphy 2016-05-17 16:05:15 UTC
It might be at least as much bug as feature request, but my hesitation is whether it's a blocker and on what basis it blocks. I've found worse data loss bugs than this that have been considered not release blocking even though UI literally entices the user into a data loss scenario.

1. Blocking on a Fedora specific systemd-* path to mask a Fedora specific Anaconda deficiency seems confusing. systemd folks consider resume= to be required.[1] Most other distros use it, rather than depend on dracut.

So it's not unreasonable to assume a required thing is actually present rather than having to go see if it's really there *and* is correct.

2. Why is it blocking to check if resume= is present, but it's not blocking to check if it's pointing to something that's valid? i.e. does it exist, is it formatted, does it have the correct partition type?

I'm not against fixing this, even blocking on it. I would like to hear what systemd folks think of it being done this way though.

[1]
https://lists.freedesktop.org/archives/systemd-devel/2016-April/036353.html

Comment 23 James Hogarth 2016-05-17 16:57:45 UTC
(In reply to Chris Murphy from comment #22)
> It might be at least as much bug as feature request, but my hesitation is
> whether it's a blocker and on what basis it blocks. I've found worse data
> loss bugs than this that have been considered not release blocking even
> though UI literally entices the user into a data loss scenario.
> 

In that case I'd argue that we should have blocked before, going by the criteria, especially if there were patches waiting that would have fixed a known root cause for them. And previous behaviour where we can improve on shouldn't be the basis for current.


> 1. Blocking on a Fedora specific systemd-* path to mask a Fedora specific
> Anaconda deficiency seems confusing. systemd folks consider resume= to be
> required.[1] Most other distros use it, rather than depend on dracut.
> 
> So it's not unreasonable to assume a required thing is actually present
> rather than having to go see if it's really there *and* is correct.
> 

Ignore the anaconda question for the time being. I suspect that's one that will take a while to play out and is less valid for blocker/FE criteria for the previous reasons discussed.

Let's just focus with this on what the UI tells the user and how to make that smarter.

> 2. Why is it blocking to check if resume= is present, but it's not blocking
> to check if it's pointing to something that's valid? i.e. does it exist, is
> it formatted, does it have the correct partition type?
> 

At my workplace we have the concept of "minimal viable product" to get a task itself to complete and avoid scope creep... With a view to iterating on it and improving in future.

You'll see in the patch I already check if the resume= points to a block device. The existing code already checked size of it. There's no reason we can't iterate with further improvements but I'd consider that scope creep and outside of the context of this specific bug.


> I'm not against fixing this, even blocking on it. I would like to hear what
> systemd folks think of it being done this way though.
> 
> [1]
> https://lists.freedesktop.org/archives/systemd-devel/2016-April/036353.html

Right I agree I definitely want their input as I've stated twice. I've also asked on IRC, issued a pull request and emailed privately. Not much more I can do on that front to try and get their input.

Hopefully one of the team are able to check in and respond here before the next blocker review meeting.

Comment 24 Paul W. Frields 2016-05-20 19:09:40 UTC
As Adam pointed out in https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/7XQ3PPESMEL22ZU6KJUFH3WIY5G3AJGG/ -- this is not a blocker per se.  Frankly from my POV, it would be better not to lie to people about hibernating, and instead power off.

I consulted with jboyer, who noted we don't even allow hibernating on machines with Secure Boot enabled.  It seems to me that putting in a partial fix that works only for some, but still perpetuates the problem for others, isn't the right thing to do here.  Option 1 seems more viable to me, and doesn't create two classes of users based on issues we can't control.

Regardless, hibernate bugs are not blockers AFAICT, and post-Beta isn't the right time to change or dither on criteria.

Comment 25 Chris Murphy 2016-05-20 19:49:16 UTC
(In reply to Paul W. Frields from comment #24)
> Frankly from my POV, it would be better not to lie to people about
> hibernating, and instead power off.

I think it is this misleading notification in the context of data loss that's more relevant than it being a hibernation bug specifically.
 
> I consulted with jboyer, who noted we don't even allow hibernating on
> machines with Secure Boot enabled.

When 'mokutil --sb-state' returns SecureBoot enabled, then also:

# cat /sys/power/state
freeze mem
# cat /sys/power/disk
[disabled]

And therefore systemd-logind should report that hibernation isn't supported and the DE won't mislead the user. jhogarth?


>  It seems to me that putting in a partial
> fix that works only for some, but still perpetuates the problem for others,
> isn't the right thing to do here.  Option 1 seems more viable to me, and
> doesn't create two classes of users based on issues we can't control.

I don't see two classes being created with the proposed systemd patch.

> Regardless, hibernate bugs are not blockers AFAICT, and post-Beta isn't the
> right time to change or dither on criteria.

Is it a hibernate bug? Or is it misleading UI in the DE as a result of missing information from systemd-logind?

Due to the missing required resume= parameter, resume is impossible. I'd say it's not at all a hibernation bug per se, there is no attempt to resume. From a user perspective, it's a UI/Ux bug, that just so  happens to be caused by a component that's assuming a required parameter is actually there but isn't. Systemd does other tests to determine if hibernation is possible, this is a rather minor additional test to help the DE not mislead the user into a data loss scenario.

The proposed fix also does not increase the pool of users depending on hibernation, so it's not like it's scope creep.

I guess the alternative is that release blocking DE's on Fedora need to carry Fedora specific patches that always hide any UI related to hibernation, because it's knowable in advance hibernation stuff doesn't work out of the box. I'm ok with that solution also but it seems a lot more invasive and unlikely to get fixed for Fedora 24.

Comment 26 Michael Catanzaro 2016-05-20 19:55:10 UTC
I would also be satisfied if we notified the user that the computer will automatically power off instead of notifying the user that the computer will hibernate. In the meantime, consider this a severe user data loss bug. When the laptop says "I'm gonna hibernate," rational users think "OK I'll keep working as long as I can, because my data will be safe when the laptop hibernates." Users need to be warned to save data and power off instead. There's a decent case to be made that this is a blocker under the corruption of user data criterion. That criterion only requires that the issue be fixed *or documented*, however.

Comment 27 James Hogarth 2016-05-20 20:20:04 UTC
(In reply to Chris Murphy from comment #25)
> (In reply to Paul W. Frields from comment #24)
> > Frankly from my POV, it would be better not to lie to people about
> > hibernating, and instead power off.
> 
> I think it is this misleading notification in the context of data loss
> that's more relevant than it being a hibernation bug specifically.
>  

Yes indeed, this bug is very specifically not about fixing the broken hibernation resume in Fedora but solely about misleading a user with a critical battery notification into data loss.

> > I consulted with jboyer, who noted we don't even allow hibernating on
> > machines with Secure Boot enabled.
> 
> When 'mokutil --sb-state' returns SecureBoot enabled, then also:
> 
> # cat /sys/power/state
> freeze mem
> # cat /sys/power/disk
> [disabled]
> 
> And therefore systemd-logind should report that hibernation isn't supported
> and the DE won't mislead the user. jhogarth?
> 

That's right. The kernel blocks the ability and reports it not available through the /sys vfs, which logind reads and use in responding to the CanHibernate DBus call.


> 
> >  It seems to me that putting in a partial
> > fix that works only for some, but still perpetuates the problem for others,
> > isn't the right thing to do here.  Option 1 seems more viable to me, and
> > doesn't create two classes of users based on issues we can't control.
> 
> I don't see two classes being created with the proposed systemd patch.
> 

Right this is putting everyone on equal footing with logind reporting 'na' to CanHibernate reported to all users on a default install.

> > Regardless, hibernate bugs are not blockers AFAICT, and post-Beta isn't the
> > right time to change or dither on criteria.
> 
> Is it a hibernate bug? Or is it misleading UI in the DE as a result of
> missing information from systemd-logind?
> 
> Due to the missing required resume= parameter, resume is impossible. I'd say
> it's not at all a hibernation bug per se, there is no attempt to resume.
> From a user perspective, it's a UI/Ux bug, that just so  happens to be
> caused by a component that's assuming a required parameter is actually there
> but isn't. Systemd does other tests to determine if hibernation is possible,
> this is a rather minor additional test to help the DE not mislead the user
> into a data loss scenario.
> 

Right I don't see this bug as a hibernate bug and i sunny think it should be viewed as such.


> The proposed fix also does not increase the pool of users depending on
> hibernation, so it's not like it's scope creep.
> 

On a side note to this Zbyszek indicated in the github PR he'd be willing to carry the first patch as a Fedora maintainer patch whilst the bigger hibernate questions are resolved in systemd ... as such perhaps rather than a blocker a FE is in order on this?


> I guess the alternative is that release blocking DE's on Fedora need to
> carry Fedora specific patches that always hide any UI related to
> hibernation, because it's knowable in advance hibernation stuff doesn't work
> out of the box. I'm ok with that solution also but it seems a lot more
> invasive and unlikely to get fixed for Fedora 24.

This would also make a significant burden for users to undo should they have a hibernate capable system and have configured resume= themselves per the common bug workaround. Plus it would all have to be undone once the power management questions have been resolved and hibernate was fixed out the box, rather than this patch being the minimal possible touch after many, many hours of testing to achieve that result with minimal required for a user to enable.

Comment 28 James Hogarth 2016-05-20 21:31:36 UTC
(In reply to Michael Catanzaro from comment #26)
> I would also be satisfied if we notified the user that the computer will
> automatically power off instead of notifying the user that the computer will
> hibernate. In the meantime, consider this a severe user data loss bug. When
> the laptop says "I'm gonna hibernate," rational users think "OK I'll keep
> working as long as I can, because my data will be safe when the laptop
> hibernates." Users need to be warned to save data and power off instead.
> There's a decent case to be made that this is a blocker under the corruption
> of user data criterion. That criterion only requires that the issue be fixed
> *or documented*, however.

Note that in the desktop mailing list in response to Chris the only reply there has been that they don't see it a good thing to work around a logind bug where it gives incorrect information so they don't see it as a positive thing to change Upower.conf ... this would again lend weight to the logind proposed fix in the attachments.

Comment 29 Zbigniew Jędrzejewski-Szmek 2016-05-23 04:53:48 UTC
Created attachment 1160438 [details]
simplified version of the F24/rawhide patch

[Assuming that this is voted as a freeze exception tomorrow]

I tweaked James' patch to remove the configuration parameter, and simply refuse hibernation if resume= is not specified. I think this is a reasonable work-around that prevents data loss until better autodetection of the resume device is implemented. I also added a warning if resume is missing=.

# with resume=/dev/mapper/fedora-swap in /proc/cmdline
$ ./test-sleep
Standby configured: no
Suspend configured: yes
Hibernate configured: yes
Hibernate+Suspend (Hybrid-Sleep) configured: yes
Hibernate+Reboot configured: yes
Hibernate+Platform configured: yes
Hibernate+Shutdown configured: yes
Freeze configured: yes
Suspend configured and possible: yes
Hibernation configured and possible: yes
Hybrid-sleep configured and possible: yes

# without resume= specified
$ ./test-sleep
Standby configured: no
Suspend configured: yes
Hibernate configured: yes
Hibernate+Suspend (Hybrid-Sleep) configured: yes
Hibernate+Reboot configured: yes
Hibernate+Platform configured: yes
Hibernate+Shutdown configured: yes
Freeze configured: yes
Suspend configured and possible: yes
No resume= argument specified in the kernel command line, disabling hibernation.
Hibernation configured and possible: no
No resume= argument specified in the kernel command line, disabling hibernation.
Hybrid-sleep configured and possible: no

On a real system the warnings will appear in the journal, so inquisitive users can figure out what is needed to enable hibernation.

If I get the green light, I'll recompile systemd with this patch for F24 (F23 and rawhide to follow).

Comment 30 Geoffrey Marr 2016-05-23 18:20:19 UTC
Discussed during the 2016-05-23 blocker review meeting: [1]

This bug has been classified as a RejectedBlocker and an AcceptedFreezeException as, while the results of the bug are unfortunate, they do not constitute a violation of the blocker criteria. The data loss scenario described is a deliberate in a case like this and the precedent among suspend/hibernate issues is to not classify them as blockers. In this case, this bug is better classified as a freeze exception.

[1] https://meetbot-raw.fedoraproject.org/fedora-blocker-review/2016-05-23/f24-blocker-review.2016-05-23-16.00.txt

Comment 31 Kamil Páral 2016-05-24 06:57:14 UTC
(In reply to Zbigniew Jędrzejewski-Szmek from comment #29)
> If I get the green light, I'll recompile systemd with this patch for F24
> (F23 and rawhide to follow).

Please do. Thanks a lot, Zbigniew.

(Adding Blocks:FinalFreezeException because Geoffrey forgot about it).

Comment 32 Fedora Update System 2016-05-31 12:08:25 UTC
systemd-229-8.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-5f8a34340d

Comment 33 Fedora Update System 2016-06-01 19:54:07 UTC
systemd-229-8.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.

Comment 34 James Hogarth 2016-06-03 15:35:55 UTC
Reopening to make sure this is tracked for rawhide appropriately ...

Looking at the current spec the patch is being applied against F24 but was not applied to the rawhide spec to be included in the future f25.

http://pkgs.fedoraproject.org/cgit/rpms/systemd.git/tree/systemd.spec

Comment 35 Zbigniew Jędrzejewski-Szmek 2016-06-03 22:51:49 UTC
Removing FinalFreezeException because this is now about rawhide.

Comment 36 Kamil Páral 2016-06-06 11:19:55 UTC
James, could you please verify whether the new systemd works as intended in F24 (i.e. the system does not auto-hibernate on critical batter when resume= is not present, but hibernation starts working when you manually add it)? Thanks a lot.

Comment 37 James Hogarth 2016-06-06 11:39:39 UTC
(In reply to Kamil Páral from comment #36)
> James, could you please verify whether the new systemd works as intended in
> F24 (i.e. the system does not auto-hibernate on critical batter when resume=
> is not present, but hibernation starts working when you manually add it)?
> Thanks a lot.

Just gave it a quick test and dbus reports that it cannot hibernate if I strip out the resume= from the kernel command line, adding the resume= back and it reports hibernate is supported.

I don't have time to do a battery drain test but based on all the research I did before this will behave as we'd prefer (ie no resume= user informed of impending shutdown event so save work rather than assume a full session restore will happen). 

Version presently installed: systemd-229-8.fc24.x86_64

Comment 38 Jan Kurik 2016-07-26 05:13:32 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 25 development cycle.
Changing version to '25'.

Comment 39 James Hogarth 2016-09-20 09:12:56 UTC
Just tested this on F25...

The current systemd there does not have the patch applied and it's behaving as per the previous F24 build pre-fix.

So this is now a regression against F24.

Given the past history and that this was a Final FreezeException for F24 proposing the same status for F25.

Comment 40 Fedora Blocker Bugs Application 2016-09-20 09:18:15 UTC
Proposed as a Freeze Exception for 25-final by Fedora user jhogarth using the blocker tracking app because:

 The regression of hibernate capability being adequately tested results in the user being informed that a hibernation event is about to take place when reaching critical battery levels, which then results in data loss if the user does not save their documents as the hibernate cannot resume without the missing resume parameter.

This was a FinalFreezeException for F24 and the patch applied to systemd in f24 has been left off the f25 spec. 

I've just tested a system and the behaviour at present is as per the pre-fixed F24 version.

Since this was agreed to be a FreezeException then I propose the same status for F25.

Comment 41 Kamil Páral 2016-09-20 10:10:59 UTC
(In reply to James Hogarth from comment #39)
> Just tested this on F25...
> 
> The current systemd there does not have the patch applied and it's behaving
> as per the previous F24 build pre-fix.

Since this is a long bug report, can you please quickly summarize how exactly F25 behaves at the moment? (or link to a relevant comment). Thanks.

Comment 42 James Hogarth 2016-09-20 10:22:17 UTC
(In reply to Kamil Páral from comment #41)
> 
> Since this is a long bug report, can you please quickly summarize how
> exactly F25 behaves at the moment? (or link to a relevant comment). Thanks.

No problem ...

It's at present the same behaviour as in comments #6 #7 and #8

Default install:
Critical battery claims it will hibernate rather than shutdown.
It completes the hibernation sequence.
Does not find the resume device so starts up with a fresh session losing any unsaved data.

With resume= added:
Critical battery claims it will hibernate.
It completes the hibernation sequence.
On start it finds the resume device and picks the session back up.

With Upower told that critical action is to PowerOff:
Critical battery says a shutdown will happen, save work or risk losing it.
Powerdown happens
On start fresh session happens.

Comment 43 James Hogarth 2016-09-20 10:26:13 UTC
Comment links:

comment 5 <-- detailed investigation for F24 now valid in F25 again

comment 6 <-- screenshot of hibernation claim in default config

comment 8 <-- screenshot with Upower told to poweroff rather than hybridsleep or hibernate

Comment 44 Geoffrey Marr 2016-10-24 18:19:59 UTC
Discussed during the 2016-10-24 blocker review meeting: [1]

The decision to classify this bug as an AcceptedFreezeException was made as the precedent of this bug being classified as such was made during the F24-release-cycle.

[1] https://meetbot.fedoraproject.org/fedora-blocker-review/2016-10-24/f25-blocker-review.2016-10-24-16.01.txt

Comment 45 Fedora End Of Life 2017-11-16 19:04:07 UTC
This message is a reminder that Fedora 25 is nearing its end of life.
Approximately 4 (four) weeks from now Fedora will stop maintaining
and issuing updates for Fedora 25. It is Fedora's policy to close all
bug reports from releases that are no longer maintained. At that time
this bug will be closed as EOL if it remains open with a Fedora  'version'
of '25'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version'
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not
able to fix it before Fedora 25 is end of life. If you would still like
to see this bug fixed and are able to reproduce it against a later version
of Fedora, you are encouraged  change the 'version' to a later Fedora
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's
lifetime, sometimes those efforts are overtaken by events. Often a
more recent Fedora release includes newer upstream software that fixes
bugs or makes them obsolete.

Comment 46 Fedora End Of Life 2017-12-12 10:47:31 UTC
Fedora 25 changed to end-of-life (EOL) status on 2017-12-12. Fedora 25 is
no longer maintained, which means that it will not receive any further
security or bug fix updates. As a result we are closing this bug.

If you can reproduce this bug against a currently maintained version of
Fedora please feel free to reopen this bug against that version. If you
are unable to reopen this bug, please file a new report against the
current release. If you experience problems, please add a comment to this
bug.

Thank you for reporting this bug and we are sorry it could not be fixed.