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

[Dominik Csapak <d.csapak@proxmox.com>]

On 6/3/22 14:31, Dominik Csapak wrote:
> just saw an additional thing, comment inline
> 
> On 6/2/22 13:22, Stefan Hrdlicka wrote:
>> 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 | 40 +++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/PVE/API2/Disks/ZFS.pm b/PVE/API2/Disks/ZFS.pm
>> index eeb9f48..63946d2 100644
>> --- a/PVE/API2/Disks/ZFS.pm
>> +++ b/PVE/API2/Disks/ZFS.pm
>> @@ -299,12 +299,27 @@ __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 => 'Number of dRAID data stripes.',
>> +        },
>> +        draidspares => {
>> +        type => 'integer',
>> +        minimum => 0,
>> +        optional => 1,
>> +        description => 'Number of dRAID spares.',
>> +        },
>>           ashift => {
>>           type => 'integer',
>>           minimum => 9,
>> @@ -339,6 +354,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 +371,9 @@ __PACKAGE__->register_method ({
>>           raidz => 3,
>>           raidz2 => 4,
>>           raidz3 => 5,
>> +        draid => 3,
>> +        draid2 => 4,
>> +        draid3 => 5,
>>       };
>>       # sanity checks
>> @@ -366,6 +386,19 @@ __PACKAGE__->register_method ({
>>       die "$raidlevel needs at least $mindisks->{$raidlevel} disks\n"
>>           if $numdisks < $mindisks->{$raidlevel};
>> +    # draid checks
>> +    if ($raidlevel =~ m/^draid/) {
>> +        # bare minium 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;
> 
> isn't that calculation wrong?
> if i set draid and no data/spares, i just have to give a single disk?
> (so we should initialize draid_data with 1 in that case probably?)
> 
> also why do you set min draid => 3, subtract 2 then add some again?
> why not have draid => 1, and just add the other things?

thats wrong of course, since we check the amount of disks additionally
at the beginning, sorry for the noise

> 
> 
>> +
>> +        die "At least $draidmin disks needed for current dRAID config\n"
>> +        if $numdisks < $draidmin;
>> +    }
>> +
>>       my $code = sub {
>>           for my $dev (@$devs) {
>>           PVE::Diskmanage::assert_disk_unused($dev);
>> @@ -402,6 +435,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;
>>           }
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>