Bug 141958

Summary: include fan control in lm_sensors service script
Product: [openSUSE] SUSE LINUX 10.0 Reporter: Volker Kuhlmann <bugz57>
Component: BasesystemAssignee: Dr. Werner Fink <werner>
Status: RESOLVED FIXED QA Contact: E-mail List <qa-bugs>
Severity: Enhancement    
Priority: P5 - None    
Version: Final   
Target Milestone: ---   
Hardware: x86   
OS: SuSE Linux 10.0   
Whiteboard:
Found By: Other Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---
Attachments: Patch to init.d/lm_sensors and fancontrol
patch to pwmconfig, needed for some fans
/etc/init.d/lm_sensors

Description Volker Kuhlmann 2006-01-08 01:12:42 UTC
These two patches include fan control into the lm_sensors service script. They have no effect until the user explicitly configures fan control (/etc/fancontrol exists and the fancontrol script is executable).
Comment 1 Volker Kuhlmann 2006-01-08 01:14:54 UTC
Created attachment 62271 [details]
Patch to init.d/lm_sensors and fancontrol
Comment 2 Volker Kuhlmann 2006-01-08 01:15:28 UTC
Created attachment 62272 [details]
patch to pwmconfig, needed for some fans
Comment 3 Dr. Werner Fink 2006-01-10 13:07:40 UTC
AFAIS the script /usr/sbin/fancontrol does provide a /var/run/fancontrol.pid
therefore the /var/lock/subsys/sensors-fan is not required.
Comment 4 Dr. Werner Fink 2006-01-10 13:24:06 UTC
Created attachment 62647 [details]
/etc/init.d/lm_sensors

Does this script work for you?
Comment 5 Dr. Werner Fink 2006-01-10 13:37:06 UTC
The script /usr/sbin/fancontrol is missing a

     declare -i fcvcount

before the first usage of the variable fcvcount.
This because the trap later uses a loop to
restore the fan speeds.   Also the shell
function restorefans() used for this loop
should use `exit 0' at the end.

With this the sleep/wait construct should not
needed anymore. 
Comment 6 Dr. Werner Fink 2006-01-10 14:05:50 UTC
Please provide feedback on comment #3 and comment #5
Comment 7 Volker Kuhlmann 2006-01-10 22:55:28 UTC
The init.d/lm_sensors (comment #3) works fine - stop/start/restart/status.
I would tend to route the sbin/fancontrol output to /dev/null, esp for the 15 lines of output generated during start, but you decide. 

However if you do leave the fancontrol output mixed in with the service script output, add newlines to both the start and stop output of the service script when fancontrol is in use. Remove the -n from lines 72 and 98 of your script.
Comment 8 Volker Kuhlmann 2006-01-10 23:12:39 UTC
In inserted the declare -i as third non-comment line in sbin/fancontrol, and changed the exit 1 to exit 0 in restorefans(). It works fine with the sleep $INTERVAL &; wait $!, the change I made. With the original sleep $INTERVAL in the foreground, the script becomes uninterruptible, i.e. init.d/lm_sensors stop waits until the sleep terminates, and then the fan speed is NOT restored to full, I guess because the script is already dead. This is with a total change of (from SUSE 10.0)

--- /usr/sbin/fancontrol.orig   2005-09-13 12:34:49.000000000 +1200
+++ /usr/sbin/fancontrol        2006-01-11 12:06:50.000000000 +1300
@@ -46,0 +47,2 @@
+declare -i fcvcount
+
@@ -160 +162 @@
-       exit 1
+       exit 0

With the additional

@@ -280 +282,4 @@
-       sleep $INTERVAL
+       # The sleep in the foreground isn't interruptible by kill, which is
+       # unsuitable for system service scripts.  -VK 8Jan06
+       sleep $INTERVAL &
+       wait $!

it works fine, i.e. it terminates when stopped and restores the fan to full speed. I don't see how declaring fcvcount integer can affect the interrupt behaviour.
Comment 9 Dr. Werner Fink 2006-01-11 13:25:03 UTC
Just add a `-q' as option for startproc in /etc/init.d/lm_sensors
to supress the output of the fancontrol script.

The TERM problem with the fancontrol script is annoying because
there is a trap set on SIGTERM/SIGHUP which should avoid that
the script is terminated before the signal handler is done.
Only if the value of $INTERVAL is greater than 5 seconds the
killproc utility send a SIGKILL to terminate the script.
Comment 10 Dr. Werner Fink 2006-01-11 13:37:48 UTC
OK, it seems that the line

   read < <(exec sleep $INTERVAL)

does the same job.  This is a named FIFO
written in bash syntax, if the FIFO close
the read is finished.
Comment 11 Dr. Werner Fink 2006-01-11 13:47:23 UTC
FIXED for next release of SuSE Linux.