Re: [PATCH manager] fix #8193: ceph: pool set: keep min_size when changing size

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

thank you for the patch, some mostly bureaucratic issues left, see
further reply inline.

Am 27.10.26 um 20:00 schrieb Parker Wahle:
> Since commit 76ed4900 ("fix #5010: ceph: pool set only changed
> properties"), set_pool reads the pool's properties once, before
> applying anything, and skips every requested setting that matches
> them. Ceph resets min_size whenever size changes, which is why
> set_pool applies size first. But a requested min_size equal to the
> old one was still compared with the value from before that reset and
> skipped, so Ceph's reset value stayed in place: for example, size=3
> min_size=1 on a size 2 / min_size 1 pool left min_size at 2, and
> size=3 min_size=3 on a size 4 / min_size 3 pool lowered it to 2.
> 
> This code change has PVE re-read the pool's properties after setting
> the size, so the remaining settings are compared with the pool's
> current values.
> 
> An LLM was used to isolate the cause of bug #8193 and draft a fix.
> The fix was then tested against a reproducer on a live node and
> found to work.

this note above would be nicer down in the diff-stat section ...

> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

We currently do not accept these anthropic trailers as is, they do
not really provide any value themselves (without prompt and other
environment context and any guarantee the same model is available
with the same params and interference engine version – which can be
only guaranteed for open weight models – it cannot be replicated,
and at that point it's rather just noise and/or free advertising, and
they already got lots of code to train on from free on from FLOSS
projects so I do not really want to give even more exposure here.

Anyway, as the change is rather straight forward and nothing intricate
you could drop this.

> Signed-off-by: Parker Wahle <regulad@regulad.xyz>

thx, I could not find any CLA on record, did you send one? See

https://pve.proxmox.com/wiki/Developer_Documentation#Software_License_and_Copyright

for details. If it's too much hassle for you we can also take this in
and  credit you with an Originally-by trailer here given that its a
straight forward change without anything novel for copyright to actually
matter – whatever you prefer.

> ---

... here – can be relevant context but not relevant to have permanently
recorded in the git history.

>  PVE/Ceph/Tools.pm | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/PVE/Ceph/Tools.pm b/PVE/Ceph/Tools.pm
> index 94177709..ad5ba687 100644
> --- a/PVE/Ceph/Tools.pm
> +++ b/PVE/Ceph/Tools.pm
> @@ -304,6 +304,8 @@ sub set_pool {
>              print "$err";
>          } else {
>              delete $param->{$setting};
> +            # Ceph resets min_size when size changes, so re-read before comparing the rest
> +            $current_properties = get_pool_properties($pool, $rados) if $setting eq 'size';
>          }
>      }
>