[pve-devel] [PATCH manager] fix #4808: ceph: use setting names with underscores

[pve-devel] [PATCH manager] fix #4808: ceph: use setting names with underscores

[Maximiliano Sandoval]

As suggested in [1], it is recommended to use `_` in all cases when
dealing with config files.

[1] https://docs.ceph.com/en/reef/rados/configuration/ceph-conf/#option-names

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 PVE/API2/Ceph/MDS.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Ceph/MDS.pm b/PVE/API2/Ceph/MDS.pm
index 1cb0b74f..6fc0ae45 100644
--- a/PVE/API2/Ceph/MDS.pm
+++ b/PVE/API2/Ceph/MDS.pm
@@ -153,10 +153,10 @@ __PACKAGE__->register_method ({
 	    }
 
 	    $cfg->{$section}->{host} = $nodename;
-	    $cfg->{$section}->{"mds standby for name"} = 'pve';
+	    $cfg->{$section}->{'mds_standby_for_name'} = 'pve';
 
 	    if ($param->{hotstandby}) {
-		$cfg->{$section}->{"mds standby replay"} = 'true';
+		$cfg->{$section}->{'mds_standby_replay'} = 'true';
 	    }
 
 	    cfs_write_file('ceph.conf', $cfg);
-- 
2.39.2

[pve-devel] applied: [PATCH manager] fix #4808: ceph: use setting names with underscores

[Thomas Lamprecht]

Am 05/09/2023 um 10:53 schrieb Maximiliano Sandoval:
> As suggested in [1], it is recommended to use `_` in all cases when
> dealing with config files.
> 
> [1] https://docs.ceph.com/en/reef/rados/configuration/ceph-conf/#option-names

The first thing I asked myself was "how is the priority though if both exist?"

Then I had to check the code and saw that this is only affecting
MDS creation, where we enforce that the config section doesn't exist,
so it's OK (but still relevant info, so I added it to the commit
message).

Anyhow, it would be still good to know how ceph handles the situation
where both exist, e.g.:

  mds standby for name = foo
  mds_standby_for_name = bar
  # what about mixing them?
  mds_standy for name

Also, did you checked for other such occurrences in our API?

Manually tracing all call sites where cfs_write_file('ceph.conf', ..) is
called should allow you to find most potential call sites using
the undesired style.

> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>  PVE/API2/Ceph/MDS.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
>

applied, thanks!

Re: [pve-devel] applied: [PATCH manager] fix #4808: ceph: use setting names with underscores

[Maximiliano Sandoval]

> Also, did you checked for other such occurrences in our API?

I found another place where they are used in pve-manager's PVE/API2/Ceph module.
I will send a patch, but most interesting is that we set

     $cfg->{global}->{'osd pg bits'} = $param->{pg_bits};
     $cfg->{global}->{'osd pgp bits'} = $param->{pg_bits};

and expose them in the API. This setting is documented in the Pacific docs [1],
but not in the Quincy docs [2]. I could not find much info about this. It is not
mentioned on the Quincy release notes [3] as far as I can tell.

[1] https://docs.ceph.com/en/pacific/rados/configuration/pool-pg-config-ref/
[2] https://docs.ceph.com/en/quincy/rados/configuration/pool-pg-config-ref/
[3] https://docs.ceph.com/en/latest/releases/quincy/#v17-2-0-quincy

Re: [pve-devel] applied: [PATCH manager] fix #4808: ceph: use setting names with underscores

[Maximiliano Sandoval]

> and expose them in the API. This setting is documented in the Pacific docs [1],
> but not in the Quincy docs [2]. I could not find much info about this. It is not
> mentioned on the Quincy release notes [3] as far as I can tell.
Found a commit [1] as part of [2] which is on the v13.0.2 tag.

[1] https://github.com/ceph/ceph/commit/e6acf2d1d528a2395947d446a57bec04a3a002dc
[2] https://github.com/ceph/ceph/pull/20172