LinuxQuestions.org
Share your knowledge at the LQ Wiki.
Go Back   LinuxQuestions.org > Forums > Linux Forums > Linux - Newbie
User Name
Password
Linux - Newbie This Linux forum is for members that are new to Linux.
Just starting out and have a question? If it is not in the man pages or the how-to's this is the place!

Notices


Reply
  Search this Thread
Old 04-10-2014, 03:36 AM   #1
niaban
LQ Newbie
 
Registered: Apr 2014
Distribution: Debian Testing
Posts: 3

Rep: Reputation: Disabled
Improving a bash script


Hi to all,
I started writing bash scripts recently, and my first serious attempt is this one.
Code:
 
#!/bin/bash

for disk in $(ls -la /dev/disk/by-uuid/ |grep -o "sd.*1");

do
	LoadCycle=$(smartctl -A /dev/$disk |grep Load_Cycle |awk '{print $10}')
	PowerOnHours=$(smartctl -A /dev/$disk |grep Power_On_Hours |awk '{print $10}')

	myvar=$(echo "scale=1; (60 * $PowerOnHours) / $LoadCycle" | bc)
	
	#echo $(awk '{print strtonum($myvar)}')
	echo $disk $myvar
done
Output
Code:
sdg1 1527.2
sdk1 24.4
sda1 2048.5
sdh1 3.5
sde1 7413.2
(standard_in) 2: syntax error
sdc1
(standard_in) 2: syntax error
sdb1
sdi1 242.7
sdj1 4645.4
sdd1 480.0
sdf1 1072.2
Background and Goal:
I have assembled a server from old hardware just for samba with 12 HD. The last 2 years, 4 of these disks fell apart (WD Greens) due to the well known problem of aggressive head parking.
So i replaced them, tweaked the firmware (wdidle), and i now want to observe how often the heads are parking.
I wrote the above script. The output is in minutes. For example sdk1 24.4 says that sdk1 parks the head every 24.4 mins (on average).

My questions are two.
1. sdb1 and sdc1 are old ATA drives, so they do not report Load_Cycle through smart. So i get (standard_in) 2: syntax error. I tried to use awk strtonum to turn the error string to zero value but i couldnt manage it. Do you have any suggestions?

2. The script runs fine but it needs 4-5 secs to run. Do you have any suggestions to improve its speed? At a later point I want to feed the numbers to munin, in order to have a graphical display of the values.

Thank you in advance for your help.
 
Old 04-10-2014, 08:37 AM   #2
rtmistler
Moderator
 
Registered: Mar 2011
Location: Sutton, MA. USA
Distribution: MINT Debian, Angstrom, SUSE, Ubuntu
Posts: 4,096
Blog Entries: 10

Rep: Reputation: 1522Reputation: 1522Reputation: 1522Reputation: 1522Reputation: 1522Reputation: 1522Reputation: 1522Reputation: 1522Reputation: 1522Reputation: 1522Reputation: 1522
Welcome to LQ niaban, and I think you've done a very good job at writing a script.

I'm sure there are more sophisticated ways to do this. I say try to run smartctl on disks you know work and on disks you know don't work and check the return to the shell:
Code:
smartctl -i /dev/sdi1
echo $?
smartctl -i /dev/sdc1
echo $?
Presuming that works; if you see a different result for the attempt on sdc1 then you can use the variable $? in your script as a test variable.
Code:
#!/bin/bash

for disk in $(ls -la /dev/disk/by-uuid/ |grep -o "sd.*1");

do
        smartctl -i /dev/$disk
        if [ $? == 0 ]; then
	    LoadCycle=$(smartctl -A /dev/$disk |grep Load_Cycle |awk '{print $10}')
	    PowerOnHours=$(smartctl -A /dev/$disk |grep Power_On_Hours |awk '{print $10}')

	    myvar=$(echo "scale=1; (60 * $PowerOnHours) / $LoadCycle" | bc)
	
	    #echo $(awk '{print strtonum($myvar)}')
	    echo $disk $myvar
        fi
done
You can also just assign the output of the smartctl -i command to a variable and test that variable; that is what the shell does and the variable is $?.
 
Old 04-10-2014, 08:51 AM   #3
grail
LQ Guru
 
Registered: Sep 2009
Location: Perth
Distribution: Manjaro
Posts: 9,252

Rep: Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685
I agree with the above, except that if you only care about success or failure, then let 'if' do the work for you.

