2014-11-05 17:01:14

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 00/15] A few enhancements to mountstats.py

The following patches add a couple of enhancements to mountstats.py. I
also fixed a few bugs I encountered along the way. Highlights include:

- added support for -f/--file to allow stats to be parsed from an
aritrary input file instead of /proc/self/mountstats

- added support for -S/--since to show just the changes that have
occurred between the current and a previous set of statisics (works
with and without the -f option)

- added support for -R/--raw to generate 'raw' statistics (i.e. in the
same format as /proc/self/mountstats). It's intended to be used with
the -f and -S options.

- implemented the ms-nfsstat command to generate client-side
nfsstat-like statisics (only works with a single mountpoint)

My motivation for these changes was so that I could take various copies
of /proc/self/mountstats and massage them into data that I could feed
into the 'report' option of Dros's nfsometer tool for scenarios where
it's not feasible to run nfsometer itself (e.g. systems where we can't
start with an 'idle' state (i.e. no NFS filesystems initially
mounted), systems with multiple NFS filesystems mounted, and workloads
that can't easily be boiled down into an nfsometer workload file or run
via the custom workload environment variables).

Scott Mayhew (15):
mountstats: Fix up NFS event counters
mountstats: Add lists of various counters
mountstats: Refactor __parse_nfs_line and __parse_rpc_line
mountstats: Refactor compare_iostats
mountstats: Convert existing option parsing to use the getopt module
mountstats: Make ms-iostat output match that of nfs-iostat.py
mountstats: Make print_iostat_summary handle newly appearing mounts
mountstats: Add support for -f/--file to the mountstats and ms-iostat
commands
mountstats: Add support for -S/--since to the mountstats and ms-iostat
commands
mountstats: Fix IndexError in __parse_nfs_line
mountstats: Allow mountstats_command to take a variable number of
mountpoints
mountstats: Add support for -R/--raw to mountstats_command
mountstats: Implement nfsstat_command
mountstats: Remove the --start and --end options
mountstats: Update the help output

tools/mountstats/mountstats.py | 730 ++++++++++++++++++++++++++++++-----------
1 file changed, 544 insertions(+), 186 deletions(-)
mode change 100644 => 100755 tools/mountstats/mountstats.py

--
1.9.3



2014-11-05 17:01:15

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 05/15] mountstats: Convert existing option parsing to use the getopt module

Signed-off-by: Scott Mayhew <[email protected]>
---
tools/mountstats/mountstats.py | 62 +++++++++++++++++++++---------------------
1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index 2fe1362..272a8f9 100755
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -24,6 +24,7 @@ MA 02110-1301 USA
"""

import sys, os, time
+import getopt

Mountstats_version = '0.2'

@@ -547,37 +548,33 @@ def print_mountstats_help(name):
def mountstats_command():
"""Mountstats command
"""
+ try:
+ opts, args = getopt.getopt(sys.argv[1:], "ehnrsv", ["end", "help", "nfs", "rpc", "start", "version"])
+ except getopt.GetoptError as err:
+ print_mountstats_help(prog)
+
mountpoints = []
nfs_only = False
rpc_only = False

- for arg in sys.argv:
- if arg in ['-h', '--help', 'help', 'usage']:
+ for o, a in opts:
+ if o in ("-e", "--end"):
+ raise Exception('Sampling is not yet implemented')
+ elif o in ("-h", "--help"):
print_mountstats_help(prog)
- return
-
- if arg in ['-v', '--version', 'version']:
- print('%s version %s' % (sys.argv[0], Mountstats_version))
sys.exit(0)
-
- if arg in ['-n', '--nfs']:
+ elif o in ("-n", "--nfs"):
nfs_only = True
- continue
-
- if arg in ['-r', '--rpc']:
+ elif o in ("-r", "--rpc"):
rpc_only = True
- continue
-
- if arg in ['-s', '--start']:
- raise Exception('Sampling is not yet implemented')
-
- if arg in ['-e', '--end']:
+ elif o in ("-s", "--start"):
raise Exception('Sampling is not yet implemented')
-
- if arg == sys.argv[0]:
- continue
-
- mountpoints += [arg]
+ elif o in ("-v", "--version"):
+ print('%s version %s' % (sys.argv[0], Mountstats_version))
+ sys.exit(0)
+ else:
+ assert False, "unhandled option"
+ mountpoints += args

if mountpoints == []:
print_mountstats_help(prog)
@@ -662,23 +659,26 @@ def print_iostat_summary(old, new, devices, time):
def iostat_command():
"""iostat-like command for NFS mount points
"""
+ try:
+ opts, args = getopt.getopt(sys.argv[1:], "hv", ["help", "version"])
+ except getopt.GetoptError as err:
+ print_iostat_help(prog)
mountstats = parse_stats_file('/proc/self/mountstats')
devices = []
interval_seen = False
count_seen = False

- for arg in sys.argv:
- if arg in ['-h', '--help', 'help', 'usage']:
+ for o, a in opts:
+ if o in ("-h", "--help"):
print_iostat_help(prog)
- return
-
- if arg in ['-v', '--version', 'version']:
+ sys.exit(0)
+ elif o in ("-v", "--version"):
print('%s version %s' % (sys.argv[0], Mountstats_version))
- return
-
- if arg == sys.argv[0]:
- continue
+ sys.exit(0)
+ else:
+ assert False, "unhandled option"

+ for arg in args:
if arg in mountstats:
devices += [arg]
elif not interval_seen:
--
1.9.3


2014-11-06 01:50:06

by Chuck Lever III

[permalink] [raw]
Subject: Re: [nfs-utils RFC PATCH 09/15] mountstats: Add support for -S/--since to the mountstats and ms-iostat commands


On Nov 5, 2014, at 12:01 PM, Scott Mayhew <[email protected]> wrote:

> Add support for -S/--since to the mountstats and ms-iostat commands to
> display the delta between a previous copy of /proc/self/mountstats and
> the current /proc/self/mountstats file. Can be combined with the -f
> option to show the statistics between two different points in time.

This is exactly what ?start and ?send were supposed to do.
Why not use use them?

> Signed-off-by: Scott Mayhew <[email protected]>
> ---
> tools/mountstats/mountstats.py | 61 +++++++++++++++++++++++++++++-------------
> 1 file changed, 43 insertions(+), 18 deletions(-)
>
> diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
> index 23def92..b3231b2 100755
> --- a/tools/mountstats/mountstats.py
> +++ b/tools/mountstats/mountstats.py
> @@ -562,13 +562,28 @@ def print_mountstats_help(name):
> print(' --rpc display only the RPC statistics')
> print(' --start sample and save statistics')
> print(' --end resample statistics and compare them with saved')
> + print(' --since <file> shows difference between current stats and those in \'file\'')
> print()
>
> +def print_mountstats(stats, nfs_only, rpc_only):
> + if nfs_only:
> + stats.display_nfs_options()
> + stats.display_nfs_events()
> + stats.display_nfs_bytes()
> + elif rpc_only:
> + stats.display_rpc_generic_stats()
> + stats.display_rpc_op_stats()
> + else:
> + stats.display_nfs_options()
> + stats.display_nfs_bytes()
> + stats.display_rpc_generic_stats()
> + stats.display_rpc_op_stats()
> +
> def mountstats_command():
> """Mountstats command
> """
> try:
> - opts, args = getopt.getopt(sys.argv[1:], "ef:hnrsv", ["end", "file=", "help", "nfs", "rpc", "start", "version"])
> + opts, args = getopt.getopt(sys.argv[1:], "ef:hnrsvS:", ["end", "file=", "help", "nfs", "rpc", "start", "version", "since="])
> except getopt.GetoptError as err:
> print_mountstats_help(prog)
>
> @@ -576,6 +591,7 @@ def mountstats_command():
> nfs_only = False
> rpc_only = False
> infile = None
> + since = None
>
> for o, a in opts:
> if o in ("-e", "--end"):
> @@ -594,6 +610,8 @@ def mountstats_command():
> elif o in ("-v", "--version"):
> print('%s version %s' % (sys.argv[0], Mountstats_version))
> sys.exit(0)
> + elif o in ("-S", "--since"):
> + since = a
> else:
> assert False, "unhandled option"
> mountpoints += args
> @@ -610,6 +628,9 @@ def mountstats_command():
> infile = '/proc/self/mountstats'
> mountstats = parse_stats_file(infile)
>
> + if since:
> + old_mountstats = parse_stats_file(since)
> +
> for mp in mountpoints:
> if mp not in mountstats:
> print('Statistics for mount point %s not found' % mp)
> @@ -622,18 +643,15 @@ def mountstats_command():
> print('Mount point %s exists but is not an NFS mount' % mp)
> continue
>
> - if nfs_only:
> - stats.display_nfs_options()
> - stats.display_nfs_events()
> - stats.display_nfs_bytes()
> - elif rpc_only:
> - stats.display_rpc_generic_stats()
> - stats.display_rpc_op_stats()
> + if not since:
> + print_mountstats(stats, nfs_only, rpc_only)
> + elif since and mp not in old_mountstats:
> + print_mountstats(stats, nfs_only, rpc_only)
> else:
> - stats.display_nfs_options()
> - stats.display_nfs_bytes()
> - stats.display_rpc_generic_stats()
> - stats.display_rpc_op_stats()
> + old_stats = DeviceData()
> + old_stats.parse_stats(old_mountstats[mp])
> + diff_stats = stats.compare_iostats(old_stats)
> + print_mountstats(diff_stats, nfs_only, rpc_only)
>
> def print_nfsstat_help(name):
> print('usage: %s [ options ]' % name)
> @@ -684,7 +702,7 @@ def iostat_command():
> """iostat-like command for NFS mount points
> """
> try:
> - opts, args = getopt.getopt(sys.argv[1:], "f:hv", ["file=", "help", "version"])
> + opts, args = getopt.getopt(sys.argv[1:], "f:hvS:", ["file=", "help", "version", "since="])
> except getopt.GetoptError as err:
> print_iostat_help(prog)
> devices = []
> @@ -692,6 +710,7 @@ def iostat_command():
> count_seen = False
> infile_seen = False
> infile = None
> + since = None
>
> for o, a in opts:
> if o in ("-f", "--file"):
> @@ -703,6 +722,8 @@ def iostat_command():
> elif o in ("-v", "--version"):
> print('%s version %s' % (sys.argv[0], Mountstats_version))
> sys.exit(0)
> + elif o in ("-S", "--since"):
> + since = a
> else:
> assert False, "unhandled option"
> if not infile:
> @@ -715,8 +736,8 @@ def iostat_command():
> elif not interval_seen:
> interval = int(arg)
> if interval > 0:
> - if infile_seen:
> - print('interval may not be used with the -f option')
> + if infile_seen or since:
> + print('interval may not be used with the -f or -S options')
> return
> else:
> interval_seen = True
> @@ -726,8 +747,8 @@ def iostat_command():
> elif not count_seen:
> count = int(arg)
> if count > 0:
> - if infile_seen:
> - print('count may not be used with the -f option')
> + if infile_seen or since:
> + print('count may not be used with the -f or -S options')
> return
> else:
> count_seen = True
> @@ -735,6 +756,11 @@ def iostat_command():
> print('Illegal <count> value')
> return
>
> + if since:
> + old_mountstats = parse_stats_file(since)
> + else:
> + old_mountstats = None
> +
> # make certain devices contains only NFS mount points
> if len(devices) > 0:
> check = []
> @@ -754,7 +780,6 @@ def iostat_command():
> print('No NFS mount points were found')
> return
>
> - old_mountstats = None
> sample_time = 0
>
> if not interval_seen:
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com




2014-11-05 17:01:15

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 07/15] mountstats: Make print_iostat_summary handle newly appearing mounts

Signed-off-by: Scott Mayhew <[email protected]>
---
tools/mountstats/mountstats.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index d08384e..67bca51 100755
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -666,7 +666,7 @@ def print_iostat_summary(old, new, devices, time):
for device in devices:
stats = DeviceData()
stats.parse_stats(new[device])
- if not old:
+ if not old or device not in old:
stats.display_iostats(time)
else:
old_stats = DeviceData()
--
1.9.3


2014-11-06 14:45:01

by Scott Mayhew

[permalink] [raw]
Subject: Re: [nfs-utils RFC PATCH 05/15] mountstats: Convert existing option parsing to use the getopt module

On Wed, 05 Nov 2014, Chuck Lever wrote:

>
> On Nov 5, 2014, at 12:01 PM, Scott Mayhew <[email protected]> wrote:
>
> I?m not clear why you need to replace this code. But if
> you really do need to replace it, why not use the more
> Python-esque argparse module, which would replace both
> the option parsing and help text?

This started out as a hacked up version of the mountstats program that I
was really planning on sending anywhere, and originally I had only done
the nfsstat options using getopt. When I redid the changes in git I
decided to convert the rest of them to use getopt as well. I can change
it back to the original parsing or I can use argparse. Let me know
which you prefer (I have no preference either way).

>
> > Signed-off-by: Scott Mayhew <[email protected]>
> > ---
> > tools/mountstats/mountstats.py | 62 +++++++++++++++++++++---------------------
> > 1 file changed, 31 insertions(+), 31 deletions(-)
> >
> > diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
> > index 2fe1362..272a8f9 100755
> > --- a/tools/mountstats/mountstats.py
> > +++ b/tools/mountstats/mountstats.py
> > @@ -24,6 +24,7 @@ MA 02110-1301 USA
> > """
> >
> > import sys, os, time
> > +import getopt
> >
> > Mountstats_version = '0.2'
> >
> > @@ -547,37 +548,33 @@ def print_mountstats_help(name):
> > def mountstats_command():
> > """Mountstats command
> > """
> > + try:
> > + opts, args = getopt.getopt(sys.argv[1:], "ehnrsv", ["end", "help", "nfs", "rpc", "start", "version"])
> > + except getopt.GetoptError as err:
> > + print_mountstats_help(prog)
> > +
> > mountpoints = []
> > nfs_only = False
> > rpc_only = False
> >
> > - for arg in sys.argv:
> > - if arg in ['-h', '--help', 'help', 'usage']:
> > + for o, a in opts:
> > + if o in ("-e", "--end"):
> > + raise Exception('Sampling is not yet implemented')
> > + elif o in ("-h", "--help"):
> > print_mountstats_help(prog)
> > - return
> > -
> > - if arg in ['-v', '--version', 'version']:
> > - print('%s version %s' % (sys.argv[0], Mountstats_version))
> > sys.exit(0)
> > -
> > - if arg in ['-n', '--nfs']:
> > + elif o in ("-n", "--nfs"):
> > nfs_only = True
> > - continue
> > -
> > - if arg in ['-r', '--rpc']:
> > + elif o in ("-r", "--rpc"):
> > rpc_only = True
> > - continue
> > -
> > - if arg in ['-s', '--start']:
> > - raise Exception('Sampling is not yet implemented')
> > -
> > - if arg in ['-e', '--end']:
> > + elif o in ("-s", "--start"):
> > raise Exception('Sampling is not yet implemented')
> > -
> > - if arg == sys.argv[0]:
> > - continue
> > -
> > - mountpoints += [arg]
> > + elif o in ("-v", "--version"):
> > + print('%s version %s' % (sys.argv[0], Mountstats_version))
> > + sys.exit(0)
> > + else:
> > + assert False, "unhandled option"
> > + mountpoints += args
> >
> > if mountpoints == []:
> > print_mountstats_help(prog)
> > @@ -662,23 +659,26 @@ def print_iostat_summary(old, new, devices, time):
> > def iostat_command():
> > """iostat-like command for NFS mount points
> > """
> > + try:
> > + opts, args = getopt.getopt(sys.argv[1:], "hv", ["help", "version"])
> > + except getopt.GetoptError as err:
> > + print_iostat_help(prog)
> > mountstats = parse_stats_file('/proc/self/mountstats')
> > devices = []
> > interval_seen = False
> > count_seen = False
> >
> > - for arg in sys.argv:
> > - if arg in ['-h', '--help', 'help', 'usage']:
> > + for o, a in opts:
> > + if o in ("-h", "--help"):
> > print_iostat_help(prog)
> > - return
> > -
> > - if arg in ['-v', '--version', 'version']:
> > + sys.exit(0)
> > + elif o in ("-v", "--version"):
> > print('%s version %s' % (sys.argv[0], Mountstats_version))
> > - return
> > -
> > - if arg == sys.argv[0]:
> > - continue
> > + sys.exit(0)
> > + else:
> > + assert False, "unhandled option"
> >
> > + for arg in args:
> > if arg in mountstats:
> > devices += [arg]
> > elif not interval_seen:
> > --
> > 1.9.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>
>
>

