Re: [pve-devel] [PATCH v2 container 2/7] status: add pool usage fields

[Daniel Kral <d.kral@proxmox.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3249 bytes --]

On 16/04/2024 14:20, Fabian Grünbichler wrote:
> these are similar to existing ones, but with slightly different semantics.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>  src/PVE/LXC.pm | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index e688ea6..9b03a97 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -138,6 +138,18 @@ our $vmstatus_return_properties = {
>  	optional => 1,
>  	renderer => 'bytes',
>      },
> +    confmem => {
> +	description => "Configured amount of memory (inc. swap), might be higher than 'maxmem'.",

nit: a little redundant, but other descriptions in this description
     schema append a 'in bytes', i.e.
     "Configured amount of memory in bytes [...]"

> +	type => 'integer',
> +	optional => 1,
> +	renderer => 'bytes',
> +    },
> +    runmem => {
> +	description => "Currently configured amount of memory (inc. swap).",

same nit as for "confmem": append 'in bytes'.

> +	type => 'integer',
> +	optional => 1,
> +	renderer => 'bytes',
> +    },
>      maxdisk => {
>  	description => "Root disk size in bytes.",
>  	type => 'integer',
> @@ -160,6 +172,16 @@ our $vmstatus_return_properties = {
>  	type => 'number',
>  	optional => 1,
>      },
> +    confcpus => {
> +	description => "Configured amount of CPUs, might be higher than 'cpus'.",
> +	type => 'integer',
> +	optional => 1,
> +    },
> +    runcpus => {
> +	description => "Currently used amount of CPUs.",

I think 'configured' instead of 'used' would make more sense for
consistency with the description of 'runmem' and also to underline that
this value is only configured as visible to the container and not the
actual amount of cpus used at the moment (or am I wrong about this?).

> +	type => 'integer',
> +	optional => 1,
> +    },
>      lock => {
>  	description => "The current config lock, if any.",
>  	type => 'string',
> @@ -200,6 +222,7 @@ sub vmstatus {
>  	my $conf = PVE::Cluster::cfs_read_file($cfspath) || {};
>  
>  	$unprivileged->{$vmid} = $conf->{unprivileged};
> +	my $usage = PVE::LXC::Config->get_pool_usage($conf);
>  
>  	$d->{name} = $conf->{'hostname'} || "CT$vmid";
>  	$d->{name} =~ s/[\s]//g;
> @@ -207,6 +230,9 @@ sub vmstatus {
>  	$d->{cpus} = $conf->{cores} || $conf->{cpulimit};
>  	$d->{cpus} = $cpucount if !$d->{cpus};
>  
> +	$d->{confcpus} = $usage->{cpu};
> +	$d->{runcpus} = $conf->{cores} || $cpucount;
> +
>  	$d->{tags} = $conf->{tags} if defined($conf->{tags});
>  
>  	if ($d->{pid}) {
> @@ -229,6 +255,9 @@ sub vmstatus {
>  	$d->{maxmem} = ($conf->{memory}||512)*1024*1024;
>  	$d->{maxswap} = ($conf->{swap}//0)*1024*1024;
>  
> +	$d->{confmem} = $usage->{mem};
> +	$d->{runmem} = $conf->{maxmem} + $conf->{swap};

Shouldn't the `$conf->{maxmem}` be a `$d->{maxmem}`?

Perl complains about this line for me in the journal logs:

Use of uninitialized value in addition (+) at
	/usr/share/perl5/PVE/LXC.pm line 301.

> +
>  	$d->{uptime} = 0;
>  	$d->{cpu} = 0;
>  
> -- 
> 2.39.2



[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel