Re: [PATCH ha-manager v3 35/40] implement automatic rebalancing

["Michael Köppl" <m.koeppl@proxmox.com>]

2 comments inline

On Mon Mar 30, 2026 at 4:30 PM CEST, Daniel Kral wrote:

[snip]

> +    my $candidates = $self->get_resource_migration_candidates();
> +
> +    my $result;
> +    if ($method eq 'bruteforce') {
> +        $result = $online_node_usage->select_best_balancing_migration($candidates);
> +    } elsif ($method eq 'topsis') {
> +        $result = $online_node_usage->select_best_balancing_migration_topsis($candidates);
> +    }
> +
> +    # happens if $candidates is empty or $method isn't handled above
> +    return if !$result;
> +
> +    my ($migration, $target_imbalance) = $result->@{qw(migration imbalance)};
> +
> +    my $relative_change = ($imbalance - $target_imbalance) / $imbalance;

Since you get $imbalance from a function that returns 0.0 for the case
that the cluster load is perfectly balanced (?), you could run into
division by 0 here, no?

> +    return if $relative_change < $margin;
> +
> +    my ($sid, $source, $target) = $migration->@{qw(sid source-node target-node)};
> +
> +    my (undef, $type, $id) = $haenv->parse_sid($sid);
> +    my $task = $type eq 'vm' ? "migrate" : "relocate";
> +    my $cmd = "$task $sid $target";
> +
> +    my $target_imbalance_str = int(100 * $target_imbalance + 0.5) / 100;
> +    $haenv->log(
> +        'info',
> +        "auto rebalance - $task $sid to $target (expected target imbalance: $target_imbalance_str)",
> +    );
> +
> +    $self->queue_resource_motion($cmd, $task, $sid, $target);
> +}
> +

[snip]

>      $self->{all_lrms_disarmed} = 0;
> diff --git a/src/PVE/HA/Usage.pm b/src/PVE/HA/Usage.pm
> index 43feb041..659ab30a 100644
> --- a/src/PVE/HA/Usage.pm
> +++ b/src/PVE/HA/Usage.pm
> @@ -60,6 +60,40 @@ sub remove_service_usage {
>      die "implement in subclass";
>  }
>  
> +sub calculate_node_imbalance {
> +    my ($self) = @_;
> +
> +    die "implement in subclass";
> +}
> +
> +sub score_best_balancing_migrations {
> +    my ($self, $migration_candidates, $limit) = @_;
> +
> +    die "implement in subclass";
> +}
> +
> +sub select_best_balancing_migration {
> +    my ($self, $migration_candidates) = @_;
> +
> +    my $migrations = $self->score_best_balancing_migrations($migration_candidates, 1);
> +
> +    return $migrations->[0];

If an error occurs in the following call in
score_best_balancing_migrations

    my $migrations = eval {
        $self->{scheduler}
            ->score_best_balancing_migration_candidates_topsis($migration_candidates, $limit);
    };

you'd return an undefined $migrations, which would result in a
dereference error here.

> +}
> +
> +sub score_best_balancing_migrations_topsis {
> +    my ($self, $migration_candidates, $limit) = @_;
> +
> +    die "implement in subclass";
> +}
> +
> +sub select_best_balancing_migration_topsis {
> +    my ($self, $migration_candidates) = @_;
> +
> +    my $migrations = $self->score_best_balancing_migrations_topsis($migration_candidates, 1);
> +
> +    return $migrations->[0];
> +}
> +
>  # Returns a hash with $nodename => $score pairs. A lower $score is better.
>  sub score_nodes_to_start_service {
>      my ($self, $sid) = @_;
> diff --git a/src/PVE/HA/Usage/Dynamic.pm b/src/PVE/HA/Usage/Dynamic.pm
> index 24c85a41..76d0feaa 100644
> --- a/src/PVE/HA/Usage/Dynamic.pm
> +++ b/src/PVE/HA/Usage/Dynamic.pm
> @@ -104,6 +104,39 @@ sub remove_service_usage {
>      $self->{haenv}->log('warning', "unable to remove service '$sid' usage - $@") if $@;
>  }
>  

[snip]