Re: [PATCH] nfsstat.c: Print diff stats every N seconds

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Greg Banks wrote:
Kevin Constantine wrote:
This patch to nfsstat.c allows for an optional argument to --sleep or
-Z.  This optional argument is the interval at which differences in
the nfsstat block are printed.

[...]

Moving to a listed output format instead of the traditional nfsstat
output makes it trivial with a simple grep to watch the  stats that
you really care about and ignore the rest.
Perhaps you could make that output format change a separate option, in a
separate patch from the optional argument to --sleep.  That way we can
argue their merits separately.  Also, a user could get the new output
format without specifying --sleep=N; if the new format is useful it's
worth having that.

Should the --list option patch be written against the --sleep[#] patch, or against the current repo?



@@ -223,7 +230,9 @@ void usage(char *name)
   -v, --verbose, --all\tSame as '-o all'\n\
   -r, --rpc\t\tShow RPC statistics\n\
   -n, --nfs\t\tShow NFS statistics\n\
-  -Z, --sleep\t\tSaves stats, pauses, diffs current and saved\n\
+  -Z[#], --sleep[=#]    Saves stats, pauses, diffs current and saved.\n\
+                      if # is provided, stats will be output every\n\
+                # seconds\n\
   -S, --since file\tShows difference between current stats and those
in 'file'\n\
   --version\t\tShow program version\n\
   --help\t\tWhat you just did\n\

This help text was already pretty clunky and clearly written by a
programmer.  Perhaps you could re-express it while you're here.
@@ -245,7 +254,7 @@ static struct option longopts[] =

@@ -258,6 +267,7 @@ main(int argc, char **argv)

@@ -279,7 +289,7 @@ main(int argc, char **argv)

@@ -311,6 +321,9 @@ main(int argc, char **argv)

@@ -384,7 +397,7 @@ main(int argc, char **argv)

All this option handling looks good.

@@ -404,7 +417,33 @@ main(int argc, char **argv)
             diff_stats(clientinfo_tmp, clientinfo, 0);
         }
     }
+    if(sleep_time) {
+        while(1) {
+            if (opt_srv) {
+                get_stats(NFSSRVSTAT, serverinfo_tmp , &opt_srv,
opt_clt, 1);
+                diff_stats(serverinfo_tmp, serverinfo, 1);
+            }
+            if (opt_clt) {
+                get_stats(NFSCLTSTAT, clientinfo_tmp, &opt_clt,
opt_srv, 0);
+                diff_stats(clientinfo_tmp, clientinfo, 0);
+            }
+            print_stats_list(opt_prt);
+            fflush(stdout);
+
+            update_old_counters(clientinfo_tmp, clientinfo);
+            sleep(sleep_time);
+ } + }
+    else {
+        print_server_stats(opt_srv, opt_prt);
+        print_client_stats(opt_clt, opt_prt);
+    }

The nfs-utils coding style is

                        if (...) {
                                ...
                        } else {
                                ...
                        }


+    return 0;
+}
+
+static void
+print_server_stats(int opt_srv, int opt_prt) {

The nfs-utils coding style puts the opening brace { of a function on
it's own line.
@@ -515,10 +556,42 @@ main(int argc, char **argv)
                 );
         }
     }
-
-    return 0;
 }

+static void
+print_stats_list(int opt_prt) {

Looks ok on a quick scan.
+
+    for (i = 0, srvtotal = 0, clttotal = 0; i < nr; i++) {
+        if (srvinfo[i])
+            srvtotal += srvinfo[i];
+        if (cltinfo[i])
+            clttotal += cltinfo[i];
The if()s here are superfluous.

I don't see an update to the manpage.

--
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

[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux