On April 11, 2024 11:17 am, Wolfgang Bumiller wrote: > On Wed, Apr 10, 2024 at 03:13:06PM +0200, Fabian Grünbichler wrote: >> one for combining the per-node broadcasted values, one for checking a pool's >> limit, and one specific helper for checking guest-related actions such as >> starting a VM. >> >> Signed-off-by: Fabian Grünbichler <f.gruenbich...@proxmox.com> >> --- >> src/PVE/GuestHelpers.pm | 190 ++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 190 insertions(+) >> >> 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); >> } >> >> +# 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) = @_; >> + >> + my $res = {}; >> + my $included_guests = {}; >> + for my $node (keys $usage->%*) { >> + my $node_usage = 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 =~ >> m!^$pool/! || $pool =~ m!^$poolid/!); >> + >> + my $d = $res->{$poolid} //= { >> + cpu => { >> + run => 0, >> + config => 0, >> + }, >> + mem => { >> + run => 0, >> + config => 0, >> + }, >> + }; >> + >> + my $pool_usage = $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} = 1; >> + >> + my $vm_data = $pool_usage->{$vmid}; >> + for my $key (keys $vm_data->%*) { >> + next if $key eq 'running'; >> + $d->{$key}->{run} += $vm_data->{$key}->{run} if >> $vm_data->{running}; >> + $d->{$key}->{config} += $vm_data->{$key}->{config}; > > 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? ;) > ... > >> + } >> + } >> + >> + if (my $parent = $pools->{$poolid}->{parent}) { >> + $res->{$parent} //= { >> + cpu => { >> + run=> 0, >> + config => 0, >> + }, >> + mem => { >> + run => 0, >> + config => 0, >> + }, >> + }; > > ... > 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. > ... > >> + $res->{$parent}->{cpu}->{run} += $d->{cpu}->{run}; >> + $res->{$parent}->{mem}->{run} += $d->{mem}->{run}; >> + $res->{$parent}->{cpu}->{config} += $d->{cpu}->{config}; >> + $res->{$parent}->{mem}->{config} += $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 starts, >> if >> +# set, only the limits with changes are checked (see check_guest_pool_limit) >> +# >> +# return value indicates whether any limit was overstepped or not (if >> $noerr is set) >> +sub check_pool_limits { >> + my ($usage, $limits, $noerr, $changes) = @_; >> + >> + my $over = {}; >> + my $only_changed = defined($changes); >> + >> + my $check_limit = sub { >> + my ($key, $running, $limit, $change) = @_; >> + >> + return if $only_changed && $change == 0; >> + >> + my $kind = $running ? 'run' : 'config'; >> + >> + my $value = $usage->{$key}->{$kind}; >> + $value = int($value); >> + $value += $change; >> + $value = $value / (1024*1024) if $key eq 'mem'; >> + if ($limit < $value) { >> + $over->{$key}->{$kind}->{change} = $change if $change; >> + $over->{$key}->{$kind}->{over} = 1; >> + } >> + }; >> + >> + my $get_change = sub { >> + my ($key, $running) = @_; >> + >> + return 0 if !defined($changes); >> + >> + my $check_running = defined($changes->{running}) && $changes->{running} >> ? 1 : 0; >> + >> + if ($running == $check_running) { >> + return $changes->{$key} // 0; >> + } else { >> + return 0; >> + } >> + }; >> + >> + if (my $limit = $limits->{"mem-run"}) { >> + my $change = $get_change->('mem', 1); >> + $check_limit->('mem', 1, $limit, $change); >> + } >> + >> + if (my $limit = $limits->{"mem-config"}) { >> + my $change = $get_change->('mem', 0); >> + $check_limit->('mem', 0, $limit, $change); >> + } > > ... > Similarly this could then iterate over the list. > >> + >> + if (my $limit = $limits->{"cpu-run"}) { >> + my $change = $get_change->('cpu', 1); >> + $check_limit->('cpu', 1, $limit, $change); >> + } >> + >> + if (my $limit = $limits->{"cpu-config"}) { >> + my $change = $get_change->('cpu', 0); >> + $check_limit->('cpu', 0, $limit, $change); >> + } >> + >> + if (!$noerr) { >> + my $msg = ''; >> + for my $key (keys $over->%*) { >> + for my $kind (keys $over->{$key}->%*) { >> + my $value = $usage->{$key}->{$kind}; >> + $value = $value / (1024*1024) if $key eq 'mem'; >> + my $change = $over->{$key}->{$kind}->{change}; >> + if ($change) { >> + $change = $change / (1024*1024) if $key eq 'mem'; >> + $value = "$value + $change" if $change; >> + } >> + my $limit = $limits->{"$key-$kind"}; >> + $msg .= "($kind) $key: $value over $limit, "; >> + } >> + } >> + if ($msg) { >> + $msg =~ 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 checked >> +# - 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 current >> 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) = @_; >> + >> + my $user_cfg = PVE::Cluster::cfs_read_file("user.cfg"); >> + >> + $poolid = $user_cfg->{vms}->{$vmid} if !defined($poolid); >> + if ($poolid) { >> + my $pool = $user_cfg->{pools}->{$poolid}; >> + >> + my $limits = $pool->{limits}; >> + return if !$limits; >> + >> + my $skip = {}; >> + $skip->{$vmid} = 1 if $changes && $changes->{absolute}; >> + my $usage = PVE::Cluster::get_node_kv('pool-usage'); >> + >> + $usage = PVE::GuestHelpers::get_pool_usage($user_cfg->{pools}, $usage, >> $poolid, $skip); > > ^ This is already inside PVE::GuestHelpers ;-) ack :) > >> + check_pool_limits($usage->{$poolid}, $limits, 0, $changes); >> + } >> +} >> + >> 1; >> -- >> 2.39.2 > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel