[pve-devel] [PATCH V4 SERIES storage/manager/docs 0/3] fix #3967: add ZFS dRAID creation

[pve-devel] [PATCH V4 SERIES storage/manager/docs 0/3] fix #3967: add ZFS dRAID creation

[Stefan Hrdlicka]

V1 -> V2:
# pve-storage
* formating change
* fixing typos & wording
* added check if dRAID options draidspares & draiddata are used for something 
else then setting up a dRAID

# pve-manager
* add a viewModel and use a binding for visiblity
* remove controller

# pve-docs
* fix many typos
* reword last paragraph to make it (hopefully :)) more helpful


V2 -> V3:
# pve-storage
* use a draid-config format

# pve-manager
* change due to draid config format (submit text field all with draid config values)
* add hidden field for submitting values --> added addtional binding
* the data & spares fields are now required to be selected in the GUI
** via the API the two config params are not required for now

# pve-docs
* openZFS replaced with OpenZFS

V3 -> V4:
# pve-docs
* added note to explain why the GUI expects one more disk then the
  minimum that dRAID would allow


Stefan Hrdlicka (1):
  fix #3967: enable ZFS dRAID creation via API

 PVE/API2/Disks/ZFS.pm | 55 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

---
 www/manager6/node/ZFS.js | 69 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

---
 local-zfs.adoc | 44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

-- 
2.30.2

[pve-devel] [PATCH V4 storage 1/3] fix #3967: enable ZFS dRAID creation via API

[Stefan Hrdlicka]

It is possible to set the number of spares and the size of
data stripes via draidspares & dreaddata parameters.

Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
---
 PVE/API2/Disks/ZFS.pm | 55 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Disks/ZFS.pm b/PVE/API2/Disks/ZFS.pm
index a4d4aa2..d1bcbcb 100644
--- a/PVE/API2/Disks/ZFS.pm
+++ b/PVE/API2/Disks/ZFS.pm
@@ -285,6 +285,19 @@ __PACKAGE__->register_method ({
 	return $pool;
     }});
 
