Bug 4607 - Typo in 'condrestart' and 'status' methods in /etc/rc.d/init.d/cpufreq
Summary: Typo in 'condrestart' and 'status' methods in /etc/rc.d/init.d/cpufreq
Status: RESOLVED FIXED
Alias: None
Product: Mageia
Classification: Unclassified
Component: RPM Packages (show other bugs)
Version: 1
Hardware: x86_64 Linux
Priority: Normal normal
Target Milestone: ---
Assignee: Kamil Rytarowski
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-20 21:41 CET by Jan Damborsky
Modified: 2012-07-11 20:52 CEST (History)
6 users (show)

See Also:
Source RPM: cpufreq-1.0-35.mga1.src.rpm
CVE:
Status comment:


Attachments

Description Jan Damborsky 2012-02-20 21:41:55 CET
Description of problem:

When playing with cpufreq init script, I have noticed that its 'status' method
returns '1' (error) even if cpufreq subsystem was successfully initialized:

$ ls -l /var/lock/subsys/cpufreq 
-rw-r--r-- 1 root root 0 Feb 20 19:54 /var/lock/subsys/cpufreq
$ /etc/rc.d/init.d/cpufreq status ; echo $?
1
$

Looking at cpufreq init script, the problem is that 'status' method (as well as
'condrestart' one) checks for existence of /var/log/subsys/cpufreq file
while /var/lock/subsys/cpufreq should be checked instead.

After I manually modified cpufreq script in following way, it started to work
as expected:

$ diff -u /tmp/cpufreq.orig /tmp/cpufreq.fixed 
--- /tmp/cpufreq.orig	2012-02-20 21:34:58.558134198 +0100
+++ /tmp/cpufreq.fixed	2012-02-20 21:33:37.981979412 +0100
@@ -48,12 +48,12 @@
 		start
 		;;
 	condrestart)
-		if [ -f /var/log/subsys/cpufreq ]; then
+		if [ -f /var/lock/subsys/cpufreq ]; then
 			restart
 		fi
 		;;
 	status)
-		[ -f /var/log/subsys/cpufreq ]
+		[ -f /var/lock/subsys/cpufreq ]
 		RETVAL=$?
 		;;
 	*)
$

How reproducible:
Always

Steps to Reproduce:

See Description above
Jan Damborsky 2012-02-20 21:43:27 CET

CC: (none) => dambi

Manuel Hiebel 2012-02-21 01:28:04 CET

CC: (none) => eandry, n54

Comment 1 Kamil Rytarowski 2012-02-21 07:10:35 CET
I'm on it.
Comment 2 Kamil Rytarowski 2012-02-21 08:03:52 CET
Fixed.
It's in 1 core/updates_testing
Kamil Rytarowski 2012-02-21 08:05:31 CET

Assignee: bugsquad => qa-bugs

