[pve-devel] [PATCH storage v3 2/2] pvesm status: improve output and its format

[Fabian Grünbichler]

On Tue, Jun 13, 2017 at 10:36:03AM +0200, Emmanuel Kasper wrote:
> +1 for the idea as the ouput of this command is not totally clear, and
> we use it in pvereport
> 
> minor nitpick downwards
> 
> On 05/24/2017 12:45 PM, Thomas Lamprecht wrote:
> > Add column names at top of output, this allows easier understanding
> > of what each column means.
> > 
> > Use leading spaces on the percentage column so that this is lined up.
> > 
> > Switch out the 1/0 from the active column with the actual status
> > (active, inactive, disabled)
> > 
> > Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
> > ---
> > 
> > changes v2 -> v3:
> > * rename status column to active
> > * output status (active, inactive, disabled) in status column
> > 
> >  PVE/CLI/pvesm.pm | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
> > index 8af4e7f..3bc98b4 100755
> > --- a/PVE/CLI/pvesm.pm
> > +++ b/PVE/CLI/pvesm.pm
> > @@ -134,14 +134,19 @@ my $print_status = sub {
> >      }
> >      $maxlen+=1;
> >  
> > +    printf "%-${maxlen}s %10s %10s %15s %15s %15s %8s\n", 'name', 'type',
> > +	'status', 'total', 'available', 'used', '%';
> 
> can we maybe capitalize the header to better distinguish it from the
> data ? ( like qm list or lsblk)
> 
> >      foreach my $res (sort { $a->{storage} cmp $b->{storage} } @$res) {
> >  	my $storeid = $res->{storage};
> >  
> >  	my $sum = $res->{used} + $res->{avail};
> >  	my $per = $sum ? (0.5 + ($res->{used}*100)/$sum) : 100;
> > +	my $active = $res->{active} ? 'active' : 'inactive';
> 
> we should fix also fix the error message in activate_storage()
> "storage '$storeid' is not online\n" if we use the term 'inactive' here
> (or use online/unreachable everywhere )

but that message is for an explicit connection check (which has failed)?
that is not the only reason why a storage can be "inactive" (or fail to
activate ;)), and IMHO this differentiation does make sense.