2014-11-06 14:41:08

by Scott Mayhew

[permalink] [raw]
Subject: Re: [nfs-utils RFC PATCH 09/15] mountstats: Add support for -S/--since to the mountstats and ms-iostat commands

On Wed, 05 Nov 2014, Chuck Lever wrote:

>
> On Nov 5, 2014, at 12:01 PM, Scott Mayhew <[email protected]> wrote:
>
> > Add support for -S/--since to the mountstats and ms-iostat commands to
> > display the delta between a previous copy of /proc/self/mountstats and
> > the current /proc/self/mountstats file. Can be combined with the -f
> > option to show the statistics between two different points in time.
>
> This is exactly what ?start and ?send were supposed to do.
> Why not use use them?

The help output for --start and --end didn't show arguments, so I was
under the impression that these were something you'd use on the same
system where the stats are being generated (as opposed to capturing the
raw data so you can generate the output on some other system).

Plus I was thinking that a neat use for --start and --end would be to
use those with an interval to capture periodic stats that could be then
used to generate line graphs. But I guess that could just as easily be
done with cp or cat in cron job or a shell loop.

Finally I was mimicking the -S/--since option from nfsstat.c.

>
> > Signed-off-by: Scott Mayhew <[email protected]>
> > ---
> > tools/mountstats/mountstats.py | 61 +++++++++++++++++++++++++++++-------------
> > 1 file changed, 43 insertions(+), 18 deletions(-)
> >
> > diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
> > index 23def92..b3231b2 100755
> > --- a/tools/mountstats/mountstats.py
> > +++ b/tools/mountstats/mountstats.py
> > @@ -562,13 +562,28 @@ def print_mountstats_help(name):
> > print(' --rpc display only the RPC statistics')
> > print(' --start sample and save statistics')
> > print(' --end resample statistics and compare them with saved')
> > + print(' --since <file> shows difference between current stats and those in \'file\'')
> > print()
> >
> > +def print_mountstats(stats, nfs_only, rpc_only):
> > + if nfs_only:
> > + stats.display_nfs_options()
> > + stats.display_nfs_events()
> > + stats.display_nfs_bytes()
> > + elif rpc_only:
> > + stats.display_rpc_generic_stats()
> > + stats.display_rpc_op_stats()
> > + else:
> > + stats.display_nfs_options()
> > + stats.display_nfs_bytes()
> > + stats.display_rpc_generic_stats()
> > + stats.display_rpc_op_stats()
> > +
> > def mountstats_command():
> > """Mountstats command
> > """
> > try:
> > - opts, args = getopt.getopt(sys.argv[1:], "ef:hnrsv", ["end", "file=", "help", "nfs", "rpc", "start", "version"])
> > + opts, args = getopt.getopt(sys.argv[1:], "ef:hnrsvS:", ["end", "file=", "help", "nfs", "rpc", "start", "version", "since="])
> > except getopt.GetoptError as err:
> > print_mountstats_help(prog)
> >
> > @@ -576,6 +591,7 @@ def mountstats_command():
> > nfs_only = False
> > rpc_only = False
> > infile = None
> > + since = None
> >
> > for o, a in opts:
> > if o in ("-e", "--end"):
> > @@ -594,6 +610,8 @@ def mountstats_command():
> > elif o in ("-v", "--version"):
> > print('%s version %s' % (sys.argv[0], Mountstats_version))
> > sys.exit(0)
> > + elif o in ("-S", "--since"):
> > + since = a
> > else:
> > assert False, "unhandled option"
> > mountpoints += args
> > @@ -610,6 +628,9 @@ def mountstats_command():
> > infile = '/proc/self/mountstats'
> > mountstats = parse_stats_file(infile)
> >
> > + if since:
> > + old_mountstats = parse_stats_file(since)
> > +
> > for mp in mountpoints:
> > if mp not in mountstats:
> > print('Statistics for mount point %s not found' % mp)
> > @@ -622,18 +643,15 @@ def mountstats_command():
> > print('Mount point %s exists but is not an NFS mount' % mp)
> > continue
> >
> > - if nfs_only:
> > - stats.display_nfs_options()
> > - stats.display_nfs_events()
> > - stats.display_nfs_bytes()
> > - elif rpc_only:
> > - stats.display_rpc_generic_stats()
> > - stats.display_rpc_op_stats()
> > + if not since:
> > + print_mountstats(stats, nfs_only, rpc_only)
> > + elif since and mp not in old_mountstats:
> > + print_mountstats(stats, nfs_only, rpc_only)
> > else:
> > - stats.display_nfs_options()
> > - stats.display_nfs_bytes()
> > - stats.display_rpc_generic_stats()
> > - stats.display_rpc_op_stats()
> > + old_stats = DeviceData()
> > + old_stats.parse_stats(old_mountstats[mp])
> > + diff_stats = stats.compare_iostats(old_stats)
> > + print_mountstats(diff_stats, nfs_only, rpc_only)
> >
> > def print_nfsstat_help(name):
> > print('usage: %s [ options ]' % name)
> > @@ -684,7 +702,7 @@ def iostat_command():
> > """iostat-like command for NFS mount points
> > """
> > try:
> > - opts, args = getopt.getopt(sys.argv[1:], "f:hv", ["file=", "help", "version"])
> > + opts, args = getopt.getopt(sys.argv[1:], "f:hvS:", ["file=", "help", "version", "since="])
> > except getopt.GetoptError as err:
> > print_iostat_help(prog)
> > devices = []
> > @@ -692,6 +710,7 @@ def iostat_command():
> > count_seen = False
> > infile_seen = False
> > infile = None
> > + since = None
> >
> > for o, a in opts:
> > if o in ("-f", "--file"):
> > @@ -703,6 +722,8 @@ def iostat_command():
> > elif o in ("-v", "--version"):
> > print('%s version %s' % (sys.argv[0], Mountstats_version))
> > sys.exit(0)
> > + elif o in ("-S", "--since"):
> > + since = a
> > else:
> > assert False, "unhandled option"
> > if not infile:
> > @@ -715,8 +736,8 @@ def iostat_command():
> > elif not interval_seen:
> > interval = int(arg)
> > if interval > 0:
> > - if infile_seen:
> > - print('interval may not be used with the -f option')
> > + if infile_seen or since:
> > + print('interval may not be used with the -f or -S options')
> > return
> > else:
> > interval_seen = True
> > @@ -726,8 +747,8 @@ def iostat_command():
> > elif not count_seen:
> > count = int(arg)
> > if count > 0:
> > - if infile_seen:
> > - print('count may not be used with the -f option')
> > + if infile_seen or since:
> > + print('count may not be used with the -f or -S options')
> > return
> > else:
> > count_seen = True
> > @@ -735,6 +756,11 @@ def iostat_command():
> > print('Illegal <count> value')
> > return
> >
> > + if since:
> > + old_mountstats = parse_stats_file(since)
> > + else:
> > + old_mountstats = None
> > +
> > # make certain devices contains only NFS mount points
> > if len(devices) > 0:
> > check = []
> > @@ -754,7 +780,6 @@ def iostat_command():
> > print('No NFS mount points were found')
> > return
> >
> > - old_mountstats = None
> > sample_time = 0
> >
> > if not interval_seen:
> > --
> > 1.9.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>
>
>

2014-11-06 15:02:47

by Chuck Lever III

[permalink] [raw]
Subject: Re: [nfs-utils RFC PATCH 05/15] mountstats: Convert existing option parsing to use the getopt module


On Nov 6, 2014, at 9:44 AM, Scott Mayhew <[email protected]> wrote:

> On Wed, 05 Nov 2014, Chuck Lever wrote:
>
>>
>> On Nov 5, 2014, at 12:01 PM, Scott Mayhew <[email protected]> wrote:
>>
>> I?m not clear why you need to replace this code. But if
>> you really do need to replace it, why not use the more
>> Python-esque argparse module, which would replace both
>> the option parsing and help text?
>
> This started out as a hacked up version of the mountstats program that I
> was really planning on sending anywhere, and originally I had only done
> the nfsstat options using getopt. When I redid the changes in git I
> decided to convert the rest of them to use getopt as well. I can change
> it back to the original parsing or I can use argparse. Let me know
> which you prefer (I have no preference either way).

I wrote mountstats almost 10 years ago, and Python has changed a lot
since then. It makes sense to modernize this a bit. I think argparse:

+ is more idiomatic Python

+ allows you to fix up the help text and attach it directly to each option

+ maybe makes it easier to manage the coexistence of the different commands
in the same source code?

Give it a try. Here?s some sample code:

http://git.linux-nfs.org/?p=cel/fedfs-utils.git;a=blob;f=src/domainroot/fedfs-domainroot.in;h=6db32eb0fdb47b90e25189929ec9588dbf342eda;hb=HEAD

If it starts looking crazy, then stick with getopt.


