[pve-devel] [PATCH V2 SERIES storage/manager/docs 0/3] add ZFS dRAID creation

[pve-devel] [PATCH V2 SERIES storage/manager/docs 0/3] 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
 
----

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

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

--

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

--

 local-zfs.adoc | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)


-- 
2.30.2

[pve-devel] [PATCH V2 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 | 44 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Disks/ZFS.pm b/PVE/API2/Disks/ZFS.pm
index eeb9f48..571bc90 100644
--- a/PVE/API2/Disks/ZFS.pm
+++ b/PVE/API2/Disks/ZFS.pm
@@ -299,12 +299,28 @@ __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.',
 	    },
+	    draiddata => {
+		type => 'integer',
+		minimum => 1,
+		optional => 1,
+		description => 'The number of data devices per redundancy group. (dRAID)',
+	    },
+	    draidspares => {
+		type => 'integer',
+		minimum => 0,
+		optional => 1,
+		description => 'Number of dRAID spares.',
+	    },
 	    ashift => {
 		type => 'integer',
 		minimum => 9,
@@ -339,6 +355,8 @@ __PACKAGE__->register_method ({
 	my $devs = [PVE::Tools::split_list($param->{devices})];
 	my $raidlevel = $param->{raidlevel};
 	my $compression = $param->{compression} // 'on';
+	my $draid_data = $param->{draiddata};
+	my $draid_spares = $param->{draidspares};
 
 	for my $dev (@$devs) {
 	    $dev = PVE::Diskmanage::verify_blockdev_path($dev);
@@ -354,6 +372,9 @@ __PACKAGE__->register_method ({
 	    raidz => 3,
 	    raidz2 => 4,
 	    raidz3 => 5,
+	    draid => 3,
+	    draid2 => 4,
+	    draid3 => 5,
 	};
 
 	# sanity checks
@@ -366,6 +387,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);
@@ -402,6 +439,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] [PATCH V2 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 | 44 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/www/manager6/node/ZFS.js b/www/manager6/node/ZFS.js
index 5b3bdbda..5276ff84 100644
--- a/www/manager6/node/ZFS.js
+++ b/www/manager6/node/ZFS.js
@@ -9,6 +9,17 @@ Ext.define('PVE.node.CreateZFS', {
     isCreate: true,
     width: 800,
 
+    viewModel: {
+	data: {
+	    raidLevel: 'single',
+	},
+	formulas: {
+	    nodraid: function(get) {
+		return !get('raidLevel').startsWith("draid");
+	    },
+	},
+    },
+
     initComponent: function() {
 	let me = this;
 
@@ -49,7 +60,39 @@ Ext.define('PVE.node.CreateZFS', {
 				['raidz', 'RAIDZ'],
 				['raidz2', 'RAIDZ2'],
 				['raidz3', 'RAIDZ3'],
+				['draid', 'dRAID'],
+				['draid2', 'dRAID2'],
+				['draid3', 'dRAID3'],
 			    ],
+			    bind: {
+				value: '{raidLevel}',
+			    },
+			},
+			{
+			    xtype: 'proxmoxintegerfield',
+			    fieldLabel: gettext('DRAID data devices'),
+			    minValue: 1,
+			    disabled: true,
+			    hidden: true,
+			    bind: {
+				disabled: '{nodraid}',
+				hidden: '{nodraid}',
+			    },
+			    reference: 'draiddata',
+			    name: 'draiddata',
+			},
+			{
+			    xtype: 'proxmoxintegerfield',
+			    fieldLabel: gettext('DRAID spares'),
+			    minValue: 0,
+			    disabled: true,
+			    hidden: true,
+			    bind: {
+				disabled: '{nodraid}',
+				hidden: '{nodraid}',
+			    },
+			    reference: 'draidspares',
+			    name: 'draidspares',
 			},
 			{
 			    xtype: 'proxmoxKVComboBox',
@@ -101,6 +144,7 @@ Ext.define('PVE.node.CreateZFS', {
 
         me.callParent();
     },
+
 });
 
 Ext.define('PVE.node.ZFSList', {
-- 
2.30.2

[pve-devel] [PATCH V2 pve-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 | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/local-zfs.adoc b/local-zfs.adoc
index ab0f6ad..b2d3863 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,44 @@ 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.
+
+ * `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

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

[Thomas Lamprecht]

Am 08/06/2022 um 17:34 schrieb Stefan Hrdlicka:
> It is possible to set the number of spares and the size of
> data stripes via draidspares & dreaddata parameters.
> 

looks functional but din't test it, some api design suggestions and small style nits
inline.

> Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
> ---
>  PVE/API2/Disks/ZFS.pm | 44 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/Disks/ZFS.pm b/PVE/API2/Disks/ZFS.pm
> index eeb9f48..571bc90 100644
> --- a/PVE/API2/Disks/ZFS.pm
> +++ b/PVE/API2/Disks/ZFS.pm
> @@ -299,12 +299,28 @@ __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.',
>  	    },
> +	    draiddata => {
> +		type => 'integer',
> +		minimum => 1,
> +		optional => 1,
> +		description => 'The number of data devices per redundancy group. (dRAID)',
> +	    },
> +	    draidspares => {
> +		type => 'integer',
> +		minimum => 0,
> +		optional => 1,
> +		description => 'Number of dRAID spares.',
> +	    },

I'd like above two to be a single format string parameter to avoid bloating the API too much
with specialized/context-dependent params.

my $draid_config_format = {
    spares => {
        type => 'integer',
        minimum => 0,
        description => 'Number of dRAID spares.',
    },
    data => {
        ....
    },
};

(note that I made the format options non-optional by design).

'draid-config' => {
    type => 'string',
    format => $draid_config_format,
    optional => 1,
}

could be also more generically named than draid specific, but I don't see that
many settings in the same spirit still coming in the future.
FWIW, we could also deprecate raidlevel and use a new "raid" named parameter with "type" + the
above config keys as parameter, but that's creeping up the scope of this a bit too much, can
also be something for a next major version.

>  	    ashift => {
>  		type => 'integer',
>  		minimum => 9,
> @@ -339,6 +355,8 @@ __PACKAGE__->register_method ({
>  	my $devs = [PVE::Tools::split_list($param->{devices})];
>  	my $raidlevel = $param->{raidlevel};
>  	my $compression = $param->{compression} // 'on';
> +	my $draid_data = $param->{draiddata};
> +	my $draid_spares = $param->{draidspares};
>  
>  	for my $dev (@$devs) {
>  	    $dev = PVE::Diskmanage::verify_blockdev_path($dev);
> @@ -354,6 +372,9 @@ __PACKAGE__->register_method ({
>  	    raidz => 3,
>  	    raidz2 => 4,
>  	    raidz3 => 5,
> +	    draid => 3,
> +	    draid2 => 4,
> +	    draid3 => 5,
>  	};
>  
>  	# sanity checks
> @@ -366,6 +387,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);

style nit, parenthesis are not required for such simple post-if and we
more commonly use `||` over `or`

> +	}
> +
>  	my $code = sub {
>  	    for my $dev (@$devs) {
>  		PVE::Diskmanage::assert_disk_unused($dev);
> @@ -402,6 +439,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;
>  	    }

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

[Matthias Heiserer]

On 08.06.2022 17:34, Stefan Hrdlicka wrote:
8<--

> +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
The initial "O" in OpenZFS should be capitalised :)

Rest looks good to me

8<--