[pve-devel] [PATCH V2 pve-common] add get_pressure_stat

[pve-devel] [PATCH V2 pve-common] add get_pressure_stat

[Alexandre Derumier]

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 src/PVE/CGroup.pm | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/src/PVE/CGroup.pm b/src/PVE/CGroup.pm
index 71d0846..cbd77cb 100644
--- a/src/PVE/CGroup.pm
+++ b/src/PVE/CGroup.pm
@@ -365,6 +365,48 @@ sub get_memory_stat {
     return $res;
 }
 
+sub get_pressure_stat {
+    my ($self) = @_;
+
+    my $res = {
+	cpu => {
+	    some => { avg10 => 0, avg60 => 0, avg300 => 0 }
+	},
+	memory => {
+	    some => { avg10 => 0, avg60 => 0, avg300 => 0 },
+	    full => { avg10 => 0, avg60 => 0, avg300 => 0 }
+	},
+	io => {
+	    some => { avg10 => 0, avg60 => 0, avg300 => 0 },
+	    full => { avg10 => 0, avg60 => 0, avg300 => 0 }
+	},
+    };
+
+    my ($path, $ver) = $self->get_path(undef, 1);
+    if (!defined($path)) {
+	# container or VM most likely isn't running
+	return undef;
+    } elsif ($ver == 2) {
+
+	foreach my $type (qw(cpu memory io)) {
+	    if (my $fh = IO::File->new ("$path/$type.pressure", "r")) {
+		while (defined (my $line = <$fh>)) {
+		    if ($line =~ /^(some|full)\s+avg10\=(\d+\.\d+)\s+avg60\=(\d+\.\d+)\s+avg300\=(\d+\.\d+)\s+total\=(\d+)/) {
+			$res->{$type}->{$1}->{avg10} = $2;
+			$res->{$type}->{$1}->{avg60} = $3;
+			$res->{$type}->{$1}->{avg300} = $4;
+		    }
+		}
+		$fh->close;
+	    }
+	}
+    } else {
+	die "bad cgroup version: $ver\n";
+    }
+
+    return $res;
+}
+
 # Change the memory limit for this container.
 #
 # Dies on error (including a not-running or currently-shutting-down guest).
-- 
2.20.1

Re: [pve-devel] [PATCH V2 pve-common] add get_pressure_stat

[Thomas Lamprecht]

On 07.02.21 14:36, Alexandre Derumier wrote:
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> ---
>  src/PVE/CGroup.pm | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/src/PVE/CGroup.pm b/src/PVE/CGroup.pm
> index 71d0846..cbd77cb 100644
> --- a/src/PVE/CGroup.pm
> +++ b/src/PVE/CGroup.pm
> @@ -365,6 +365,48 @@ sub get_memory_stat {
>      return $res;
>  }
>  
> +sub get_pressure_stat {
> +    my ($self) = @_;
> +
> +    my $res = {
> +	cpu => {
> +	    some => { avg10 => 0, avg60 => 0, avg300 => 0 }
> +	},
> +	memory => {
> +	    some => { avg10 => 0, avg60 => 0, avg300 => 0 },
> +	    full => { avg10 => 0, avg60 => 0, avg300 => 0 }
> +	},
> +	io => {
> +	    some => { avg10 => 0, avg60 => 0, avg300 => 0 },
> +	    full => { avg10 => 0, avg60 => 0, avg300 => 0 }
> +	},
> +    };
> +
> +    my ($path, $ver) = $self->get_path(undef, 1);
> +    if (!defined($path)) {
> +	# container or VM most likely isn't running
> +	return undef;

or return $res? it has already the zero defaults we'd set in pve-container
anyway?

> +    } elsif ($ver == 2) {
> +
> +	foreach my $type (qw(cpu memory io)) {
> +	    if (my $fh = IO::File->new ("$path/$type.pressure", "r")) {
> +		while (defined (my $line = <$fh>)) {
> +		    if ($line =~ /^(some|full)\s+avg10\=(\d+\.\d+)\s+avg60\=(\d+\.\d+)\s+avg300\=(\d+\.\d+)\s+total\=(\d+)/) {
> +			$res->{$type}->{$1}->{avg10} = $2;
> +			$res->{$type}->{$1}->{avg60} = $3;
> +			$res->{$type}->{$1}->{avg300} = $4;
> +		    }
> +		}
> +		$fh->close;
> +	    }
> +	}
> +    } else {
> +	die "bad cgroup version: $ver\n";

So if the v1 controller does not support it this results in die's in
the vmstatus sub in pve-container where this is used without eval?

That would be not so ideal, as the v1 one is currently the default.

Maybe just return undef or the defaults instead?

(sorry, did not saw this when checking the v1)
> +    }
> +
> +    return $res;
> +}
> +
>  # Change the memory limit for this container.
>  #
>  # Dies on error (including a not-running or currently-shutting-down guest).
> 

[pve-devel] applied: [PATCH V2 pve-common] add get_pressure_stat

[Thomas Lamprecht]

On 07.02.21 14:36, Alexandre Derumier wrote:
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> ---
>  src/PVE/CGroup.pm | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
>

I now actually applied this patch, but followed up with two commits:

* unify parsing from host and cgroup pressure stats into a common sub
  in ProcFSTools

* return undef for the v1 controller case and the zeroed default $res
  if no path is found (i.e., vm/ct not running)

hope that works for you and I made no dumb error anywhere.

Re: [pve-devel] applied: [PATCH V2 pve-common] add get_pressure_stat

[aderumier]

Hi,

Le lundi 08 février 2021 à 16:07 +0100, Thomas Lamprecht a écrit :
> On 07.02.21 14:36, Alexandre Derumier wrote:
> > Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> > ---
> >  src/PVE/CGroup.pm | 42 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> > 
> > 
> 
> I now actually applied this patch, but followed up with two commits:
> 
> * unify parsing from host and cgroup pressure stats into a common sub
>   in ProcFSTools
> 
Great, I was planning to do it after this commit apply.

> * return undef for the v1 controller case and the zeroed default $res
>   if no path is found (i.e., vm/ct not running)
> 

it shouldn't return v1 , because of undef in:
my ($path, $ver) = $self->get_path(undef, 1);

(I have done this, because pressure stats are already available in v2
path, even if other cpu,mem,disk accounting stats are still in v1 path)
But no problem, better to have a check if v1 could be return.




> hope that works for you and I made no dumb error anywhere.
> 

Thanks again !