* [PATCH 3/4] nfs-utils: nfs-iostat.py autofs cleanup and option to sort by ops/s
@ 2009-09-15 4:57 Lans Carstensen
0 siblings, 0 replies; 6+ messages in thread
From: Lans Carstensen @ 2009-09-15 4:57 UTC (permalink / raw)
To: linux-nfs
Introduce optparse for managing command usage/help and the statistics
options. This change helps more cleanly add new options such as --sort
while preserving the iostat-like interval, count, and mount point
positional arguments.
Signed-off-by: Lans Carstensen <Lans.Carstensen-hCDZnVt6e3JSwrhanM7KvQ@public.gmane.org>
---
tools/nfs-iostat/nfs-iostat.py | 112
++++++++++++++++++++-------------------
1 files changed, 57 insertions(+), 55 deletions(-)
diff --git a/tools/nfs-iostat/nfs-iostat.py b/tools/nfs-iostat/nfs-iostat.py
index b3f335a..fc2cac8 100755
--- a/tools/nfs-iostat/nfs-iostat.py
+++ b/tools/nfs-iostat/nfs-iostat.py
@@ -21,6 +21,7 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston,
MA 02111-1307 USA
"""
import sys, os, time
+from optparse import OptionParser, OptionGroup
Iostats_version = '0.2'
@@ -395,33 +396,6 @@ class DeviceData:
# Functions
#
-def print_iostat_help(name):
- print 'usage: %s [ <interval> [ <count> ] ] [ <options> ] [ <mount
point> ] ' % name
- print
- print ' Version %s' % Iostats_version
- print
- print ' Sample iostat-like program to display NFS client per-mount
statistics.'
- print
- print ' The <interval> parameter specifies the amount of time in
seconds between'
- print ' each report. The first report contains statistics for the
time since each'
- print ' file system was mounted. Each subsequent report contains
statistics'
- print ' collected during the interval since the previous report.'
- print
- print ' If the <count> parameter is specified, the value of <count>
determines the'
- print ' number of reports generated at <interval> seconds apart.
If the interval'
- print ' parameter is specified without the <count> parameter, the
command generates'
- print ' reports continuously.'
- print
- print ' Options include "--attr", which displays statistics related
to the attribute'
- print ' cache, "--dir", which displays statistics related to
directory operations,'
- print ' and "--page", which displays statistics related to the page
cache.'
- print ' By default, if no option is specified, statistics related
to file I/O are'
- print ' displayed.'
- print
- print ' If one or more <mount point> names are specified,
statistics for only these'
- print ' mount points will be displayed. Otherwise, all NFS mount
points on the'
- print ' client are listed.'
-
def parse_stats_file(filename):
"""pop the contents of a mountstats file into a dictionary,
keyed by mount point. each value object is a list of the
@@ -491,30 +465,50 @@ def iostat_command(name):
mountstats = parse_stats_file('/proc/self/mountstats')
devices = []
origdevices = []
- which = 0
interval_seen = False
count_seen = False
- for arg in sys.argv:
- if arg in ['-h', '--help', 'help', 'usage']:
- print_iostat_help(name)
- return
-
- if arg in ['-v', '--version', 'version']:
- print '%s version %s' % (name, Iostats_version)
- return
-
- if arg in ['-a', '--attr']:
- which = 1
- continue
-
- if arg in ['-d', '--dir']:
- which = 2
- continue
-
- if arg in ['-p', '--page']:
- which = 3
- continue
+ mydescription= """
+Sample iostat-like program to display NFS client per-mount'
+statistics. The <interval> parameter specifies the amount of time in
seconds
+between each report. The first report contains statistics for the time
since
+each file system was mounted. Each subsequent report contains statistics
+collected during the interval since the previous report. If the <count>
+parameter is specified, the value of <count> determines the number of
reports
+generated at <interval> seconds apart. If the interval parameter is
specified
+without the <count> parameter, the command generates reports continuously.
+If one or more <mount point> names are specified, statistics for only these
+mount points will be displayed. Otherwise, all NFS mount points on the
+client are listed.
+"""
+ parser = OptionParser(
+ usage="usage: %prog [ <interval> [ <count> ] ] [ <options> ] [
<mount point> ]",
+ description=mydescription,
+ version='version %s' % Iostats_version)
+ parser.set_defaults(which=0)
+
+ statgroup = OptionGroup(parser, "Statistics Options",
+ 'File I/O is displayed unless one of the
following is specified:')
+ statgroup.add_option('-a', '--attr',
+ action="store_const",
+ dest="which",
+ const=1,
+ help='displays statistics related to the
attribute cache')
+ statgroup.add_option('-d', '--dir',
+ action="store_const",
+ dest="which",
+ const=2,
+ help='displays statistics related to
directory operations')
+ statgroup.add_option('-p', '--page',
+ action="store_const",
+ dest="which",
+ const=3,
+ help='displays statistics related to the
page cache')
+ parser.add_option_group(statgroup)
+
+ (options, args) = parser.parse_args(sys.argv)
+
+ for arg in args:
if arg == sys.argv[0]:
continue
@@ -522,18 +516,26 @@ def iostat_command(name):
if arg in mountstats:
origdevices += [arg]
elif not interval_seen:
- interval = int(arg)
+ try:
+ interval = int(arg)
+ except:
+ print 'Illegal <interval> value %s' % arg
+ return
if interval > 0:
interval_seen = True
else:
- print 'Illegal <interval> value'
+ print 'Illegal <interval> value %s' % arg
return
elif not count_seen:
- count = int(arg)
+ try:
+ count = int(arg)
+ except:
+ print 'Ilegal <count> value %s' % arg
+ return
if count > 0:
count_seen = True
else:
- print 'Illegal <count> value'
+ print 'Illegal <count> value %s' % arg
return
# make certain devices contains only NFS mount points
@@ -547,12 +549,12 @@ def iostat_command(name):
sample_time = 0.0
if not interval_seen:
- print_iostat_summary(old_mountstats, mountstats, devices,
sample_time, which)
+ print_iostat_summary(old_mountstats, mountstats, devices,
sample_time, options.which)
return
if count_seen:
while count != 0:
- print_iostat_summary(old_mountstats, mountstats, devices,
sample_time, which)
+ print_iostat_summary(old_mountstats, mountstats, devices,
sample_time, options.which)
old_mountstats = mountstats
time.sleep(interval)
sample_time = interval
@@ -566,7 +568,7 @@ def iostat_command(name):
count -= 1
else:
while True:
- print_iostat_summary(old_mountstats, mountstats, devices,
sample_time, which)
+ print_iostat_summary(old_mountstats, mountstats, devices,
sample_time, options.which)
old_mountstats = mountstats
time.sleep(interval)
sample_time = interval
--
1.5.5.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] nfs-utils: nfs-iostat.py autofs cleanup and option to sort by ops/s
@ 2009-08-26 4:59 Lans Carstensen
2009-08-26 14:30 ` Chuck Lever
2009-08-26 21:12 ` Chuck Lever
0 siblings, 2 replies; 6+ messages in thread
From: Lans Carstensen @ 2009-08-26 4:59 UTC (permalink / raw)
To: NFS list
commit ef558b7b978418ac3da35e36dcf02a223e4ebe2d
Author: Lans Carstensen <Lans.Carstensen@dreamworks.com>
Date: Tue Aug 25 21:52:49 2009 -0700
Update list of mount points to parse stats for and drop mount
points from stats collection when they are umounted. This
ensures proper stats collection for autofs mountpoints.
diff --git a/tools/nfs-iostat/nfs-iostat.py b/tools/nfs-iostat/nfs-iostat.py
index 6ce31fc..9f3b3eb 100644
--- a/tools/nfs-iostat/nfs-iostat.py
+++ b/tools/nfs-iostat/nfs-iostat.py
@@ -447,7 +447,14 @@ def parse_stats_file(filename):
return ms_dict
def print_iostat_summary(old, new, devices, time, ac):
- for device in devices:
+ if old:
+ # Trim device list to only include intersection of old and new
data,
+ # this addresses umounts due to autofs mountpoints
+ devicelist = filter(lambda x:x in devices,old)
+ else:
+ devicelist = devices
+
+ for device in devicelist:
stats = DeviceData()
stats.parse_stats(new[device])
if not old:
@@ -458,11 +465,35 @@ def print_iostat_summary(old, new, devices, time, ac):
diff_stats = stats.compare_iostats(old_stats)
diff_stats.display_iostats(time, ac)
+def list_nfs_mounts(givenlist, mountstats):
+ """return a list of NFS mounts given a list to validate or
+ return a full list if the given list is empty
+ """
+ list = []
+ if len(givenlist) > 0:
+ for device in givenlist:
+ stats = DeviceData()
+ stats.parse_stats(mountstats[device])
+ if stats.is_nfs_mountpoint():
+ list += [device]
+ else:
+ for device, descr in mountstats.iteritems():
+ stats = DeviceData()
+ stats.parse_stats(descr)
+ if stats.is_nfs_mountpoint():
+ list += [device]
+ if len(list) == 0:
+ print 'No NFS mount points were found'
+ return
+
+ return list
+
def iostat_command(name):
"""iostat-like command for NFS mount points
"""
mountstats = parse_stats_file('/proc/self/mountstats')
devices = []
+ origdevices = []
which = 0
interval_seen = False
count_seen = False
@@ -492,7 +523,7 @@ def iostat_command(name):
continue
if arg in mountstats:
- devices += [arg]
+ origdevices += [arg]
elif not interval_seen:
interval = int(arg)
if interval > 0:
@@ -509,23 +540,7 @@ def iostat_command(name):
return
# make certain devices contains only NFS mount points
- if len(devices) > 0:
- check = []
- for device in devices:
- stats = DeviceData()
- stats.parse_stats(mountstats[device])
- if stats.is_nfs_mountpoint():
- check += [device]
- devices = check
- else:
- for device, descr in mountstats.iteritems():
- stats = DeviceData()
- stats.parse_stats(descr)
- if stats.is_nfs_mountpoint():
- devices += [device]
- if len(devices) == 0:
- print 'No NFS mount points were found'
- return
+ devices = list_nfs_mounts(origdevices, mountstats)
old_mountstats = None
sample_time = 0.0
@@ -541,6 +556,9 @@ def iostat_command(name):
time.sleep(interval)
sample_time = interval
mountstats = parse_stats_file('/proc/self/mountstats')
+ # automount mountpoints add and drop, if automount is involved
+ # we need to recheck the devices list when reparsing
+ devices = list_nfs_mounts(origdevices,mountstats)
count -= 1
else:
while True:
@@ -549,6 +567,9 @@ def iostat_command(name):
time.sleep(interval)
sample_time = interval
mountstats = parse_stats_file('/proc/self/mountstats')
+ # automount mountpoints add and drop, if automount is involved
+ # we need to recheck the devices list when reparsing
+ devices = list_nfs_mounts(origdevices,mountstats)
#
# Main
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3/4] nfs-utils: nfs-iostat.py autofs cleanup and option to sort by ops/s
2009-08-26 4:59 Lans Carstensen
@ 2009-08-26 14:30 ` Chuck Lever
2009-08-26 21:12 ` Chuck Lever
1 sibling, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2009-08-26 14:30 UTC (permalink / raw)
To: Lans Carstensen; +Cc: NFS list
I'd like to try this one out so I'm sure I understand what's going on.
On Aug 26, 2009, at 12:59 AM, Lans Carstensen wrote:
> commit ef558b7b978418ac3da35e36dcf02a223e4ebe2d
> Author: Lans Carstensen <Lans.Carstensen@dreamworks.com>
> Date: Tue Aug 25 21:52:49 2009 -0700
>
> Update list of mount points to parse stats for and drop mount
> points from stats collection when they are umounted. This
> ensures proper stats collection for autofs mountpoints.
>
> diff --git a/tools/nfs-iostat/nfs-iostat.py b/tools/nfs-iostat/nfs-
> iostat.py
> index 6ce31fc..9f3b3eb 100644
> --- a/tools/nfs-iostat/nfs-iostat.py
> +++ b/tools/nfs-iostat/nfs-iostat.py
> @@ -447,7 +447,14 @@ def parse_stats_file(filename):
> return ms_dict
>
> def print_iostat_summary(old, new, devices, time, ac):
> - for device in devices:
> + if old:
> + # Trim device list to only include intersection of old and
> new data,
> + # this addresses umounts due to autofs mountpoints
> + devicelist = filter(lambda x:x in devices,old)
> + else:
> + devicelist = devices
> +
> + for device in devicelist:
> stats = DeviceData()
> stats.parse_stats(new[device])
> if not old:
> @@ -458,11 +465,35 @@ def print_iostat_summary(old, new, devices,
> time, ac):
> diff_stats = stats.compare_iostats(old_stats)
> diff_stats.display_iostats(time, ac)
>
> +def list_nfs_mounts(givenlist, mountstats):
> + """return a list of NFS mounts given a list to validate or
> + return a full list if the given list is empty
> + """
> + list = []
> + if len(givenlist) > 0:
> + for device in givenlist:
> + stats = DeviceData()
> + stats.parse_stats(mountstats[device])
> + if stats.is_nfs_mountpoint():
> + list += [device]
> + else:
> + for device, descr in mountstats.iteritems():
> + stats = DeviceData()
> + stats.parse_stats(descr)
> + if stats.is_nfs_mountpoint():
> + list += [device]
> + if len(list) == 0:
> + print 'No NFS mount points were found'
> + return
> +
> + return list
> +
> def iostat_command(name):
> """iostat-like command for NFS mount points
> """
> mountstats = parse_stats_file('/proc/self/mountstats')
> devices = []
> + origdevices = []
> which = 0
> interval_seen = False
> count_seen = False
> @@ -492,7 +523,7 @@ def iostat_command(name):
> continue
>
> if arg in mountstats:
> - devices += [arg]
> + origdevices += [arg]
> elif not interval_seen:
> interval = int(arg)
> if interval > 0:
> @@ -509,23 +540,7 @@ def iostat_command(name):
> return
>
> # make certain devices contains only NFS mount points
> - if len(devices) > 0:
> - check = []
> - for device in devices:
> - stats = DeviceData()
> - stats.parse_stats(mountstats[device])
> - if stats.is_nfs_mountpoint():
> - check += [device]
> - devices = check
> - else:
> - for device, descr in mountstats.iteritems():
> - stats = DeviceData()
> - stats.parse_stats(descr)
> - if stats.is_nfs_mountpoint():
> - devices += [device]
> - if len(devices) == 0:
> - print 'No NFS mount points were found'
> - return
> + devices = list_nfs_mounts(origdevices, mountstats)
>
> old_mountstats = None
> sample_time = 0.0
> @@ -541,6 +556,9 @@ def iostat_command(name):
> time.sleep(interval)
> sample_time = interval
> mountstats = parse_stats_file('/proc/self/mountstats')
> + # automount mountpoints add and drop, if automount is
> involved
> + # we need to recheck the devices list when reparsing
> + devices = list_nfs_mounts(origdevices,mountstats)
> count -= 1
> else:
> while True:
> @@ -549,6 +567,9 @@ def iostat_command(name):
> time.sleep(interval)
> sample_time = interval
> mountstats = parse_stats_file('/proc/self/mountstats')
> + # automount mountpoints add and drop, if automount is
> involved
> + # we need to recheck the devices list when reparsing
> + devices = list_nfs_mounts(origdevices,mountstats)
>
> #
> # Main
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3/4] nfs-utils: nfs-iostat.py autofs cleanup and option to sort by ops/s
2009-08-26 4:59 Lans Carstensen
2009-08-26 14:30 ` Chuck Lever
@ 2009-08-26 21:12 ` Chuck Lever
2009-08-26 23:57 ` Lans Carstensen
1 sibling, 1 reply; 6+ messages in thread
From: Chuck Lever @ 2009-08-26 21:12 UTC (permalink / raw)
To: Lans Carstensen; +Cc: NFS list
On Aug 26, 2009, at 12:59 AM, Lans Carstensen wrote:
> commit ef558b7b978418ac3da35e36dcf02a223e4ebe2d
> Author: Lans Carstensen <Lans.Carstensen@dreamworks.com>
> Date: Tue Aug 25 21:52:49 2009 -0700
>
> Update list of mount points to parse stats for and drop mount
> points from stats collection when they are umounted. This
> ensures proper stats collection for autofs mountpoints.
>
> diff --git a/tools/nfs-iostat/nfs-iostat.py b/tools/nfs-iostat/nfs-
> iostat.py
> index 6ce31fc..9f3b3eb 100644
> --- a/tools/nfs-iostat/nfs-iostat.py
> +++ b/tools/nfs-iostat/nfs-iostat.py
> @@ -447,7 +447,14 @@ def parse_stats_file(filename):
> return ms_dict
>
> def print_iostat_summary(old, new, devices, time, ac):
> - for device in devices:
> + if old:
> + # Trim device list to only include intersection of old and
> new data,
> + # this addresses umounts due to autofs mountpoints
> + devicelist = filter(lambda x:x in devices,old)
> + else:
> + devicelist = devices
> +
> + for device in devicelist:
> stats = DeviceData()
> stats.parse_stats(new[device])
> if not old:
> @@ -458,11 +465,35 @@ def print_iostat_summary(old, new, devices,
> time, ac):
> diff_stats = stats.compare_iostats(old_stats)
> diff_stats.display_iostats(time, ac)
>
> +def list_nfs_mounts(givenlist, mountstats):
> + """return a list of NFS mounts given a list to validate or
> + return a full list if the given list is empty
> + """
> + list = []
> + if len(givenlist) > 0:
> + for device in givenlist:
> + stats = DeviceData()
> + stats.parse_stats(mountstats[device])
> + if stats.is_nfs_mountpoint():
> + list += [device]
> + else:
> + for device, descr in mountstats.iteritems():
> + stats = DeviceData()
> + stats.parse_stats(descr)
> + if stats.is_nfs_mountpoint():
> + list += [device]
> + if len(list) == 0:
> + print 'No NFS mount points were found'
> + return
You refactored this logic from the main routine. I think this last
"return" was OK in the main routine, but in a subroutine it may not do
exactly what you want. A NoneType object is passed to
print_iostat_summary in this case.
[cel@matisse tmp]$ ./nfs-iostat.py
No NFS mount points were found
Traceback (most recent call last):
File "./nfs-iostat.py", line 580, in <module>
iostat_command(prog)
File "./nfs-iostat.py", line 549, in iostat_command
print_iostat_summary(old_mountstats, mountstats, devices,
sample_time, which)
File "./nfs-iostat.py", line 457, in print_iostat_summary
for device in devicelist:
TypeError: 'NoneType' object is not iterable
> +
> + return list
> +
> def iostat_command(name):
> """iostat-like command for NFS mount points
> """
> mountstats = parse_stats_file('/proc/self/mountstats')
> devices = []
> + origdevices = []
> which = 0
> interval_seen = False
> count_seen = False
> @@ -492,7 +523,7 @@ def iostat_command(name):
> continue
>
> if arg in mountstats:
> - devices += [arg]
> + origdevices += [arg]
> elif not interval_seen:
> interval = int(arg)
> if interval > 0:
> @@ -509,23 +540,7 @@ def iostat_command(name):
> return
>
> # make certain devices contains only NFS mount points
> - if len(devices) > 0:
> - check = []
> - for device in devices:
> - stats = DeviceData()
> - stats.parse_stats(mountstats[device])
> - if stats.is_nfs_mountpoint():
> - check += [device]
> - devices = check
> - else:
> - for device, descr in mountstats.iteritems():
> - stats = DeviceData()
> - stats.parse_stats(descr)
> - if stats.is_nfs_mountpoint():
> - devices += [device]
> - if len(devices) == 0:
> - print 'No NFS mount points were found'
> - return
> + devices = list_nfs_mounts(origdevices, mountstats)
>
> old_mountstats = None
> sample_time = 0.0
> @@ -541,6 +556,9 @@ def iostat_command(name):
> time.sleep(interval)
> sample_time = interval
> mountstats = parse_stats_file('/proc/self/mountstats')
> + # automount mountpoints add and drop, if automount is
> involved
> + # we need to recheck the devices list when reparsing
> + devices = list_nfs_mounts(origdevices,mountstats)
> count -= 1
> else:
> while True:
> @@ -549,6 +567,9 @@ def iostat_command(name):
> time.sleep(interval)
> sample_time = interval
> mountstats = parse_stats_file('/proc/self/mountstats')
> + # automount mountpoints add and drop, if automount is
> involved
> + # we need to recheck the devices list when reparsing
> + devices = list_nfs_mounts(origdevices,mountstats)
>
> #
> # Main
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3/4] nfs-utils: nfs-iostat.py autofs cleanup and option to sort by ops/s
2009-08-26 21:12 ` Chuck Lever
@ 2009-08-26 23:57 ` Lans Carstensen
2009-08-27 14:09 ` Chuck Lever
0 siblings, 1 reply; 6+ messages in thread
From: Lans Carstensen @ 2009-08-26 23:57 UTC (permalink / raw)
To: Chuck Lever; +Cc: NFS list
Chuck Lever wrote:
>
> On Aug 26, 2009, at 12:59 AM, Lans Carstensen wrote:
>
>> commit ef558b7b978418ac3da35e36dcf02a223e4ebe2d
>> Author: Lans Carstensen <Lans.Carstensen@dreamworks.com>
>> Date: Tue Aug 25 21:52:49 2009 -0700
>>
>> Update list of mount points to parse stats for and drop mount
>> points from stats collection when they are umounted. This
>> ensures proper stats collection for autofs mountpoints.
>>
>> diff --git a/tools/nfs-iostat/nfs-iostat.py
>> b/tools/nfs-iostat/nfs-iostat.py
>> index 6ce31fc..9f3b3eb 100644
>> --- a/tools/nfs-iostat/nfs-iostat.py
>> +++ b/tools/nfs-iostat/nfs-iostat.py
>> @@ -447,7 +447,14 @@ def parse_stats_file(filename):
>> return ms_dict
>>
>> def print_iostat_summary(old, new, devices, time, ac):
>> - for device in devices:
>> + if old:
>> + # Trim device list to only include intersection of old and
>> new data,
>> + # this addresses umounts due to autofs mountpoints
>> + devicelist = filter(lambda x:x in devices,old)
>> + else:
>> + devicelist = devices
>> +
>> + for device in devicelist:
>> stats = DeviceData()
>> stats.parse_stats(new[device])
>> if not old:
>> @@ -458,11 +465,35 @@ def print_iostat_summary(old, new, devices,
>> time, ac):
>> diff_stats = stats.compare_iostats(old_stats)
>> diff_stats.display_iostats(time, ac)
>>
>> +def list_nfs_mounts(givenlist, mountstats):
>> + """return a list of NFS mounts given a list to validate or
>> + return a full list if the given list is empty
>> + """
>> + list = []
>> + if len(givenlist) > 0:
>> + for device in givenlist:
>> + stats = DeviceData()
>> + stats.parse_stats(mountstats[device])
>> + if stats.is_nfs_mountpoint():
>> + list += [device]
>> + else:
>> + for device, descr in mountstats.iteritems():
>> + stats = DeviceData()
>> + stats.parse_stats(descr)
>> + if stats.is_nfs_mountpoint():
>> + list += [device]
>> + if len(list) == 0:
>> + print 'No NFS mount points were found'
>> + return
>
> You refactored this logic from the main routine. I think this last
> "return" was OK in the main routine, but in a subroutine it may not do
> exactly what you want. A NoneType object is passed to
> print_iostat_summary in this case
>
> [cel@matisse tmp]$ ./nfs-iostat.py
> No NFS mount points were found
> Traceback (most recent call last):
> File "./nfs-iostat.py", line 580, in <module>
> iostat_command(prog)
> File "./nfs-iostat.py", line 549, in iostat_command
> print_iostat_summary(old_mountstats, mountstats, devices,
> sample_time, which)
> File "./nfs-iostat.py", line 457, in print_iostat_summary
> for device in devicelist:
> TypeError: 'NoneType' object is not iterable
Sigh. Thanks. I'll test more thoroughly and resubmit. Based on the
--list= argument change it'll probably be easier to refactor the script
to use optparse, so I'll end up regenerating the last two patches as
probably three new ones.
-- Lans Carstensen, Systems Engineering, Dreamworks Animation
Because they consistently observe and listen, the humble improve.
-- Wynton Marsalis
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3/4] nfs-utils: nfs-iostat.py autofs cleanup and option to sort by ops/s
2009-08-26 23:57 ` Lans Carstensen
@ 2009-08-27 14:09 ` Chuck Lever
0 siblings, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2009-08-27 14:09 UTC (permalink / raw)
To: Lans Carstensen; +Cc: NFS list
On Aug 26, 2009, at 7:57 PM, Lans Carstensen wrote:
> Chuck Lever wrote:
>> On Aug 26, 2009, at 12:59 AM, Lans Carstensen wrote:
>>> commit ef558b7b978418ac3da35e36dcf02a223e4ebe2d
>>> Author: Lans Carstensen <Lans.Carstensen@dreamworks.com>
>>> Date: Tue Aug 25 21:52:49 2009 -0700
>>>
>>> Update list of mount points to parse stats for and drop mount
>>> points from stats collection when they are umounted. This
>>> ensures proper stats collection for autofs mountpoints.
>>>
>>> diff --git a/tools/nfs-iostat/nfs-iostat.py b/tools/nfs-iostat/nfs-
>>> iostat.py
>>> index 6ce31fc..9f3b3eb 100644
>>> --- a/tools/nfs-iostat/nfs-iostat.py
>>> +++ b/tools/nfs-iostat/nfs-iostat.py
>>> @@ -447,7 +447,14 @@ def parse_stats_file(filename):
>>> return ms_dict
>>>
>>> def print_iostat_summary(old, new, devices, time, ac):
>>> - for device in devices:
>>> + if old:
>>> + # Trim device list to only include intersection of old
>>> and new data,
>>> + # this addresses umounts due to autofs mountpoints
>>> + devicelist = filter(lambda x:x in devices,old)
>>> + else:
>>> + devicelist = devices
>>> +
>>> + for device in devicelist:
>>> stats = DeviceData()
>>> stats.parse_stats(new[device])
>>> if not old:
>>> @@ -458,11 +465,35 @@ def print_iostat_summary(old, new, devices,
>>> time, ac):
>>> diff_stats = stats.compare_iostats(old_stats)
>>> diff_stats.display_iostats(time, ac)
>>>
>>> +def list_nfs_mounts(givenlist, mountstats):
>>> + """return a list of NFS mounts given a list to validate or
>>> + return a full list if the given list is empty
>>> + """
>>> + list = []
>>> + if len(givenlist) > 0:
>>> + for device in givenlist:
>>> + stats = DeviceData()
>>> + stats.parse_stats(mountstats[device])
>>> + if stats.is_nfs_mountpoint():
>>> + list += [device]
>>> + else:
>>> + for device, descr in mountstats.iteritems():
>>> + stats = DeviceData()
>>> + stats.parse_stats(descr)
>>> + if stats.is_nfs_mountpoint():
>>> + list += [device]
>>> + if len(list) == 0:
>>> + print 'No NFS mount points were found'
>>> + return
>> You refactored this logic from the main routine. I think this last
>> "return" was OK in the main routine, but in a subroutine it may not
>> do exactly what you want. A NoneType object is passed to
>> print_iostat_summary in this case
>> [cel@matisse tmp]$ ./nfs-iostat.py
>> No NFS mount points were found
>> Traceback (most recent call last):
>> File "./nfs-iostat.py", line 580, in <module>
>> iostat_command(prog)
>> File "./nfs-iostat.py", line 549, in iostat_command
>> print_iostat_summary(old_mountstats, mountstats, devices,
>> sample_time, which)
>> File "./nfs-iostat.py", line 457, in print_iostat_summary
>> for device in devicelist:
>> TypeError: 'NoneType' object is not iterable
>
> Sigh. Thanks. I'll test more thoroughly and resubmit. Based on
> the --list= argument change it'll probably be easier to refactor the
> script to use optparse, so I'll end up regenerating the last two
> patches as probably three new ones.
Thanks, these are good changes.
When you resubmit, can you add a Signed-off-by: line at the bottom of
each patch description? You can look at this page for more information:
http://kerneltrap.org/taxonomy/term/245
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-09-15 5:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-09-15 4:57 [PATCH 3/4] nfs-utils: nfs-iostat.py autofs cleanup and option to sort by ops/s Lans Carstensen
-- strict thread matches above, loose matches on Subject: below --
2009-08-26 4:59 Lans Carstensen
2009-08-26 14:30 ` Chuck Lever
2009-08-26 21:12 ` Chuck Lever
2009-08-26 23:57 ` Lans Carstensen
2009-08-27 14:09 ` Chuck Lever
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.