Comment 3 Bit Twister 2012-02-21 14:33:57 CET
(In reply to comment #1)
> I'm on it.

Would the cpufreq rpm be the one to fix bug 4196 ?

CC: (none) => junk_no_spam

Comment 4 Dave Hodgins 2012-02-22 00:08:06 CET
(In reply to comment #2)
> Fixed.
> It's in 1 core/updates_testing

One more change needed
# service cpufreq condrestart
/etc/init.d/cpufreq: line 52: restart: command not found

The restart command should be changed to real_stop.

CC: (none) => davidwhodgins

Comment 5 Dave Hodgins 2012-02-28 04:51:06 CET
Re-assigning back to developer.  Please re-assign to qa after taking
care of comment 4.

Assignee: qa-bugs => n54
CC: (none) => qa-bugs

Comment 6 Kamil Rytarowski 2012-02-28 07:12:49 CET
I will try to fix it again,
Comment 7 Kamil Rytarowski 2012-03-13 02:33:21 CET
Fixed in Cauldron, please test.
Bit Twister 2012-03-13 03:04:46 CET

CC: junk_no_spam => (none)

Kamil Rytarowski 2012-03-13 10:00:29 CET

Assignee: n54 => qa-bugs

Comment 8 José Jorge 2012-03-13 22:09:54 CET
(In reply to comment #7)
> Fixed in Cauldron, please test.

This is a MGA1 bug, not CAuldron!

CC: (none) => lists.jjorge

Comment 9 Kamil Rytarowski 2012-03-13 22:15:05 CET
(In reply to comment #8)
> (In reply to comment #7)
> > Fixed in Cauldron, please test.
> 
> This is a MGA1 bug, not CAuldron!

It is valid for Cauldron too.
Comment 10 José Jorge 2012-03-13 22:18:03 CET
(In reply to comment #9)
> It is valid for Cauldron too.

Ok, but saying please test in this bug report, without anything in updates_testing won't help.
Comment 11 Dave Hodgins 2012-03-15 23:31:39 CET
(In reply to comment #7)
> Fixed in Cauldron, please test.

I've tested the version in cauldron, and all options work.
The status doesn't provide any output, but now terminates
with a return code of zero.

Now we just need the Mageia 1 version updated.
Comment 12 Kamil Rytarowski 2012-03-15 23:36:49 CET
(In reply to comment #11)
> (In reply to comment #7)
> > Fixed in Cauldron, please test.
> 
> I've tested the version in cauldron, and all options work.
> The status doesn't provide any output, but now terminates
> with a return code of zero.
> 
> Now we just need the Mageia 1 version updated.

Thank you, I will push it into 1.
Comment 13 Kamil Rytarowski 2012-03-16 00:02:04 CET
(In reply to comment #11)
> Now we just need the Mageia 1 version updated.

OK, the package is pushed into 1/updates_testing. Please validate it.
Comment 14 Manuel Hiebel 2012-03-16 00:04:18 CET
it's a recent patch for fixing this bug that make a critical one ?
https://bugs.mageia.org/show_bug.cgi?id=4772
Comment 15 Kamil Rytarowski 2012-03-16 00:32:09 CET
(In reply to comment #14)
> it's a recent patch for fixing this bug that make a critical one ?
> https://bugs.mageia.org/show_bug.cgi?id=4772

I really doubt it. It could be related to kernel or hardware.
Comment 16 Kamil Rytarowski 2012-03-16 00:33:21 CET
(In reply to comment #14)
> it's a recent patch for fixing this bug that make a critical one ?
> https://bugs.mageia.org/show_bug.cgi?id=4772

The patch isn't changing anything just condrestart and status.
Comment 17 Dave Hodgins 2012-03-16 03:34:57 CET
The latest update also changed the default from performance
to on-demand.  That should be mentioned in an advisory.

Testing complete on i586 for the srpm
cpufreq-1.0-35.2.mga1.src.rpm

Checked "service cpufreq start|stop|restart|condrestart|status"

Although the status doesn't return any output, running
echo $?
right after the service cpufreq status is returning zero.
Comment 18 José Jorge 2012-03-16 18:06:59 CET
(In reply to comment #17)
> The latest update also changed the default from performance
> to on-demand.  That should be mentioned in an advisory.
> 
What? cpufreq point is to activate on_demand on processors to support it. It has never defaulted to performace, as this is simply default without cpufreq...
Comment 19 Dave Hodgins 2012-03-16 21:30:57 CET
(In reply to comment #18)
> (In reply to comment #17)
> > The latest update also changed the default from performance
> > to on-demand.  That should be mentioned in an advisory.
> > 
> What? cpufreq point is to activate on_demand on processors to support it. It
> has never defaulted to performace, as this is simply default without cpufreq...

Sorry, my mistake.  I must have modified it previously and forgotten.
Comment 20 claire robinson 2012-03-22 15:21:26 CET
Testing x86_64

# service cpufreq start
Setting CPU frequency settings:                                   [  OK  ]
# service cpufreq stop
# service cpufreq status
# service cpufreq condrestart
Resetting CPU frequency settings:                                 [  OK  ]
Setting CPU frequency settings:                                   [  OK  ]
# service cpufreq condrestart
Resetting CPU frequency settings:                                 [  OK  ]
Setting CPU frequency settings:                                   [  OK  ]
# service cpufreq status
# service cpufreq restart
Resetting CPU frequency settings:                                 [  OK  ]
Setting CPU frequency settings:                                   [  OK  ]
# service cpufreq status
#

I don't know if this means it is working properly. Condrestart now does something, as does start and restart. Status and Stop appear to do nothing.

# ll /var/lock/subsys/cpufreq
-rw-r--r-- 1 root root 0 Mar 22 14:12 /var/lock/subsys/cpufreq
# service cpufreq stop
# ll /var/lock/subsys/cpufreq
-rw-r--r-- 1 root root 0 Mar 22 14:12 /var/lock/subsys/cpufreq

The stop case is empty in the init script, I think it should probably also be real_stop
Comment 21 claire robinson 2012-03-22 15:24:03 CET
After changing it to real_stop..

# service cpufreq stop
Resetting CPU frequency settings:                                 [  OK  ]
# ll /var/lock/subsys/cpufreq
ls: cannot access /var/lock/subsys/cpufreq: No such file or directory
Comment 22 claire robinson 2012-04-15 18:24:57 CEST
Please reassign when you've had a chance to look at this Kamil. Thanks.

Assignee: qa-bugs => n54

Comment 23 Kamil Rytarowski 2012-04-15 18:49:01 CEST
OK, I will have a look.
Comment 24 Marja Van Waes 2012-07-06 15:06:02 CEST
Please look at the bottom of this mail to see whether you're the assignee of this  bug, if you don't already know whether you are.


If you're the assignee:

We'd like to know for sure whether this bug was assigned correctly. Please change status to ASSIGNED if it is, or put OK on the whiteboard instead.

If you don't have a clue and don't see a way to find out, then please put NEEDHELP on the whiteboard.

Please assign back to Bug Squad or to the correct person to solve this bug if we were wrong to assign it to you, and explain why.

Thanks :)

**************************** 

@ the reporter and persons in the cc of this bug:

If you have any new information that wasn't given before (like this bug being valid for another version of Mageia, too, or it being solved) please tell us.

@ the reporter of this bug

If you didn't reply yet to a request for more information, please do so within two weeks from now.

Thanks all :-D
Comment 25 José Jorge 2012-07-11 20:52:29 CEST
As far as I've followed, this was fixed.

Status: NEW => RESOLVED
Resolution: (none) => FIXED


Note You need to log in before you can comment on or make changes to this bug.