>>
>>> Signed-off-by: Scott Mayhew <[email protected]>
>>> ---
>>> tools/mountstats/mountstats.py | 62 +++++++++++++++++++++---------------------
>>> 1 file changed, 31 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
>>> index 2fe1362..272a8f9 100755
>>> --- a/tools/mountstats/mountstats.py
>>> +++ b/tools/mountstats/mountstats.py
>>> @@ -24,6 +24,7 @@ MA 02110-1301 USA
>>> """
>>>
>>> import sys, os, time
>>> +import getopt
>>>
>>> Mountstats_version = '0.2'
>>>
>>> @@ -547,37 +548,33 @@ def print_mountstats_help(name):
>>> def mountstats_command():
>>> """Mountstats command
>>> """
>>> + try:
>>> + opts, args = getopt.getopt(sys.argv[1:], "ehnrsv", ["end", "help", "nfs", "rpc", "start", "version"])
>>> + except getopt.GetoptError as err:
>>> + print_mountstats_help(prog)
>>> +
>>> mountpoints = []
>>> nfs_only = False
>>> rpc_only = False
>>>
>>> - for arg in sys.argv:
>>> - if arg in ['-h', '--help', 'help', 'usage']:
>>> + for o, a in opts:
>>> + if o in ("-e", "--end"):
>>> + raise Exception('Sampling is not yet implemented')
>>> + elif o in ("-h", "--help"):
>>> print_mountstats_help(prog)
>>> - return
>>> -
>>> - if arg in ['-v', '--version', 'version']:
>>> - print('%s version %s' % (sys.argv[0], Mountstats_version))
>>> sys.exit(0)
>>> -
>>> - if arg in ['-n', '--nfs']:
>>> + elif o in ("-n", "--nfs"):
>>> nfs_only = True
>>> - continue
>>> -
>>> - if arg in ['-r', '--rpc']:
>>> + elif o in ("-r", "--rpc"):
>>> rpc_only = True
>>> - continue
>>> -
>>> - if arg in ['-s', '--start']:
>>> - raise Exception('Sampling is not yet implemented')
>>> -
>>> - if arg in ['-e', '--end']:
>>> + elif o in ("-s", "--start"):
>>> raise Exception('Sampling is not yet implemented')
>>> -
>>> - if arg == sys.argv[0]:
>>> - continue
>>> -
>>> - mountpoints += [arg]
>>> + elif o in ("-v", "--version"):
>>> + print('%s version %s' % (sys.argv[0], Mountstats_version))
>>> + sys.exit(0)
>>> + else:
>>> + assert False, "unhandled option"
>>> + mountpoints += args
>>>
>>> if mountpoints == []:
>>> print_mountstats_help(prog)
>>> @@ -662,23 +659,26 @@ def print_iostat_summary(old, new, devices, time):
>>> def iostat_command():
>>> """iostat-like command for NFS mount points
>>> """
>>> + try:
>>> + opts, args = getopt.getopt(sys.argv[1:], "hv", ["help", "version"])
>>> + except getopt.GetoptError as err:
>>> + print_iostat_help(prog)
>>> mountstats = parse_stats_file('/proc/self/mountstats')
>>> devices = []
>>> interval_seen = False
>>> count_seen = False
>>>
>>> - for arg in sys.argv:
>>> - if arg in ['-h', '--help', 'help', 'usage']:
>>> + for o, a in opts:
>>> + if o in ("-h", "--help"):
>>> print_iostat_help(prog)
>>> - return
>>> -
>>> - if arg in ['-v', '--version', 'version']:
>>> + sys.exit(0)
>>> + elif o in ("-v", "--version"):
>>> print('%s version %s' % (sys.argv[0], Mountstats_version))
>>> - return
>>> -
>>> - if arg == sys.argv[0]:
>>> - continue
>>> + sys.exit(0)
>>> + else:
>>> + assert False, "unhandled option"
>>>
>>> + for arg in args:
>>> if arg in mountstats:
>>> devices += [arg]
>>> elif not interval_seen:
>>> --
>>> 1.9.3
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> --
>> Chuck Lever
>> chuck[dot]lever[at]oracle[dot]com
>>
>>
>>

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com




2014-11-05 17:01:14

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 01/15] mountstats: Fix up NFS event counters

The event counters in the mountstats program aren't in sync with the
event counters in the kernel. Removed syncinodes and added
vfsupdatepage, vfssetattr, congestionwait, pnfsreads, and pnfswrites.

Signed-off-by: Scott Mayhew <[email protected]>
---
tools/mountstats/mountstats.py | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)
mode change 100644 => 100755 tools/mountstats/mountstats.py

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
old mode 100644
new mode 100755
index 9a6ec43..5770f2a
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -73,25 +73,29 @@ class DeviceData:
self.__nfs_data['dentryrevalidates'] = int(words[2])
self.__nfs_data['datainvalidates'] = int(words[3])
self.__nfs_data['attrinvalidates'] = int(words[4])
- self.__nfs_data['syncinodes'] = int(words[5])
- self.__nfs_data['vfsopen'] = int(words[6])
- self.__nfs_data['vfslookup'] = int(words[7])
- self.__nfs_data['vfspermission'] = int(words[8])
+ self.__nfs_data['vfsopen'] = int(words[5])
+ self.__nfs_data['vfslookup'] = int(words[6])
+ self.__nfs_data['vfspermission'] = int(words[7])
+ self.__nfs_data['vfsupdatepage'] = int(words[8])
self.__nfs_data['vfsreadpage'] = int(words[9])
self.__nfs_data['vfsreadpages'] = int(words[10])
self.__nfs_data['vfswritepage'] = int(words[11])
self.__nfs_data['vfswritepages'] = int(words[12])
self.__nfs_data['vfsreaddir'] = int(words[13])
- self.__nfs_data['vfsflush'] = int(words[14])
- self.__nfs_data['vfsfsync'] = int(words[15])
- self.__nfs_data['vfslock'] = int(words[16])
- self.__nfs_data['vfsrelease'] = int(words[17])
- self.__nfs_data['setattrtrunc'] = int(words[18])
- self.__nfs_data['extendwrite'] = int(words[19])
- self.__nfs_data['sillyrenames'] = int(words[20])
- self.__nfs_data['shortreads'] = int(words[21])
- self.__nfs_data['shortwrites'] = int(words[22])
- self.__nfs_data['delay'] = int(words[23])
+ self.__nfs_data['vfssetattr'] = int(words[14])
+ self.__nfs_data['vfsflush'] = int(words[15])
+ self.__nfs_data['vfsfsync'] = int(words[16])
+ self.__nfs_data['vfslock'] = int(words[17])
+ self.__nfs_data['vfsrelease'] = int(words[18])
+ self.__nfs_data['congestionwait'] = int(words[19])
+ self.__nfs_data['setattrtrunc'] = int(words[20])
+ self.__nfs_data['extendwrite'] = int(words[21])
+ self.__nfs_data['sillyrenames'] = int(words[22])
+ self.__nfs_data['shortreads'] = int(words[23])
+ self.__nfs_data['shortwrites'] = int(words[24])
+ self.__nfs_data['delay'] = int(words[25])
+ self.__nfs_data['pnfsreads'] = int(words[26])
+ self.__nfs_data['pnfswrites'] = int(words[27])
elif words[0] == 'bytes:':
self.__nfs_data['normalreadbytes'] = int(words[1])
self.__nfs_data['normalwritebytes'] = int(words[2])
@@ -201,7 +205,6 @@ class DeviceData:
print('Cache events:')
print(' data cache invalidated %d times' % self.__nfs_data['datainvalidates'])
print(' attribute cache invalidated %d times' % self.__nfs_data['attrinvalidates'])
- print(' inodes synced %d times' % self.__nfs_data['syncinodes'])
print()
print('VFS calls:')
print(' VFS requested %d inode revalidations' % self.__nfs_data['inoderevalidates'])
--
1.9.3


2014-11-05 17:01:17

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 08/15] mountstats: Add support for -f/--file to the mountstats and ms-iostat commands

Add support for the -f/--file option to allow parsing of data from an
arbitrary file instead of /proc/self/mountstats.

Signed-off-by: Scott Mayhew <[email protected]>
---
tools/mountstats/mountstats.py | 35 ++++++++++++++++++++++++++++-------
1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index 67bca51..23def92 100755
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -557,6 +557,7 @@ def print_mountstats_help(name):
print(' Display NFS client per-mount statistics.')
print()
print(' --version display the version of this command')
+ print(' --file <file> read stats from \'file\' instead of /proc/self/mountstats')
print(' --nfs display only the NFS statistics')
print(' --rpc display only the RPC statistics')
print(' --start sample and save statistics')
@@ -567,17 +568,20 @@ def mountstats_command():
"""Mountstats command
"""
try:
- opts, args = getopt.getopt(sys.argv[1:], "ehnrsv", ["end", "help", "nfs", "rpc", "start", "version"])
+ opts, args = getopt.getopt(sys.argv[1:], "ef:hnrsv", ["end", "file=", "help", "nfs", "rpc", "start", "version"])
except getopt.GetoptError as err:
print_mountstats_help(prog)

mountpoints = []
nfs_only = False
rpc_only = False
+ infile = None

for o, a in opts:
if o in ("-e", "--end"):
raise Exception('Sampling is not yet implemented')
+ elif o in ("-f", "--file"):
+ infile = a
elif o in ("-h", "--help"):
print_mountstats_help(prog)
sys.exit(0)
@@ -602,7 +606,9 @@ def mountstats_command():
print_mountstats_help(prog)
return

- mountstats = parse_stats_file('/proc/self/mountstats')
+ if not infile:
+ infile = '/proc/self/mountstats'
+ mountstats = parse_stats_file(infile)

for mp in mountpoints:
if mp not in mountstats:
@@ -678,16 +684,20 @@ def iostat_command():
"""iostat-like command for NFS mount points
"""
try:
- opts, args = getopt.getopt(sys.argv[1:], "hv", ["help", "version"])
+ opts, args = getopt.getopt(sys.argv[1:], "f:hv", ["file=", "help", "version"])
except getopt.GetoptError as err:
print_iostat_help(prog)
- mountstats = parse_stats_file('/proc/self/mountstats')
devices = []
interval_seen = False
count_seen = False
+ infile_seen = False
+ infile = None

for o, a in opts:
- if o in ("-h", "--help"):
+ if o in ("-f", "--file"):
+ infile = a
+ infile_seen = True
+ elif o in ("-h", "--help"):
print_iostat_help(prog)
sys.exit(0)
elif o in ("-v", "--version"):
@@ -695,6 +705,9 @@ def iostat_command():
sys.exit(0)
else:
assert False, "unhandled option"
+ if not infile:
+ infile = '/proc/self/mountstats'
+ mountstats = parse_stats_file(infile)

for arg in args:
if arg in mountstats:
@@ -702,14 +715,22 @@ def iostat_command():
elif not interval_seen:
interval = int(arg)
if interval > 0:
- interval_seen = True
+ if infile_seen:
+ print('interval may not be used with the -f option')
+ return
+ else:
+ interval_seen = True
else:
print('Illegal <interval> value')
return
elif not count_seen:
count = int(arg)
if count > 0:
- count_seen = True
+ if infile_seen:
+ print('count may not be used with the -f option')
+ return
+ else:
+ count_seen = True
else:
print('Illegal <count> value')
return
--
1.9.3


2014-11-05 17:01:18

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 12/15] mountstats: Add support for -R/--raw to mountstats_command

This option displays the mountstats in raw format (i.e. in the same
format as /proc/self/mountstats). It's intended to be used mainly with
the -S/--since option.

Signed-off-by: Scott Mayhew <[email protected]>
---
tools/mountstats/mountstats.py | 50 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index 281f4be..2d6ca95 100755
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -305,6 +305,41 @@ class DeviceData:
return True
return False

+ def display_raw_stats(self):
+ """Prints out stats in the same format as /proc/self/mountstats
+ """
+ print('device %s mounted on %s with fstype %s %s' % \
+ (self.__nfs_data['export'], self.__nfs_data['mountpoint'], \
+ self.__nfs_data['fstype'], self.__nfs_data['statvers']))
+ print('\topts:\t%s' % ','.join(self.__nfs_data['mountoptions']))
+ print('\tage:\t%d' % self.__nfs_data['age'])
+ print('\tcaps:\t%s' % ','.join(self.__nfs_data['servercapabilities']))
+ print('\tsec:\tflavor=%d,pseudoflavor=%d' % (self.__nfs_data['flavor'], \
+ self.__nfs_data['pseudoflavor']))
+ print('\tevents:\t%s' % " ".join([str(self.__nfs_data[key]) for key in NfsEventCounters]))
+ print('\tbytes:\t%s' % " ".join([str(self.__nfs_data[key]) for key in NfsByteCounters]))
+ print('\tRPC iostats version: %1.1f p/v: %s (nfs)' % (self.__rpc_data['statsvers'], \
+ self.__rpc_data['programversion']))
+ if self.__rpc_data['protocol'] == 'udp':
+ print('\txprt:\tudp %s' % " ".join([str(self.__rpc_data[key]) for key in XprtUdpCounters]))
+ elif self.__rpc_data['protocol'] == 'tcp':
+ print('\txprt:\ttcp %s' % " ".join([str(self.__rpc_data[key]) for key in XprtTcpCounters]))
+ elif self.__rpc_data['protocol'] == 'rdma':
+ print('\txprt:\trdma %s' % " ".join([str(self.__rpc_data[key]) for key in XprtRdmaCounters]))
+ else:
+ raise Exception('Unknown RPC transport protocol %s' % self.__rpc_data['protocol'])
+ print('\tper-op statistics')
+ prog, vers = self.__rpc_data['programversion'].split('/')
+ if vers == '3':
+ for op in Nfsv3ops:
+ print('\t%12s: %s' % (op, " ".join(str(x) for x in self.__rpc_data[op])))
+ elif vers == '4':
+ for op in Nfsv4ops:
+ print('\t%12s: %s' % (op, " ".join(str(x) for x in self.__rpc_data[op])))
+ else:
+ print('\tnot implemented for version %d' % vers)
+ print()
+
def display_nfs_options(self):
"""Pretty-print the NFS options
"""
@@ -565,7 +600,7 @@ def print_mountstats_help(name):
print(' --since <file> shows difference between current stats and those in \'file\'')
print()

-def print_mountstats(stats, nfs_only, rpc_only):
+def print_mountstats(stats, nfs_only, rpc_only, raw):
if nfs_only:
stats.display_nfs_options()
stats.display_nfs_events()
@@ -573,6 +608,8 @@ def print_mountstats(stats, nfs_only, rpc_only):
elif rpc_only:
stats.display_rpc_generic_stats()
stats.display_rpc_op_stats()
+ elif raw:
+ stats.display_raw_stats()
else:
stats.display_nfs_options()
stats.display_nfs_bytes()
@@ -583,13 +620,14 @@ def mountstats_command():
"""Mountstats command
"""
try:
- opts, args = getopt.getopt(sys.argv[1:], "ef:hnrsvS:", ["end", "file=", "help", "nfs", "rpc", "start", "version", "since="])
+ opts, args = getopt.getopt(sys.argv[1:], "ef:hnrsvS:R", ["end", "file=", "help", "nfs", "rpc", "start", "version", "since=", "raw"])
except getopt.GetoptError as err:
print_mountstats_help(prog)

mountpoints = []
nfs_only = False
rpc_only = False
+ raw = False
infile = None
since = None

@@ -612,6 +650,8 @@ def mountstats_command():
sys.exit(0)
elif o in ("-S", "--since"):
since = a
+ elif o in ("-R", "--raw"):
+ raw = True
else:
assert False, "unhandled option"
mountpoints += args
@@ -650,14 +690,14 @@ def mountstats_command():
stats = DeviceData()
stats.parse_stats(mountstats[mp])
if not since:
- print_mountstats(stats, nfs_only, rpc_only)
+ print_mountstats(stats, nfs_only, rpc_only, raw)
elif since and mp not in old_mountstats:
- print_mountstats(stats, nfs_only, rpc_only)
+ print_mountstats(stats, nfs_only, rpc_only, raw)
else:
old_stats = DeviceData()
old_stats.parse_stats(old_mountstats[mp])
diff_stats = stats.compare_iostats(old_stats)
- print_mountstats(diff_stats, nfs_only, rpc_only)
+ print_mountstats(diff_stats, nfs_only, rpc_only, raw)

def print_nfsstat_help(name):
print('usage: %s [ options ]' % name)
--
1.9.3


2014-11-05 17:01:18

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 11/15] mountstats: Allow mountstats_command to take a variable number of mountpoints

Allow the mountstats command to take a variable number of mountpoints
(including none, in which case it will print stats for all NFS
mountpoints it finds).

Signed-off-by: Scott Mayhew <[email protected]>
---
tools/mountstats/mountstats.py | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index 77975ad..281f4be 100755
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -616,10 +616,6 @@ def mountstats_command():
assert False, "unhandled option"
mountpoints += args

- if mountpoints == []:
- print_mountstats_help(prog)
- return
-
if rpc_only == True and nfs_only == True:
print_mountstats_help(prog)
return
@@ -628,21 +624,31 @@ def mountstats_command():
infile = '/proc/self/mountstats'
mountstats = parse_stats_file(infile)

+ # make certain devices contains only NFS mount points
+ if len(mountpoints) > 0:
+ check = []
+ for device in mountpoints:
+ stats = DeviceData()
+ stats.parse_stats(mountstats[device])
+ if stats.is_nfs_mountpoint():
+ check += [device]
+ mountpoints = check
+ else:
+ for device, descr in mountstats.items():
+ stats = DeviceData()
+ stats.parse_stats(descr)
+ if stats.is_nfs_mountpoint():
+ mountpoints += [device]
+ if len(mountpoints) == 0:
+ print('No NFS mount points were found')
+ return
+
if since:
old_mountstats = parse_stats_file(since)

for mp in mountpoints:
- if mp not in mountstats:
- print('Statistics for mount point %s not found' % mp)
- continue
-
stats = DeviceData()
stats.parse_stats(mountstats[mp])
-
- if not stats.is_nfs_mountpoint():
- print('Mount point %s exists but is not an NFS mount' % mp)
- continue
-
if not since:
print_mountstats(stats, nfs_only, rpc_only)
elif since and mp not in old_mountstats:
--
1.9.3


2014-11-05 17:01:17

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 09/15] mountstats: Add support for -S/--since to the mountstats and ms-iostat commands

Add support for -S/--since to the mountstats and ms-iostat commands to
display the delta between a previous copy of /proc/self/mountstats and
the current /proc/self/mountstats file. Can be combined with the -f
option to show the statistics between two different points in time.

Signed-off-by: Scott Mayhew <[email protected]>
---
tools/mountstats/mountstats.py | 61 +++++++++++++++++++++++++++++-------------
1 file changed, 43 insertions(+), 18 deletions(-)

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index 23def92..b3231b2 100755
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -562,13 +562,28 @@ def print_mountstats_help(name):
print(' --rpc display only the RPC statistics')
print(' --start sample and save statistics')
print(' --end resample statistics and compare them with saved')
+ print(' --since <file> shows difference between current stats and those in \'file\'')
print()

+def print_mountstats(stats, nfs_only, rpc_only):
+ if nfs_only:
+ stats.display_nfs_options()
+ stats.display_nfs_events()
+ stats.display_nfs_bytes()
+ elif rpc_only:
+ stats.display_rpc_generic_stats()
+ stats.display_rpc_op_stats()
+ else:
+ stats.display_nfs_options()
+ stats.display_nfs_bytes()
+ stats.display_rpc_generic_stats()
+ stats.display_rpc_op_stats()
+
def mountstats_command():
"""Mountstats command
"""
try:
- opts, args = getopt.getopt(sys.argv[1:], "ef:hnrsv", ["end", "file=", "help", "nfs", "rpc", "start", "version"])
+ opts, args = getopt.getopt(sys.argv[1:], "ef:hnrsvS:", ["end", "file=", "help", "nfs", "rpc", "start", "version", "since="])
except getopt.GetoptError as err:
print_mountstats_help(prog)

@@ -576,6 +591,7 @@ def mountstats_command():
nfs_only = False
rpc_only = False
infile = None
+ since = None

for o, a in opts:
if o in ("-e", "--end"):
@@ -594,6 +610,8 @@ def mountstats_command():
elif o in ("-v", "--version"):
print('%s version %s' % (sys.argv[0], Mountstats_version))
sys.exit(0)
+ elif o in ("-S", "--since"):
+ since = a
else:
assert False, "unhandled option"
mountpoints += args
@@ -610,6 +628,9 @@ def mountstats_command():
infile = '/proc/self/mountstats'
mountstats = parse_stats_file(infile)

+ if since:
+ old_mountstats = parse_stats_file(since)
+
for mp in mountpoints:
if mp not in mountstats:
print('Statistics for mount point %s not found' % mp)
@@ -622,18 +643,15 @@ def mountstats_command():
print('Mount point %s exists but is not an NFS mount' % mp)
continue

- if nfs_only:
- stats.display_nfs_options()
- stats.display_nfs_events()
- stats.display_nfs_bytes()
- elif rpc_only:
- stats.display_rpc_generic_stats()
- stats.display_rpc_op_stats()
+ if not since:
+ print_mountstats(stats, nfs_only, rpc_only)
+ elif since and mp not in old_mountstats:
+ print_mountstats(stats, nfs_only, rpc_only)
else:
- stats.display_nfs_options()
- stats.display_nfs_bytes()
- stats.display_rpc_generic_stats()
- stats.display_rpc_op_stats()
+ old_stats = DeviceData()
+ old_stats.parse_stats(old_mountstats[mp])
+ diff_stats = stats.compare_iostats(old_stats)
+ print_mountstats(diff_stats, nfs_only, rpc_only)

def print_nfsstat_help(name):
print('usage: %s [ options ]' % name)
@@ -684,7 +702,7 @@ def iostat_command():
"""iostat-like command for NFS mount points
"""
try:
- opts, args = getopt.getopt(sys.argv[1:], "f:hv", ["file=", "help", "version"])
+ opts, args = getopt.getopt(sys.argv[1:], "f:hvS:", ["file=", "help", "version", "since="])
except getopt.GetoptError as err:
print_iostat_help(prog)
devices = []
@@ -692,6 +710,7 @@ def iostat_command():
count_seen = False
infile_seen = False
infile = None
+ since = None

for o, a in opts:
if o in ("-f", "--file"):
@@ -703,6 +722,8 @@ def iostat_command():
elif o in ("-v", "--version"):
print('%s version %s' % (sys.argv[0], Mountstats_version))
sys.exit(0)
+ elif o in ("-S", "--since"):
+ since = a
else:
assert False, "unhandled option"
if not infile:
@@ -715,8 +736,8 @@ def iostat_command():
elif not interval_seen:
interval = int(arg)
if interval > 0:
- if infile_seen:
- print('interval may not be used with the -f option')
+ if infile_seen or since:
+ print('interval may not be used with the -f or -S options')
return
else:
interval_seen = True
@@ -726,8 +747,8 @@ def iostat_command():
elif not count_seen:
count = int(arg)
if count > 0:
- if infile_seen:
- print('count may not be used with the -f option')
+ if infile_seen or since:
+ print('count may not be used with the -f or -S options')
return
else:
count_seen = True
@@ -735,6 +756,11 @@ def iostat_command():
print('Illegal <count> value')
return

+ if since:
+ old_mountstats = parse_stats_file(since)
+ else:
+ old_mountstats = None
+
# make certain devices contains only NFS mount points
if len(devices) > 0:
check = []
@@ -754,7 +780,6 @@ def iostat_command():
print('No NFS mount points were found')
return

- old_mountstats = None
sample_time = 0

if not interval_seen:
--
1.9.3


2014-11-05 17:01:18

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 14/15] mountstats: Remove the --start and --end options

All they do is throw 'not implemented' exceptions anyway.

Signed-off-by: Scott Mayhew <[email protected]>
---
tools/mountstats/mountstats.py | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index 389f241..f9f5eac 100755
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -631,8 +631,6 @@ def print_mountstats_help(name):
print(' --file <file> read stats from \'file\' instead of /proc/self/mountstats')
print(' --nfs display only the NFS statistics')
print(' --rpc display only the RPC statistics')
- print(' --start sample and save statistics')
- print(' --end resample statistics and compare them with saved')
print(' --since <file> shows difference between current stats and those in \'file\'')
print()

@@ -656,7 +654,7 @@ def mountstats_command():
"""Mountstats command
"""
try:
- opts, args = getopt.getopt(sys.argv[1:], "ef:hnrsvS:R", ["end", "file=", "help", "nfs", "rpc", "start", "version", "since=", "raw"])
+ opts, args = getopt.getopt(sys.argv[1:], "f:hnrvS:R", ["file=", "help", "nfs", "rpc", "version", "since=", "raw"])
except getopt.GetoptError as err:
print_mountstats_help(prog)

@@ -668,9 +666,7 @@ def mountstats_command():
since = None

for o, a in opts:
- if o in ("-e", "--end"):
- raise Exception('Sampling is not yet implemented')
- elif o in ("-f", "--file"):
+ if o in ("-f", "--file"):
infile = a
elif o in ("-h", "--help"):
print_mountstats_help(prog)
@@ -679,8 +675,6 @@ def mountstats_command():
nfs_only = True
elif o in ("-r", "--rpc"):
rpc_only = True
- elif o in ("-s", "--start"):
- raise Exception('Sampling is not yet implemented')
elif o in ("-v", "--version"):
print('%s version %s' % (sys.argv[0], Mountstats_version))
sys.exit(0)
--
1.9.3


2014-11-06 01:52:44

by Chuck Lever III

[permalink] [raw]
Subject: Re: [nfs-utils RFC PATCH 05/15] mountstats: Convert existing option parsing to use the getopt module


On Nov 5, 2014, at 12:01 PM, Scott Mayhew <[email protected]> wrote:

I?m not clear why you need to replace this code. But if
you really do need to replace it, why not use the more
Python-esque argparse module, which would replace both
the option parsing and help text?

> Signed-off-by: Scott Mayhew <[email protected]>
> ---
> tools/mountstats/mountstats.py | 62 +++++++++++++++++++++---------------------
> 1 file changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
> index 2fe1362..272a8f9 100755
> --- a/tools/mountstats/mountstats.py
> +++ b/tools/mountstats/mountstats.py
> @@ -24,6 +24,7 @@ MA 02110-1301 USA
> """
>
> import sys, os, time
> +import getopt
>
> Mountstats_version = '0.2'
>
> @@ -547,37 +548,33 @@ def print_mountstats_help(name):
> def mountstats_command():
> """Mountstats command
> """
> + try:
> + opts, args = getopt.getopt(sys.argv[1:], "ehnrsv", ["end", "help", "nfs", "rpc", "start", "version"])
> + except getopt.GetoptError as err:
> + print_mountstats_help(prog)
> +
> mountpoints = []
> nfs_only = False
> rpc_only = False
>
> - for arg in sys.argv:
> - if arg in ['-h', '--help', 'help', 'usage']:
> + for o, a in opts:
> + if o in ("-e", "--end"):
> + raise Exception('Sampling is not yet implemented')
> + elif o in ("-h", "--help"):
> print_mountstats_help(prog)
> - return
> -
> - if arg in ['-v', '--version', 'version']:
> - print('%s version %s' % (sys.argv[0], Mountstats_version))
> sys.exit(0)
> -
> - if arg in ['-n', '--nfs']:
> + elif o in ("-n", "--nfs"):
> nfs_only = True
> - continue
> -
> - if arg in ['-r', '--rpc']:
> + elif o in ("-r", "--rpc"):
> rpc_only = True
> - continue
> -
> - if arg in ['-s', '--start']:
> - raise Exception('Sampling is not yet implemented')
> -
> - if arg in ['-e', '--end']:
> + elif o in ("-s", "--start"):
> raise Exception('Sampling is not yet implemented')
> -
> - if arg == sys.argv[0]:
> - continue
> -
> - mountpoints += [arg]
> + elif o in ("-v", "--version"):
> + print('%s version %s' % (sys.argv[0], Mountstats_version))
> + sys.exit(0)
> + else:
> + assert False, "unhandled option"
> + mountpoints += args
>
> if mountpoints == []:
> print_mountstats_help(prog)
> @@ -662,23 +659,26 @@ def print_iostat_summary(old, new, devices, time):
> def iostat_command():
> """iostat-like command for NFS mount points
> """
> + try:
> + opts, args = getopt.getopt(sys.argv[1:], "hv", ["help", "version"])
> + except getopt.GetoptError as err:
> + print_iostat_help(prog)
> mountstats = parse_stats_file('/proc/self/mountstats')
> devices = []
> interval_seen = False
> count_seen = False
>
> - for arg in sys.argv:
> - if arg in ['-h', '--help', 'help', 'usage']:
> + for o, a in opts:
> + if o in ("-h", "--help"):
> print_iostat_help(prog)
> - return
> -
> - if arg in ['-v', '--version', 'version']:
> + sys.exit(0)
> + elif o in ("-v", "--version"):
> print('%s version %s' % (sys.argv[0], Mountstats_version))
> - return
> -
> - if arg == sys.argv[0]:
> - continue
> + sys.exit(0)
> + else:
> + assert False, "unhandled option"
>
> + for arg in args:
> if arg in mountstats:
> devices += [arg]
> elif not interval_seen:
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com




