On 1/30/25 9:19 AM, Frank Sorenson wrote: > This patchset is intended to fix several bugs in nfsiostat and > 'mountstats iostat', in particular when an <interval> is specified. > > Specifically, the patches address the following issues when printing > multiple times: > > * both nfsiostat and 'mountstats iostat': > - if <count> is also specified, the scripts sleep an additional > <interval> after printing <count> times, and parse mountstats > unnecessarily before checking the <count> > > - if no nfs mounts are present when printing, the scripts output > a message that there are no nfs mounts, and exit immediately. > > However, if multiple intervals are indicated, it makes more sense > to output the message and sleep until the next iteration; there > may be nfs mounts the next time through. > > - if mountpoints are specified on the command-line, but are not > present during an iteration, the scripts crash. > > > * 'mountstats iostat': > - if an nfs filesystem is unmounted between iterations, the > script will crash attempting to reference the non-existent > mountpoint in the new mountstats file (or if another filesystem > type is mounted at that location, will instead crash while > comparing old and new stats) > > - new nfs mounts are not detected > > > * nfsiostat: > - if a new nfs filesystem is mounted between iterations, the > script will crash attempting to reference the non-existent > mountpoint in the old mountstats file > > - if a mountpoint is specified on the command-line, but topmost > mount there is not nfs, the script crashes while trying to > compare old and new stats > > > To address these issues, the patches do the following: > * when printing diff stats from previous iteration: > - verify that a device exists in the old mountstats before referencing it > - verify that both old and new fstypes are the same > > * when filtering the current mountstats file: > - verify the device exists in the mountstats file before referencing it > - filter the list of nfs mountpoints each iostat iteration > > * check for empty device list and output the 'No NFS mount points found' > message, but don't immediately exit the script > > * merge the infinite loop and counted loop, and (for the counted loop) > decrement and check the count before sleeping and parsing the mountstats > file > > > Frank Sorenson (8): > mountstats/nfsiostat: add a function to return the fstype > mountstats: when printing iostats, verify that old and new types are > the same > nfsiostat: mirror how mountstats iostat prints the stats > nfsiostat: fix crash when filtering mountstats after unmount > nfsiostat: make comment explain mount/unmount more broadly > mountstats: filter for nfs mounts in a function, each iostat iteration > mountstats/nfsiostat: Move the checks for empty mountpoint list into > the print function > mountstats/nfsiostat: merge and rework the infinite and counted loops > > tools/mountstats/mountstats.py | 102 ++++++++++++++++-------------- > tools/nfs-iostat/nfs-iostat.py | 110 +++++++++++++-------------------- > 2 files changed, 100 insertions(+), 112 deletions(-) > Hi Frank, I browsed this series briefly. I'm not sure that counts as a full review, but it all seems sensible to me. Thanks for the clear explanations! -- Chuck Lever
