[pve-devel] [PATCH] qemuserver : vm_status : add extended stats (disks, nics, memory) V2

[Dietmar Maurer]

comments inline

> On June 16, 2015 at 11:18 AM Alexandre Derumier <aderumier at odiso.com> wrote:
> 
> 
> Add extended stats results for each nics,disks and memory on full stats mode
> only.
> 
> Signed-off-by: Alexandre Derumier <aderumier at odiso.com>
> ---
>  PVE/QemuServer.pm | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index e89308c..892b051 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -2347,6 +2347,12 @@ sub vmstatus {
>  
>  	$d->{netout} += $netdev->{$dev}->{receive};
>  	$d->{netin} += $netdev->{$dev}->{transmit};
> +
> +	if ($full) {
> +	    $res->{$vmid}->{nics}->{$dev}->{netout} = $netdev->{$dev}->{receive};
> +	    $res->{$vmid}->{nics}->{$dev}->{netin} = $netdev->{$dev}->{transmit};
> +	}
> +

Can't we use $d, like:

$d->{nics}->{$dev}->{netout} = $netdev->{$dev}->{receive};
$d->{nics}->{$dev}->{netin} = $netdev->{$dev}->{transmit};

>      }
>  
>      my $ctime = gettimeofday;
> @@ -2415,6 +2421,7 @@ sub vmstatus {
>  	    $d->{freemem} = $info->{free_mem};
>  	}
>  
> +	$d->{ballooninfo} = $info;
>      };
>  
>      my $blockstatscb = sub {
> @@ -2422,9 +2429,24 @@ sub vmstatus {
>  	my $data = $resp->{'return'} || [];
>  	my $totalrdbytes = 0;
>  	my $totalwrbytes = 0;
> +
> +	my $cfspath = cfs_config_path($vmid);
> +	my $conf = PVE::Cluster::cfs_read_file($cfspath);
> +	my $storecfg = PVE::Storage::config();
> +

Can we do above outside the callback (only once)? Or can we avoid that?

>  	for my $blockstat (@$data) {
>  	    $totalrdbytes = $totalrdbytes + $blockstat->{stats}->{rd_bytes};
>  	    $totalwrbytes = $totalwrbytes + $blockstat->{stats}->{wr_bytes};
> +
> +	    $blockstat->{device} =~ s/drive-//;
> +	    my $drive = parse_drive($blockstat->{device},
> $conf->{$blockstat->{device}});
> +	    next if PVE::QemuServer::drive_is_cdrom($drive);

why do you want to skip CDROMS?

> +	    my $volid = $drive->{file};
> +	    my ($storeid, $volname) = PVE::Storage::parse_volume_id($volid);
> +	    my ($vtype, $name, $owner) = PVE::Storage::parse_volname($storecfg,
> $volid);
> +
> +	    $res->{$vmid}->{blockstat}->{$storeid}->{$name} = $blockstat->{stats};
> +

I never wanted to have such complex code here. 

$blockstat->{device} =~ s/drive-//;
$res->{$vmid}->{blockstat}->{$blockstat->{device}} = $blockstat->{stats};

would be much simpler?