pm-utils doesn't reload hdparm.conf after a suspend

Disk-idleing (aka "laptop-mode") and power states are quite separate things.
https://wiki.ubuntu.com/PowerManagement

Since ubuntu by default ships with laptop-mode-tools (thus disabled) which handles all the things associated with disk-idleing,

(like mount options, hdparm options, low battery exceptions, disk-idleing on AC when on the road, etc.)

laptop-mode-tools will handle it.

The ubuntu package does not work yet as well as the debian package, though.
Package laptop-mode-tools needs to put scripts into pm-tools directories. #244844

(The current laptop-mode disk-idleing approach seems to be a left-over from before the ubuntu-laptop-mode package was droped for laptop-mode-tools.)

Also package acpi-support is still preveting much of laptop-mode-tools working.
#244833 #244831 #244832 #244839 #244836 #244838

Yes, this is hardcoded in /usr/lib/pm-utils/power.d/95hdparm-apm.

Here is a patch to add a new option to hdparm.conf, apm_battery, that will enable us to eliminate the hard-coded apm values in pm-utils. Rationale:

- as long as laptop-mode is not enabled by default in Ubuntu, hdparm.conf is the only place that we can sensibly configure the apm battery values and eliminate the hard-coding, and that requires a new config option.
- hdparm is the only package that has a udev rule setting the values on newly-attached hard drives - if we want settings applied consistently to devices that are connected after boot, it has to be done here.

Since this is new feature development in pursuit of a bugfix, I'm proposing this as a feature freeze exception for karmic.

Benefits:
 - user configuration of disk apm levels will be respected by pm-utils on suspend/resume and on power events

Risks:
 - Debian rejects this additional syntax or we decide it's the wrong place to do this (because we decide to use laptop-mode by default?) and we have to revert this post-karmic, resulting in user confusion
 - I've overlooked some way that this takes effect by default at boot time where it didn't before, impacting our boot performance
(risk is minimal; before and after this change, the udev rule shouldn't call hdparm unless there are uncommented options in /etc/hdparm.conf)

I've tested the correctness of this change with and without an hdparm.conf file that includes these settings, confirming that I get the expected apm settings on boot and resume.

If this FFe is accepted, a corresponding patch to pm-utils will be forthcoming.

I reviewed the patch. Thinking aloud..

 * hdparm_is_on_battery doesn't check whether on_ac_power actually exists (it's only recommended), but since it explicitly tests for $? == 1, and "command not found" yields 127, this should be okay. It means that if powermgmt-base is not installed, we default to the current behaviour, which makes sense.

 * The new hdparm_apm_option_for_disk() function has the same structure as in /lib/udev/hdparm, except it only looks out for some particular parameters. If this is accepted in Debian, this probably needs some refactoring. In anticipation of this, could hdparm-functions perhaps be installed into /lib, not /usr/lib/? Then the udev script could use it as well.

 * hdparm_apm_option_for_disk() could do with a comment describing parameters and return value, and mention that the result will go to stdout

 * There seems to be an inconsistency between the manpage (saying that the default value of apm is 255), and the code (which defaults to 254).

In general I'm in favour of this, since it's pretty much a bug fix. The code duplication is okay for now since it (a) avoids structural changes in existing code, and (b) it does not make sense to work on this until Debian agrees to that change in general.

I was conscious of the code duplication between hdparm_apm_option_for_disk() and the udev rule. The challenge is that pm-utils is looking to be given one specific option for the device, and udev wants to apply all the options specified in /etc/hdparm.conf, so I felt implementing a duplicate, stripped-down parser would be the lesser evil here for a FFe.

Since the code duplication is ok, I think we should leave this as-is in /usr/ilb for now; it's going to be an API change regardless if we refactor the code later, so I don't think there's any benefit to placing the file in /lib yet.

The code defaults to 254 because it's implementing our policy, which up to now was hard-coded in pm-utils (and acpi-support before that). The manpage doesn't say that 255 is a default, it just presents 255 as an example value; probably a bad example since this apm value has a special meaning, but I think that's out of scope for the FFe.

Will add a comment to hdparm_apm_option_for_disk() as suggested.

Steve Langasek [2009-10-06 19:56 -0000]:
> I felt implementing a duplicate, stripped-down parser would be the
> lesser evil here for a FFe.

Agreed.

> The manpage doesn't say that 255 is a default, it just presents 255
> as an example value

I see. This wasn't clear from the limited context of the patch.

> Will add a comment to hdparm_apm_option_for_disk() as suggested.

Thanks. So this is fine, approved.

This bug was fixed in the package hdparm - 9.15-1ubuntu4

---------------
hdparm (9.15-1ubuntu4) karmic; urgency=low

  * Recommend powermgmt-base, so we can use on_ac_power to detect if we're on
    battery.
  * Add a new option to hdparm.conf, apm_battery, which is used in place of
    apm when we detect that we're on battery power.
  * debian/hdparm.udev-script, debian/hdparm.init: only use the 'apm' setting
    when we're not on battery; when we are on battery, apply the separate
    'apm_battery' setting.
  * debian/hdparm-functions: break out a barebones parser for
    /etc/hdparm.conf and install it to /usr/lib/pm-utils, for use in
    /usr/lib/pm-utils/sleep.d/95hdparm-apm. LP: #238555.

 -- Steve Langasek <email address hidden> Mon, 05 Oct 2009 22:57:20 +0000

This bug was fixed in the package pm-utils - 1.2.5-2ubuntu7

---------------
pm-utils (1.2.5-2ubuntu7) karmic; urgency=low

  * debian/95hdparm-apm: use /usr/lib/pm-utils/hdparm-functions from hdparm
    (>= 9.15-1ubuntu4) to determine what apm value to use, instead of
    hard-coding settings that users then can't override. LP: #238555

 -- Steve Langasek <email address hidden> Tue, 06 Oct 2009 21:11:53 +0000

Would not it make sense to handle spindown_time the same way, adding a spindown_time_battery so that the user can configure different timeouts for hard drives?
I used to have 20 minutes when on AC and 3 when on battery. I don't understand how to control it with pm-utils