[pve-devel] [PATCH storage 2/2] add API for ZFS management

Dietmar Maurer dietmar at proxmox.com
Wed Aug 8 08:10:21 CEST 2018


applied, comments inline

> On August 7, 2018 at 4:51 PM Dominik Csapak <d.csapak at proxmox.com> wrote:
> 
> 
> a list, a detail and a create api call
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  PVE/API2/Disks.pm       |   7 +
>  PVE/API2/Disks/Makefile |   1 +
>  PVE/API2/Disks/ZFS.pm   | 351
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 359 insertions(+)
>  create mode 100644 PVE/API2/Disks/ZFS.pm
> 
> diff --git a/PVE/API2/Disks.pm b/PVE/API2/Disks.pm
> index b501c60..e335a13 100644
> --- a/PVE/API2/Disks.pm
> +++ b/PVE/API2/Disks.pm
> @@ -11,6 +11,7 @@ use PVE::JSONSchema qw(get_standard_option);
>  use PVE::API2::Disks::LVM;
>  use PVE::API2::Disks::LVMThin;
>  use PVE::API2::Disks::Directory;
> +use PVE::API2::Disks::ZFS;
>  
>  use PVE::RESTHandler;
>  
> @@ -32,6 +33,11 @@ __PACKAGE__->register_method ({
>  });
>  
>  __PACKAGE__->register_method ({
> +   subclass => "PVE::API2::Disks::ZFS",
> +   path => 'zfs',
> +});
> +
> +__PACKAGE__->register_method ({
>      name => 'index',
>      path => '',
>      method => 'GET',
> @@ -62,6 +68,7 @@ __PACKAGE__->register_method ({
>  	    { name => 'lvm' },
>  	    { name => 'lvmthin' },
>  	    { name => 'directory' },
> +	    { name => 'zfs' },
>  	    ];
>  
>  	return $result;
> diff --git a/PVE/API2/Disks/Makefile b/PVE/API2/Disks/Makefile
> index ccbe004..9152aed 100644
> --- a/PVE/API2/Disks/Makefile
> +++ b/PVE/API2/Disks/Makefile
> @@ -1,6 +1,7 @@
>  
>  SOURCES= LVM.pm\
>  	 LVMThin.pm\
> +	 ZFS.pm\
>  	 Directory.pm
>  
>  .PHONY: install
> diff --git a/PVE/API2/Disks/ZFS.pm b/PVE/API2/Disks/ZFS.pm
> new file mode 100644
> index 0000000..0a5a557
> --- /dev/null
> +++ b/PVE/API2/Disks/ZFS.pm
> @@ -0,0 +1,351 @@
> +package PVE::API2::Disks::ZFS;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::Diskmanage;
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::API2::Storage::Config;
> +use PVE::Storage;
> +use PVE::Tools qw(run_command lock_file trim);
> +
> +use PVE::RPCEnvironment;
> +use PVE::RESTHandler;
> +
> +use base qw(PVE::RESTHandler);
> +
> +my $ZPOOL = '/sbin/zpool';
> +my $ZFS = '/sbin/zfs';
> +
> +__PACKAGE__->register_method ({
> +    name => 'index',
> +    path => '',
> +    method => 'GET',
> +    proxyto => 'node',
> +    protected => 1,
> +    permissions => {
> +	check => ['perm', '/', ['Sys.Audit', 'Datastore.Audit'], any => 1],
> +    },
> +    description => "List Zpools.",
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	},
> +    },
> +    returns => {
> +	type => 'array',
> +	items => {
> +	    type => 'object',
> +	    properties => {
> +		name => {
> +		    type => 'string',
> +		    description => "",
> +		},
> +		size => {
> +		    type => 'integer',
> +		    description => "",
> +		},
> +		alloc => {
> +		    type => 'integer',
> +		    description => "",
> +		},
> +		free => {
> +		    type => 'integer',
> +		    description => "",
> +		},
> +		frag => {
> +		    type => 'integer',
> +		    description => "",
> +		},
> +		dedup => {
> +		    type => 'number',
> +		    description => "",
> +		},
> +		health => {
> +		    type => 'string',
> +		    description => "",
> +		},
> +	    },
> +	},
> +	links => [ { rel => 'child', href => "{name}" } ],
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	if (!-f $ZPOOL) {
> +	    die "zfsutils-linux not installed\n";
> +	}
> +
> +	my $propnames = [qw(name size alloc free frag dedup health)];
> +	my $numbers = {
> +	    size => 1,
> +	    alloc => 1,
> +	    free => 1,
> +	    frag => 1,
> +	    dedup => 1,
> +	};
> +
> +	my $cmd = [$ZPOOL,'list', '-HpPLo', join(',', @$propnames)];
> +
> +	my $pools = [];
> +
> +	run_command($cmd, outfunc => sub {
> +	    my ($line) = @_;
> +
> +		my @props = split('\s+', trim($line));
> +		my $pool = {};
> +		for (my $i = 0; $i < scalar(@$propnames); $i++) {
> +		    if ($numbers->{$propnames->[$i]}) {
> +			$pool->{$propnames->[$i]} = $props[$i] + 0;
> +		    } else {
> +			$pool->{$propnames->[$i]} = $props[$i];
> +		    }
> +		}
> +
> +		push @$pools, $pool;
> +	});
> +
> +	return $pools;
> +    }});
> +
> +sub preparetree {
> +    my ($el) = @_;
> +    delete $el->{lvl};
> +    if ($el->{children} && scalar(@{$el->{children}})) {
> +	$el->{leaf} = 0;
> +	foreach my $child (@{$el->{children}}) {
> +	    preparetree($child);
> +	}
> +    } else {
> +	$el->{leaf} = 1;
> +    }
> +}
> +
> +
> +__PACKAGE__->register_method ({
> +    name => 'detail',
> +    path => '{name}',
> +    method => 'GET',
> +    proxyto => 'node',
> +    protected => 1,
> +    permissions => {
> +	check => ['perm', '/', ['Sys.Audit', 'Datastore.Audit'], any => 1],
> +    },
> +    description => "Get details about a zpool.",
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	    name => get_standard_option('pve-storage-id'),
> +	},
> +    },
> +    returns => {
> +	type => 'object'

return property definitions missing.

> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	if (!-f $ZPOOL) {
> +	    die "zfsutils-linux not installed\n";
> +	}
> +
> +	my $cmd = [$ZPOOL, 'status', '-P', $param->{name}];
> +
> +	my $pool = {
> +	    lvl => 0,
> +	};
> +	my $vdevs = [];
> +
> +	my $curfield;
> +	my $config = 0;
> +
> +	my $stack = [$pool];
> +	my $curlvl = 0;
> +
> +	run_command($cmd, outfunc => sub {
> +	    my ($line) = @_;
> +
> +	    if ($line =~ m/^\s*(\S+): (\S+.*)$/) {
> +		$curfield = $1;
> +		$pool->{$curfield} = $2;
> +
> +		$config = 0 if $curfield eq 'errors';
> +	    } elsif (!$config && $line =~ m/^\s+(\S+.*)$/) {
> +		$pool->{$curfield} .= " " . $1;
> +	    } elsif (!$config && $line =~ m/^\s*config:/) {
> +		$config = 1;
> +	    } elsif ($config && $line =~
> m/^(\s+)(\S+)\s+(\S+)\s+(\S+)\s+(\S+)\s+(\S+)\s*(.*)$/) {
> +		my ($space, $name, $state, $read, $write, $cksum, $msg) = ($1, $2, $3, $4,
> $5, $6, $7);
> +		if ($space  =~ m/^\t(\s+)$/) {
> +		    my $lvl= length($1)/2; # two spaces per level
> +		    my $vdev = {
> +			name => $name,
> +			state => $state,
> +			read => $read + 0,
> +			write => $write + 0,
> +			cksum => $cksum + 0,
> +			msg => $msg,
> +			lvl => $lvl,
> +		    };
> +
> +		    my $cur = pop @$stack;
> +
> +		    if ($lvl > $curlvl) {
> +			$cur->{children} = [ $vdev ];
> +			push @$stack, $cur;
> +			push @$stack, $vdev;
> +		    } elsif ($lvl == $curlvl) {
> +			$cur = pop @$stack;
> +			push @{$cur->{children}}, $vdev;
> +			push @$stack, $cur;
> +			push @$stack, $vdev;
> +		    } else {
> +			while ($lvl <= $cur->{lvl}) {
> +			    $cur = pop @$stack;
> +			}
> +			push @{$cur->{children}}, $vdev;
> +			push @$stack, $cur;
> +			push @$stack, $vdev;
> +		    }
> +		    $curlvl = $lvl;
> +		}
> +	    }
> +	});
> +
> +	# change treenodes for extjs tree
> +	$pool->{name} = delete $pool->{pool};
> +	preparetree($pool);
> +
> +	return $pool;
> +    }});
> +
> +__PACKAGE__->register_method ({
> +    name => 'create',
> +    path => '',
> +    method => 'POST',
> +    proxyto => 'node',
> +    protected => 1,
> +    permissions => {
> +	check => ['perm', '/', ['Sys.Modify', 'Datastore.Allocate']],
> +    },
> +    description => "Create an LVM thinpool",
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	    name => get_standard_option('pve-storage-id'),
> +	    raidlevel => {
> +		type => 'string',
> +		description => 'The RAID level to use, for single disk, use raid1.',

description =~ s/raid1/mirror/ (already fixed that)

> +		enum => ['mirror', 'raid10', 'raidz', 'raidz2', 'raidz3'],
> +	    },
> +	    devices => {
> +		type => 'string-list',
> +		description => 'The block device you want to create the thinpool on',

why 'thinpool'? I thought we create a zfs pool here?

> +	    },
> +	    ashift => {
> +		type => 'integer',
> +		minimum => 9,
> +		maximum => 16,
> +		optional => 1,
> +		default => 12,
> +		description => 'Pool sector size exponent. '

trailing space?

> +	    },
> +	    compression => {
> +		type => 'string',
> +		description => 'The compression algorithm to use.',
> +		enum => ['on', 'off', 'gzip', 'lz4', 'lzjb', 'zle'],
> +		optional => 1,
> +		default => 'on',
> +	    },
> +	    add_storage => {
> +		description => "Configure storage using the thinpool",

missing dot.

> +		type => 'boolean',
> +		optional => 1,
> +		default => 0,
> +	    },
> +	},
> +    },
> +    returns => { type => 'string' },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $rpcenv = PVE::RPCEnvironment::get();
> +	my $user = $rpcenv->get_user();
> +
> +	my $name = $param->{name};
> +	my $devs = [PVE::Tools::split_list($param->{devices})];
> +	my $raidlvl = $param->{raidlevel};