2014-11-05 17:01:18

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 15/15] mountstats: Update the help output

Updated the help output for mounstats, ms-iostat, and ms-nfsstat to make
them constistent.

Signed-off-by: Scott Mayhew <[email protected]>
---
tools/mountstats/mountstats.py | 43 ++++++++++++++++++++++++++++++++++++------
1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index f9f5eac..50dcc10 100755
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -627,11 +627,25 @@ def print_mountstats_help(name):
print()
print(' Display NFS client per-mount statistics.')
print()
- print(' --version display the version of this command')
- print(' --file <file> read stats from \'file\' instead of /proc/self/mountstats')
- print(' --nfs display only the NFS statistics')
- print(' --rpc display only the RPC statistics')
- print(' --since <file> shows difference between current stats and those in \'file\'')
+ print('Options:')
+ print()
+ print(' -n, --nfs')
+ print(' Display only the NFS statistics')
+ print()
+ print(' -r, --rpc')
+ print(' Display only the RPC statistics')
+ print()
+ print(' -f <file>, --file <file>')
+ print(' Read stats from \'file\' instead of /proc/self/mountstats')
+ print()
+ print(' -S <file>, --since <file>')
+ print(' Shows difference between current stats and those in \'file\'')
+ print()
+ print(' -h, --help')
+ print(' Show the help message')
+ print()
+ print(' -v, --version')
+ print(' Display the version of this command')
print()

