[pve-devel] [PATCH pve-storage] RBDPlugin: add missing check for external ceph cluster

[pve-devel] [PATCH pve-storage] RBDPlugin: add missing check for external ceph cluster

[Hannes Duerr]

In this commit [0] the ceph config creation was packed into a new
function and checked whether the installation is an external Ceph
cluster or not. However, a check was forgotten in the RBDPlugin which we
are now adding.

Without this check a configuration in /etc/pve/priv/ceph/<pool>.conf is
created and pvestatd complains

 pvestatd[1144]: ignoring custom ceph config for storage 'pool', 'monhost' is not set (assuming pveceph managed cluster)! because the file /etc/pve/priv/ceph/pool.conf

[0] https://git.proxmox.com/?p=pve-storage.git;a=commitdiff;h=7684225baccf0de07504ac21a25cbf576d60a766

Signed-off-by: Hannes Duerr <h.duerr@proxmox.com>
---
 src/PVE/Storage/RBDPlugin.pm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index ce7db50..bf2e358 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -457,8 +457,12 @@ sub options {
 sub on_add_hook {
     my ($class, $storeid, $scfg, %param) = @_;
 
+    my $pveceph_managed = !defined($scfg->{monhost});
+
     PVE::CephConfig::ceph_create_keyfile($scfg->{type}, $storeid, $param{keyring});
-    PVE::CephConfig::ceph_create_configuration($scfg->{type}, $storeid);
+    
+    PVE::CephConfig::ceph_create_configuration($scfg->{type}, $storeid) if (!$pveceph_managed);
+
 
     return;
 }
-- 
2.47.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH pve-storage] RBDPlugin: add missing check for external ceph cluster

[Fiona Ebner]

Thank you for the fix!

Am 16.07.25 um 13:43 schrieb Hannes Duerr:
> In this commit [0] the ceph config creation was packed into a new

Please simply mention the commit ID (or better short ID + title) instead
of adding a link to the git web interface.

> function and checked whether the installation is an external Ceph
> cluster or not. However, a check was forgotten in the RBDPlugin which we
> are now adding.

to avoid the "we", you could use e.g. "which is now added."

> 
> Without this check a configuration in /etc/pve/priv/ceph/<pool>.conf is
> created and pvestatd complains
> 
>  pvestatd[1144]: ignoring custom ceph config for storage 'pool', 'monhost' is not set (assuming pveceph managed cluster)! because the file /etc/pve/priv/ceph/pool.conf
> 
> [0] https://git.proxmox.com/?p=pve-storage.git;a=commitdiff;h=7684225baccf0de07504ac21a25cbf576d60a766
> 

Please add:

Fixes: 7684225 ("ceph/rbd: set 'keyring' in ceph configuration for
externally managed RBD storages")

> Signed-off-by: Hannes Duerr <h.duerr@proxmox.com>

With improvements to the commit message and the style nits below addressed:

Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>

> ---
>  src/PVE/Storage/RBDPlugin.pm | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
> index ce7db50..bf2e358 100644
> --- a/src/PVE/Storage/RBDPlugin.pm
> +++ b/src/PVE/Storage/RBDPlugin.pm
> @@ -457,8 +457,12 @@ sub options {
>  sub on_add_hook {
>      my ($class, $storeid, $scfg, %param) = @_;
>  
> +    my $pveceph_managed = !defined($scfg->{monhost});
> +
>      PVE::CephConfig::ceph_create_keyfile($scfg->{type}, $storeid, $param{keyring});
> -    PVE::CephConfig::ceph_create_configuration($scfg->{type}, $storeid);
> +    

Style nit: line above has trailing spaces

> +    PVE::CephConfig::ceph_create_configuration($scfg->{type}, $storeid) if (!$pveceph_managed);

Stlye nit: superfluous parentheses in post-if

> +
>  
>      return;
>  }



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel