acpi-support's 90-hdparm.sh overwrites hdparm.conf's apm settings

Note, the hard drive in /etc/hdparm.conf can be specified by one of its several names, possibly a different name from that which acpi-support is using, e.g. /dev/disk/by-id/ata-ST320423A_3EJ0GMSY versus /dev/sdb.

I've created a small patch that hopefully fixes this.
It uses readlink to overcome the problem of custom names in hdparm.conf.

This patch tries to make sure that /etc/acpi/*.d/90-hdparm.sh does not overwrite settings made by laptop-mode-tools or hdparm. Each harddisk is checked individually; APM settings are only applied if the drive is neither controlled by /etc/laptop-mode/laptop-mode.conf nor by a matching section in /etc/hdparm.conf.

However, the proposed patch does NOT check whether the device section in /etc/hdparm.conf actually contains an APM setting. This would require a lot of functionality that is already present in /lib/udev/hdparm. Maybe this functionality should be exported to a small parser lib for the hdparm.conf file format?
Note that this problem only affects people who created a drive section in hdparm.conf and left out the APM value (which seems unlikely; most people seem to edit /etc/hdparm.conf because they *DO* want to set a specific APM value). The default behaviour does not differ from the current implementation of 90-hdparm.sh

Thanks for trying to please everybody. :-) A couple of minor comments. "egrep '^/dev[-_/[:alnum:]]*[-_[:alnum:]]+ {" -- can there be spaces at the start of the line, or anything other than one space before the open brace? controlled_by_LMT() and controlled_by_HDPARM() both echo 0 or 1 to stdout, any reason they can't just "return 0" or "return 1"? It's supported by /bin/sh (dash). Logically, it would make sense then if controlled_by_*() returned 0 for true, as is normal for shell commands.

Thanks for your quick response. You are right, there might be spaces at the start of the line and before the curly brace, so I changed the regex to: '^[[:space:]]*/dev[-_/[:alnum:]]*[-_[:alnum:]]+[[:space:]]+{'
I also fixed the return-values and that _very_ stupid "echo $HDPARM" error... Sorry :)

This patch implies that if a single value is configured in /etc/hdparm.conf, it will be used in place of acpi-support's power-aware handling. I don't think that's appropriate; at most we should read the value from hdparm.conf and use it when on AC power while continuing to apply a power-saving mode when on battery.

I wrote the patch because my laptop hard drive was heating up to about 50°C because acpi-support disables the disk's APM while the laptop is on AC power.

I think if the user explicitly sets a value in /etc/hdparm.conf, acpi-support should not overwrite this setting, even if the power state changes. Arbitrarily applying the settings from /etc/hdparm.conf while on AC but using acpi-support's default while on battery doesn't seem appropriate either; while user A might be happy, user B might expect the exact opposite behavior.
Furthermore, customized power-aware handling of APM values can already be done via laptop-mode-tools.
In my oppinion, only if both /etc/hdparm.conf and l-m-t are disabled, it's acpi-support's turn to provide decent default values.

In fact there is no need for a patch, this behavior is controlled by laptop-mode. A desktop pc is something like a laptop with ac power adapter and if you want that acpi don't control hard drives apm you should edit /etc/laptop-mode/laptop-mode.conf and set CONTROL_HD_POWERMGMT from 1 to 0.

The real problem is very big, I think the whole acpi subsystem is a little messy. For example, is defined when a computer is a laptop but in some scripts for example the ones involved here don't try to see if it's a laptop with ac batt or a normal pc.

I wish with the new API for devices DeviceKit the hardware administration in gnu/linux will become more powerful and also coherent design.

Other example I forget to say of this real mess is that in the /etc/laptop-mode/laptop-mode.conf there are variables to set diferent APM values for hard drives depending if there ac power, battery... but the scripts don't use it, the have hardcoded values instead and other issue also reported here en ubuntu launchpad is apm values not being restored after suspend, take a look in the acpi scripts and you will see that is the same problem but with another acpi script.

90-hdparm.sh sets the apm value when laptop-mode does *not* control the hd apm setting. Good.
It should also leave the apm value alone when it is set by hdparm.conf but it doesn't. Bad.
That's the problem, that's what is fixed by Günther's patch.

Yes I explained myself bad. acpi takes the laptop-mode config if its running or not, I think could be better to take the config values then instead of hardcoding them and use laptop mode config to set apm, it's very bad to have many places to do the same thing because then appears problems like this. The patch is a solution yes, but is not the solution, the problem is more than this.

I expect i have explained now right.

This bug was fixed in the package acpi-support - 0.127

---------------
acpi-support (0.127) karmic; urgency=low

  [ Lionel Le Folgoc ]
  * lib/policy-funcs: Recognize xfce4-power-manager as a power manager.
    LP: #425155

  [ Steve Langasek ]
  * Drop /etc/acpi/start.d/90-hdparm.sh: this is redundant because we're
    already calling pm-powersave on start (either via /etc/acpi/power.sh,
    or by one of the desktop power managers as enumerated in
    lib/policy-funcs) so there's no reason we should be reapplying the
    policy here. LP: #443992, #438355, #318980.
  * Add guidance-power-manager to the list of known desktop power managers.
    LP: #154910

  [ Michael Terry ]
  * lib/policy-funcs: Recognize dalston as a power manager. LP: #432578

 -- Steve Langasek <email address hidden> Mon, 05 Oct 2009 20:11:12 -0700

Well solved in karmic, but what about other releases? The same in another bug, but without response since half a year, look the second comment https://bugs.launchpad.net/ubuntu/+source/ocaml/+bug/215000. What is the sense to have LTS releases if they don't get know fixes? Hardy in alredy supported due to its LTS nature, also Jaunty is already supported and I think Intrepid also but this last I'm not sure.

Then the LTS tag is only three letters to add in a release name and nothing else?

David, LTS refers to the longer support cycle for security fixes and other high-impact problems. It does not mean that we fix every bug found in the LTS that gets fixed in the latest release.

Ocaml bug consequence no 64 bits application written in ocaml is possible to be run properly, this is not an high-impact problem?

This bug, prevents an user from setting a less aggressive apm and in some hard-drive models could prevent from stopping his/her hard drives and, making impossible to reduce noise for certain hard drives and consume from 6-8 Watts (depends on the model) less per disk, this is not an high-impact problem?

This bug still exists in Natty. The code the patch addresses has migrated to /lib/hdparm/hdparm-functions (about line 99).

> This bug still exists in Natty.

No, it does not. The /etc/acpi/start.d/90-hdparm.sh script was removed in karmic and has not been seen since.

> The code the patch addresses has migrated to /lib/hdparm/hdparm-functions (about line 99).

This bug report was about *overriding* of hdparm.conf settings. The section of /lib/hdparm/hdparm-functions in question implements *support* for setting the hdparm option via hdparm.conf.