From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <f.gruenbichler@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id EE641962F7
 for <pve-devel@lists.proxmox.com>; Mon, 15 Apr 2024 14:37:21 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id F3AA997CF
 for <pve-devel@lists.proxmox.com>; Mon, 15 Apr 2024 14:36:50 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pve-devel@lists.proxmox.com>; Mon, 15 Apr 2024 14:36:49 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id B34DB44974
 for <pve-devel@lists.proxmox.com>; Mon, 15 Apr 2024 14:36:49 +0200 (CEST)
Date: Mon, 15 Apr 2024 14:36:45 +0200
From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= <f.gruenbichler@proxmox.com>
To: Wolfgang Bumiller <w.bumiller@proxmox.com>
Cc: pve-devel@lists.proxmox.com
References: <20240410131316.1208679-1-f.gruenbichler@proxmox.com>
 <20240410131316.1208679-12-f.gruenbichler@proxmox.com>
 <ydtgpstjnnzm2cwlhx2q63gtnk625lklvxaudpmwejxclss2ox@wwv2uzkejcuw>
In-Reply-To: <ydtgpstjnnzm2cwlhx2q63gtnk625lklvxaudpmwejxclss2ox@wwv2uzkejcuw>
MIME-Version: 1.0
User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid)
Message-Id: <1713183846.hlodlsxnxs.astroid@yuna.none>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.056 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 manager 2/4] pvestatd: collect and broadcast
 pool usage
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>
X-List-Received-Date: Mon, 15 Apr 2024 12:37:22 -0000

On April 11, 2024 11:32 am, Wolfgang Bumiller wrote:
> On Wed, Apr 10, 2024 at 03:13:08PM +0200, Fabian Gr=C3=BCnbichler wrote:
>> so that other nodes can query it and both block changes that would viola=
te the
>> limits, and mark pools which are violating it currently accordingly.
>>=20
>> Signed-off-by: Fabian Gr=C3=BCnbichler <f.gruenbichler@proxmox.com>
>> ---
>>  PVE/Service/pvestatd.pm | 59 ++++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 55 insertions(+), 4 deletions(-)
>>=20
>> diff --git a/PVE/Service/pvestatd.pm b/PVE/Service/pvestatd.pm
>> index 2515120c6..d7e4755e9 100755
>> --- a/PVE/Service/pvestatd.pm
>> +++ b/PVE/Service/pvestatd.pm
>> @@ -231,7 +231,7 @@ sub auto_balloning {
>>  }
>> =20
>>  sub update_qemu_status {
>> -    my ($status_cfg) =3D @_;
>> +    my ($status_cfg, $pool_membership, $pool_usage) =3D @_;
>> =20
>>      my $ctime =3D time();
>>      my $vmstatus =3D PVE::QemuServer::vmstatus(undef, 1);
>> @@ -242,6 +242,21 @@ sub update_qemu_status {
>>      my $transactions =3D PVE::ExtMetric::transactions_start($status_cfg=
);
>>      foreach my $vmid (keys %$vmstatus) {
>>  	my $d =3D $vmstatus->{$vmid};
>> +
>> +	if (my $pool =3D $pool_membership->{$vmid}) {
>> +	    $pool_usage->{$pool}->{$vmid} =3D {
>> +		cpu =3D> {
>> +		    config =3D> ($d->{confcpus} // 0),
>> +		    run =3D> ($d->{runcpus} // 0),
>> +		},
>> +		mem =3D> {
>> +		    config =3D> ($d->{confmem} // 0),
>> +		    run =3D> ($d->{runmem} // 0),
>> +		},
>=20
> I feel like it should be possible to build this hash given the `keys` in
> the limit hash... The `cpu-run/config` vs `{cpu}->{run}` vs `runcpu`
> naming feels a bit awkward to me.

not really, unless you want me to introduce a helper that is longer than
the hard-coded variant here?

I already have one for limit key -> usage hash (keys) for places where
we are mostly 'key agnostic' (i.e., PVE::GuestHelpers), but the vmstatus
hash has different keys again (see reply there) and those are only
relevant (in relation to the other limit/usage stuff) for the two subs
here, so if we extend the vmstatus return schema (e.g., with fields for
disk limits), then we'd need to extend the helper here anyway, just like
we'd need to extend the hard-coded variant, so I don't really see a
benefit?

adding a new limit "category" will require changes to:
- pve-access-control (for the user.cfg schema)
- qemu-server/pve-container (for actually providing the usage data via
  vmstatus, and checking the limits in all the places where current
  usage would change)
- pve-manager (for displaying/editing/.. and pvestatd conversion between
  vmstatus and usage, i.e., this part here)