def print_mountstats(stats, nfs_only, rpc_only, raw):
@@ -748,7 +762,8 @@ def print_nfsstat_help(name):
print(' Shows difference between current stats and those in \'file\'')
print()
print(' -h, --help')
- print(' What you just did')
+ print(' Show the help message')
+ print()

def nfsstat_command():
"""nfsstat-like command for NFS mount points
@@ -810,6 +825,22 @@ def print_iostat_help(name):
print(' mount points will be displayed. Otherwise, all NFS mount points on the')
print(' client are listed.')
print()
+ print('Options:')
+ print()
+ print(' -f <file>, --file <file>')
+ print(' Read stats from \'file\' instead of /proc/self/mountstats.')
+ print(' This may not be combined with the <interval> and <count> parameters.')
+ print()
+ print(' -S <file>, --since <file>')
+ print(' Shows difference between current stats and those in \'file\'')
+ print(' This may not be combined with the <interval> and <count> parameters.')
+ print()
+ print(' -h, --help')
+ print(' Show the help message')
+ print()
+ print(' -v, --version')
+ print(' Display the version of this command')
+ print()

def print_iostat_summary(old, new, devices, time):
for device in devices:
--
1.9.3


2014-11-05 17:01:16

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 10/15] mountstats: Fix IndexError in __parse_nfs_line

If __parse_nfs_line is is called on a line that has 'fstype nfsd', it'll
raise an IndexError trying to parse a nonexistent statvers entry.

Signed-off-by: Scott Mayhew <[email protected]>
---
tools/mountstats/mountstats.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index b3231b2..77975ad 100755
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -215,13 +215,13 @@ class DeviceData:
self.__nfs_data['export'] = words[1]
self.__nfs_data['mountpoint'] = words[4]
self.__nfs_data['fstype'] = words[7]
- if words[7].find('nfs') != -1:
+ if words[7].find('nfs') != -1 and words[7] != 'nfsd':
self.__nfs_data['statvers'] = words[8]
elif 'nfs' in words or 'nfs4' in words:
self.__nfs_data['export'] = words[0]
self.__nfs_data['mountpoint'] = words[3]
self.__nfs_data['fstype'] = words[6]
- if words[6].find('nfs') != -1:
+ if words[6].find('nfs') != -1 and words[6] != 'nfsd':
self.__nfs_data['statvers'] = words[7]
elif words[0] == 'age:':
self.__nfs_data['age'] = int(words[1])
--
1.9.3


2014-11-06 14:59:07

by Chuck Lever III

[permalink] [raw]
Subject: Re: [nfs-utils RFC PATCH 11/15] mountstats: Allow mountstats_command to take a variable number of mountpoints


On Nov 6, 2014, at 9:46 AM, Scott Mayhew <[email protected]> wrote:

> On Wed, 05 Nov 2014, Chuck Lever wrote:
>
>>
>> On Nov 5, 2014, at 12:01 PM, Scott Mayhew <[email protected]> wrote:
>>
>>> Allow the mountstats command to take a variable number of mountpoints
>>> (including none, in which case it will print stats for all NFS
>>> mountpoints it finds).
>>
>> Cool idea.
>
> Thanks! It looks like I need to add some sort of a header when only the
> rpc stats are being printed though.

You mean a header visually separating the group of stats for each
mount point. Yeah, that would be helpful. (I typically have just one
mount point under test on my system, so I didn?t notice this before).

>>
>>> Signed-off-by: Scott Mayhew <[email protected]>
>>> ---
>>> tools/mountstats/mountstats.py | 32 +++++++++++++++++++-------------
>>> 1 file changed, 19 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
>>> index 77975ad..281f4be 100755
>>> --- a/tools/mountstats/mountstats.py
>>> +++ b/tools/mountstats/mountstats.py
>>> @@ -616,10 +616,6 @@ def mountstats_command():
>>> assert False, "unhandled option"
>>> mountpoints += args
>>>
>>> - if mountpoints == []:
>>> - print_mountstats_help(prog)
>>> - return
>>> -
>>> if rpc_only == True and nfs_only == True:
>>> print_mountstats_help(prog)
>>> return
>>> @@ -628,21 +624,31 @@ def mountstats_command():
>>> infile = '/proc/self/mountstats'
>>> mountstats = parse_stats_file(infile)
>>>
>>> + # make certain devices contains only NFS mount points
>>> + if len(mountpoints) > 0:
>>> + check = []
>>> + for device in mountpoints:
>>> + stats = DeviceData()
>>> + stats.parse_stats(mountstats[device])
>>> + if stats.is_nfs_mountpoint():
>>> + check += [device]
>>> + mountpoints = check
>>> + else:
>>> + for device, descr in mountstats.items():
>>> + stats = DeviceData()
>>> + stats.parse_stats(descr)
>>> + if stats.is_nfs_mountpoint():
>>> + mountpoints += [device]
>>> + if len(mountpoints) == 0:
>>> + print('No NFS mount points were found')
>>> + return
>>> +
>>> if since:
>>> old_mountstats = parse_stats_file(since)
>>>
>>> for mp in mountpoints:
>>> - if mp not in mountstats:
>>> - print('Statistics for mount point %s not found' % mp)
>>> - continue
>>> -
>>> stats = DeviceData()
>>> stats.parse_stats(mountstats[mp])
>>> -
>>> - if not stats.is_nfs_mountpoint():
>>> - print('Mount point %s exists but is not an NFS mount' % mp)
>>> - continue
>>> -
>>> if not since:
>>> print_mountstats(stats, nfs_only, rpc_only)
>>> elif since and mp not in old_mountstats:
>>> --
>>> 1.9.3
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> --
>> Chuck Lever
>> chuck[dot]lever[at]oracle[dot]com
>>
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com




2014-11-05 17:01:19

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 13/15] mountstats: Implement nfsstat_command

Displays nfssstat-like statistics for a single mountpoint (client
statistics only).

Signed-off-by: Scott Mayhew <[email protected]>
---
tools/mountstats/mountstats.py | 87 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 86 insertions(+), 1 deletion(-)

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index 2d6ca95..389f241 100755
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -445,6 +445,42 @@ class DeviceData:
print('\ttotal execute time: %f (milliseconds)' % \
(float(stats[7]) / count))

+ def display_nfsstat_stats(self):
+ """Pretty-print nfsstat-style stats
+ """
+ sends = 0
+ trans = 0
+ for op in self.__rpc_data['ops']:
+ sends += self.__rpc_data[op][0]
+ trans += self.__rpc_data[op][1]
+ retrans = trans - sends
+ print('Client rpc stats:')
+ print('calls retrans authrefrsh')
+ # authrefresh stats don't actually get captured in
+ # /proc/self/mountstats, so we fudge it here
+ print('%-11u%-11u%-11u' % (sends, retrans, sends))
+ if not sends:
+ return
+ print()
+ prog, vers = self.__rpc_data['programversion'].split('/')
+ print('Client nfs v%d' % int(vers))
+ info = []
+ for op in self.__rpc_data['ops']:
+ print('%-13s' % str.lower(op), end='')
+ count = self.__rpc_data[op][0]
+ pct = (count * 100) / sends
+ info.append((count, pct))
+ if (self.__rpc_data['ops'].index(op) + 1) % 6 == 0:
+ print()
+ for (count, pct) in info:
+ print('%-8u%3u%% ' % (count, pct), end='')
+ print()
+ info = []
+ print()
+ for (count, pct) in info:
+ print('%-8u%3u%% ' % (count, pct), end='')
+ print()
+
def compare_iostats(self, old_stats):
"""Return the difference between two sets of stats
"""
@@ -706,9 +742,58 @@ def print_nfsstat_help(name):
print()
print(' nfsstat-like program that uses NFS client per-mount statistics.')
print()
+ print('Options:')
+ print()
+ print(' -m <mountpoint>, --mountpount <mountpoint>')
+ print(' Show stats for \'mountpoint\'')
+ print()
+ print(' -f <file>, --file <file>')
+ print(' Read stats from \'file\' instead of /proc/self/mountstats')
+ print()
+ print(' -S <file>, --since <file>')
+ print(' Shows difference between current stats and those in \'file\'')
+ print()
+ print(' -h, --help')
+ print(' What you just did')

def nfsstat_command():
- print_nfsstat_help(prog)
+ """nfsstat-like command for NFS mount points
+ """
+ try:
+ opts, args = getopt.getopt(sys.argv[1:], "f:hm:S:", ["file=", "help", "mountpoint=", "since="])
+ except getopt.GetoptError as err:
+ print_nfsstat_help(prog)
+ infile = None
+ mp = None
+ since = None
+ for o, a in opts:
+ if o in ("-f", "--file"):
+ infile = a
+ elif o in ("-h", "--help"):
+ print_nfsstat_help(prog)
+ sys.exit()
+ elif o in ("-m", "--mountpoint"):
+ mp = a
+ elif o in ("-S", "--since"):
+ since = a
+ else:
+ assert False, "unhandled option"
+ if not mp:
+ print_nfsstat_help(prog)
+ sys.exit()
+ if not infile:
+ infile = '/proc/self/mountstats'
+ mountstats = parse_stats_file(infile)
+ stats = DeviceData()
+ stats.parse_stats(mountstats[mp])
+ if not since:
+ stats.display_nfsstat_stats()
+ else:
+ old_mountstats = parse_stats_file(since)
+ oldstats = DeviceData()
+ oldstats.parse_stats(old_mountstats[mp])
+ diff_stats = stats.compare_iostats(oldstats)
+ diff_stats.display_nfsstat_stats()

def print_iostat_help(name):
print('usage: %s [ <interval> [ <count> ] ] [ <mount point> ] ' % name)
--
1.9.3


2014-11-06 02:09:56

by Chuck Lever III

[permalink] [raw]
Subject: Re: [nfs-utils RFC PATCH 11/15] mountstats: Allow mountstats_command to take a variable number of mountpoints


On Nov 5, 2014, at 12:01 PM, Scott Mayhew <[email protected]> wrote:

> Allow the mountstats command to take a variable number of mountpoints
> (including none, in which case it will print stats for all NFS
> mountpoints it finds).

Cool idea.

> Signed-off-by: Scott Mayhew <[email protected]>
> ---
> tools/mountstats/mountstats.py | 32 +++++++++++++++++++-------------
> 1 file changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
> index 77975ad..281f4be 100755
> --- a/tools/mountstats/mountstats.py
> +++ b/tools/mountstats/mountstats.py
> @@ -616,10 +616,6 @@ def mountstats_command():
> assert False, "unhandled option"
> mountpoints += args
>
> - if mountpoints == []:
> - print_mountstats_help(prog)
> - return
> -
> if rpc_only == True and nfs_only == True:
> print_mountstats_help(prog)
> return
> @@ -628,21 +624,31 @@ def mountstats_command():
> infile = '/proc/self/mountstats'
> mountstats = parse_stats_file(infile)
>
> + # make certain devices contains only NFS mount points
> + if len(mountpoints) > 0:
> + check = []
> + for device in mountpoints:
> + stats = DeviceData()
> + stats.parse_stats(mountstats[device])
> + if stats.is_nfs_mountpoint():
> + check += [device]
> + mountpoints = check
> + else:
> + for device, descr in mountstats.items():
> + stats = DeviceData()
> + stats.parse_stats(descr)
> + if stats.is_nfs_mountpoint():
> + mountpoints += [device]
> + if len(mountpoints) == 0:
> + print('No NFS mount points were found')
> + return
> +
> if since:
> old_mountstats = parse_stats_file(since)
>
> for mp in mountpoints:
> - if mp not in mountstats:
> - print('Statistics for mount point %s not found' % mp)
> - continue
> -
> stats = DeviceData()
> stats.parse_stats(mountstats[mp])
> -
> - if not stats.is_nfs_mountpoint():
> - print('Mount point %s exists but is not an NFS mount' % mp)
> - continue
> -
> if not since:
> print_mountstats(stats, nfs_only, rpc_only)
> elif since and mp not in old_mountstats:
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com




2014-11-05 20:34:41

by Steve Dickson

[permalink] [raw]
Subject: Re: [nfs-utils RFC PATCH 00/15] A few enhancements to mountstats.py



On 11/05/2014 12:00 PM, Scott Mayhew wrote:
> The following patches add a couple of enhancements to mountstats.py. I
> also fixed a few bugs I encountered along the way. Highlights include:
>
> - added support for -f/--file to allow stats to be parsed from an
> aritrary input file instead of /proc/self/mountstats
>
> - added support for -S/--since to show just the changes that have
> occurred between the current and a previous set of statisics (works
> with and without the -f option)
>
> - added support for -R/--raw to generate 'raw' statistics (i.e. in the
> same format as /proc/self/mountstats). It's intended to be used with
> the -f and -S options.
The mountstats(8) will need to be updated with these new flags...
You could probably just use the verbiage from the usage messages

Ping me if want some help with the archaic nroff notation ;-)
>
> - implemented the ms-nfsstat command to generate client-side
> nfsstat-like statisics (only works with a single mountpoint)
>
> My motivation for these changes was so that I could take various copies
> of /proc/self/mountstats and massage them into data that I could feed
> into the 'report' option of Dros's nfsometer tool for scenarios where
> it's not feasible to run nfsometer itself (e.g. systems where we can't
> start with an 'idle' state (i.e. no NFS filesystems initially
> mounted), systems with multiple NFS filesystems mounted, and workloads
> that can't easily be boiled down into an nfsometer workload file or run
> via the custom workload environment variables).
Thanks for doing this!

steved.
>
> Scott Mayhew (15):
> mountstats: Fix up NFS event counters
> mountstats: Add lists of various counters
> mountstats: Refactor __parse_nfs_line and __parse_rpc_line
> mountstats: Refactor compare_iostats
> mountstats: Convert existing option parsing to use the getopt module
> mountstats: Make ms-iostat output match that of nfs-iostat.py
> mountstats: Make print_iostat_summary handle newly appearing mounts
> mountstats: Add support for -f/--file to the mountstats and ms-iostat
> commands
> mountstats: Add support for -S/--since to the mountstats and ms-iostat
> commands
> mountstats: Fix IndexError in __parse_nfs_line
> mountstats: Allow mountstats_command to take a variable number of
> mountpoints
> mountstats: Add support for -R/--raw to mountstats_command
> mountstats: Implement nfsstat_command
> mountstats: Remove the --start and --end options
> mountstats: Update the help output
>
> tools/mountstats/mountstats.py | 730 ++++++++++++++++++++++++++++++-----------
> 1 file changed, 544 insertions(+), 186 deletions(-)
> mode change 100644 => 100755 tools/mountstats/mountstats.py
>

2014-11-05 18:08:57

by Chuck Lever III

[permalink] [raw]
Subject: Re: [nfs-utils RFC PATCH 00/15] A few enhancements to mountstats.py

Hi Scott-

> On Nov 5, 2014, at 12:00 PM, Scott Mayhew <[email protected]> wrote:
>
> The following patches add a couple of enhancements to mountstats.py. I
> also fixed a few bugs I encountered along the way. Highlights include:
>
> - added support for -f/--file to allow stats to be parsed from an
> aritrary input file instead of /proc/self/mountstats
>
> - added support for -S/--since to show just the changes that have
> occurred between the current and a previous set of statisics (works
> with and without the -f option)
>
> - added support for -R/--raw to generate 'raw' statistics (i.e. in the
> same format as /proc/self/mountstats). It's intended to be used with
> the -f and -S options.
>
> - implemented the ms-nfsstat command to generate client-side
> nfsstat-like statisics (only works with a single mountpoint)
>
> My motivation for these changes was so that I could take various copies
> of /proc/self/mountstats and massage them into data that I could feed
> into the 'report' option of Dros's nfsometer tool for scenarios where
> it's not feasible to run nfsometer itself (e.g. systems where we can't
> start with an 'idle' state (i.e. no NFS filesystems initially
> mounted), systems with multiple NFS filesystems mounted, and workloads
> that can't easily be boiled down into an nfsometer workload file or run
> via the custom workload environment variables).

Do you have a git repo I can pull these from to try out?


> Scott Mayhew (15):
> mountstats: Fix up NFS event counters
> mountstats: Add lists of various counters
> mountstats: Refactor __parse_nfs_line and __parse_rpc_line
> mountstats: Refactor compare_iostats
> mountstats: Convert existing option parsing to use the getopt module
> mountstats: Make ms-iostat output match that of nfs-iostat.py
> mountstats: Make print_iostat_summary handle newly appearing mounts
> mountstats: Add support for -f/--file to the mountstats and ms-iostat
> commands
> mountstats: Add support for -S/--since to the mountstats and ms-iostat
> commands
> mountstats: Fix IndexError in __parse_nfs_line
> mountstats: Allow mountstats_command to take a variable number of
> mountpoints
> mountstats: Add support for -R/--raw to mountstats_command
> mountstats: Implement nfsstat_command
> mountstats: Remove the --start and --end options
> mountstats: Update the help output
>
> tools/mountstats/mountstats.py | 730 ++++++++++++++++++++++++++++++-----------
> 1 file changed, 544 insertions(+), 186 deletions(-)
> mode change 100644 => 100755 tools/mountstats/mountstats.py
>
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-11-05 22:07:52

by Scott Mayhew

[permalink] [raw]
Subject: Re: [nfs-utils RFC PATCH 00/15] A few enhancements to mountstats.py

Hi Chuck,

On Wed, 05 Nov 2014, Chuck Lever wrote:

> Hi Scott-
>
> > On Nov 5, 2014, at 12:00 PM, Scott Mayhew <[email protected]> wrote:
> >
> > The following patches add a couple of enhancements to mountstats.py. I
> > also fixed a few bugs I encountered along the way. Highlights include:
> >
> > - added support for -f/--file to allow stats to be parsed from an
> > aritrary input file instead of /proc/self/mountstats
> >
> > - added support for -S/--since to show just the changes that have
> > occurred between the current and a previous set of statisics (works
> > with and without the -f option)
> >
> > - added support for -R/--raw to generate 'raw' statistics (i.e. in the
> > same format as /proc/self/mountstats). It's intended to be used with
> > the -f and -S options.
> >
> > - implemented the ms-nfsstat command to generate client-side
> > nfsstat-like statisics (only works with a single mountpoint)
> >
> > My motivation for these changes was so that I could take various copies
> > of /proc/self/mountstats and massage them into data that I could feed
> > into the 'report' option of Dros's nfsometer tool for scenarios where
> > it's not feasible to run nfsometer itself (e.g. systems where we can't
> > start with an 'idle' state (i.e. no NFS filesystems initially
> > mounted), systems with multiple NFS filesystems mounted, and workloads
> > that can't easily be boiled down into an nfsometer workload file or run
> > via the custom workload environment variables).
>
> Do you have a git repo I can pull these from to try out?

You can pull them from https://github.com/scottmayhew/nfs-utils.git
commit ids f9c256040^..6a43a1599

-Scott

>
>
> > Scott Mayhew (15):
> > mountstats: Fix up NFS event counters
> > mountstats: Add lists of various counters
> > mountstats: Refactor __parse_nfs_line and __parse_rpc_line
> > mountstats: Refactor compare_iostats
> > mountstats: Convert existing option parsing to use the getopt module
> > mountstats: Make ms-iostat output match that of nfs-iostat.py
> > mountstats: Make print_iostat_summary handle newly appearing mounts
> > mountstats: Add support for -f/--file to the mountstats and ms-iostat
> > commands
> > mountstats: Add support for -S/--since to the mountstats and ms-iostat
> > commands
> > mountstats: Fix IndexError in __parse_nfs_line
> > mountstats: Allow mountstats_command to take a variable number of
> > mountpoints
> > mountstats: Add support for -R/--raw to mountstats_command
> > mountstats: Implement nfsstat_command
> > mountstats: Remove the --start and --end options
> > mountstats: Update the help output
> >
> > tools/mountstats/mountstats.py | 730 ++++++++++++++++++++++++++++++-----------
> > 1 file changed, 544 insertions(+), 186 deletions(-)
> > mode change 100644 => 100755 tools/mountstats/mountstats.py
> >
> > --
> > 1.9.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html


2014-11-05 17:01:16

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 06/15] mountstats: Make ms-iostat output match that of nfs-iostat.py

Changes mostly lifted straight from nfs-iostat.py.

Signed-off-by: Scott Mayhew <[email protected]>
---
tools/mountstats/mountstats.py | 104 ++++++++++++++++++++++++-----------------
1 file changed, 61 insertions(+), 43 deletions(-)

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index 272a8f9..d08384e 100755
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -449,60 +449,78 @@ class DeviceData:
result.__nfs_data[key] -= old_stats.__nfs_data[key]
return result

+ def __print_rpc_op_stats(self, op, sample_time):
+ """Print generic stats for one RPC op
+ """
+ if op not in self.__rpc_data:
+ return
+
+ rpc_stats = self.__rpc_data[op]
+ ops = float(rpc_stats[0])
+ retrans = float(rpc_stats[1] - rpc_stats[0])
+ kilobytes = float(rpc_stats[3] + rpc_stats[4]) / 1024
+ rtt = float(rpc_stats[6])
+ exe = float(rpc_stats[7])
+
+ # prevent floating point exceptions
+ if ops != 0:
+ kb_per_op = kilobytes / ops
+ retrans_percent = (retrans * 100) / ops
+ rtt_per_op = rtt / ops
+ exe_per_op = exe / ops
+ else:
+ kb_per_op = 0.0
+ retrans_percent = 0.0
+ rtt_per_op = 0.0
+ exe_per_op = 0.0
+
+ op += ':'
+ print(format(op.lower(), '<16s'), end='')
+ print(format('ops/s', '>8s'), end='')
+ print(format('kB/s', '>16s'), end='')
+ print(format('kB/op', '>16s'), end='')
+ print(format('retrans', '>16s'), end='')
+ print(format('avg RTT (ms)', '>16s'), end='')
+ print(format('avg exe (ms)', '>16s'))
+
+ print(format((ops / sample_time), '>24.3f'), end='')
+ print(format((kilobytes / sample_time), '>16.3f'), end='')
+ print(format(kb_per_op, '>16.3f'), end='')
+ retransmits = '{0:>10.0f} ({1:>3.1f}%)'.format(retrans, retrans_percent).strip()
+ print(format(retransmits, '>16'), end='')
+ print(format(rtt_per_op, '>16.3f'), end='')
+ print(format(exe_per_op, '>16.3f'))
+
def display_iostats(self, sample_time):
"""Display NFS and RPC stats in an iostat-like way
"""
sends = float(self.__rpc_data['rpcsends'])
if sample_time == 0:
sample_time = float(self.__nfs_data['age'])
+ # sample_time could still be zero if the export was just mounted.
+ # Set it to 1 to avoid divide by zero errors in this case since we'll
+ # likely still have relevant mount statistics to show.
+ #
+ if sample_time == 0:
+ sample_time = 1;
+ if sends != 0:
+ backlog = (float(self.__rpc_data['backlogutil']) / sends) / sample_time
+ else:
+ backlog = 0.0

print()
print('%s mounted on %s:' % \
(self.__nfs_data['export'], self.__nfs_data['mountpoint']))
+ print()

- print('\top/s\trpc bklog')
- print('\t%.2f' % (sends / sample_time), end=' ')
- if sends != 0:
- print('\t%.2f' % \
- ((float(self.__rpc_data['backlogutil']) / sends) / sample_time))
- else:
- print('\t0.00')
-
- # reads: ops/s, kB/s, avg rtt, and avg exe
- # XXX: include avg xfer size and retransmits?
- read_rpc_stats = self.__rpc_data['READ']
- ops = float(read_rpc_stats[0])
- kilobytes = float(self.__nfs_data['serverreadbytes']) / 1024
- rtt = float(read_rpc_stats[6])
- exe = float(read_rpc_stats[7])
-
- print('\treads:\tops/s\t\tkB/s\t\tavg RTT (ms)\tavg exe (ms)')
- print('\t\t%.2f' % (ops / sample_time), end=' ')
- print('\t\t%.2f' % (kilobytes / sample_time), end=' ')
- if ops != 0:
- print('\t\t%.2f' % (rtt / ops), end=' ')
- print('\t\t%.2f' % (exe / ops))
- else:
- print('\t\t0.00', end=' ')
- print('\t\t0.00')
-
- # writes: ops/s, kB/s, avg rtt, and avg exe
- # XXX: include avg xfer size and retransmits?
- write_rpc_stats = self.__rpc_data['WRITE']
- ops = float(write_rpc_stats[0])
- kilobytes = float(self.__nfs_data['serverwritebytes']) / 1024
- rtt = float(write_rpc_stats[6])
- exe = float(write_rpc_stats[7])
-
- print('\twrites:\tops/s\t\tkB/s\t\tavg RTT (ms)\tavg exe (ms)')
- print('\t\t%.2f' % (ops / sample_time), end=' ')
- print('\t\t%.2f' % (kilobytes / sample_time), end=' ')
- if ops != 0:
- print('\t\t%.2f' % (rtt / ops), end=' ')
- print('\t\t%.2f' % (exe / ops))
- else:
- print('\t\t0.00', end=' ')
- print('\t\t0.00')
+ print(format('ops/s', '>16') + format('rpc bklog', '>16'))
+ print(format((sends / sample_time), '>16.3f'), end='')
+ print(format(backlog, '>16.3f'))
+ print()
+
+ self.__print_rpc_op_stats('READ', sample_time)
+ self.__print_rpc_op_stats('WRITE', sample_time)
+ sys.stdout.flush()

def parse_stats_file(filename):
"""pop the contents of a mountstats file into a dictionary,
--
1.9.3


