2007-08-07 23:52:23

by david m. richter

[permalink] [raw]
Subject: [PATCH 0/5] nfsstat: more --diff-stat/--sleep items

Hello, Neil,

This set of patches fills in some holes from the initial
--diff-stat work. The first patch maybe should be separate, but the third
patch makes use of it (but could easily be reworked).


- patch 1 adds three counters to the client v4 ops display -- GETACL,
SETACL, and FS_LOCATIONS, which are available in /proc/net/rpc/nfs but
haven't been used.

- patch 2 adds, at your suggestion, a comment clarifying what has_stats()
is actually doing.

- patch 3 fixes a bug in diff_stats() that causes false-positives in
has_stats().

- patch 4, as Bruce Fields suggested, renames -D/--diff-stat to
the (hopefully) more intuitive -Z/--sleep.

- patch 5 adds -Z/--sleep to the nfsstat manpage.


thanks kindly,

d
.

_____
david m. richter
CITI -- Center for Information Technology Integration
http://www.citi.umich.edu

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs


2007-08-09 01:14:03

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH 0/5] nfsstat: more --diff-stat/--sleep items

On Tuesday August 7, [email protected] wrote:
> Hello, Neil,
>
> This set of patches fills in some holes from the initial
> --diff-stat work. The first patch maybe should be separate, but the third
> patch makes use of it (but could easily be reworked).
>
>
> - patch 1 adds three counters to the client v4 ops display -- GETACL,
> SETACL, and FS_LOCATIONS, which are available in /proc/net/rpc/nfs but
> haven't been used.
>
> - patch 2 adds, at your suggestion, a comment clarifying what has_stats()
> is actually doing.
>
> - patch 3 fixes a bug in diff_stats() that causes false-positives in
> has_stats().
>
> - patch 4, as Bruce Fields suggested, renames -D/--diff-stat to
> the (hopefully) more intuitive -Z/--sleep.
>
> - patch 5 adds -Z/--sleep to the nfsstat manpage.
>

Thanks. All applied (after I wraps your comments to 80 columns, and
removed white space at the end of lines).

NeilBrown

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-08-09 15:43:41

by david m. richter

[permalink] [raw]
Subject: Re: [PATCH 0/5] nfsstat: more --diff-stat/--sleep items

On Thu, 9 Aug 2007, Neil Brown wrote:
<snip>
> Thanks. All applied (after I wraps your comments to 80 columns, and
> removed white space at the end of lines).
>
> NeilBrown


thank you -- and sorry about that; i'll be more assiduous in style
issues in the future. :-/

i'd like some input on your idea of adding a timeout to --sleep: i
took 5 minutes and dropped it in yesterday, but ran into a getopt_long(3)
issue. i had wanted to just add the timeout as an optional argument to
--sleep, as opposed to adding a separate timeout flag (ugh).

unless i'm doing something wrong (and i'll poke at it now, maybe
i'm being dumb), it seems that getopt wants to see those optional args in
the forms:

nfsstat --sleep=5
nfsstat -Z5

.. but doesn't grok the optarg in these, which are more natural to me:

nfsstat --sleep 5
nfsstat -Z 5

if i'm not doing something wrong, what's the feeling about going
with -Z5 versus adding -t/--timeout 5? i don't like an extra flag like
that, but the main reason i don't like the -Z5/--sleep=5 requirement is
that getopt seems to silently swallow the time argument if one does -Z
5/--sleep 5; that is, it just pauses and waits for ^C -- and so it may
appear to a user like the timeout thing is just broken/unimplemented.


thanks kindly,

d
.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-08-10 05:13:53

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH 0/5] nfsstat: more --diff-stat/--sleep items

On Thursday August 9, [email protected] wrote:
>
> i'd like some input on your idea of adding a timeout to --sleep: i
> took 5 minutes and dropped it in yesterday, but ran into a getopt_long(3)
> issue. i had wanted to just add the timeout as an optional argument to
> --sleep, as opposed to adding a separate timeout flag (ugh).
>
> unless i'm doing something wrong (and i'll poke at it now, maybe
> i'm being dumb), it seems that getopt wants to see those optional args in
> the forms:
>
> nfsstat --sleep=5
> nfsstat -Z5
>
> .. but doesn't grok the optarg in these, which are more natural to me:
>
> nfsstat --sleep 5
> nfsstat -Z 5
>
> if i'm not doing something wrong, what's the feeling about going
> with -Z5 versus adding -t/--timeout 5? i don't like an extra flag like
> that, but the main reason i don't like the -Z5/--sleep=5 requirement is
> that getopt seems to silently swallow the time argument if one does -Z
> 5/--sleep 5; that is, it just pauses and waits for ^C -- and so it may
> appear to a user like the timeout thing is just broken/unimplemented.

You are not doing anything wrong. If the arg to --sleep is
optional, then the 5 in
--sleep 5
must be interpreted as a separate option. You need something
unambiguous like an '=' to say that there really is an arg.

I'm quite comfortable with --sleep=5 for myself.

However having followed the discussion, I really like the idea of
storing info in a file.
Something like:

nfsstat --since=/some/file

where the semantics are:
- load the stats from the kernel (as 'new').
- if /some/file exists and contains stats, load them (as 'old') and
print the difference between 'old' and 'new'.
- write 'new' to /some/file

Then you could
watch -n10 nfsstat --since=/tmp/mystats

to see stats collected over 10second intervals.
(maybe you could have --no-update to inhibit the write...)

I think that would be more useful to scripts even than --sleep=N

Thanks,
NeilBrown

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs