prakashsurya requested changes on this pull request.
LGTM.
Can you also add a comment where we call `snap_stats` from `init`? To me, that
block:
```
# check if L2ARC exists
snap_stats();
if (defined $cur{"l2_size"}) {
$l2exist = 1;
}
```
makes it seem like we only call `snap_stats` so we can set `$l2exist`, but
really (at least with this change), we're calling it so we properly print the
status since boot. IMO, it'd be helpful to briefly explain that as a comment;
otherwise, if (for whatever reason) this conditional was removed, I could
envision somebody also removing the call to `snap_stats` and not realizing the
mistake.
I'm thinking of something along the lines of this:
```
--- a/usr/src/cmd/stat/arcstat/arcstat.pl
+++ b/usr/src/cmd/stat/arcstat/arcstat.pl
@@ -166,8 +166,11 @@ sub init {
detailed_usage() if $vflag;
@hdr = @xhdr if $xflag; #reset headers to xhdr
- # check if L2ARC exists
+ # we want to capture the stats here, so that we can use them to check
+ # if an L2ARC device exists; but more importantly, so that we print
+ # the stats since boot as the first line of output from main().
snap_stats();
+
if (defined $cur{"l2_size"}) {
$l2exist = 1;
}
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/581#pullrequestreview-101329650
------------------------------------------
openzfs: openzfs-developer
Permalink:
https://openzfs.topicbox.com/groups/developer/discussions/T7bc7a5f9745bd2c1-Mc720b477c8d49812ddf4765f
Delivery options: https://openzfs.topicbox.com/groups
