power: Add suspend option to battery-critial actions #12680
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
some say that including "suspend" in the set of available battery critical actions is dangerous: power might ran out during suspend and machine state would be lost.
however:
the really dangerous action is "do nothing", and it is still allowed in the UI. suspend might loose state, but is way way safer than "do nothing": suspend will flush all caches to disks, making sure that all recently saved data is persistent. it also forwards filesystems to a point where -though still mounted- they are effectively clean: filesystems are guaranteed to be in a sane state on-disk, even if they do not journal. running out of power during suspend is infinitely more "graceful" and safe than running out of power while the CPU is running: the latter is allowed, but the former is not. go figure...
suspend can loose state, yes, but this can happen any time the user suspends. if suspending, the system will not wake up and do the critical action (eg, shutdown) when power is critical. by this token, suspend should never be allowed then. but we allow it because it makes sense: we assume users know what they are doing.
the current UI has a bug by which, if the user chooses suspend in dconf (as i do; i also elevate the critical trigger percentage), the UI shows the setting as blank and corrupts it immediately if the user simply touches it. the bug was reported here: Buggy handling of battery critical action setting by the cinnamon settings UI #11983. this PR fixes that issue (for suspend only, the most likely trigger by far). there are other ways to fix this (discussed in the bug report), but over a year passed and no fixes were proposed, so i propose merging this.
thank you!