Can't we simply use the same name:

my $raidlevel = $param->{raidlevel};

> +	my $node = $param->{node};
> +	my $ashift = $param->{ashift} // 12;
> +	my $compression = $param->{compression} // 'on';
> +
> +	foreach my $dev (@$devs) {
> +	    $dev = PVE::Diskmanage::verify_blockdev_path($dev);
> +	    die "device $dev is already in use\n" if
> PVE::Diskmanage::disk_is_used($dev);
> +	}
> +
> +	my $cfg = PVE::Storage::config();
> +
> +	if (my $scfg = PVE::Storage::storage_config($cfg, $name, 1)) {
> +	    die "storage ID '$name' already defined\n";
> +	}
> +
> +	my $numdisks = scalar(@$devs);
> +	my $mindisks = {
> +	    mirror => 1,
> +	    raid10 => 4,
> +	    raidz => 3,
> +	    raidz2 => 4,
> +	    raidz3 => 5,
> +	};
> +
> +	# sanity checks
> +	die "raid10 needs an even number of disks\n"
> +	    if $raidlvl eq 'raid10' && $numdisks % 2 != 0;
> +
> +	die "$raidlvl needs at least $mindisks->{$raidlvl} disks\n"
> +	    if $numdisks < $mindisks->{$raidlvl};
> +
> +	my $worker = sub {
> +	    lock_file('/run/lock/pve-diskmanage.lck', 10, sub {

Please can we factor out this "lock_file('/run/lock/pve-diskmanage.lck', ..."
and remove the 
hardcoded timeout.




More information about the pve-devel mailing list