Arch Linux

Please read this before reporting a bug:
https://wiki.archlinux.org/title/Bug_reporting_guidelines

Do NOT report bugs when a package is just outdated, or it is in the AUR. Use the 'flag out of date' link on the package page, or the Mailing List.

REPEAT: Do NOT report bugs for outdated packages!
Tasklist

FS#18177 - [lm_sensors] fancontrol daemon won't report failure

Attached to Project: Arch Linux
Opened by Pericles (watsonalgas) - Friday, 05 February 2010, 12:57 GMT
Last edited by Dan Griffiths (Ghost1227) - Monday, 10 May 2010, 20:37 GMT
Task Type Bug Report
Category Packages: Extra
Status Closed
Assigned To Eric Belanger (Snowman)
Dan Griffiths (Ghost1227)
Architecture x86_64
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 1
Private No

Details

Description:

I reported the fancontrol  FS#17775  earlier, and thought I had a proper fix, but now with the lm_sensors upgrade, fancontrol was failing but the daemon reported "Done". I'm sure it has to do with that same fix for the other bug, so it's just a problem with the daemon script's error handling.

Additional info:
* package version -- 3.1.2-1

Steps to reproduce:
If you upgrade lm_sensors, and run fancontrol without updating your /etc/fancontrol config, the daemon will run but fancontrol itself will fail to run.
This task depends upon

Closed by  Dan Griffiths (Ghost1227)
Monday, 10 May 2010, 20:37 GMT
Reason for closing:  Implemented
Comment by Eric Belanger (Snowman) - Friday, 05 February 2010, 21:08 GMT
I don't have the hardware to test. Try this and report if it works.
Replace:

[ -z "$PID" ] && /usr/sbin/fancontrol >/dev/null 2>&1 &
if [ $? -gt 0 ]; then

by:

[ -z "$PID" ] && /usr/sbin/fancontrol >/dev/null 2>&1 &
if [ $? -gt 0 -o -z "$PID" ]; then

Comment by Pericles (watsonalgas) - Friday, 05 February 2010, 21:41 GMT
No, it didn't work. With that it reports fail even if it starts successfully.
Comment by Eric Belanger (Snowman) - Friday, 05 February 2010, 22:25 GMT
I'll probably need to ask another dev to look into it. Just curious: If you change it back to what it was before the previous bug report, does it work?

[ -z "$PID" ] && /usr/sbin/fancontrol >/dev/null 2>&1 &
if [ -z "$PID" -o $? -gt 0 ]; then
Comment by Pericles (watsonalgas) - Saturday, 06 February 2010, 03:17 GMT
With that it says it fails even though it starts successfully.
Comment by PyroPeter (pyropeter) - Sunday, 28 February 2010, 01:54 GMT
This works for me:

-------
if [ $? -gt 0 -o -n "$PID" ] ; then
-------

But it does not output a "fail" if fancontrol actually fails. (This is caused by starting the deamon in the background, which makes it impossible to determine its exit-status)

The only good way to fix this is to modify the fancontrol script.
It should get a "-d" argument, which makes it start, run all checks, then crash and return >0, or fork to the background. (I don't have a clue if bash is capable of forking in a script, but as a workaround it could re-call itself in the background and exit. the second instance should most likely find valid working conditions.)
Comment by PyroPeter (pyropeter) - Sunday, 28 February 2010, 18:33 GMT
Here is a patch that adds the above mentioned commandline argument to fancontrol.

Its just a kind of POC and not tested very well.

My current initscript using the patched fancontrol looks like this:

------
PID=$(pidof -o %PPID -x /usr/sbin/fancontrol2)
case "$1" in
start)
stat_busy "Starting fancontrol"
[ -z "$PID" ] && /usr/sbin/fancontrol2 -D &>/dev/null
if [ $? -gt 0 -o -n "$PID" ] ; then
------
Comment by Paul Mattal (paul) - Saturday, 06 March 2010, 14:28 GMT
Those with the problem-- what sort of hardware _do_ we need to test this? I admit to not knowing much about lm_sensors, but if I have relevant hardware, I can try to help.
Comment by PyroPeter (pyropeter) - Sunday, 07 March 2010, 00:53 GMT
I think lm_sensors is mostly used for CPUs or mainboards. If you use a CPU in your computer, you should take a look at http://www.lm-sensors.org/wiki/Devices .

Attached is an improvment of the above posted patch. (It also covers the initscript)

Loading...