[pve-devel] [PATCH manager v2 1/4] ceph tools: set_pools: filter settings for erasure code pools

[pve-devel] [PATCH manager v2 1/4] ceph tools: set_pools: filter settings for erasure code pools

[Aaron Lauterer]

Erasure code pools cannot change certain settings after creation.
Trying to set them will cause errors on Cephs side.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
changes since v1:
* check if setting is present in params before removing them
* create new rados object if needed in get_pool_type

 PVE/Ceph/Tools.pm | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/PVE/Ceph/Tools.pm b/PVE/Ceph/Tools.pm
index 7bd3eedc..a1458b40 100644
--- a/PVE/Ceph/Tools.pm
+++ b/PVE/Ceph/Tools.pm
@@ -205,7 +205,7 @@ sub check_ceph_enabled {
 }
 
 my $set_pool_setting = sub {
-    my ($pool, $setting, $value) = @_;
+    my ($pool, $setting, $value, $rados) = @_;
 
     my $command;
     if ($setting eq 'application') {
@@ -224,7 +224,7 @@ my $set_pool_setting = sub {
 	};
     }
 
-    my $rados = PVE::RADOS->new();
+    $rados = PVE::RADOS->new() if !$rados;
     eval { $rados->mon_command($command); };
     return $@ ? $@ : undef;
 };
