Re: [pve-devel] [PATCH manager 1/4] ceph tools: set_pools: filter settings for erasure code pools

[Dominik Csapak <d.csapak@proxmox.com>]

one minor nit inline

On 4/29/22 16:16, Aaron Lauterer wrote:
> Erasure code pools cannot change certain settings after creation.
> Trying to set them will cause errors on Cephs side.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>   PVE/Ceph/Tools.pm | 22 +++++++++++++++++++---
>   1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/Ceph/Tools.pm b/PVE/Ceph/Tools.pm
> index 7bd3eedc..65589820 100644
> --- a/PVE/Ceph/Tools.pm
> +++ b/PVE/Ceph/Tools.pm
> @@ -205,7 +205,7 @@ sub check_ceph_enabled {
>   }
>   
>   my $set_pool_setting = sub {
> -    my ($pool, $setting, $value) = @_;
> +    my ($pool, $setting, $value, $rados) = @_;
>   
>       my $command;
>       if ($setting eq 'application') {
> @@ -224,7 +224,7 @@ my $set_pool_setting = sub {
>   	};
>       }
>   
> -    my $rados = PVE::RADOS->new();
> +    $rados = PVE::RADOS->new() if !$rados;
>       eval { $rados->mon_command($command); };
>       return $@ ? $@ : undef;
>   };
> @@ -232,6 +232,16 @@ my $set_pool_setting = sub {
>   sub set_pool {
>       my ($pool, $param) = @_;
>   
> +    my $rados = PVE::RADOS->new();
> +
> +    if (get_pool_type($pool, $rados) eq 'erasure') {
> +	#remove parameters that cannot be changed for erasure coded pools
> +	my $ignore_params = ['size', 'crush_rule'];
> +	for my $setting (@$ignore_params) {
> +	    print "cannot set '${setting}' for erasure coded pool\n";

i'd only print that line if there was actually a value there, else
every one gets those warnings even if they did not attempt to set e.g.
the size

> +	    delete $param->{$setting};
> +	}
> +    }
>       # by default, pool size always resets min_size, so set it as first item
>       # https://tracker.ceph.com/issues/44862
>       my $keys = [ grep { $_ ne 'size' } sort keys %$param ];
> @@ -241,7 +251,7 @@ sub set_pool {
>   	my $value = $param->{$setting};
>   
>   	print "pool $pool: applying $setting = $value\n";
> -	if (my $err = $set_pool_setting->($pool, $setting, $value)) {
> +	if (my $err = $set_pool_setting->($pool, $setting, $value, $rados)) {
>   	    print "$err";
>   	} else {
>   	    delete $param->{$setting};
> @@ -267,6 +277,12 @@ sub get_pool_properties {
>       return $rados->mon_command($command);
>   }
>   
> +sub get_pool_type {
> +    my ($pool, $rados) = @_;
> +    return 'erasure' if get_pool_properties($pool, $rados)->{erasure_code_profile};
> +    return 'replicated';
> +}
> +
>   sub create_pool {
>       my ($pool, $param, $rados) = @_;
>       $rados = PVE::RADOS->new() if !defined($rados);