/lib/udev/hdparm Compares Against $DEVNAME.

Bug #222458 reported by Ralph Corderoy
80
This bug affects 18 people
Affects Status Importance Assigned to Milestone
hdparm (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

Binary package hint: hdparm

Ubuntu 7.10, hdparm 7.5-1ubuntu1. My /etc/hdparm.conf has entries like

    /dev/disk/by-id/ata-ST3160023A_4JS03MMK {
        spindown_time = 60
    }

which work just fine with /etc/init.d/hdparm. However, it appears init.d/hdparm is on its way out and has been replaced with /lib/udev/hdparm. That only executes hdparm(8) for bits of /etc/hdparm.conf that match $DEVNAME, presumably passed in by udev. I guess it'll have values like /dev/sda or /dev/hda. However, I don't necessarily know what the /dev name will be, so I've specified it by /dev/disk/by-id instead which is a symlink.

    $ stat -c %N /dev/disk/by-id/ata-ST3160023A_4JS03MMK
    `/dev/disk/by-id/ata-ST3160023A_4JS03MMK' -> `../../hda'

It seems that /lib/udev/hdparm needs to do some readlink(1) or similar for each of the discs specified in /etc/hdparm.conf, e.g. `readlink -e /dev/disk/by-id/ata-ST3160023A_4JS03MMK' gives /dev/hda, and only then compare against $DEVNAME.

Revision history for this message
Jakob Unterwurzacher (jakobunt) wrote :

Sounds quite useful.

Would you test if the attached /lib/udev/hdparm does what you want?

diff /lib/udev/hdparm /tmp/hdparm
93c93,98
< DISC=$KEY
---
> if [ -L $KEY ]
> then
> DISC=$(readlink -m $KEY)
> else
> DISC=$KEY

Revision history for this message
Jakob Unterwurzacher (jakobunt) wrote :

> fi

(missed that line on copy-paste)

Revision history for this message
Ralph Corderoy (ralph-inputplus) wrote : Re: [Bug 222458] Re: /lib/udev/hdparm Compares Against $DEVNAME.

> Would you test if the attached /lib/udev/hdparm does what you want?
>
> > if [ -L $KEY ]

I'm not in a position to test it right now, although looking at it in
context, I think it should work just fine. One comment,
/lib/udev/hdparm is a /bin/sh script which is dash on 8.04 but dash(1)
says -L is only there for old time's sake and -h should be used instead.

Revision history for this message
Ralph Corderoy (ralph-inputplus) wrote :

Whilst you're in that script. What purpose does $ret have? AFAICS it is assigned to but never used. Either it should be used or it could be deleted altogether for clarity.

Revision history for this message
Martien Verbruggen (martien.verbruggen) wrote :

I've run into this same problem, but I actually use /dev/disk/by-uuid/* links, which all link to a partition on the given device, rather than the device itself. I suggest the following as a fix for this as well as the earlier suggested fix:

93c93,100
< DISC=$KEY
---
> if [ -L $KEY ]
> then
> DISC=$(readlink -m $KEY)
> # if this is a link to a partition, strip off the digit
> DISC=${DISC%[0-9]}
> else
> DISC=$KEY
> fi

Revision history for this message
Ralph Corderoy (ralph-inputplus) wrote :

As long as $DISC won't end in a digit unless it's a partition digit to
be stripped off, that looks fine. But it should be -h not -L in dash(1)
as I said earlier.

Revision history for this message
Martien Verbruggen (martien.verbruggen) wrote :

I'm pretty sure that all /dev/[sh]d[a-z] devices would not have a digit at the end unless they're partitions.

I don't mind using -h instead of -L.

I need to do some more testing. While the above worked fine when running /lib/udev/hdparm from command line. i am not entirely convinced yet that it will work during boot. I am not sure that these scripts are being called _after_ the /dev/disk hierarchy has been built or before.

I'll play with a test installation tomorrow, when I have some time to confirm.

Revision history for this message
Martien Verbruggen (martien.verbruggen) wrote :

I've just been trying to determine where, during the boot cycle, /lib/udev/hdparm is being called for the permanently attached disks. While I can see the devices in /var/log/udev, I don't think that the /lib/udev/hdparm script is run correctly, or maybe it's called too early in the boot process. It's hard to tell, because there STILL is no boot.log.

I think the script is called during boot before the file systems have been mounted, and therefore /sbin/hdparm isn't even there yet. I'm beginning to think that this is not the right place to try to call hdparm for the permanent disks.

I'll probably just add a script to init.d that calls hdparm explicitly, and give up on this.

Revision history for this message
Jools Wills (jools) wrote :

Is this related to the problem I am having ?

See:

https://bugs.launchpad.net/ubuntu/+source/hdparm/+bug/227705

I'm having to continue using the init.d script as it isn't working from udev. Note that the init.d script is still useful. How else are users supposed to check whether their hdparm.conf settings are working nicely or correct ?

Revision history for this message
Ralph Corderoy (ralph-inputplus) wrote :

Yes, bug #227705 may be a duplicate of this one, but I'm puzzled why it
still didn't work when you switched from the symlink to /dev/sda1. Did
you try the drive rather than the partition? /dev/sda.

I agree that, compared to init.d, the udev stuff seems hard to debug.
Perhaps we just don't have a good model of how it works and where to
look compared with straightforward shell scripts where one can follow a
path. Anyone know of a good udev debugging/overview resource?

Revision history for this message
Bas Ploeger (basploeger) wrote :

I confirm this bug, it's quite annoying. I have two drives and cannot predict which one will be /dev/sda and which one will be /dev/sdb after booting. Hence, the only thing I can do is select by the ids in /dev/disk/by-id but this is not supported by the current udev-rules.

Revision history for this message
Bas Ploeger (basploeger) wrote :

Addition: the proposed bugfix of Jakob works for me. After rebooting, the disk parameters are indeed set to the values in /etc/hdparm.conf. Thanks!

Revision history for this message
Jakob Unterwurzacher (jakobunt) wrote :

This uses -h instead of -L as suggested by Ralph. Additionally, it uses some quoting for safety (empty $KEY, spaces inside).

Revision history for this message
Jakob Unterwurzacher (jakobunt) wrote :

Bas, does that mean that you don't experience bug #318980 ? Which Ubuntu version do you use?

Revision history for this message
Bas Ploeger (basploeger) wrote :

Hello Jakob,

I'm using Ubuntu Intrepid. I don't know whether I experience that bug, I have not looked at the apm settings yet. But I can do so if that's helping anyone.

I currently only use /etc/hdparm.conf to set spindown_time (-S) and acoustic_management (-M) on one of my drives (viz. the one that is used only every now and then for storing backups).

Thanks for the patch, I'll apply it when I have time.

Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in hdparm (Ubuntu):
status: New → Confirmed
Revision history for this message
Ewano (i-launchpad-ewano-net) wrote :

Not sure if I've done this right, but I've marked Bug #799312 as a duplicate of this bug. Perhaps I should have marked this as the duplicate, but I'm not sure how launchpad works just yet.

It appears to be dealing with the same issue however a more up to date patch has been generated which works for 10.10, 11.04, 11.10, and also 12.04 beta.

My concern is that no form of this fix patch has been incorporated in the hdparm source since it was first noted here in #5 in 2008. How do I go about letting a code maintainer know that this patch exists so it can be incorporated into the main code base of hdparm as used under Precise.

I use this patch to make sure that powered external USB HDD's which don't have reasonable APM settings will spin down after being idle for 30 min - ie. after the machine they are attached to powers down. I don't want updates to overwrite the patch I have applied to a client machine, and leave his external hard drive running 24/7..

Revision history for this message
Ewano (i-launchpad-ewano-net) wrote :

I have also marked Bug #227705 as a duplicate of this bug as the patch supplied in this bug seems to deal with the same issue and has been patched more recently in Bug #799312.

Like I say, I'm not too sure if I should have marked this bug (222458) and 227705 as duplicates of 799312 - as it appears to have the more recent fix - but hopefully by marking these as duplicates it will raise the profile of issue a little higher to the dev team and perhaps they can integrate the most recent patch:

https://launchpadlibrarian.net/73763550/hdparm-functions.patch

into the code base.

Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (4.3 KiB)

This bug was fixed in the package hdparm - 9.42-1ubuntu1

---------------
hdparm (9.42-1ubuntu1) raring; urgency=low

  * Merge from Debian unstable:
    + Adds support to the udev script for hdparm.conf stanzas that point at
      a symlink of the device rather than the device itself. LP: #222458.
  * Remaining changes:
    + debian/control:
      - Add Breaks on old udev
      - Do not depend on lsb-base (we no longer use init script)
    + debian/hpdarm.dirs:
      - Drop unused /etc/udev/rules.d
    + debian/rules, debian/hdparm.udev-script:
      - Install udev-script, drop init file stuff
    + debian/rules:
      - Install a suspend.d symlink to also call the pm-utils hook on
        resuming.
  * Dropped changes, included in Debian:
    + debian/control:
      - Add Homepage
      - cdbs build-dependency bump
      - Recommend powermgmt-base
    + debian/rules:
      - Pass --priority=85 to dh_installudev
    + debian/hdparm.postinst, debian/rules: move udev rules to /lib/udev
    + Addition of debian/95hdparm-apm and debian/hdparm-functions
    + 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.
      - Don't call hdparm -f for every drive
      - don't call sync
    + debian/hdparm.install:
      - Use hpdarm.udev instead of .rules
    + debian/hdparm.conf, debian/hdparm.conf.5: drop 'command_line' example
  * Dropped changes:
    - pm-utils Breaks/Replaces: Ubuntu-specific and no longer needed for
      upgrades.
    - adding lib/udev for our udev-script: dh_installudev takes care of this
      for us.
    - Remove broken symlink on upgrade: pre-lucid upgrades, no longer needed.
  * Don't trigger the udev rule on sd[a-z]*; yes, this means we aren't
    handling /dev/sdaa, but it also means we avoid spurious triggers on
    /dev/sda{1,2,...}

hdparm (9.42-1) unstable; urgency=low

  * New upstream release
  * Allow udev rule to process large numbers of devices (closes: #675624)
  * Changed hdparm-functions to allow links instead of devices in hdparm.conf.
  * Taken from Ubuntu:
    Set spindown policy back to -B128 instead of -B127. Too many drives
    misbehave too badly with this setting, possibly leading to drive failure.
    We should revisit this once we understand why people's drives are spinning
    back *up* so frequently (closes: #684241)

hdparm (9.39-1) unstable; urgency=low

  * New upstream version (closes: #602522, #631614, #639660)
  * Added myself to uploaders
  * Added Homepage field (closes: #533190, #545005)
  * Changed linking stage to not strip binary, otherwise the debug package
    would be empty
  * Renamed XC-Package-Type to Package-Type
  * No longer install idectl, that relies on options that were removed, and
    ultrabayd, that relies on idectl (closes: #511283)
  * Install UDEV rule to /lib/udev/rules.d (closes: #563762, #622568)
  * Bumped build dependency on debhelper for correct udev handling.
  * Install wiper.sh into its own subdirectory...

Read more...

Changed in hdparm (Ubuntu):
status: Confirmed → Fix Released
Revision history for this message
Tom Fields (udzelem) wrote :

Hi Guys,

I know this bug has been fixed one year ago, but here is one further point for the /wishlist/:

==> Add documentation of the new features (specify devices by UUID e.g.) to hdparm manual page
        and to the description header text in /etc/hdparm.conf...

Revision history for this message
Ralph Corderoy (ralph-inputplus) wrote :

Hi Tom, This bug is "Fix Released". No further work will be done on it. Your comment will be ignored. You need to find a closely related open bug to request it or open a new one.

Revision history for this message
Tom Fields (udzelem) wrote :

Hi Ralph,

done: Bug #1268452

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.