Re: [PATCH proxmox 02/15] resource-scheduling: scheduler: abstract post-migration stats logic

["Daniel Kral" <d.kral@proxmox.com>]

Nice!

On Wed Oct 21, 2026 at 10:59 AM CEST, Dominik Rusovac wrote:
> DRY and hide repeatedly used closure inside reuasble function.

nit: s/reuasble/reusable/

>
> Signed-off-by: Dominik Rusovac <d.rusovac@proxmox.com>
> ---
>  proxmox-resource-scheduling/src/scheduler.rs | 41 ++++++++------------
>  1 file changed, 16 insertions(+), 25 deletions(-)
>
> diff --git a/proxmox-resource-scheduling/src/scheduler.rs b/proxmox-resource-scheduling/src/scheduler.rs
> index 9078fa72..a12cd519 100644
> --- a/proxmox-resource-scheduling/src/scheduler.rs
> +++ b/proxmox-resource-scheduling/src/scheduler.rs
> @@ -56,6 +56,19 @@ fn calculate_node_imbalance(nodes: &[NodeUsage], to_load: impl Fn(&NodeUsage) ->
>      cv / max_cv
>  }
>  
> +/// Returns the hypothetical [`NodeStats`] of `node` with respect to the migration `candidate`.
> +fn node_stats_after_migration(candidate: &MigrationCandidate, node: &NodeUsage) -> NodeStats {
> +    let mut new_stats = node.stats;
> +
> +    if node.name == candidate.migration.source_node {
> +        new_stats.remove_running_resource(&candidate.stats);
> +    } else if node.name == candidate.migration.target_node {
> +        new_stats.add_running_resource(&candidate.stats);
> +    }
> +
> +    new_stats
> +}
> +
>  criteria_struct! {
>      /// A given alternative.
>      struct PveTopsisAlternative {
> @@ -216,15 +229,7 @@ impl Scheduler {
>      /// See [`calculate_node_imbalance`] for more information.
>      fn node_imbalance_with_migration_candidate(&self, candidate: &MigrationCandidate) -> f64 {
>          calculate_node_imbalance(&self.nodes, |node| {
> -            let mut new_stats = node.stats;
> -
> -            if node.name == candidate.migration.source_node {
> -                new_stats.remove_running_resource(&candidate.stats);
> -            } else if node.name == candidate.migration.target_node {
> -                new_stats.add_running_resource(&candidate.stats);
> -            }
> -
> -            new_stats.load()
> +            node_stats_after_migration(candidate, node).load()
>          })

nit: if it fits on a line, could drop the braces {} here

>      }
>  
> @@ -288,22 +293,8 @@ impl Scheduler {
>          let matrix = candidates
>              .iter()
>              .map(|candidate| {
> -                let resource_stats = &candidate.stats;
> -                let source_node = &candidate.migration.source_node;
> -                let target_node = &candidate.migration.target_node;
> -
> -                self.topsis_alternative_with(|node| {
> -                    let mut new_stats = node.stats;
> -
> -                    if &node.name == source_node {
> -                        new_stats.remove_running_resource(resource_stats);
> -                    } else if &node.name == target_node {
> -                        new_stats.add_running_resource(resource_stats);
> -                    }
> -
> -                    new_stats
> -                })
> -                .into()
> +                self.topsis_alternative_with(|node| node_stats_after_migration(candidate, node))
> +                    .into()
>              })
>              .collect::<Vec<_>>();
>  

Otherwise, consider as:

Reviewed-by: Daniel Kral <d.kral@proxmox.com>