2014-11-05 17:01:14

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 03/15] mountstats: Refactor __parse_nfs_line and __parse_rpc_line

Iterate over the newly added counter lists instead of having a ton of
assignment statements.

Signed-off-by: Scott Mayhew <[email protected]>
---
tools/mountstats/mountstats.py | 92 +++++++++++-------------------------------
1 file changed, 23 insertions(+), 69 deletions(-)

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index 534196c..69c94d8 100755
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -237,40 +237,18 @@ class DeviceData:
if self.__nfs_data['flavor'] == 6:
self.__nfs_data['pseudoflavor'] = int(keys[1].split('=')[1])
elif words[0] == 'events:':
- self.__nfs_data['inoderevalidates'] = int(words[1])
- self.__nfs_data['dentryrevalidates'] = int(words[2])
- self.__nfs_data['datainvalidates'] = int(words[3])
- self.__nfs_data['attrinvalidates'] = int(words[4])
- self.__nfs_data['vfsopen'] = int(words[5])
- self.__nfs_data['vfslookup'] = int(words[6])
- self.__nfs_data['vfspermission'] = int(words[7])
- self.__nfs_data['vfsupdatepage'] = int(words[8])
- self.__nfs_data['vfsreadpage'] = int(words[9])
- self.__nfs_data['vfsreadpages'] = int(words[10])
- self.__nfs_data['vfswritepage'] = int(words[11])
- self.__nfs_data['vfswritepages'] = int(words[12])
- self.__nfs_data['vfsreaddir'] = int(words[13])
- self.__nfs_data['vfssetattr'] = int(words[14])
- self.__nfs_data['vfsflush'] = int(words[15])
- self.__nfs_data['vfsfsync'] = int(words[16])
- self.__nfs_data['vfslock'] = int(words[17])
- self.__nfs_data['vfsrelease'] = int(words[18])
- self.__nfs_data['congestionwait'] = int(words[19])
- self.__nfs_data['setattrtrunc'] = int(words[20])
- self.__nfs_data['extendwrite'] = int(words[21])
- self.__nfs_data['sillyrenames'] = int(words[22])
- self.__nfs_data['shortreads'] = int(words[23])
- self.__nfs_data['shortwrites'] = int(words[24])
- self.__nfs_data['delay'] = int(words[25])
- self.__nfs_data['pnfsreads'] = int(words[26])
- self.__nfs_data['pnfswrites'] = int(words[27])
+ i = 1
+ for key in NfsEventCounters:
+ try:
+ self.__nfs_data[key] = int(words[i])
+ except IndexError as err:
+ self.__nfs_data[key] = 0
+ i += 1
elif words[0] == 'bytes:':
- self.__nfs_data['normalreadbytes'] = int(words[1])
- self.__nfs_data['normalwritebytes'] = int(words[2])
- self.__nfs_data['directreadbytes'] = int(words[3])
- self.__nfs_data['directwritebytes'] = int(words[4])
- self.__nfs_data['serverreadbytes'] = int(words[5])
- self.__nfs_data['serverwritebytes'] = int(words[6])
+ i = 1
+ for key in NfsByteCounters:
+ self.__nfs_data[key] = int(words[i])
+ i += 1

