2009-03-17 07:46:32

by Kevin Constantine

[permalink] [raw]
Subject: [PATCH 2/2] nfsstat: Add --list flag

nfsstat.c: Adds the --list flag to print information in a list format instead of the standard multi-column format
nfsstat.man: Updates the manpage to include the --list flag.

Signed-off-by: Kevin Constantine <kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
---
utils/nfsstat/nfsstat.c | 80 +++++++++++++++++++++++++++++++++++++++++++--
utils/nfsstat/nfsstat.man | 3 ++
2 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/utils/nfsstat/nfsstat.c b/utils/nfsstat/nfsstat.c
index a447cf9..630bf14 100644
--- a/utils/nfsstat/nfsstat.c
+++ b/utils/nfsstat/nfsstat.c
@@ -170,10 +170,13 @@ DECLARE_CLT(cltinfo, _old);
static void print_all_stats(int, int, int);
static void print_server_stats(int, int);
static void print_client_stats(int, int);
+static void print_stats_list(int);
static void print_numbers(const char *, unsigned int *,
unsigned int);
static void print_callstats(const char *, const char **,
unsigned int *, unsigned int);
+static void print_callstats_list(const char *, const char **,
+ unsigned int *, unsigned int);
static int parse_raw_statfile(const char *, struct statinfo *);
static int parse_pretty_statfile(const char *, struct statinfo *);

@@ -232,6 +235,7 @@ void usage(char *name)
If # is provided, stats will be output every\n\
# seconds.\n\
-S, --since file Shows difference between current stats and those in 'file'\n\
+ -l, --list Prints stats in list format\n\
--version Show program version\n\
--help What you just did\n\
\n", name);
@@ -254,6 +258,7 @@ static struct option longopts[] =
{ "version", 0, 0, '\2' },
{ "sleep", 2, 0, 'Z' },
{ "since", 1, 0, 'S' },
+ { "list", 0, 0, 'l' },
{ NULL, 0, 0, 0 }
};

@@ -266,6 +271,7 @@ main(int argc, char **argv)
opt_prt = 0,
opt_sleep = 0,
sleep_time = 0,
+ opt_list =0,
opt_since = 0;
int c;
char *progname,
@@ -287,7 +293,7 @@ main(int argc, char **argv)
else
progname = argv[0];

- while ((c = getopt_long(argc, argv, "234acmno:Z::S:vrsz\1\2", longopts, NULL)) != EOF) {
+ while ((c = getopt_long(argc, argv, "234acmno:Z::S:vrslz\1\2", longopts, NULL)) != EOF) {
switch (c) {
case 'a':
fprintf(stderr, "nfsstat: nfs acls are not yet supported.\n");
@@ -345,6 +351,9 @@ main(int argc, char **argv)
case 's':
opt_srv = 1;
break;
+ case 'l':
+ opt_list = 1;
+ break;
case 'z':
fprintf(stderr, "nfsstat: zeroing of nfs statistics "
"not yet supported\n");
@@ -425,7 +434,11 @@ main(int argc, char **argv)
get_stats(NFSCLTSTAT, clientinfo_tmp, &opt_clt, opt_srv, 0);
diff_stats(clientinfo_tmp, clientinfo, 0);
}
- print_all_stats(opt_srv, opt_clt, opt_prt);
+ if (opt_list) {
+ print_stats_list(opt_prt);
+ } else {
+ print_all_stats(opt_srv, opt_clt, opt_prt);
+ }
printf("\n\n");
fflush(stdout);

@@ -433,7 +446,11 @@ main(int argc, char **argv)
sleep(sleep_time);
}
} else {
- print_all_stats(opt_srv, opt_clt, opt_prt);
+ if (opt_list) {
+ print_stats_list(opt_prt);
+ } else {
+ print_all_stats(opt_srv, opt_clt, opt_prt);
+ }
}

return 0;
@@ -564,6 +581,44 @@ print_client_stats(int opt_clt, int opt_prt)
}
}

