In comment #2 I wrote: > Comment on attachment 68712 [details] [review] >... > Although you could make the penultimate line as follows I don't recommend it > because it would hide any problems like the escaping issue that could cause > HIBERNATE_MODE_SAVE to be invalid: > > [ -n "$HIBERNATE_MODE_SAVE" ] && echo -n "$HIBERNATE_MODE_SAVE" > > > /sys/power/disk
In fact there are 2 cases as the code is used now: 1 HIBERNATE_MODE unset => normal hibernate 2 HIBERNATE_MODE = suspend => suspend-hybrid Given which, I've revised my comment above and propose a new version of the modified do_hibernate() as follows: do_hibernate() { local hibernate_mode_save ret [ -n "${HIBERNATE_MODE}" ] && \ grep -qw "${HIBERNATE_MODE}" /sys/power/disk && \ hibernate_mode_save=$(cat /sys/power/disk) && \ hibernate_mode_save="${hibernate_mode_save##*\[}" && \ hibernate_mode_save="${hibernate_mode_save%%\]*}" && \ [ "$hibernate_mode_save" != "${HIBERNATE_MODE}" ] || \ hibernate_mode_save="" [ -n "$hibernate_mode_save" ] && \ echo -n "${HIBERNATE_MODE}" > /sys/power/disk echo -n "disk" > /sys/power/state ret=$? [ -n "$hibernate_mode_save" ] && \ echo -n "$hibernate_mode_save" > /sys/power/disk return $ret } The key points: - hibernate_mode_save is only set if the current HIBERNATE_MODE is being changed (which only happens, if it does, in the suspend-hybrid case); - on resume the hibernate mode is only restored if hibernate_mode_save was set. This fixes: - the failure to restore the hibernate mode with suspend-hybrid; - "sh: I/O error" on resume from suspend-hybrid - "sh: I/O error" on resume from hibernate. Finally, the pm_functions script uses "echo -n" (from line 318, including the above patch) and local declarations while the comment against function log() implies that the script is aiming for POSIX conformance and yet other functions use non-POSIX local declarations. local and "echo -n" usages are fine for Debian and derived environments. To achieve POSIX conformance such usages would have to be reviewed and modified; "echo -n" could be replaced with printf (the parameters to be echoed in each case being plain text not containing formatting commands); or a shell function echo() can be added based on log(). -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to pm-utils in Ubuntu. https://bugs.launchpad.net/bugs/1172692 Title: Please implement in-kernel suspend to both Status in OEM Priority Project: Fix Released Status in OEM Priority Project precise series: Fix Released Status in pm-utils: Confirmed Status in pm-utils package in Ubuntu: Fix Released Status in pm-utils source package in Precise: Fix Released Status in pm-utils source package in Quantal: Fix Released Status in pm-utils source package in Trusty: Fix Released Status in pm-utils package in CentOS: Unknown Bug description: From kernel-3.6 there is in-kernel support for suspend to both (AKA hybrid suspend). Working patches can be found at https://bugs.freedesktop.org/show_bug.cgi?id=52572 (or links). [Impact] * Ubuntu 12.04 implements hybrid suspend differently. It suspends first and wakes up the computer for hibernation 15 minutes later. This is risky since the computer may be carried when the wakeup happens. The hard disk may experience physical shocks and get damaged. * Thus, it is desirable to have a real hybrid suspend implementation. In-kernel hybrid suspend has been supported since kernel 3.6+. pm-utils only needs a small patch to enable this feature. [Test Case] * Ensure all Ubuntu 12.04 packages are up-to-date in the test environment. Install pm-utils 1.4.1-9ubuntu1 Install linux-image-lts-raring * Reboot the computer with lts-raring kernel. Run the command: 'pm-suspend-hybrid' from a terminal. After the computer suspends, press the power button. It should be able to resume from suspension correctly. * Run the command above again. After it suspends, remove and reconnect its power supply (or its battery). Press the power button. It should be able to resume from hibernation correctly. * Reboot with the default 3.2 kernel, Run the command above. The computer should be able to suspend and then wake up for hibernation 15 minutes later. [Regression Potential] * This patch won't affect users who still use 3.2 kernel. It only enables in-kernel hybrid suspend if the option 'suspend' is available from /sys/power/disk. To manage notifications about this bug go to: https://bugs.launchpad.net/oem-priority/+bug/1172692/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : touch-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp