Re: [pve-devel] [PATCH manager] fix #5010: ceph: pool set only changed properties

[Christoph Heiss <c.heiss@proxmox.com>]

Tested it on a simple 3-node cluster - first without setting
`nosizechange` on a pool, then w/ the patch applied. Does what it says
on the tin, avoids a task error while allowing setting other properties
when `nosizechange` is active.

On Tue, Jul 09, 2024 at 01:41:16PM GMT, Aaron Lauterer wrote:
> By only setting propterties that have changed, we can avoid potential
> errors in the task.
>
> For example, if one configures the "nosizechange" property on a pool, to
> prevent accidential size changes, the task will now only error if the
> user is actually trying to change the size.
>
> Prior to this patch, we would always try to set all parameters, even if
> they were the same value. In the above example, this would result in the
> task ending in error state, as we are not allowed to change the size.
>
> To disable size changing you can run the following command:
> ceph osd pool set {pool} nosizechange 1
>
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>

> ---
>
> I am not sure if we want to log every property that is skipped. It makes
> visible what is done, but is also a bit noisy.

I didn't really perceive them as annoying or such, as the task log is
only a few lines anyway in both cases. We do not log such things anywhere
else AFAIK, so would be fine without these messages too. But not really
worth sending a v2 IMO.

>
>  PVE/Ceph/Tools.pm | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/PVE/Ceph/Tools.pm b/PVE/Ceph/Tools.pm
> index 9b97171e..dd27e7ce 100644
> --- a/PVE/Ceph/Tools.pm
> +++ b/PVE/Ceph/Tools.pm
> @@ -262,9 +262,17 @@ sub set_pool {
>      my $keys = [ grep { $_ ne 'size' } sort keys %$param ];
>      unshift @$keys, 'size' if exists $param->{size};
>
> +    my $current_properties = get_pool_properties($pool, $rados);
> +
>      for my $setting (@$keys) {
>  	my $value = $param->{$setting};
>
> +	if (defined($current_properties->{$setting}) && $value eq $current_properties->{$setting}) {
> +	    print "skipping '${setting}', did not change\n";
> +	    delete $param->{$setting};
> +	    next;
> +	}
> +
>  	print "pool $pool: applying $setting = $value\n";
>  	if (my $err = $set_pool_setting->($pool, $setting, $value, $rados)) {
>  	    print "$err";
> --
> 2.39.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel