Re: [PATCH perl-rs v2 15/40] pve-rs: resource-scheduling: implement pve_dynamic bindings

Dominik Rusovac
2026-03-27T14:15:13Z

lgtm, consider modulo one nit

On Tue Mar 24, 2026 at 7:29 PM CET, Daniel Kral wrote:
> The implementation is similar to pve_static, but extends the node and
> resource stats with sampled runtime usage statistics, i.e., the actual
> usage on the nodes and the actual usages of the resources.
>
> In the case of users repeatedly calling score_nodes_to_start_resource()
> and then adding them as starting resources with add_resource(), these
> starting resources need to be accumulated on top of these nodes actual
> current usages to prevent score_nodes_to_start_resource() to favor the
> currently least loaded node(s) for all starting resources.
>
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
> changes v1 -> v2:
> - move this patch one before 'expose auto rebalancing methods' as this
> is the same change order as done in pve-ha-manager, making it easier
> to separate the feature of using dynamic usage information and
> afterwards allowing rebalancing methods with static and dynamic usage
> information
> - adapt patch message accordingly
> - s/service/resource/ for any new struct and method as this is more
> consistent with the naming in the HA Manager and the name of the
> crate/module itself; can change this back if it's better in the other
> way, but as these are new API endpoints, I thought it's better to do
> it now than later
>

[snip]

> +impl UsageAggregator for StartingAsStartedResourceAggregator {
> + fn aggregate(usage: &Usage) -> Vec<NodeUsage> {
> + usage
> + .nodes_iter()
> + .map(|(nodename, node)| {

nice fold!

nit: by making `node_stats` mutable in the first place, variable
shadowing can be avoided, see:

let stats = node.resources_iter().fold(node.stats(), |mut node_stats, sid| {

if let Some(resource) = usage.get_resource(sid) {
node_stats.add_started_resource(&resource.stats());
}

node_stats
});

> + let stats = node.resources_iter().fold(node.stats(), |node_stats, sid| {
> + let mut node_stats = node_stats;
> +
> + if let Some(resource) = usage.get_resource(sid)
> + && resource.state() == ResourceState::Starting
> + {
> + node_stats.add_started_resource(&resource.stats());
> + }
> +
> + node_stats
> + });
> +
> + NodeUsage {
> + name: nodename.to_string(),
> + stats,
> + }
> + })
> + .collect()
> + }
> +}

Reviewed-by: Dominik Rusovac <d.rusovac@proxmox.com>