Re: [PATCH ha-manager 2/7] env: dynamic service stats: use host-side memory footprint of guests

["Dominik Rusovac" <d.rusovac@proxmox.com>]

thank you for sending this patch! 

one nit inline, otherwise lgtm

NOTE: as mentioned in the cover letter, this overlaps with
https://lore.proxmox.com/all/20260828123239.1110595-2-d.rusovac@proxmox.com/

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

On Sun Sep 20, 2026 at 6:22 PM CEST, Michael Ryom wrote:
> The dynamic service stats used the 'mem' RRD column of the guests, which
> is the guest-reported memory usage (total_mem - free_mem) whenever the
> balloon driver is active, and can be considerably lower than the actual
> host-side footprint of the guest process (QEMU overhead, page cache
> still mapped, etc.). The node stats, on the other hand, always contain
> the full host view.
>
> The scheduler simulates a migration by subtracting exactly the
> service's stats from the source node and adding them to the target
> node. With the guest-reported value, the predicted post-migration state
> is systematically off by the difference between guest-reported and
> host-side usage (over 1 GiB for a mid-size VM). This bias is of the
> same order of magnitude as the trigger condition of the automatic load
> balancer on a two-node cluster with a single dominant resource, and
> contributes to a resource being moved back and forth.
>
> Use the 'memhost' column (host-side cgroup usage, without KSM) instead
> and fall back to 'mem' if it is unset: containers do not broadcast
> 'memhost', because their 'mem' already is the host-side cgroup value.
>
> Signed-off-by: Michael Ryom <Michael@RyomHerold.dk>
> ---
>  src/PVE/HA/Env/PVE2.pm | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm
> index 782d19d..8c2b03d 100644
> --- a/src/PVE/HA/Env/PVE2.pm
> +++ b/src/PVE/HA/Env/PVE2.pm
> @@ -46,6 +46,7 @@ use constant {
>      RRD_VM_INDEX_CPU => 6,
>      RRD_VM_INDEX_MAXMEM => 7,
>      RRD_VM_INDEX_MEM => 8,
> +    RRD_VM_INDEX_MEMHOST => 15,
>  };
>  
>  # rrd entry indices for PVE nodes
> @@ -598,11 +599,22 @@ sub get_dynamic_service_stats {
>          # NOTE the guests' broadcasted vmstatus() caps maxcpu at the node's maxcpu
>          my $maxcpu = ($rrdentry->[RRD_VM_INDEX_MAXCPU] || 0.0) + 0.0;
>  
> +        # prefer the host-side memory footprint over 'mem', which is the
> +        # guest-reported usage (total_mem - free_mem) when the balloon driver
> +        # is active and can be considerably lower than what running the guest
> +        # actually consumes on the node; the node stats contain the full host
> +        # view, so mixing in the guest view makes the scheduler's prediction
> +        # of the post-migration state systematically off
> +        #
> +        # 'memhost' is only broadcasted for VMs, for CTs 'mem' already is the
> +        # host-side (cgroup) usage
> +        my $mem = int($rrdentry->[RRD_VM_INDEX_MEMHOST] || $rrdentry->[RRD_VM_INDEX_MEM] || 0);

nit: while this kind of declaration shouldn't cause problems, since it
can be assumed that 

	$rrdentry->[RRD_VM_INDEX_MEMHOST] >= $rrdentry->[RRD_VM_INDEX_MEM], 

technically, we aim to fall back to 'mem' only if 'memhost' is undef and
not if 'memhost' is 0.

[snip]