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

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

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.