! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
"Spin down hard disks" code does not work, multiple issues
Status:
RESOLVED: WONTFIX
Product:
Xfce4-power-manager
Component:
General

Comments

Description Chris Bainbridge 2013-08-27 13:02:55 CEST
As reported downstream  at http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=714660 unchecking "Spin down hard disks" on battery power does not
work. It seems to be a bad interaction with pm-utils, which is odd because these packages are always installed together. 

hdparm -I /dev/sda shows:
    IDLE_IMMEDIATE with UNLOAD is enabled
    Advanced power management level: 127

Shouldn't one of these have changed?

Load_Cycle_Count in 'smartctl -d ata -a /dev/sda' shows disk is spinning down

With AC all is fine and the drive does not spin down. On battery it
spins down. laptop-mode-tools is not installed.

The problem seems to be interaction with pm-utils: from
/var/log/pm-powersave.log I see that when I unplug AC pm-utils
immediately runs /usr/lib/pm-utils/power.d/95hdparm-apm, which sets
"APM_level = 127" on /dev/sda.

xfce4-power-manager depends on upower which depends on pm-utils, so
this isn't some unusual package interaction, these packages are always
going to be installed together.
Comment 1 Chris Bainbridge 2013-10-16 20:55:27 CEST
How is the xfce4-power-manager spin down code meant to work? It appears to be completely broken.

src/xfpm-disks.c contains code that only enables spin down if the timeout > 120:


else if ( timeout != 0 && timeout > 120 && !disks->priv->set)
    {
    xfpm_disks_enable_spin_down_timeouts (disks, timeout);,
    }


But this code is never going to run, because the timeout defaults to 120 in src/xfpm-xfconf.c:


g_object_class_install_property (object_class,
                                     PROP_SPIN_DOWN_DISK_ON_BATTERY_TIMEOUT,
                                     g_param_spec_uint (SPIN_DOWN_ON_BATTERY_TIMEOUT,
                                                        NULL, NULL,
                            120,
                            G_MAXUINT,
                            120,
                                                        G_PARAM_READWRITE));

So, besides the interaction with pm-utils, it appears the code could never work as intended.
Comment 2 Chris Bainbridge 2013-10-16 21:15:44 CEST
It seems like this code has the battery state inverted when it runs, so it uses the wrong enabled and timeout values (AC for battery and vice versa). xfpm-disks.c contains this code:


    g_object_get (G_OBJECT (disks->priv->power),
          "on-battery", &on_battery,
          NULL);

    if ( !on_battery )
    {
    g_object_get (G_OBJECT (disks->priv->conf),
              SPIN_DOWN_ON_AC, &enabled,
              SPIN_DOWN_ON_AC_TIMEOUT, &timeout,
              NULL);
    }
    else
    {
    g_object_get (G_OBJECT (disks->priv->conf),
              SPIN_DOWN_ON_BATTERY, &enabled,
              SPIN_DOWN_ON_BATTERY_TIMEOUT, &timeout,
              NULL);
    }

    XFPM_DEBUG ("On Battery=%d spin_down_enabled=%d timeout=%d\n", on_battery, enabled, timeout);


Run 'xfce4-power-manager --no-daemon --debug' and watch the log. 

Unplug AC:

TRACE[xfpm-battery.c:148] xfpm_battery_refresh_visible(): visible=TRUE: ((XfpmShowIcon) SHOW_ICON_WHEN_BATTERY_PRESENT)
TRACE[xfpm-battery.c:284] xfpm_battery_refresh_icon(): Battery state 2
TRACE[xfpm-disks.c:147] xfpm_disks_set_spin_timeouts(): On Battery=0 spin_down_enabled=0 timeout=800

Note that system is now running on battery but it reports "On Battery = 0"!

Plug in AC:

TRACE[xfpm-battery.c:148] xfpm_battery_refresh_visible(): visible=TRUE: ((XfpmShowIcon) SHOW_ICON_WHEN_BATTERY_PRESENT)
TRACE[xfpm-battery.c:284] xfpm_battery_refresh_icon(): Battery state 1
TRACE[xfpm-disks.c:147] xfpm_disks_set_spin_timeouts(): On Battery=1 spin_down_enabled=1 timeout=120

We are now running on AC but it reports "On Battery=1"
Comment 3 Eric Koegel editbugs 2014-07-16 19:47:29 CEST
With the hard drive code being somewhat broken as you point out, and being UDisks1 which isn't maintained anymore, and Linux only, and with UDisks2 requiring a rewrite anyway, the code was dropped. Marking this bug as won't fix.

Bug #10317

Reported by:
Chris Bainbridge
Reported on: 2013-08-27
Last modified on: 2014-07-16

People

Assignee:
Ali Abdallah
CC List:
1 user

Version

Version:
1.0.11

Attachments

Additional information