| Message ID | 20250130142008.3600334-1-sorenson@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mountstats/nfsiostat: bugfixes for iostat | expand |
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!
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(-)