Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 3 Jun 2020 10:57:55 -0400
From:   Scott Mayhew <smayhew@redhat.com>
To:     Rohan Sable <rsable@redhat.com>
Cc:     linux-nfs@vger.kernel.org, steved@redhat.com
Subject: Re: [PATCH] mountstats: Adding Day:Hour:Min:Sec format along with
 age to "mountstats --raw" for ease of understanding.
Message-ID: <20200603145755.GF108002@aion.usersys.redhat.com>
References: <20200602092919.GA42177@fedora.rsable.com>
MIME-Version: 1.0
In-Reply-To: <20200602092919.GA42177@fedora.rsable.com>
Content-Type: text/plain; charset=WINDOWS-1252
Content-Transfer-Encoding: 8BIT
Content-Disposition: inline
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

Hi Rohan,

On Tue, 02 Jun 2020, Rohan Sable wrote:

> The output will look something like this :
> 
> From :
> age:    2215
> 
> To   :
> age:    2267; 0 Day(s) 0 Hour(s) 37 Min(s) 47 Sec(s)
> 
> Signed-off-by: Rohan Sable <rsable@redhat.com>
> ---
>  tools/mountstats/mountstats.py | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
> index 6ac83ccb..d9b5af1b 100755
> --- a/tools/mountstats/mountstats.py
> +++ b/tools/mountstats/mountstats.py
> @@ -228,6 +228,15 @@ Nfsv4ops = [
>      'CLONE'
>  ]
>  
> +def sec_conv(rem):
> +    day = int(rem / (24 * 3600))
> +    rem %= (24 * 3600)
> +    hrs = int(rem / 3600)
> +    rem %= 3600
> +    min = int(rem / 60)
> +    sec = rem % 60
> +    print(day, "Day(s)", hrs, "Hour(s)", min, "Min(s)", sec, "Sec(s)")
> +
>  class DeviceData:
>      """DeviceData objects provide methods for parsing and displaying
>      data for a single mount grabbed from /proc/self/mountstats
> @@ -349,7 +358,8 @@ class DeviceData:
>              (self.__nfs_data['export'], self.__nfs_data['mountpoint'], \
>              self.__nfs_data['fstype'], self.__nfs_data['statvers']))
>          print('\topts:\t%s' % ','.join(self.__nfs_data['mountoptions']))
> -        print('\tage:\t%d' % self.__nfs_data['age'])
> +        print('\tage:\t%d' % self.__nfs_data['age'], end="; ")
> +        sec_conv(self.__nfs_data['age'])

If you write this raw output to a file, you can no longer use the
resulting file with 'mountstats --file' or 'mountstats --since':

[smayhew@aion nfs-utils]$ tools/mountstats/mountstats.py --file b1 --since a1
Traceback (most recent call last):
  File "tools/mountstats/mountstats.py", line 1130, in <module>
    res = main()
  File "tools/mountstats/mountstats.py", line 1119, in main
    return args.func(args)
  File "tools/mountstats/mountstats.py", line 846, in mountstats_command
    stats.parse_stats(descr)
  File "tools/mountstats/mountstats.py", line 333, in parse_stats
    self.__parse_nfs_line(words)
  File "tools/mountstats/mountstats.py", line 263, in __parse_nfs_line
    self.__nfs_data['age'] = int(words[1])
ValueError: invalid literal for int() with base 10: '366;'


That was the original intended purpose for the raw option.

I think it would be better leave display_raw_stats() alone and either
add the age (including the call to sec_conv()) to display_stats_header()
(in which case it would always appear except for when the --raw option
is used), or add it to display_nfs_options() (in which case it would
only be shown when mountstats is invoked with no options or with the
--nfs option).

-Scott

>          print('\tcaps:\t%s' % ','.join(self.__nfs_data['servercapabilities']))
>          print('\tsec:\tflavor=%d,pseudoflavor=%d' % (self.__nfs_data['flavor'], \
>              self.__nfs_data['pseudoflavor']))
> -- 
> 2.25.4
>