Do not use the output of ls, as it can be unpredictable. Why not just glob straight from /dev?

The use of grep and awk is not required as awk can already perform the matching task.
Code:
#!/bin/bash

for disk in /dev/sd*1
do
        if smartctl -i "$disk"; then
	    LoadCycle=$(smartctl -A "$disk" |awk '/Load_Cycle/{print $10}')
	    PowerOnHours=$(smartctl -A "$disk" | awk '/Power_On_Hours/{print $10}')

	    myvar=$(echo "scale=1; (60 * $PowerOnHours) / $LoadCycle" | bc)
	
	    #echo $(awk '{print strtonum($myvar)}')
	    echo $disk $myvar
        fi
done
Now I am not familiar with smartctl, so the if may also need to be redirected to /dev/null if you do not wish to see any output.


Edit: on a short after thought, as you run the awk on the exact same data, why not let it do the calculation as it can already handle floating point:
Code:
myvar=$(smartctl -A "$disk" |awk '/Load_Cycle/{lc = $10}/Power_On_Hours/{ph = $10}END{print (60 * ph) / lc}')

Last edited by grail; 04-10-2014 at 08:54 AM.
 
Old 04-10-2014, 01:18 PM   #4
niaban
LQ Newbie
 
Registered: Apr 2014
Distribution: Debian Testing
Posts: 3

Original Poster
Rep: Reputation: Disabled
Thank you both very much.
As, all disks are smart enabled, smartctl -i /dev/sdXX gives a result for all of them.
So, i have to stick with smartctl -A /dev/$disk | grep Load_Cycle

Indeed, if i let awk handle the calculation (as grail suggested), script goes from 5" to 2.5".
That was great!!!!

On the other hand i still had an error message for division by zero.
I followed your suggestions for the if statement and execution time went from 2.5" to 4"
At least i dont have the error.

Final code
Code:
 
for disk in $(ls /dev/sd*1);
do
  smartctl -A $disk | grep Load_Cycle >/dev/null
  if [ $? == 0 ]; then
     myvar=$(smartctl -A $disk |awk '/Load_Cycle/{lc = $10}/Power_On_Hours/{ph = $10}END{print (60 * ph) / lc}')
     echo $disk $myvar
  fi
done
 
Old 04-10-2014, 03:16 PM   #5
grail
LQ Guru
 
Registered: Sep 2009
Location: Perth
Distribution: Manjaro
Posts: 9,252

Rep: Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685Reputation: 2685
I think you missed the part about do not use ls. Also in the case where you have used it, it is completely pointless as the glob will already expand to all the devices.

As for the divide by zero, remove the if all together and have awk deliver a zero when lc is zero
Code:
for disk in /dev/sd*1
do
   echo "$disk $(smartctl -A $disk |awk '/Load_Cycle/{lc = $10}/Power_On_Hours/{ph = $10}END{if(lc)print (60 * ph) / lc; else print 0}')"
done
 
1 members found this post helpful.
Old 04-10-2014, 11:25 PM   #6
niaban
LQ Newbie
 
Registered: Apr 2014
Distribution: Debian Testing
Posts: 3

Original Poster
Rep: Reputation: Disabled
thank you both very much
 
  


Reply


Thread Tools Search this Thread
Search this Thread:

Advanced Search

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is Off
HTML code is Off



Similar Threads
Thread Thread Starter Forum Replies Last Post
[SOLVED] Improving a wrapper script threezerous Linux - Newbie 6 04-09-2014 09:07 AM
[SOLVED] [BASH]first script, suggestions for improving! RaptorX Programming 9 08-29-2009 06:11 PM
LXer: Improving bash LXer Syndicated Linux News 0 02-02-2007 03:33 AM
improving my script lpn1160 Linux - Newbie 4 06-19-2006 09:09 PM
Improving the startx script Woodsman Slackware 14 11-05-2005 09:08 AM


All times are GMT -5. The time now is 08:45 AM.

Main Menu
Advertisement
My LQ
Write for LQ
LinuxQuestions.org is looking for people interested in writing Editorials, Articles, Reviews, and more. If you'd like to contribute content, let us know.
Main Menu
Syndicate
RSS1  Latest Threads
RSS1  LQ News
Twitter: @linuxquestions
Facebook: linuxquestions Google+: linuxquestions
Open Source Consulting | Domain Registration