Re: [PATCH v2 17/17] tools/kvm_stat: add '%Total' column

["Radim Krčmář" <rkrcmar@redhat.com>]

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
>