Re: [nfs-utils PATCH 0/8] mountstats/nfsiostat: bugfixes for iostat

[Steve Dickson <steved@redhat.com>]

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(-)
> 
Frank... nice work... but I hate to break the news... with all
these changes... You now own these commands! :-) Just ask mrchuck ;-)

Committed... (tag nfs-utils-2-8-3-rc5)

steved.