From mboxrd@z Thu Jan  1 00:00:00 1970
From: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
Subject: Re: [PATCH v2 17/17] tools/kvm_stat: add '%Total' column
Date: Wed, 15 Mar 2017 22:52:39 +0100
Message-ID: <20170315215239.GH14081@potion>
References: <20170310124016.96319-1-raspl@linux.vnet.ibm.com>
 <20170310124016.96319-18-raspl@linux.vnet.ibm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, pbonzini@redhat.com,
        frankja@linux.vnet.ibm.com
To: Stefan Raspl <raspl@linux.vnet.ibm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:54218 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1751277AbdCOVwt (ORCPT <rfc822;kvm@vger.kernel.org>);
        Wed, 15 Mar 2017 17:52:49 -0400
Content-Disposition: inline
In-Reply-To: <20170310124016.96319-18-raspl@linux.vnet.ibm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

2017-03-10 13:40+0100, Stefan Raspl:
> Add column '%Total' next to 'Total' for easier comparison of numbers between
> hosts.
> 
> Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
> Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
> ---
>  tools/kvm/kvm_stat/kvm_stat | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
> index f8c48f8..8f74ed8 100755
> --- a/tools/kvm/kvm_stat/kvm_stat
> +++ b/tools/kvm/kvm_stat/kvm_stat
> @@ -973,7 +973,9 @@ class Tui(object):
>          self.screen.addstr(2, 1, 'Event')
>          self.screen.addstr(2, 1 + LABEL_WIDTH + NUMBER_WIDTH -
>                             len('Total'), 'Total')
> -        self.screen.addstr(2, 1 + LABEL_WIDTH + NUMBER_WIDTH + 8 -
> +        self.screen.addstr(2, 1 + LABEL_WIDTH + NUMBER_WIDTH + 7 -
> +                           len('%Total'), '%Total')
> +        self.screen.addstr(2, 1 + LABEL_WIDTH + NUMBER_WIDTH + 7 + 8 -
>                             len('Current'), 'Current')

I think it would look better as

  self.screen.addstr(2, 1, '%-40s%10s%7s%8s' % ('Event', 'Total', '%Total', 'Current'))

(And we could even get constants to the formatting string if we tried.)

>          self.screen.addstr(4, 1, 'Collecting data...')
>          self.screen.refresh()
> @@ -989,6 +991,9 @@ class Tui(object):
>                  return (-stats[x][1], -stats[x][0])
>              else:
>                  return (0, -stats[x][0])
> +        total = 0.
> +        for val in stats.values():
> +            total += val[0]
>          for key in sorted(stats.keys(), key=sortkey):
>  
>              if row >= self.screen.getmaxyx()[0]:
> @@ -1001,6 +1006,8 @@ class Tui(object):
>              col += LABEL_WIDTH
>              self.screen.addstr(row, col, '%10d' % (values[0],))
>              col += NUMBER_WIDTH
> +            self.screen.addstr(row, col, '%7.1f' % (values[0] * 100 / total,))
                                                                              ^
What is the purpose of making it a tuple with the comma?

Thanks.

> +            col += 7
>              if values[1] is not None:
>                  self.screen.addstr(row, col, '%8d' % (values[1] / sleeptime,))

If the number ever gets full 8 digits, it would merge with the
percentage ...
offsetting one more column and doing %7d would be nicer.

(And could be done in one self.screen.addstr() too.)

>              row += 1
> -- 
> 2.8.4
>