+my $draid_config_format = {
+    spares => {
+	type => 'integer',
+	minimum => 0,
+	description => 'Number of dRAID spares.',
+    },
+    data => {
+	type => 'integer',
+	minimum => 1,
+	description => 'The number of data devices per redundancy group. (dRAID)',
+    },
+};
+
 __PACKAGE__->register_method ({
     name => 'create',
     path => '',
@@ -303,12 +316,21 @@ __PACKAGE__->register_method ({
 	    raidlevel => {
 		type => 'string',
 		description => 'The RAID level to use.',
-		enum => ['single', 'mirror', 'raid10', 'raidz', 'raidz2', 'raidz3'],
+		enum => [
+		    'single', 'mirror',
+		    'raid10', 'raidz', 'raidz2', 'raidz3',
+		    'draid', 'draid2', 'draid3',
+		],
 	    },
 	    devices => {
 		type => 'string', format => 'string-list',
 		description => 'The block devices you want to create the zpool on.',
 	    },
+	    'draid-config' => {
+		type => 'string',
+		format => $draid_config_format,
+		optional => 1,
+	    },
 	    ashift => {
 		type => 'integer',
 		minimum => 9,
@@ -344,6 +366,13 @@ __PACKAGE__->register_method ({
 	my $devs = [PVE::Tools::split_list($param->{devices})];
 	my $raidlevel = $param->{raidlevel};
 	my $compression = $param->{compression} // 'on';
+	my $draid_config = {};
+	if (exists $param->{'draid-config'}) {
+	    $draid_config = PVE::JSONSchema::parse_property_string(
+		$draid_config_format, $param->{'draid-config'});
+	}
+	my $draid_data = $draid_config->{data};
+	my $draid_spares = $draid_config->{spares};
 
 	for my $dev (@$devs) {
 	    $dev = PVE::Diskmanage::verify_blockdev_path($dev);
@@ -381,6 +410,9 @@ __PACKAGE__->register_method ({
 	    raidz => 3,
 	    raidz2 => 4,
 	    raidz3 => 5,
+	    draid => 3,
+	    draid2 => 4,
+	    draid3 => 5,
 	};
 
 	# sanity checks
@@ -393,6 +425,22 @@ __PACKAGE__->register_method ({
 	die "$raidlevel needs at least $mindisks->{$raidlevel} disks\n"
 	    if $numdisks < $mindisks->{$raidlevel};
 
+	# draid checks
+	if ($raidlevel =~ m/^draid/) {
+	    # bare minimum would be two drives:
+	    # one parity & one data drive this code doesn't allow that because
+	    # it makes no sense, at least one spare disk should be used
+	    my $draidmin = $mindisks->{$raidlevel} - 2;
+	    $draidmin += $draid_data if $draid_data;
+	    $draidmin += $draid_spares if $draid_spares;
+
+	    die "At least $draidmin disks needed for current dRAID config\n"
+		if $numdisks < $draidmin;
+	} else {
+	    die "draidspares and/or draiddata set without using dRAID"
+		if ($draid_spares or $draid_data);
+	}
+
 	my $code = sub {
 	    for my $dev (@$devs) {
 		PVE::Diskmanage::assert_disk_unused($dev);
@@ -429,6 +477,11 @@ __PACKAGE__->register_method ({
 		}
 	    } elsif ($raidlevel eq 'single') {
 		push @$cmd, $devs->[0];
+	    } elsif ($raidlevel =~ m/^draid/) {
+		my $draid_cmd = $raidlevel;
+		$draid_cmd .= ":${draid_data}d" if $draid_data;
+		$draid_cmd .= ":${draid_spares}s" if $draid_spares;
+		push @$cmd, $draid_cmd, @$devs;
 	    } else {
 		push @$cmd, $raidlevel, @$devs;
 	    }
-- 
2.30.2

[pve-devel] applied: [PATCH V4 storage 1/3] fix #3967: enable ZFS dRAID creation via API

[Thomas Lamprecht]

Am 10/11/2022 um 14:24 schrieb Stefan Hrdlicka:
> It is possible to set the number of spares and the size of
> data stripes via draidspares & dreaddata parameters.
> 
> Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
> ---
>  PVE/API2/Disks/ZFS.pm | 55 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 54 insertions(+), 1 deletion(-)
> 

applied with Lukas' T-b tag and some stylistic follow ups (see below), thanks!

> diff --git a/PVE/API2/Disks/ZFS.pm b/PVE/API2/Disks/ZFS.pm
> index a4d4aa2..d1bcbcb 100644
> --- a/PVE/API2/Disks/ZFS.pm
> +++ b/PVE/API2/Disks/ZFS.pm
> @@ -285,6 +285,19 @@ __PACKAGE__->register_method ({
>  	return $pool;
>      }});
>  
> +my $draid_config_format = {
> +    spares => {
> +	type => 'integer',
> +	minimum => 0,
> +	description => 'Number of dRAID spares.',
> +    },
> +    data => {
> +	type => 'integer',
> +	minimum => 1,
> +	description => 'The number of data devices per redundancy group. (dRAID)',

the code handles either as optional but this isn't encoded in the schema,
I added it, please re-check if that was OK.

> +    },
> +};
> +
>  __PACKAGE__->register_method ({
>      name => 'create',
>      path => '',
> @@ -303,12 +316,21 @@ __PACKAGE__->register_method ({
>  	    raidlevel => {
>  		type => 'string',
>  		description => 'The RAID level to use.',
> -		enum => ['single', 'mirror', 'raid10', 'raidz', 'raidz2', 'raidz3'],
> +		enum => [
> +		    'single', 'mirror',
> +		    'raid10', 'raidz', 'raidz2', 'raidz3',
> +		    'draid', 'draid2', 'draid3',
> +		],
>  	    },
>  	    devices => {
>  		type => 'string', format => 'string-list',
>  		description => 'The block devices you want to create the zpool on.',
>  	    },
> +	    'draid-config' => {
> +		type => 'string',
> +		format => $draid_config_format,
> +		optional => 1,
> +	    },
>  	    ashift => {
>  		type => 'integer',
>  		minimum => 9,
> @@ -344,6 +366,13 @@ __PACKAGE__->register_method ({
>  	my $devs = [PVE::Tools::split_list($param->{devices})];
>  	my $raidlevel = $param->{raidlevel};
>  	my $compression = $param->{compression} // 'on';
> +	my $draid_config = {};
> +	if (exists $param->{'draid-config'}) {

I moved the "draid-config but no draid level" assertion here, we don't need to check
spare/raid settings then explicitly, which would have been prone to forget to updating
the check below if draid-config ever got more parameters.

> +	    $draid_config = PVE::JSONSchema::parse_property_string(
> +		$draid_config_format, $param->{'draid-config'});
> +	}
> +	my $draid_data = $draid_config->{data};
> +	my $draid_spares = $draid_config->{spares};

I dropped those intermediate variables, location of declaration and usage was pretty
split and it didn't felt like we'd gain much over just using the hash directly here in
their current usage.

[pve-devel] [PATCH V4 manager 2/3] fix #3967: enable ZFS dRAID creation in WebGUI

[Stefan Hrdlicka]

add fields for additional settings required by ZFS dRAID

Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
---
 www/manager6/node/ZFS.js | 69 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/www/manager6/node/ZFS.js b/www/manager6/node/ZFS.js
index 5b3bdbda..75d7d8e1 100644
--- a/www/manager6/node/ZFS.js
+++ b/www/manager6/node/ZFS.js
@@ -9,6 +9,27 @@ Ext.define('PVE.node.CreateZFS', {
     isCreate: true,
     width: 800,
 
+    viewModel: {
+	data: {
+	    raidLevel: 'single',
+	},
+	formulas: {
+	    isdraid: function(get) {
+		return get('raidLevel').startsWith("draid");
+	    },
+	    draidconfig: function(get) {
+		if (get('isdraid')) {
+		    var config = {};
+		    config.data = get('draiddata');
+		    config.spares = get('draidspares');
+		    return PVE.Parser.printPropertyString(config);
+		} else {
+		    return "";
+		}
+	    },
+	},
+    },
+
     initComponent: function() {
 	let me = this;
 
@@ -49,7 +70,54 @@ Ext.define('PVE.node.CreateZFS', {
 				['raidz', 'RAIDZ'],
 				['raidz2', 'RAIDZ2'],
 				['raidz3', 'RAIDZ3'],
+				['draid', 'dRAID'],
+				['draid2', 'dRAID2'],
+				['draid3', 'dRAID3'],
 			    ],
+			    bind: {
+				value: '{raidLevel}',
+			    },
+			},
+			{
+			    xtype: 'proxmoxtextfield',
+			    name: 'draid-config',
+			    hidden: true,
+			    submitValue: true,
+			    bind: {
+				value: '{draidconfig}',
+			    },
+			},
+			{
+			    xtype: 'proxmoxintegerfield',
+			    fieldLabel: gettext('DRAID data devices'),
+			    minValue: 1,
+			    allowBlank: false,
+			    disabled: true,
+			    hidden: true,
+			    submitValue: false,
+			    bind: {
+				disabled: '{!isdraid}',
+				hidden: '{!isdraid}',
+				value: '{draiddata}',
+			    },
+			    reference: 'draiddata',
+			    name: 'draiddata',
+			},
+			{
+			    xtype: 'proxmoxintegerfield',
+			    fieldLabel: gettext('DRAID spares'),
+			    minValue: 0,
+			    allowBlank: false,
+			    disabled: true,
+			    hidden: true,
+			    submitValue: false,
+			    bind: {
+				disabled: '{!isdraid}',
+				hidden: '{!isdraid}',
+				value: '{draidspares}',
+			    },
+			    reference: 'draidspares',
+			    name: 'draidspares',
 			},
 			{
 			    xtype: 'proxmoxKVComboBox',
@@ -101,6 +169,7 @@ Ext.define('PVE.node.CreateZFS', {
 
         me.callParent();
     },
+
 });
 
 Ext.define('PVE.node.ZFSList', {
-- 
2.30.2

[pve-devel] applied: [PATCH V4 manager 2/3] fix #3967: enable ZFS dRAID creation in WebGUI

[Thomas Lamprecht]

Am 10/11/2022 um 14:24 schrieb Stefan Hrdlicka:
> add fields for additional settings required by ZFS dRAID
> 
> Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
> ---
>  www/manager6/node/ZFS.js | 69 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
>

applied, thanks! Note that I reworked both the layout and the config handling a bit.

Basically we can avoid quite some view-model binding/formula stuff and the extra
field just by using the onGetValues submit hook that our InputPanel provides.

Moving the fields then below just makes the form a bit more balanced, no biggie
there.

[pve-devel] [PATCH V4 docs 3/3] fix #3967: add ZFS dRAID documentation

[Stefan Hrdlicka]

add some basic explanation how ZFS dRAID works including
links to openZFS for more details

add documentation for two dRAID parameters used in code

Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
---
 local-zfs.adoc | 44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/local-zfs.adoc b/local-zfs.adoc
index a80b7d8..d3db1c5 100644
--- a/local-zfs.adoc
+++ b/local-zfs.adoc
@@ -32,7 +32,8 @@ management.
 
 * Copy-on-write clone
 
-* Various raid levels: RAID0, RAID1, RAID10, RAIDZ-1, RAIDZ-2 and RAIDZ-3
+* Various raid levels: RAID0, RAID1, RAID10, RAIDZ-1, RAIDZ-2, RAIDZ-3,
+dRAID, dRAID2, dRAID3
 
 * Can use SSD for cache
 
@@ -244,6 +245,47 @@ them, unless your environment has specific needs and characteristics where
 RAIDZ performance characteristics are acceptable.
 
 
+ZFS dRAID
+~~~~~~~~~
+
+In a ZFS dRAID (declustered RAID) the hot spare drive(s) participate in the RAID.
+Their spare capacity is reserved and used for rebuilding when one drive fails.
+This provides, depending on the configuration, faster rebuilding compared to a
+RAIDZ in case of drive failure. More information can be found in the official
+OpenZFS documentation. footnote:[OpenZFS dRAID
+https://openzfs.github.io/openzfs-docs/Basic%20Concepts/dRAID%20Howto.html]
+
+NOTE: dRAID is intended for more than 10-15 disks in a dRAID. A RAIDZ
+setup should be better for a lower amount of disks in most use cases.
+
+NOTE: The GUI requires one more disk than the minimum (i.e. dRAID1 needs 3). It
+expects that a spare disk is added as well.
+
+ * `dRAID1` or `dRAID`: requires at least 2 disks, one can fail before data is
+lost
+ * `dRAID2`: requires at least 3 disks, two can fail before data is lost
+ * `dRAID3`: requires at least 4 disks, three can fail before data is lost
+
+
+Additional information can be found on the manual page:
+
+----
+# man zpoolconcepts
+----
+
+Spares and Data
+^^^^^^^^^^^^^^^
+The number of `spares` tells the system how many disks it should keep ready in
+case of a disk failure. The default value is 0 `spares`. Without spares,
+rebuilding won't get any speed benefits.
+
+`data` defines the number of devices in a redundancy group. The default value is
+8. Except when `disks - parity - spares` equal something less than 8, the lower
+number is used. In general, a smaller number of `data` devices leads to higher
+IOPS, better compression ratios and faster resilvering, but defining fewer data
+devices reduces the available storage capacity of the pool.
+
+
 Bootloader
 ~~~~~~~~~~
 
-- 
2.30.2

[pve-devel] applied: [PATCH V4 docs 3/3] fix #3967: add ZFS dRAID documentation

[Thomas Lamprecht]

Am 10/11/2022 um 14:24 schrieb Stefan Hrdlicka:
> add some basic explanation how ZFS dRAID works including
> links to openZFS for more details
> 
> add documentation for two dRAID parameters used in code
> 
> Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
> ---
>  local-zfs.adoc | 44 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 43 insertions(+), 1 deletion(-)
> 
>

applied, thanks!

Re: [pve-devel] [PATCH V4 SERIES storage/manager/docs 0/3] fix #3967: add ZFS dRAID creation

[Lukas Wagner]

Tested-by: Lukas Wagner <l.wagner@proxmox.com>

On 11/10/22 14:24, Stefan Hrdlicka wrote:
> V1 -> V2:
> # pve-storage
> * formating change
> * fixing typos & wording
> * added check if dRAID options draidspares & draiddata are used for something
> else then setting up a dRAID
>
> # pve-manager
> * add a viewModel and use a binding for visiblity
> * remove controller
>
> # pve-docs
> * fix many typos
> * reword last paragraph to make it (hopefully :)) more helpful
>
>
> V2 -> V3:
> # pve-storage
> * use a draid-config format
>
> # pve-manager
> * change due to draid config format (submit text field all with draid config values)
> * add hidden field for submitting values --> added addtional binding
> * the data & spares fields are now required to be selected in the GUI
> ** via the API the two config params are not required for now
>
> # pve-docs
> * openZFS replaced with OpenZFS
>
> V3 -> V4:
> # pve-docs
> * added note to explain why the GUI expects one more disk then the
>    minimum that dRAID would allow
>
>
> Stefan Hrdlicka (1):
>    fix #3967: enable ZFS dRAID creation via API
>
>   PVE/API2/Disks/ZFS.pm | 55 ++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 54 insertions(+), 1 deletion(-)
>
> ---
>   www/manager6/node/ZFS.js | 69 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 69 insertions(+)
>
> ---
>   local-zfs.adoc | 44 +++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 43 insertions(+), 1 deletion(-)
>
-- 
Best Regards,
Lukas Wagner