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.
--
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
