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)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 430CB96187 for ; Mon, 15 Apr 2024 11:38:22 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 269F26939 for ; Mon, 15 Apr 2024 11:38:22 +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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 15 Apr 2024 11:38:21 +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 C774B44A8A for ; Mon, 15 Apr 2024 11:38:20 +0200 (CEST) Date: Mon, 15 Apr 2024 11:38:16 +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-10-f.gruenbichler@proxmox.com> In-Reply-To: MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1713173754.uwkriy5wgf.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 guest-common 1/1] helpers: add pool limit/usage helpers 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 09:38:22 -0000 On April 11, 2024 11:17 am, Wolfgang Bumiller wrote: > On Wed, Apr 10, 2024 at 03:13:06PM +0200, Fabian Gr=C3=BCnbichler wrote: >> one for combining the per-node broadcasted values, one for checking a po= ol's >> limit, and one specific helper for checking guest-related actions such a= s >> starting a VM. >>=20 >> Signed-off-by: Fabian Gr=C3=BCnbichler >> --- >> src/PVE/GuestHelpers.pm | 190 ++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 190 insertions(+) >>=20 >> diff --git a/src/PVE/GuestHelpers.pm b/src/PVE/GuestHelpers.pm >> index 961a7b8..36b44bb 100644 >> --- a/src/PVE/GuestHelpers.pm >> +++ b/src/PVE/GuestHelpers.pm >> @@ -416,4 +416,194 @@ sub check_vnet_access { >> if !($tag || $trunks); >> } >> =20 >> +# combines the broadcasted pool usage information to get per-pool stats >> +# >> +# $pools parsed pool info from user.cfg >> +# $usage broadcasted KV hash >> +# $pool filter for specific pool >> +# $skip skip a certain guest to ignore its current usage >> +# >> +# returns usage hash: >> +# pool -> cpu/mem/.. -> run/config -> $usage >> +sub get_pool_usage { >> + my ($pools, $usage, $pool, $skip) =3D @_; >> + >> + my $res =3D {}; >> + my $included_guests =3D {}; >> + for my $node (keys $usage->%*) { >> + my $node_usage =3D JSON::decode_json($usage->{$node} // ''); >> + >> + # long IDs first, so we can add children to their parents right away >> + for my $poolid (sort {$b cmp $a} keys $pools->%*) { >> + next if defined($pool) && !($pool eq $poolid || $poolid =3D~ m!^$p= ool/! || $pool =3D~ m!^$poolid/!); >> + >> + my $d =3D $res->{$poolid} //=3D { >> + cpu =3D> { >> + run =3D> 0, >> + config =3D> 0, >> + }, >> + mem =3D> { >> + run =3D> 0, >> + config =3D> 0, >> + }, >> + }; >> + >> + my $pool_usage =3D $node_usage->{data}->{$poolid} // {}; >> + for my $vmid (keys $pool_usage->%*) { >> + # only include once in case of migration between broadcast >> + next if $included_guests->{$vmid}; >> + next if $skip && $skip->{$vmid}; >> + $included_guests->{$vmid} =3D 1; >> + >> + my $vm_data =3D $pool_usage->{$vmid}; >> + for my $key (keys $vm_data->%*) { >> + next if $key eq 'running'; >> + $d->{$key}->{run} +=3D $vm_data->{$key}->{run} if $vm_data->{runn= ing}; >> + $d->{$key}->{config} +=3D $vm_data->{$key}->{config}; >=20 > So here we autovivify more keys inside $d, so simply receiving them in > the $pool_usage will add them well, yeah.. if they are set in pool_usage, then they represent a usage attribute and should be summed up? ;) > ... >=20 >> + } >> + } >> + >> + if (my $parent =3D $pools->{$poolid}->{parent}) { >> + $res->{$parent} //=3D { >> + cpu =3D> { >> + run=3D> 0, >> + config =3D> 0, >> + }, >> + mem =3D> { >> + run =3D> 0, >> + config =3D> 0, >> + }, >> + }; >=20 > ... > so would it make sense to just iterate over `keys %$d` here instead > of having `cpu` and `mem` hardcoded? yes > And/or maybe access-control should expose a list derived from > $pool_limits_desc directly (or helper sub) to deal with the > `-run`/`-config` suffixes that might make sense as well, yes. > ... >=20 >> + $res->{$parent}->{cpu}->{run} +=3D $d->{cpu}->{run}; >> + $res->{$parent}->{mem}->{run} +=3D $d->{mem}->{run}; >> + $res->{$parent}->{cpu}->{config} +=3D $d->{cpu}->{config}; >> + $res->{$parent}->{mem}->{config} +=3D $d->{mem}->{config}; >> + } >> + } >> + } >> + >> + return $res; >> +} >> + >> +# checks whether a pool is (or would be) over its resource limits >> +# >> +# $changes is for checking limits for config/state changes like VM star= ts, if >> +# set, only the limits with changes are checked (see check_guest_pool_l= imit) >> +# >> +# return value indicates whether any limit was overstepped or not (if $= noerr is set) >> +sub check_pool_limits { >> + my ($usage, $limits, $noerr, $changes) =3D @_; >> + >> + my $over =3D {}; >> + my $only_changed =3D defined($changes); >> + >> + my $check_limit =3D sub { >> + my ($key, $running, $limit, $change) =3D @_; >> + >> + return if $only_changed && $change =3D=3D 0; >> + >> + my $kind =3D $running ? 'run' : 'config'; >> + >> + my $value =3D $usage->{$key}->{$kind}; >> + $value =3D int($value); >> + $value +=3D $change; >> + $value =3D $value / (1024*1024) if $key eq 'mem'; >> + if ($limit < $value) { >> + $over->{$key}->{$kind}->{change} =3D $change if $change; >> + $over->{$key}->{$kind}->{over} =3D 1; >> + } >> + }; >> + >> + my $get_change =3D sub { >> + my ($key, $running) =3D @_; >> + >> + return 0 if !defined($changes); >> + >> + my $check_running =3D defined($changes->{running}) && $changes->{runni= ng} ? 1 : 0; >> + >> + if ($running =3D=3D $check_running) { >> + return $changes->{$key} // 0; >> + } else { >> + return 0; >> + } >> + }; >> + >> + if (my $limit =3D $limits->{"mem-run"}) { >> + my $change =3D $get_change->('mem', 1); >> + $check_limit->('mem', 1, $limit, $change); >> + } >> + >> + if (my $limit =3D $limits->{"mem-config"}) { >> + my $change =3D $get_change->('mem', 0); >> + $check_limit->('mem', 0, $limit, $change); >> + } >=20 > ... > Similarly this could then iterate over the list. >=20 >> + >> + if (my $limit =3D $limits->{"cpu-run"}) { >> + my $change =3D $get_change->('cpu', 1); >> + $check_limit->('cpu', 1, $limit, $change); >> + } >> + >> + if (my $limit =3D $limits->{"cpu-config"}) { >> + my $change =3D $get_change->('cpu', 0); >> + $check_limit->('cpu', 0, $limit, $change); >> + } >> + >> + if (!$noerr) { >> + my $msg =3D ''; >> + for my $key (keys $over->%*) { >> + for my $kind (keys $over->{$key}->%*) { >> + my $value =3D $usage->{$key}->{$kind}; >> + $value =3D $value / (1024*1024) if $key eq 'mem'; >> + my $change =3D $over->{$key}->{$kind}->{change}; >> + if ($change) { >> + $change =3D $change / (1024*1024) if $key eq 'mem'; >> + $value =3D "$value + $change" if $change; >> + } >> + my $limit =3D $limits->{"$key-$kind"}; >> + $msg .=3D "($kind) $key: $value over $limit, "; >> + } >> + } >> + if ($msg) { >> + $msg =3D~ s/, $//; >> + die "pool limits exhausted: $msg\n"; >> + } >> + } >> + >> + return $over->%* ? 1 : 0; >> +} >> + >> +# checks whether the given changes for a certain guest would overstep a= pool limit >> +# >> +# $changes is an optional hash containing >> +# - absolute: flag whether changes are relative or absolute >> +# - running: flag whether the config or running limits should be checke= d >> +# - cpu: change value for cpu limit >> +# - mem: change value for mem limit >> +# all elements are optional >> +# >> +# if no $changes is provided, the limits are checked against the curren= t usage >> +# >> +# $poolid allows overriding the guest's pool membership, for example in= case it >> +# is not yet properly set when creating the guest >> +sub check_guest_pool_limit { >> + my ($vmid, $changes, $poolid) =3D @_; >> + >> + my $user_cfg =3D PVE::Cluster::cfs_read_file("user.cfg"); >> + >> + $poolid =3D $user_cfg->{vms}->{$vmid} if !defined($poolid); >> + if ($poolid) { >> + my $pool =3D $user_cfg->{pools}->{$poolid}; >> + >> + my $limits =3D $pool->{limits}; >> + return if !$limits; >> + >> + my $skip =3D {}; >> + $skip->{$vmid} =3D 1 if $changes && $changes->{absolute}; >> + my $usage =3D PVE::Cluster::get_node_kv('pool-usage'); >> + >> + $usage =3D PVE::GuestHelpers::get_pool_usage($user_cfg->{pools}, $usag= e, $poolid, $skip); >=20 > ^ This is already inside PVE::GuestHelpers ;-) ack :) >=20 >> + check_pool_limits($usage->{$poolid}, $limits, 0, $changes); >> + } >> +} >> + >> 1; >> --=20 >> 2.39.2 >=20