+static void
+print_stats_list(int opt_prt)
+{
+ if (opt_prt & PRNT_CALLS) {
+ if ((opt_prt & PRNT_V2) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc2info))) {
+ print_callstats_list(
+ "nfs v2 server",
+ nfsv2name, srvproc2info + 1, sizeof(nfsv2name)/sizeof(char *));
+ printf("\n");
+ print_callstats_list(
+ "nfs v2 client",
+ nfsv2name, cltproc2info + 1, sizeof(nfsv2name)/sizeof(char *));
+ }
+ if ((opt_prt & PRNT_V3) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc3info))) {
+ print_callstats_list(
+ "nfs v3 server",
+ nfsv3name, srvproc3info + 1, sizeof(nfsv3name)/sizeof(char *));
+ printf("\n");
+ print_callstats_list(
+ "nfs v3 client",
+ nfsv3name, cltproc3info + 1, sizeof(nfsv3name)/sizeof(char *));
+ }
+ if ((opt_prt & PRNT_V4) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc4info))) {
+ print_callstats_list(
+ "nfs v4 server",
+ nfssrvproc4name, srvproc4info + 1, sizeof(nfssrvproc4name)/sizeof(char *));
+ printf("\n");
+ print_callstats_list(
+ "nfs v4 ops",
+ nfssrvproc4opname, srvproc4opsinfo + 1, sizeof(nfssrvproc4opname)/sizeof(char *));
+ printf("\n");
+ print_callstats_list(
+ "nfs v4 client",
+ nfscltproc4name, cltproc4info + 1, sizeof(nfscltproc4name)/sizeof(char *));
+ }
+ }
+}
+
static statinfo *
get_stat_info(const char *sp, struct statinfo *statp)
{
@@ -614,6 +669,25 @@ print_callstats(const char *hdr, const char **names,
printf("\n");
}

+static void
+print_callstats_list(const char *hdr, const char **names,
+ unsigned int *callinfo, unsigned int nr)
+{
+ unsigned long long calltotal;
+ int i;
+
+ for (i = 0, calltotal = 0; i < nr; i++) {
+ calltotal += callinfo[i];
+ }
+ printf("%13s %13s %8llu \n", hdr, "total:", calltotal);
+ printf("------------- ------------- --------\n");
+ for (i = 0; i < nr; i++) {
+ printf("%13s %12s: %8u \n", hdr, names[i], callinfo[i]);
+ }
+
+}
+
+
/* returns 0 on success, 1 otherwise */
static int
parse_raw_statfile(const char *name, struct statinfo *statp)
diff --git a/utils/nfsstat/nfsstat.man b/utils/nfsstat/nfsstat.man
index 461b2c0..52215a9 100644
--- a/utils/nfsstat/nfsstat.man
+++ b/utils/nfsstat/nfsstat.man
@@ -72,6 +72,9 @@ Display all of the above facilities.
.B \-v, \-\-verbose
This is equivalent to \fB\-o all\fR.
.TP
+.B \-l, \-\-list
+Print information in list form.
+.TP
.BI "\-S, \-\-since " file
Instead of printing current statistics,
.B nfsstat
--
1.6.0.4.761.g47577



2009-03-17 15:58:31

by Frank Filz

[permalink] [raw]
Subject: Re: [PATCH 2/2] nfsstat: Add --list flag

On Tue, 2009-03-17 at 08:15 -0400, Steve Dickson wrote:
> Thanks Greg for taking a look...
>
> Greg Banks wrote:
> > Kevin Constantine wrote:
> >
> > Maybe you can smooth out this difference, and make the code simpler and
> > cleaner, by making the output always be exactly two columns, one for the
> > label and one for the value. Then your earlier quoted example would be
> > something like
...
> > nfs v3 server readdir: 0
> > nfs v3 server readdirplus: 0
> > nfs v3 server fsstat: 0
> > nfs v3 server fsinfo: 0
> > nfs v3 server pathconf: 0
> > nfs v3 server commit: 0
> Finally, does it really make sense to show a stats with zero occurrences?
> Why not just show what has happen instead of including what has not happen?

I've done some playing with nfsstat and one thing I added was a -q
(--quick) option that suppressed zero entries. It does make it a lot
easier to look at stats when you only have to look at the interesting
data.

Frank



2009-03-17 16:33:37

by Kevin Constantine

[permalink] [raw]
Subject: Re: [PATCH 2/2] nfsstat: Add --list flag

Steve Dickson wrote:
> Thanks Greg for taking a look...
>
> Greg Banks wrote:
>> Kevin Constantine wrote:
>>
>> Maybe you can smooth out this difference, and make the code simpler and
>> cleaner, by making the output always be exactly two columns, one for the
>> label and one for the value. Then your earlier quoted example would be
>> something like
>>
>> nfs v3 client total: 213
>> nfs v3 client null: 0
[...]
>> nfs v3 client readdirplus: 0
>> nfs v3 client fsstat: 126
>> nfs v3 client fsinfo: 0
>> nfs v3 client pathconf: 0
>> nfs v3 client commit: 0
>> nfs v3 server total: 0
>> nfs v3 server null: 0
[...]
>> nfs v3 server commit: 0
> Question... Now why are we *not* using the information from /proc/self/mountstats
> and doing this per mount?
>
nfsstat is already parsing /proc/net/rpc/nfs[d]. Re-writing it to parse
/proc/self/mountstats seems like it should be a separate undertaking.

> Also, can I assume (not looking at the patches yet) that this same type info
> will be available v2 and v4?
>
Yes. v2 and v4 will look the same. Here's the new format.

nfs v3 server total: 0
------------- ------------- --------
nfs v3 server null: 0
nfs v3 server getattr: 0
nfs v3 server setattr: 0
nfs v3 server lookup: 0
nfs v3 server access: 0
nfs v3 server readlink: 0
nfs v3 server read: 0
nfs v3 server write: 0
nfs v3 server create: 0
nfs v3 server mkdir: 0
nfs v3 server symlink: 0
nfs v3 server mknod: 0
nfs v3 server remove: 0
nfs v3 server rmdir: 0
nfs v3 server rename: 0
nfs v3 server link: 0
nfs v3 server readdir: 0
nfs v3 server readdirplus: 0
nfs v3 server fsstat: 0
nfs v3 server fsinfo: 0
nfs v3 server pathconf: 0
nfs v3 server commit: 0

nfs v3 client total: 203
------------- ------------- --------
nfs v3 client null: 0
nfs v3 client getattr: 23
nfs v3 client setattr: 7
nfs v3 client lookup: 7
nfs v3 client access: 0
nfs v3 client readlink: 0
nfs v3 client read: 7
nfs v3 client write: 21
nfs v3 client create: 7
nfs v3 client mkdir: 0
nfs v3 client symlink: 0
nfs v3 client mknod: 0
nfs v3 client remove: 7
nfs v3 client rmdir: 0
nfs v3 client rename: 0
nfs v3 client link: 0
nfs v3 client readdir: 0
nfs v3 client readdirplus: 0
nfs v3 client fsstat: 124
nfs v3 client fsinfo: 0
nfs v3 client pathconf: 0
nfs v3 client commit: 0

> Finally, does it really make sense to show a stats with zero occurrences?
> Why not just show what has happen instead of including what has not happen?
>
The reason I left the zeroed stats in the display is so that the output
stats fairly constant with the values being the only information that
changes. If lines are popping in and out, it makes it difficult to keep
an eye on what you really care about.
> steved.
>
>

2009-03-18 02:33:54

by Greg Banks

[permalink] [raw]
Subject: Re: [PATCH 2/2] nfsstat: Add --list flag

Kevin Constantine wrote:
> nfsstat.c: Adds the --list flag to print information in a list format instead of the standard multi-column format
> nfsstat.man: Updates the manpage to include the --list flag.
>
> Signed-off-by: Kevin Constantine <kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
> ---
> utils/nfsstat/nfsstat.c | 80 +++++++++++++++++++++++++++++++++++++++++++--
> utils/nfsstat/nfsstat.man | 3 ++
> 2 files changed, 80 insertions(+), 3 deletions(-)
>
Acked-by: Greg Banks <[email protected]>

--
Greg Banks, P.Engineer, SGI Australian Software Group.
the brightly coloured sporks of revolution.
I don't speak for SGI.