@@ -232,6 +232,18 @@ my $set_pool_setting = sub {
 sub set_pool {
     my ($pool, $param) = @_;
 
+    my $rados = PVE::RADOS->new();
+
+    if (get_pool_type($pool, $rados) eq 'erasure') {
+	#remove parameters that cannot be changed for erasure coded pools
+	my $ignore_params = ['size', 'crush_rule'];
+	for my $setting (@$ignore_params) {
+	    if ($param->{$setting}) {
+		print "cannot set '${setting}' for erasure coded pool\n";
+		delete $param->{$setting};
+	    }
+	}
+    }
     # by default, pool size always resets min_size, so set it as first item
     # https://tracker.ceph.com/issues/44862
     my $keys = [ grep { $_ ne 'size' } sort keys %$param ];
@@ -241,7 +253,7 @@ sub set_pool {
 	my $value = $param->{$setting};
 
 	print "pool $pool: applying $setting = $value\n";
-	if (my $err = $set_pool_setting->($pool, $setting, $value)) {
+	if (my $err = $set_pool_setting->($pool, $setting, $value, $rados)) {
 	    print "$err";
 	} else {
 	    delete $param->{$setting};
@@ -267,6 +279,13 @@ sub get_pool_properties {
     return $rados->mon_command($command);
 }
 
+sub get_pool_type {
+    my ($pool, $rados) = @_;
+    $rados = PVE::RADOS->new() if !defined($rados);
+    return 'erasure' if get_pool_properties($pool, $rados)->{erasure_code_profile};
+    return 'replicated';
+}
+
 sub create_pool {
     my ($pool, $param, $rados) = @_;
     $rados = PVE::RADOS->new() if !defined($rados);
-- 
2.30.2

[pve-devel] [PATCH manager v2 2/4] api: ceph pools: add type to returned properties

[Aaron Lauterer]

The osd dump already contains the pool type in numerical format.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
changes since v1:
add link to ceph codebase where numerical pool types are defined for
reference

 PVE/API2/Ceph/Pools.pm | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/PVE/API2/Ceph/Pools.pm b/PVE/API2/Ceph/Pools.pm
index efdee360..6931d5f6 100644
--- a/PVE/API2/Ceph/Pools.pm
+++ b/PVE/API2/Ceph/Pools.pm
@@ -66,6 +66,11 @@ __PACKAGE__->register_method ({
 		    type => 'integer',
 		    title => 'Size',
 		},
+		type => {
+		    type => 'string',
+		    title => 'Type',
+		    enum => ['replicated', 'erasure', 'unknown'],
+		},
 		min_size => {
 		    type => 'integer',
 		    title => 'Min Size',
@@ -185,6 +190,17 @@ __PACKAGE__->register_method ({
 		$d->{bytes_used} = $s->{bytes_used};
 		$d->{percent_used} = $s->{percent_used};
 	    }
+
+	    # Cephs numerical pool types are barely documented. Found the following in the Ceph
+	    # codebase: https://github.com/ceph/ceph/blob/22c402b84e508af396252c5875253c8742b732e6/qa/tasks/ceph_manager.py#L196-L198
+	    if ($e->{type} == 1) {
+		$d->{type} = 'replicated';
+	    } elsif ($e->{type} == 3) {
+		$d->{type} = 'erasure';
+	    } else {
+		# we should never get here, but better be safe
+		$d->{type} = 'unknown';
+	    }
 	    push @$data, $d;
 	}
 
-- 
2.30.2

[pve-devel] [PATCH manager v2 3/4] ui: ceph pool edit: disable size and crush rule for erasure pools

[Aaron Lauterer]

They cannot be changed after pool creation for erasure coded pools

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 www/manager6/ceph/Pool.js | 44 ++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/www/manager6/ceph/Pool.js b/www/manager6/ceph/Pool.js
index 41df216e..16b903ce 100644
--- a/www/manager6/ceph/Pool.js
+++ b/www/manager6/ceph/Pool.js
@@ -19,21 +19,28 @@ Ext.define('PVE.CephPoolInputPanel', {
 	    allowBlank: false,
 	},
 	{
-	    xtype: 'proxmoxintegerfield',
+	    xtype: 'pmxDisplayEditField',
+	    cbind: {
+		editable: '{!isErasure}',
+	    },
 	    fieldLabel: gettext('Size'),
 	    name: 'size',
-	    value: 3,
-	    minValue: 2,
-	    maxValue: 7,
-	    allowBlank: false,
-	    listeners: {
-		change: function(field, val) {
-		    let size = Math.round(val / 2);
-		    if (size > 1) {
-			field.up('inputpanel').down('field[name=min_size]').setValue(size);
-		    }
+	    editConfig: {
+		xtype: 'proxmoxintegerfield',
+		value: 3,
+		minValue: 2,
+		maxValue: 7,
+		allowBlank: false,
+		listeners: {
+		    change: function(field, val) {
+			let size = Math.round(val / 2);
+			if (size > 1) {
+			    field.up('inputpanel').down('field[name=min_size]').setValue(size);
+			}
+		    },
 		},
 	    },
+
 	},
     ],
     column2: [
@@ -101,14 +108,18 @@ Ext.define('PVE.CephPoolInputPanel', {
 	    hidden: true,
 	},
 	{
-	    xtype: 'pveCephRuleSelector',
-	    fieldLabel: 'Crush Rule', // do not localize
-	    name: 'crush_rule',
+	    xtype: 'pmxDisplayEditField',
 	    cbind: {
+		editable: '{!isErasure}',
 		nodename: '{nodename}',
 		isCreate: '{isCreate}',
 	    },
-	    allowBlank: false,
+	    fieldLabel: 'Crush Rule', // do not localize
+	    name: 'crush_rule',
+	    editConfig: {
+		xtype: 'pveCephRuleSelector',
+		allowBlank: false,
+	    },
 	},
 	{
 	    xtype: 'proxmoxintegerfield',
@@ -203,6 +214,7 @@ Ext.define('PVE.Ceph.PoolEdit', {
 	cbind: {
 	    nodename: '{nodename}',
 	    pool_name: '{pool_name}',
+	    isErasure: '{isErasure}',
 	    isCreate: '{isCreate}',
 	},
     }],
@@ -354,6 +366,7 @@ Ext.define('PVE.node.Ceph.PoolList', {
 		title: gettext('Edit') + ': Ceph Pool',
 		nodename: nodename,
 		pool_name: rec.data.pool_name,
+		isErasure: rec.data.type === 'erasure',
 		autoShow: true,
 		listeners: {
 		    destroy: () => rstore.load(),
@@ -371,6 +384,7 @@ Ext.define('PVE.node.Ceph.PoolList', {
 			Ext.create('PVE.Ceph.PoolEdit', {
 			    title: gettext('Create') + ': Ceph Pool',
 			    isCreate: true,
+			    isErasure: false,
 			    nodename: nodename,
 			    autoShow: true,
 			    listeners: {
-- 
2.30.2

[pve-devel] [PATCH manager v2 4/4] ui: ceph pools: add pool type column

[Aaron Lauterer]

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 www/manager6/ceph/Pool.js | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/www/manager6/ceph/Pool.js b/www/manager6/ceph/Pool.js
index 16b903ce..a1f008d1 100644
--- a/www/manager6/ceph/Pool.js
+++ b/www/manager6/ceph/Pool.js
@@ -240,6 +240,13 @@ Ext.define('PVE.node.Ceph.PoolList', {
 	    sortable: true,
 	    dataIndex: 'pool_name',
 	},
+	{
+	    text: gettext('Type'),
+	    minWidth: 100,
+	    flex: 1,
+	    dataIndex: 'type',
+	    hidden: true,
+	},
 	{
 	    text: gettext('Size') + '/min',
 	    minWidth: 100,
-- 
2.30.2

[pve-devel] [PATCH manager v2 follow up 2/4] api: ceph pools: add type to returned properties

[Aaron Lauterer]

The osd dump already contains the pool type in numerical format.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
thanks to @Dominik we found a better place where the numerical pool
types are defined.

 PVE/API2/Ceph/Pools.pm | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/PVE/API2/Ceph/Pools.pm b/PVE/API2/Ceph/Pools.pm
index efdee360..70427277 100644
--- a/PVE/API2/Ceph/Pools.pm
+++ b/PVE/API2/Ceph/Pools.pm
@@ -66,6 +66,11 @@ __PACKAGE__->register_method ({
 		    type => 'integer',
 		    title => 'Size',
 		},
+		type => {
+		    type => 'string',
+		    title => 'Type',
+		    enum => ['replicated', 'erasure', 'unknown'],
+		},
 		min_size => {
 		    type => 'integer',
 		    title => 'Min Size',
@@ -185,6 +190,17 @@ __PACKAGE__->register_method ({
 		$d->{bytes_used} = $s->{bytes_used};
 		$d->{percent_used} = $s->{percent_used};
 	    }
+
+	    # Cephs numerical pool types are barely documented. Found the following in the Ceph
+	    # codebase: https://github.com/ceph/ceph/blob/ff144995a849407c258bcb763daa3e03cfce5059/src/osd/osd_types.h#L1221-L1233
+	    if ($e->{type} == 1) {
+		$d->{type} = 'replicated';
+	    } elsif ($e->{type} == 3) {
+		$d->{type} = 'erasure';
+	    } else {
+		# we should never get here, but better be safe
+		$d->{type} = 'unknown';
+	    }
 	    push @$data, $d;
 	}
 
-- 
2.30.2

Re: [pve-devel] [PATCH manager v2 1/4] ceph tools: set_pools: filter settings for erasure code pools

[Dominik Csapak]

LGTM

Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>

[pve-devel] applied-series: [PATCH manager v2 1/4] ceph tools: set_pools: filter settings for erasure code pools

[Thomas Lamprecht]

Am 5/2/22 um 10:09 schrieb Aaron Lauterer:
> Erasure code pools cannot change certain settings after creation.
> Trying to set them will cause errors on Cephs side.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> changes since v1:
> * check if setting is present in params before removing them
> * create new rados object if needed in get_pool_type
> 
>  PVE/Ceph/Tools.pm | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
>

applied with the followup for 2/4 and Dominik's R-b & T-b tags, thanks to both!