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

Michael Köppl
2026-03-31T09:07:38Z

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]