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.hrdli...@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 mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to