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.laute...@proxmox.com>
Reviewed-by: Christoph Heiss <c.he...@proxmox.com>
Tested-by: Christoph Heiss <c.he...@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
