Hi,
I've submitted the merge request for this feature. You could test it :).
This allows the Suspend as the CriticalPowerAction. Since it is an unusual configuration for ordinary systems and only for the specific use case, AllowRiskyCriticalPowerAction
is introduced to make sure this risky feature can't be easily enabled. If and only if AllowRiskyCriticalPowerAction
is set to true
and CriticalPowerAction
is set to Suspend
, this feature can be enabled. Otherwise, Power off
will be the default CriticalPowerAction
.
Moreover, if the user insists on setting the Suspend
as the CriticalPowerAction
, the warning message will be shown in the log or journal to notify the user about the risk.
Kate Hsuan (9b30e908) at 27 Mar 05:23
linux: up-backend: A safeguard for the risky CriticalPowerAction
... and 2 more commits
Kate Hsuan (da6b9402) at 22 Mar 12:02
safeguard for the risky critical power action
... and 2 more commits
Good idea.
thanks! any option is better than no option.
my $0.02 ...
those manually editing the file might not need such safeguards, but if safeguards are desired, you might want to consider these ideas:
Hi,
Thank you guys.
Here is the summary of this topic.
Hope this feature gives the user a chance to configure their upower based on the use case and will not affect the ordinary users.
If I missed something, please let me know :)
I'll put this issue and implementation in my backlog.
do you mean kernel fs corruption or app file corruption?
In my case a fsck fixed things up.
we are talking fs corruption, not data loss. data loss already happens if the system forcefully shuts down without you saving stuff, while in the middle of processing, etc.
those are corrupted files, not a corrupted fs, which is way worse.
do you mean kernel fs corruption or app file corruption?
if you mean fs corruption, please give pointers because you get me really interested there.
I think a default-off config option and journal warning is probably a pragmatic middle ground.
you are wrong: suspend flushes all filesystems in a such a way that reset after standby can never cause corruption
Not true; I've personally experienced data loss on EXT4, and have also seen several customer cases for RHEL.
the user expects power outages to be common
this is not true. i simply expect and require my systems to recover from an outage.
Shutting down avoids filesystem corruption (at least if the shutdown finishes before power is lost). That's much worse than losing unsaved data.
you are wrong in what you are not saying but implying. you are implying that shutdown cleans filesystems but standby doesn't. you are wrong: suspend flushes all filesystems in a such a way that reset after standby can never cause corruption. (this is very important and yet trivial to implement. kernel fs devs are a smart bunch, they wouldn't fall for trivial things such as this.)
some filesystems could maybe suffer some "safe" corruption as an optimization (maybe for example, not freeing some free blocks in a free list) that will be automatically fixed on next boot auto fsck, but i doubt even these cases exist. if you find fs corruption on reset after suspend, please file a kernel bug immediately.
(also, people today almost exclusively use journaling fs's, which means that -save for hardware errors (storage device firmware or design bugs)- abruptly cutting power will never corrupt the fs (again, file bug if journaling is broken). but this has no relation with the safe and trivial suspend case.)
you can do whatever you want with the UI, but forcing this in upower is just crazy.
for those impacted by this draconian imposition, take a look at the cinnamon desktop. it is a very nice and user friendly GTK desktop which was forked from gnome a long time ago. as you can see below, the backend can be configured to your liking even if the UI indeed hides the suspend option.
That's all we've been asking for since the beginning
Bump glib2 to 2.66 so we can use g_file_set_contents_full
Kate Hsuan (1e58e821) at 14 Mar 06:25
Stop using deprecated g_get_current_time
... and 1 more commit
Or, this feature can be configured through config file. We can disable it by default. The user can enable it and a horrible warning message will be shown in the log.
Is it better than the previous idea?
Shrug. Still sounds risky to me. Honestly I'd say Igel's previous comment is case in point: user thinks it's "safe enough" because ext4 has a journal and it's usually fine... except when it's not.
Hi,
Thank you for updating and verifying it.
Could you please rebase the branch to the master branch?
This is my thought, upower could provide an interface to configure the system as much as it can. The upper layer application shows the possible configuration to the user and avoids showing the dangerous configuration so the ordinary user will not be affected by this special use case. The hidden configuration options give the user a chance to configure the system to fulfil their special workload or use case.
Alternatively, upower shows a terrible warning message in the log when the user set suspend
as the critical action. The log message warns the user about the risk if they insist on using the dangerous config.
@mcatanzaro Is it good for you?
The issue should be here
in linux/up-device-supply-battery.c
...
struct _UpDeviceSupplyBattery
{
UpDeviceBattery parent; <----here
gboolean has_coldplug_values;
gboolean coldplug_units;
gdouble *energy_old;
GTimeVal *energy_old_timespec; <--- drop it
guint energy_old_first;
gdouble rate_old;
gboolean shown_invalid_voltage_warning;
gboolean ignore_system_percentage;
};
...
Since it inherited UpDeviceBattery, a parent object should be declared at the first line of the data structure. Moreover, GTimeVal *energy_old_timespec;
can be removed and it isn't used by any function.
After revising it, the unit test should pass.
Could you please help to verify this?