Re: [nfs-utils PATCH] mountstats/nfsiostat: when parsing the mountstats file, only keep the nfs mounts

[Chuck Lever <chuck.lever@xxxxxxxxxx>]

Hi Frank -

On 1/21/25 10:34 PM, Frank Sorenson wrote:
> Don't store the mountstats if the fstype is not nfs

The original intent of mountstats is to be agnostic to the file system
type. The description must provide a reason why the proposed change
needs to be made.

This patch doesn't provide a bug or email link either. There's no
context we reviewers can use to help understand the purpose of this
patch.

Generally, a patch description needs to explain, among other things, why
the change is necessary. The diff already shows /what/ the patch is
doing, thus a patch description typically does not need to repeat that.


> Signed-off-by: Frank Sorenson <sorenson@xxxxxxxxxx>
> ---
>   tools/mountstats/mountstats.py | 18 +++++++++++++-----
>   tools/nfs-iostat/nfs-iostat.py | 16 +++++++++++-----
>   2 files changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
> index 8e129c83..326b35c3 100755
> --- a/tools/mountstats/mountstats.py
> +++ b/tools/mountstats/mountstats.py
> @@ -783,25 +783,33 @@ def parse_stats_file(f):
>       """pop the contents of a mountstats file into a dictionary,
>       keyed by mount point.  each value object is a list of the
>       lines in the mountstats file corresponding to the mount
> -    point named in the key.
> +    point named in the key.  Only return nfs mounts.
>       """
>       ms_dict = dict()
>       key = ''
> +    fstype = ''
>   
>       f.seek(0)
>       for line in f.readlines():
>           words = line.split()
>           if len(words) == 0:
> +            fstype = ''
> +            continue
> +        if line.startswith("no device mounted"):
> +            fstype = ''
>               continue
>           if words[0] == 'device':
> +            if 'with fstype nfs' in line:
> +                fstype = words[-2]
> +            else:
> +                fstype = words[-1]
> +
>               key = words[4]
>               new = [ line.strip() ]
> -        elif 'nfs' in words or 'nfs4' in words:
> -            key = words[3]
> -            new = [ line.strip() ]
>           else:
>               new += [ line.strip() ]
> -        ms_dict[key] = new
> +        if fstype in ('nfs', 'nfs4'):
> +            ms_dict[key] = new
>   
>       return ms_dict
>   
> diff --git a/tools/nfs-iostat/nfs-iostat.py b/tools/nfs-iostat/nfs-iostat.py
> index 31587370..f97b23c0 100755
> --- a/tools/nfs-iostat/nfs-iostat.py
> +++ b/tools/nfs-iostat/nfs-iostat.py
> @@ -445,27 +445,33 @@ def parse_stats_file(filename):
>       """pop the contents of a mountstats file into a dictionary,
>       keyed by mount point.  each value object is a list of the
>       lines in the mountstats file corresponding to the mount
> -    point named in the key.
> +    point named in the key.  Only return nfs mounts.
>       """
>       ms_dict = dict()
>       key = ''
> +    fstype = ''
>   
>       f = open(filename)
>       for line in f.readlines():
>           words = line.split()
>           if len(words) == 0:
> +            fstype = ''
>               continue
>           if line.startswith("no device mounted"):
> +            fstype = ''
>               continue
>           if words[0] == 'device':
> +            if 'with fstype nfs' in line:
> +                fstype = words[-2]
> +            else:
> +                fstype = words[-1]
> +
>               key = words[4]
>               new = [ line.strip() ]
> -        elif 'nfs' in words or 'nfs4' in words:
> -            key = words[3]
> -            new = [ line.strip() ]
>           else:
>               new += [ line.strip() ]
> -        ms_dict[key] = new
> +        if fstype in ('nfs', 'nfs4'):
> +            ms_dict[key] = new
>       f.close
>   
>       return ms_dict


--
Chuck Lever