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