Marcus Better wrote: > Package: kpowersave > Version: 0.7.2-1 > Severity: normal > > kpowersave has code to show a notification popup if suspend-to-ram > fails. This code is quite broken. > > Suspend-to-RAM did not work on my system. When I tried to suspend, it > locked the screen (as configured) but never actually suspended. This > was not caused by kpowersave, however it should notify when something > goes wrong. > > kpowersave.cpp contains this code to check for successful suspend: > > bool ret = hwinfo->suspend(SUSPEND2RAM); > > if (ret) { > return true; > } else { > > KPassivePopup::message(i18n("WARNING"),i18n("Suspend to RAM failed"), > SmallIcon("messagebox_warning", 20), > this, i18n("Warning"), 15000); > return false; > } > > However the actual result is not returned by hwinfo->suspend(), but > through a callback from HAL. Re-compiling with debug messages enabled, > I can see the different status codes returned from HAL: > > Suspend working: > > 10:27:07:366: dbusHAL::callBackSuspend > 10:27:07:366: HardwareInfo::handleResumeSignal (int result: '0') > > Suspend not working: > > 10:38:03:161: dbusHAL::callBackSuspend > 10:38:03:161: HardwareInfo::handleResumeSignal (int result: '127') > > This callback causes the resumed() signal to be emitted, and triggers > kpowersave::handleResumeSignal(). There is a wrong assumption here, > since it will be emitted even if the suspend failed. > > And that function contains the following gem: > > // handle result of the resume/suspend > // TODO: fill with some code > if (result == 0) { > // successful resumed ... remount only in this case > if (!handleMounts(false)) { > KPassivePopup::message(i18n("WARNING"), i18n("Could not > remount (all) external storage" > " media."), > SmallIcon("messagebox_warning", 20), this, > i18n("Warning"), 15000); > } > } else if ( result == -1 ) { > myDebug ("Unknown if we successful resumed, look like a > timeout"); > setAutoSuspend(true); > } else { > // errro case > } > > Notice the unhandled last error case! > > So, to fix this properly one would check the result code above and > determine that the suspend failed, and then trigger the pop-up > warning. > > Incidentally, the reason for suspend not working is that I use > uswsusp, and my laptop is not yet in the s2ram whitelist, so the > "s2ram" command fails unless given the --force option. This makes the > HAL script return an error code, which kpowersave fails to pick up. > > This patch for HAL makes suspend work on non-listed > laptops, but it is only a quick-fix if you don't want to wait for a > new uswsusp release. (Please add your laptop model to the whitelist as > per /usr/share/doc/uswsusp/README.s2ram-whitelist.gz if you have this > problem.) > > --- /usr/lib/hal/scripts/linux/hal-system-power-suspend-linux 2007-03-07 > 10:02:48.375989744 +0100 > +++ ./hal-system-power-suspend-linux 2007-03-07 10:02:38.006060821 +0100 > @@ -91,7 +91,7 @@ > /usr/sbin/hibernate -F/etc/hibernate/ram.conf > RET=$? > elif [ -x "/usr/sbin/s2ram" ] ; then > - /usr/sbin/s2ram -f -a3 > + /usr/sbin/s2ram > RET=$? > elif [ -x "/etc/acpi/sleep.sh" ] ; then > # Use acpi-support for suspend to ram
Imo s2ram should not be called directly, but via a wrapper, like pm-utils/acpi-support/hibernate which sets the correct return codes and output to stdout/stderr. There's already a similar bug report http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=410818 The cleanest fix imo would be to remove the direct call to s2ram and have hal depend on one of the above package instead. This way we get defined behaviour. Cheers, Michael -- Why is it that all of the instruments seeking intelligent life in the universe are pointed away from Earth?
signature.asc
Description: OpenPGP digital signature