public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* SPAM: [PATCH manager] fix #8193: ceph: pool set: keep min_size when changing size
@ 2026-10-18 20:52 Parker Wahle
  2026-10-29  1:42 ` Kefu Chai
  2026-11-10 19:02 ` Thomas Lamprecht
  0 siblings, 2 replies; 3+ messages in thread
From: Parker Wahle @ 2026-10-18 20:52 UTC (permalink / raw)
  To: pve-devel; +Cc: Parker Wahle, Claude Opus 5.5

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.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Parker Wahle <regulad@regulad.xyz>
---
 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';
         }
     }
 
-- 
2.56.0




^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH manager] fix #8193: ceph: pool set: keep min_size when changing size
  2026-10-18 20:52 SPAM: [PATCH manager] fix #8193: ceph: pool set: keep min_size when changing size Parker Wahle
@ 2026-10-29  1:42 ` Kefu Chai
  2026-11-10 19:02 ` Thomas Lamprecht
  1 sibling, 0 replies; 3+ messages in thread
From: Kefu Chai @ 2026-10-29  1:42 UTC (permalink / raw)
  To: pve-devel, Parker Wahle

On Sun Oct 18, 2026 at 10:52 PM CEST, Parker Wahle wrote:
[snip]
> @@ -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';
>          }
>      }

Thanks for tracking this down! I could reproduce it on a three-node
lab cluster running Ceph 21.1.2, with osd_pool_default_min_size = 2.
Without the patch, both of your examples lose the requested min_size:

- a 2/1 pool set to size=3 min_size=1 ends up with min_size 2
- a 4/3 pool set to size=3 min_size=3 ends up with min_size 2

With the patch, both keep the min_size that was asked for. I also
tried changing only the size, which still gives Ceph's default
min_size, and changing only min_size, which works as before. The pool
is only re-read when the size actually changes, which seems fine to
me.

Tested-by: Kefu Chai <k.chai@proxmox.com>
Reviewed-by: Kefu Chai <k.chai@proxmox.com>




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH manager] fix #8193: ceph: pool set: keep min_size when changing size
  2026-10-18 20:52 SPAM: [PATCH manager] fix #8193: ceph: pool set: keep min_size when changing size Parker Wahle
  2026-10-29  1:42 ` Kefu Chai
@ 2026-11-10 19:02 ` Thomas Lamprecht
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Lamprecht @ 2026-11-10 19:02 UTC (permalink / raw)
  To: Parker Wahle; +Cc: pve-devel

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';
>          }
>      }
>  





^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-10 19:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-18 20:52 SPAM: [PATCH manager] fix #8193: ceph: pool set: keep min_size when changing size Parker Wahle
2026-10-29  1:42 ` Kefu Chai
2026-11-10 19:02 ` Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal