Re: [pve-devel] [PATCH ha-manager 07/11] env: add get_crs_settings() method

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 10/11/2022 um 15:37 schrieb Fiona Ebner:
> for reading the resource scheduler settings.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/HA/Env.pm      |  7 +++++++
>  src/PVE/HA/Env/PVE2.pm | 12 ++++++++++++
>  src/PVE/HA/Sim/Env.pm  |  9 +++++++++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/src/PVE/HA/Env.pm b/src/PVE/HA/Env.pm
> index 00e3e3c..c014ff7 100644
> --- a/src/PVE/HA/Env.pm
> +++ b/src/PVE/HA/Env.pm
> @@ -269,6 +269,13 @@ sub get_ha_settings {
>      return $self->{plug}->get_ha_settings();
>  }
>  
> +# return cluster wide resource scheduling settings
> +sub get_crs_settings {
> +    my ($self) = @_;
> +
> +    return $self->{plug}->get_crs_settings();
> +}
> +

we try to keep the Env footprint on the smaller side, if possible; I'd rather add
this to get_ha_settings, either as tuple or from a gut feeling maybe better as hash

To clarify on that we could rename it to get_datacenter_settings first, I'd still
limit it to the relevant ones to avoid info "leakage" that some future patch misuses
then so subtly that we don't notice.


It'd then return:

{
   ha => {}
   crs => {}
}

fwiw, moving in max_workers would be an option then too, but no need for that in
this series.

maybe throw then also a comment in that changes are only to be taken in between new
LRM rounds or new Manager creation (due to no active one) for stability purpose.