[PATCH manager 0/1] ceph: pool: fix pool statistics filter returning wrong pool's data

[PATCH manager 0/1] ceph: pool: fix pool statistics filter returning wrong pool's data

[Kefu Chai]

While reading the code I noticed the condition:

  next if !defined($d->{name}) && !$d->{name} ne "$pool";

I tried to simplify it using De Morgan's law:

  next if !(defined($d->{name}) || $d->{name} ne "$pool");

That immediately looked wrong -- we use the pattern
'defined($foo) && $foo ne "needle"' as a null-safe equality check in
many languages, but here the operator was '&&' combined with negation
on both sides, which doesn't match that pattern at all.

Looking closer, I realised the original condition is simply a logic
error: '&&' short-circuits when the first clause is false, so the name
comparison is never reached for any entry that has a defined name. All
pools with a defined name pass through, and the loop overwrites
$data->{statistics} on every iteration, leaving the stats of whichever
pool happens to be last in the 'ceph df' output.

The !defined() guard also intrigued me -- could 'name' ever be absent
from the response? Reading the Ceph source (PGMap.cc:dump_pool_stats_full)
shows that f->dump_string("name", pool_name) is called unconditionally
for every pool entry. Multiple mgr modules (influx, prometheus, telegraf)
also access pool['name'] directly without any None check, confirming it
is part of the established contract.

The fix removes both the logic error and the now-unnecessary !defined()
guard, simplifying the condition to a plain string comparison.

Kefu Chai (1):
  ceph: pool: fix pool statistics filter returning wrong pool's data

 PVE/API2/Ceph/Pool.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.47.3

[PATCH manager 1/1] ceph: pool: fix pool statistics filter returning wrong pool's data

[Kefu Chai]

The condition filtering the 'ceph df' response to the requested pool
had a logic error:

  next if !defined($d->{name}) && !$d->{name} ne "$pool";

With '&&', short-circuit evaluation means the name comparison is never
reached when $d->{name} is defined. All entries with a defined name
pass through, and $data->{statistics} ends up holding the stats of
whichever pool appears last in the output.

The combination of double negation and '&&' also makes the condition
difficult to reason about: a reader must mentally apply De Morgan's law
to understand what is being skipped, increasing the chance of
misreading the logic.

The !defined() guard is also unnecessary. The 'ceph df' response always
includes a 'name' field for every pool entry: PGMap.cc:dump_pool_stats_full
calls f->dump_string("name", pool_name) unconditionally for each pool in
the OSD map. Multiple mgr modules (influx, prometheus, telegraf) access
pool['name'] directly from the df output without any None/missing check,
confirming that 'name' is part of the established contract for this
response.

Simplify to a plain string comparison.

Signed-off-by: Kefu Chai <k.chai@proxmox.com>
---
 PVE/API2/Ceph/Pool.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/API2/Ceph/Pool.pm b/PVE/API2/Ceph/Pool.pm
index 73166cdf..587c6c96 100644
--- a/PVE/API2/Ceph/Pool.pm
+++ b/PVE/API2/Ceph/Pool.pm
@@ -787,7 +787,7 @@ __PACKAGE__->register_method({
 
             foreach my $d (@{ $res->{pools} }) {
                 next if !$d->{stats};
-                next if !defined($d->{name}) && !$d->{name} ne "$pool";
+                next if $d->{name} ne $pool;
                 $data->{statistics} = $d->{stats};
             }
 
-- 
2.47.3