From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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 ; 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 ; 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 ; 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?= To: Wolfgang Bumiller Cc: pve-devel@lists.proxmox.com References: <20240410131316.1208679-1-f.gruenbichler@proxmox.com> <20240410131316.1208679-12-f.gruenbichler@proxmox.com> In-Reply-To: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 >> --- >> 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)