Chuck Lever wrote:
On Aug 25, 2009, at 1:34 PM, Steve Dickson wrote:
On 08/25/2009 12:29 PM, Chuck Lever wrote:
[ Cc: changed to correct mailing list. sf.net list is deprecated. ]
On Aug 24, 2009, at 7:24 PM, Lans Carstensen wrote:
Hi,
I've recently made tools/nfs-iostat/nfs-iostat.py more useful in our
autofs environment with a variety of cleanups and am offering this
patch up for discussion and/or inclusion in nfs-utils. It does the
following:
* Adds a --top flag to sort the display of mountpoint entries by ops/s.
* Adds a --<n> flag to only display stats for the first <n> mountpoints
* Re-reads the mountpoint list on intervals since it's dynamic in an
autofs environment.
* Conforms the Python path to the LSB 3.2+ standard of /usr/bin/python
http://refspecs.freestandards.org/LSB_3.2.0/LSB-Languages/LSB-Languages/pylocation.html
A couple of overall comments.
1. These seem to be logically independent changes, so we would prefer
them in separate patches. Each logical change can be refined and voted
up or down separately.
I guess you could break this up into more patches... but overall its
by no means unruly....
Didn't mean to imply "unruly" but we do _prefer_ having logical changes
separated.
2. Sorting by ops is OK, but not sure about "--top". Since the script
doesn't generate a curses like display like "top" does, maybe "--sort
<n>" would be a better name, and --top and --<n> could be combined.
Hmm.. I kinda like like the --top flag... its pretty descriptive to
what it does..
Some people aren't so imaginative, might expect "--top" to produce a
full curses-like display, then be pissed when they get a simple list.
And it seems to me that the two new options are related and could be
combined to good effect.
I think "--top" should be reserved for an actual top-like implementation
(which might be pretty cool).
3. The distributors should weigh in on the Python path change.
Using '#!/usr/bin/python' is better than '#!/usr/bin/env python'
since you know exactly which python binary you will be using.
It was also pointed out the env convention will confuse
rpm's dependency generator
Good enough for me. Lans, it would be nice to document this last bit
(about rpm) in the patch description.
Thanks for looking at this, guys.
* I don't live in git, so it'll take me a little bit to break out the
patches - but I certainly don't mind if that helps this along.
* To aid your testing on rescanning, there is a rescan but currently no
handling of differences in the "new" and "old" mount table and there is
no re-evaluation of the list of monitored mountpoints based on changes
in the mount table. The existing implementation will throw a Python
exception when a mountpoint (say a user homedirectory) is unmounted from
a system during a trace. The displayed data is also misleading if the
list of monitored mountpoints isn't grown to reflect new automounts that
might be your current top talkers.
* "--sort" doesn't offend me and I can switch to that. The semantic of
"--top -1" was the most intuitive for a couple of us here but "--sort
-1" isn't that far off.
* I agree a real "top" implementation would be cool.
-- Lans
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