def __parse_rpc_line(self, words):
if words[0] == 'RPC':
@@ -279,44 +257,20 @@ class DeviceData:
elif words[0] == 'xprt:':
self.__rpc_data['protocol'] = words[1]
if words[1] == 'udp':
- self.__rpc_data['port'] = int(words[2])
- self.__rpc_data['bind_count'] = int(words[3])
- self.__rpc_data['rpcsends'] = int(words[4])
- self.__rpc_data['rpcreceives'] = int(words[5])
- self.__rpc_data['badxids'] = int(words[6])
- self.__rpc_data['inflightsends'] = int(words[7])
- self.__rpc_data['backlogutil'] = int(words[8])
+ i = 2
+ for key in XprtUdpCounters:
+ self.__rpc_data[key] = int(words[i])
+ i += 1
elif words[1] == 'tcp':
- self.__rpc_data['port'] = words[2]
- self.__rpc_data['bind_count'] = int(words[3])
- self.__rpc_data['connect_count'] = int(words[4])
- self.__rpc_data['connect_time'] = int(words[5])
- self.__rpc_data['idle_time'] = int(words[6])
- self.__rpc_data['rpcsends'] = int(words[7])
- self.__rpc_data['rpcreceives'] = int(words[8])
- self.__rpc_data['badxids'] = int(words[9])
- self.__rpc_data['inflightsends'] = int(words[10])
- self.__rpc_data['backlogutil'] = int(words[11])
+ i = 2
+ for key in XprtTcpCounters:
+ self.__rpc_data[key] = int(words[i])
+ i += 1
elif words[1] == 'rdma':
- self.__rpc_data['port'] = words[2]
- self.__rpc_data['bind_count'] = int(words[3])
- self.__rpc_data['connect_count'] = int(words[4])
- self.__rpc_data['connect_time'] = int(words[5])
- self.__rpc_data['idle_time'] = int(words[6])
- self.__rpc_data['rpcsends'] = int(words[7])
- self.__rpc_data['rpcreceives'] = int(words[8])
- self.__rpc_data['badxids'] = int(words[9])
- self.__rpc_data['backlogutil'] = int(words[10])
- self.__rpc_data['read_chunks'] = int(words[11])
- self.__rpc_data['write_chunks'] = int(words[12])
- self.__rpc_data['reply_chunks'] = int(words[13])
- self.__rpc_data['total_rdma_req'] = int(words[14])
- self.__rpc_data['total_rdma_rep'] = int(words[15])
- self.__rpc_data['pullup'] = int(words[16])
- self.__rpc_data['fixup'] = int(words[17])
- self.__rpc_data['hardway'] = int(words[18])
- self.__rpc_data['failed_marshal'] = int(words[19])
- self.__rpc_data['bad_reply'] = int(words[20])
+ i = 2
+ for key in XprtRdmaCounters:
+ self.__rpc_data[key] = int(words[i])
+ i += 1
elif words[0] == 'per-op':
self.__rpc_data['per-op'] = words
else:
--
1.9.3


2014-11-05 17:01:15

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 02/15] mountstats: Add lists of various counters

The NfsEventCounters and NfsByteCounters were lifted straight from
nfs-iostat.py. Also added counters for the xprt line (for udp, tcp, and
rdma) as well as for v3 and v4 NFS ops. These will allow for more
compact code in a couple of places.

Signed-off-by: Scott Mayhew <[email protected]>
---
tools/mountstats/mountstats.py | 168 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 168 insertions(+)

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index 5770f2a..534196c 100755
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -32,6 +32,174 @@ def difference(x, y):
"""
return x - y

+NfsEventCounters = [
+ 'inoderevalidates',
+ 'dentryrevalidates',
+ 'datainvalidates',
+ 'attrinvalidates',
+ 'vfsopen',
+ 'vfslookup',
+ 'vfspermission',
+ 'vfsupdatepage',
+ 'vfsreadpage',
+ 'vfsreadpages',
+ 'vfswritepage',
+ 'vfswritepages',
+ 'vfsreaddir',
+ 'vfssetattr',
+ 'vfsflush',
+ 'vfsfsync',
+ 'vfslock',
+ 'vfsrelease',
+ 'congestionwait',
+ 'setattrtrunc',
+ 'extendwrite',
+ 'sillyrenames',
+ 'shortreads',
+ 'shortwrites',
+ 'delay',
+ 'pnfsreads',
+ 'pnfswrites'
+]
+
+NfsByteCounters = [
+ 'normalreadbytes',
+ 'normalwritebytes',
+ 'directreadbytes',
+ 'directwritebytes',
+ 'serverreadbytes',
+ 'serverwritebytes',
+ 'readpages',
+ 'writepages'
+]
+
+XprtUdpCounters = [
+ 'port',
+ 'bind_count',
+ 'rpcsends',
+ 'rpcreceives',
+ 'badxids',
+ 'inflightsends',
+ 'backlogutil'
+]
+
+XprtTcpCounters = [
+ 'port',
+ 'bind_count',
+ 'connect_count',
+ 'connect_time',
+ 'idle_time',
+ 'rpcsends',
+ 'rpcreceives',
+ 'badxids',
+ 'inflightsends',
+ 'backlogutil'
+]
+
+XprtRdmaCounters = [
+ 'port',
+ 'bind_count',
+ 'connect_count',
+ 'connect_time',
+ 'idle_time',
+ 'rpcsends',
+ 'rpcreceives',
+ 'badxids',
+ 'backlogutil',
+ 'read_chunks',
+ 'write_chunks',
+ 'reply_chunks',
+ 'total_rdma_req',
+ 'total_rdma_rep',
+ 'pullup',
+ 'fixup',
+ 'hardway',
+ 'failed_marshal',
+ 'bad_reply'
+]
+
+Nfsv3ops = [
+ 'NULL',
+ 'GETATTR',
+ 'SETATTR',
+ 'LOOKUP',
+ 'ACCESS',
+ 'READLINK',
+ 'READ',
+ 'WRITE',
+ 'CREATE',
+ 'MKDIR',
+ 'SYMLINK',
+ 'MKNOD',
+ 'REMOVE',
+ 'RMDIR',
+ 'RENAME',
+ 'LINK',
+ 'READDIR',
+ 'READDIRPLUS',
+ 'FSSTAT',
+ 'FSINFO',
+ 'PATHCONF',
+ 'COMMIT'
+]
+
+Nfsv4ops = [
+ 'NULL',
+ 'READ',
+ 'WRITE',
+ 'COMMIT',
+ 'OPEN',
+ 'OPEN_CONFIRM',
+ 'OPEN_NOATTR',
+ 'OPEN_DOWNGRADE',
+ 'CLOSE',
+ 'SETATTR',
+ 'FSINFO',
+ 'RENEW',
+ 'SETCLIENTID',
+ 'SETCLIENTID_CONFIRM',
+ 'LOCK',
+ 'LOCKT',
+ 'LOCKU',
+ 'ACCESS',
+ 'GETATTR',
+ 'LOOKUP',
+ 'LOOKUP_ROOT',
+ 'REMOVE',
+ 'RENAME',
+ 'LINK',
+ 'SYMLINK',
+ 'CREATE',
+ 'PATHCONF',
+ 'STATFS',
+ 'READLINK',
+ 'READDIR',
+ 'SERVER_CAPS',
+ 'DELEGRETURN',
+ 'GETACL',
+ 'SETACL',
+ 'FS_LOCATIONS',
+ 'RELEASE_LOCKOWNER',
+ 'SECINFO',
+ 'FSID_PRESENT',
+ 'EXCHANGE_ID',
+ 'CREATE_SESSION',
+ 'DESTROY_SESSION',
+ 'SEQUENCE',
+ 'GET_LEASE_TIME',
+ 'RECLAIM_COMPLETE',
+ 'LAYOUTGET',
+ 'GETDEVICEINFO',
+ 'LAYOUTCOMMIT',
+ 'LAYOUTRETURN',
+ 'SECINFO_NO_NAME',
+ 'TEST_STATEID',
+ 'FREE_STATEID',
+ 'GETDEVICELIST',
+ 'BIND_CONN_TO_SESSION',
+ 'DESTROY_CLIENTID'
+]
+
class DeviceData:
"""DeviceData objects provide methods for parsing and displaying
data for a single mount grabbed from /proc/self/mountstats
--
1.9.3


2014-11-05 17:01:14

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils RFC PATCH 04/15] mountstats: Refactor compare_iostats

Iterate over the newly added counters instead of using repeated
assignment statements. Also compute the difference of every counter
where it makes sense -- this will allow support for -S/--since to be
implemented in the future.

Signed-off-by: Scott Mayhew <[email protected]>
---
tools/mountstats/mountstats.py | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index 69c94d8..2fe1362 100755
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -412,7 +412,11 @@ class DeviceData:
def compare_iostats(self, old_stats):
"""Return the difference between two sets of stats
"""
+ if old_stats.__nfs_data['age'] > self.__nfs_data['age']:
+ return self
+
result = DeviceData()
+ protocol = self.__rpc_data['protocol']

# copy self into result
for key, value in self.__nfs_data.items():
@@ -427,12 +431,21 @@ class DeviceData:
for op in result.__rpc_data['ops']:
result.__rpc_data[op] = list(map(difference, self.__rpc_data[op], old_stats.__rpc_data[op]))

- # update the remaining keys we care about
- result.__rpc_data['rpcsends'] -= old_stats.__rpc_data['rpcsends']
- result.__rpc_data['backlogutil'] -= old_stats.__rpc_data['backlogutil']
- result.__nfs_data['serverreadbytes'] -= old_stats.__nfs_data['serverreadbytes']
- result.__nfs_data['serverwritebytes'] -= old_stats.__nfs_data['serverwritebytes']
-
+ # update the remaining keys
+ if protocol == 'udp':
+ for key in XprtUdpCounters:
+ result.__rpc_data[key] -= old_stats.__rpc_data[key]
+ elif protocol == 'tcp':
+ for key in XprtTcpCounters:
+ result.__rpc_data[key] -= old_stats.__rpc_data[key]
+ elif protocol == 'rdma':
+ for key in XprtRdmaCounters:
+ result.__rpc_data[key] -= old_stats.__rpc_data[key]
+ result.__nfs_data['age'] -= old_stats.__nfs_data['age']
+ for key in NfsEventCounters:
+ result.__nfs_data[key] -= old_stats.__nfs_data[key]
+ for key in NfsByteCounters:
+ result.__nfs_data[key] -= old_stats.__nfs_data[key]
return result

def display_iostats(self, sample_time):
--
1.9.3


2014-11-06 14:46:25

by Scott Mayhew

[permalink] [raw]
Subject: Re: [nfs-utils RFC PATCH 11/15] mountstats: Allow mountstats_command to take a variable number of mountpoints

On Wed, 05 Nov 2014, Chuck Lever wrote:

>
> On Nov 5, 2014, at 12:01 PM, Scott Mayhew <[email protected]> wrote:
>
> > Allow the mountstats command to take a variable number of mountpoints
> > (including none, in which case it will print stats for all NFS
> > mountpoints it finds).
>
> Cool idea.

Thanks! It looks like I need to add some sort of a header when only the
rpc stats are being printed though.

>
> > Signed-off-by: Scott Mayhew <[email protected]>
> > ---
> > tools/mountstats/mountstats.py | 32 +++++++++++++++++++-------------
> > 1 file changed, 19 insertions(+), 13 deletions(-)
> >
> > diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
> > index 77975ad..281f4be 100755
> > --- a/tools/mountstats/mountstats.py
> > +++ b/tools/mountstats/mountstats.py
> > @@ -616,10 +616,6 @@ def mountstats_command():
> > assert False, "unhandled option"
> > mountpoints += args
> >
> > - if mountpoints == []:
> > - print_mountstats_help(prog)
> > - return
> > -
> > if rpc_only == True and nfs_only == True:
> > print_mountstats_help(prog)
> > return
> > @@ -628,21 +624,31 @@ def mountstats_command():
> > infile = '/proc/self/mountstats'
> > mountstats = parse_stats_file(infile)
> >
> > + # make certain devices contains only NFS mount points
> > + if len(mountpoints) > 0:
> > + check = []
> > + for device in mountpoints:
> > + stats = DeviceData()
> > + stats.parse_stats(mountstats[device])
> > + if stats.is_nfs_mountpoint():
> > + check += [device]
> > + mountpoints = check
> > + else:
> > + for device, descr in mountstats.items():
> > + stats = DeviceData()
> > + stats.parse_stats(descr)
> > + if stats.is_nfs_mountpoint():
> > + mountpoints += [device]
> > + if len(mountpoints) == 0:
> > + print('No NFS mount points were found')
> > + return
> > +
> > if since:
> > old_mountstats = parse_stats_file(since)
> >
> > for mp in mountpoints:
> > - if mp not in mountstats:
> > - print('Statistics for mount point %s not found' % mp)
> > - continue
> > -
> > stats = DeviceData()
> > stats.parse_stats(mountstats[mp])
> > -
> > - if not stats.is_nfs_mountpoint():
> > - print('Mount point %s exists but is not an NFS mount' % mp)
> > - continue
> > -
> > if not since:
> > print_mountstats(stats, nfs_only, rpc_only)
> > elif since and mp not in old_mountstats:
> > --
> > 1.9.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>
>
>

2014-11-06 15:02:56

by Chuck Lever III

[permalink] [raw]
Subject: Re: [nfs-utils RFC PATCH 09/15] mountstats: Add support for -S/--since to the mountstats and ms-iostat commands


On Nov 6, 2014, at 9:40 AM, Scott Mayhew <[email protected]> wrote:

> On Wed, 05 Nov 2014, Chuck Lever wrote:
>
>>
>> On Nov 5, 2014, at 12:01 PM, Scott Mayhew <[email protected]> wrote:
>>
>>> Add support for -S/--since to the mountstats and ms-iostat commands to
>>> display the delta between a previous copy of /proc/self/mountstats and
>>> the current /proc/self/mountstats file. Can be combined with the -f
>>> option to show the statistics between two different points in time.
>>
>> This is exactly what ?start and ?send were supposed to do.
>> Why not use use them?
>
> The help output for --start and --end didn't show arguments, so I was
> under the impression that these were something you'd use on the same
> system where the stats are being generated (as opposed to capturing the
> raw data so you can generate the output on some other system).

Why can?t you just copy /proc/self/mountstats to a permanent file?

> Plus I was thinking that a neat use for --start and --end would be to
> use those with an interval to capture periodic stats that could be then
> used to generate line graphs. But I guess that could just as easily be
> done with cp or cat in cron job or a shell loop.

That was the original purpose.

> Finally I was mimicking the -S/--since option from nfsstat.c.

Then, sounds like there?s precedent for -S.

>>
>>> Signed-off-by: Scott Mayhew <[email protected]>
>>> ---
>>> tools/mountstats/mountstats.py | 61 +++++++++++++++++++++++++++++-------------
>>> 1 file changed, 43 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
>>> index 23def92..b3231b2 100755
>>> --- a/tools/mountstats/mountstats.py
>>> +++ b/tools/mountstats/mountstats.py
>>> @@ -562,13 +562,28 @@ def print_mountstats_help(name):
>>> print(' --rpc display only the RPC statistics')
>>> print(' --start sample and save statistics')
>>> print(' --end resample statistics and compare them with saved')
>>> + print(' --since <file> shows difference between current stats and those in \'file\'')
>>> print()
>>>
>>> +def print_mountstats(stats, nfs_only, rpc_only):
>>> + if nfs_only:
>>> + stats.display_nfs_options()
>>> + stats.display_nfs_events()
>>> + stats.display_nfs_bytes()
>>> + elif rpc_only:
>>> + stats.display_rpc_generic_stats()
>>> + stats.display_rpc_op_stats()
>>> + else:
>>> + stats.display_nfs_options()
>>> + stats.display_nfs_bytes()
>>> + stats.display_rpc_generic_stats()
>>> + stats.display_rpc_op_stats()
>>> +
>>> def mountstats_command():
>>> """Mountstats command
>>> """
>>> try:
>>> - opts, args = getopt.getopt(sys.argv[1:], "ef:hnrsv", ["end", "file=", "help", "nfs", "rpc", "start", "version"])
>>> + opts, args = getopt.getopt(sys.argv[1:], "ef:hnrsvS:", ["end", "file=", "help", "nfs", "rpc", "start", "version", "since="])
>>> except getopt.GetoptError as err:
>>> print_mountstats_help(prog)
>>>
>>> @@ -576,6 +591,7 @@ def mountstats_command():
>>> nfs_only = False
>>> rpc_only = False
>>> infile = None
>>> + since = None
>>>
>>> for o, a in opts:
>>> if o in ("-e", "--end"):
>>> @@ -594,6 +610,8 @@ def mountstats_command():
>>> elif o in ("-v", "--version"):
>>> print('%s version %s' % (sys.argv[0], Mountstats_version))
>>> sys.exit(0)
>>> + elif o in ("-S", "--since"):
>>> + since = a
>>> else:
>>> assert False, "unhandled option"
>>> mountpoints += args
>>> @@ -610,6 +628,9 @@ def mountstats_command():
>>> infile = '/proc/self/mountstats'
>>> mountstats = parse_stats_file(infile)
>>>
>>> + if since:
>>> + old_mountstats = parse_stats_file(since)
>>> +
>>> for mp in mountpoints:
>>> if mp not in mountstats:
>>> print('Statistics for mount point %s not found' % mp)
>>> @@ -622,18 +643,15 @@ def mountstats_command():
>>> print('Mount point %s exists but is not an NFS mount' % mp)
>>> continue
>>>
>>> - if nfs_only:
>>> - stats.display_nfs_options()
>>> - stats.display_nfs_events()
>>> - stats.display_nfs_bytes()
>>> - elif rpc_only:
>>> - stats.display_rpc_generic_stats()
>>> - stats.display_rpc_op_stats()
>>> + if not since:
>>> + print_mountstats(stats, nfs_only, rpc_only)
>>> + elif since and mp not in old_mountstats:
>>> + print_mountstats(stats, nfs_only, rpc_only)
>>> else:
>>> - stats.display_nfs_options()
>>> - stats.display_nfs_bytes()
>>> - stats.display_rpc_generic_stats()
>>> - stats.display_rpc_op_stats()
>>> + old_stats = DeviceData()
>>> + old_stats.parse_stats(old_mountstats[mp])
>>> + diff_stats = stats.compare_iostats(old_stats)
>>> + print_mountstats(diff_stats, nfs_only, rpc_only)
>>>
>>> def print_nfsstat_help(name):
>>> print('usage: %s [ options ]' % name)
>>> @@ -684,7 +702,7 @@ def iostat_command():
>>> """iostat-like command for NFS mount points
>>> """
>>> try:
>>> - opts, args = getopt.getopt(sys.argv[1:], "f:hv", ["file=", "help", "version"])
>>> + opts, args = getopt.getopt(sys.argv[1:], "f:hvS:", ["file=", "help", "version", "since="])
>>> except getopt.GetoptError as err:
>>> print_iostat_help(prog)
>>> devices = []
>>> @@ -692,6 +710,7 @@ def iostat_command():
>>> count_seen = False
>>> infile_seen = False
>>> infile = None
>>> + since = None
>>>
>>> for o, a in opts:
>>> if o in ("-f", "--file"):
>>> @@ -703,6 +722,8 @@ def iostat_command():
>>> elif o in ("-v", "--version"):
>>> print('%s version %s' % (sys.argv[0], Mountstats_version))
>>> sys.exit(0)
>>> + elif o in ("-S", "--since"):
>>> + since = a
>>> else:
>>> assert False, "unhandled option"
>>> if not infile:
>>> @@ -715,8 +736,8 @@ def iostat_command():
>>> elif not interval_seen:
>>> interval = int(arg)
>>> if interval > 0:
>>> - if infile_seen:
>>> - print('interval may not be used with the -f option')
>>> + if infile_seen or since:
>>> + print('interval may not be used with the -f or -S options')
>>> return
>>> else:
>>> interval_seen = True
>>> @@ -726,8 +747,8 @@ def iostat_command():
>>> elif not count_seen:
>>> count = int(arg)
>>> if count > 0:
>>> - if infile_seen:
>>> - print('count may not be used with the -f option')
>>> + if infile_seen or since:
>>> + print('count may not be used with the -f or -S options')
>>> return
>>> else:
>>> count_seen = True
>>> @@ -735,6 +756,11 @@ def iostat_command():
>>> print('Illegal <count> value')
>>> return
>>>
>>> + if since:
>>> + old_mountstats = parse_stats_file(since)
>>> + else:
>>> + old_mountstats = None
>>> +
>>> # make certain devices contains only NFS mount points
>>> if len(devices) > 0:
>>> check = []
>>> @@ -754,7 +780,6 @@ def iostat_command():
>>> print('No NFS mount points were found')
>>> return
>>>
>>> - old_mountstats = None
>>> sample_time = 0
>>>
>>> if not interval_seen:
>>> --
>>> 1.9.3
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> --
>> Chuck Lever
>> chuck[dot]lever[at]oracle[dot]com
>>
>>
>>

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com