From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id AF23A1FF16F
	for <inbox@lore.proxmox.com>; Thu, 19 Dec 2024 17:03:25 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id AFEA3BF8C;
	Thu, 19 Dec 2024 17:03:25 +0100 (CET)
From: Daniel Kral <d.kral@proxmox.com>
To: f.gruenbichler@proxmox.com
Date: Thu, 19 Dec 2024 17:02:48 +0100
Message-Id: <20241219160248.183900-1-d.kral@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20240416122054.733817-4-f.gruenbichler@proxmox.com>
References: <20240416122054.733817-4-f.gruenbichler@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.004 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH v2 container 2/7] status: add pool usage
 fields
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Cc: pve-devel@lists.proxmox.com
Content-Type: multipart/mixed; boundary="===============7122854028129625777=="
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>

--===============7122854028129625777==
Content-Transfer-Encoding: quoted-printable

On 16/04/2024 14:20, Fabian Gr=C3=BCnbichler wrote:=0D
> these are similar to existing ones, but with slightly different semantics=
.=0D
> =0D
> Signed-off-by: Fabian Gr=C3=BCnbichler <f.gruenbichler@proxmox.com>=0D
> ---=0D
>  src/PVE/LXC.pm | 29 +++++++++++++++++++++++++++++=0D
>  1 file changed, 29 insertions(+)=0D
> =0D
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm=0D
> index e688ea6..9b03a97 100644=0D
> --- a/src/PVE/LXC.pm=0D
> +++ b/src/PVE/LXC.pm=0D
> @@ -138,6 +138,18 @@ our $vmstatus_return_properties =3D {=0D
>  	optional =3D> 1,=0D
>  	renderer =3D> 'bytes',=0D
>      },=0D
> +    confmem =3D> {=0D
> +	description =3D> "Configured amount of memory (inc. swap), might be hig=
her than 'maxmem'.",=0D
=0D
nit: a little redundant, but other descriptions in this description=0D
     schema append a 'in bytes', i.e.=0D
     "Configured amount of memory in bytes [...]"=0D
=0D
> +	type =3D> 'integer',=0D
> +	optional =3D> 1,=0D
> +	renderer =3D> 'bytes',=0D
> +    },=0D
> +    runmem =3D> {=0D
> +	description =3D> "Currently configured amount of memory (inc. swap).",=
=0D
=0D
same nit as for "confmem": append 'in bytes'.=0D
=0D
> +	type =3D> 'integer',=0D
> +	optional =3D> 1,=0D
> +	renderer =3D> 'bytes',=0D
> +    },=0D
>      maxdisk =3D> {=0D
>  	description =3D> "Root disk size in bytes.",=0D
>  	type =3D> 'integer',=0D
> @@ -160,6 +172,16 @@ our $vmstatus_return_properties =3D {=0D
>  	type =3D> 'number',=0D
>  	optional =3D> 1,=0D
>      },=0D
> +    confcpus =3D> {=0D
> +	description =3D> "Configured amount of CPUs, might be higher than 'cpus=
'.",=0D
> +	type =3D> 'integer',=0D
> +	optional =3D> 1,=0D
> +    },=0D
> +    runcpus =3D> {=0D
> +	description =3D> "Currently used amount of CPUs.",=0D
=0D
I think 'configured' instead of 'used' would make more sense for=0D
consistency with the description of 'runmem' and also to underline that=0D
this value is only configured as visible to the container and not the=0D
actual amount of cpus used at the moment (or am I wrong about this?).=0D
=0D
> +	type =3D> 'integer',=0D
> +	optional =3D> 1,=0D
> +    },=0D
>      lock =3D> {=0D
>  	description =3D> "The current config lock, if any.",=0D
>  	type =3D> 'string',=0D
> @@ -200,6 +222,7 @@ sub vmstatus {=0D
>  	my $conf =3D PVE::Cluster::cfs_read_file($cfspath) || {};=0D
>  =0D
>  	$unprivileged->{$vmid} =3D $conf->{unprivileged};=0D
> +	my $usage =3D PVE::LXC::Config->get_pool_usage($conf);=0D
>  =0D
>  	$d->{name} =3D $conf->{'hostname'} || "CT$vmid";=0D
>  	$d->{name} =3D~ s/[\s]//g;=0D
> @@ -207,6 +230,9 @@ sub vmstatus {=0D
>  	$d->{cpus} =3D $conf->{cores} || $conf->{cpulimit};=0D
>  	$d->{cpus} =3D $cpucount if !$d->{cpus};=0D
>  =0D
> +	$d->{confcpus} =3D $usage->{cpu};=0D
> +	$d->{runcpus} =3D $conf->{cores} || $cpucount;=0D
> +=0D
>  	$d->{tags} =3D $conf->{tags} if defined($conf->{tags});=0D
>  =0D
>  	if ($d->{pid}) {=0D
> @@ -229,6 +255,9 @@ sub vmstatus {=0D
>  	$d->{maxmem} =3D ($conf->{memory}||512)*1024*1024;=0D
>  	$d->{maxswap} =3D ($conf->{swap}//0)*1024*1024;=0D
>  =0D
> +	$d->{confmem} =3D $usage->{mem};=0D
> +	$d->{runmem} =3D $conf->{maxmem} + $conf->{swap};=0D
=0D
Shouldn't the `$conf->{maxmem}` be a `$d->{maxmem}`?=0D
=0D
Perl complains about this line for me in the journal logs:=0D
=0D
Use of uninitialized value in addition (+) at=0D
	/usr/share/perl5/PVE/LXC.pm line 301.=0D
=0D
> +=0D
>  	$d->{uptime} =3D 0;=0D
>  	$d->{cpu} =3D 0;=0D
>  =0D
> -- =0D
> 2.39.2=0D



--===============7122854028129625777==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

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

--===============7122854028129625777==--