[pve-devel] [PATCH] Add genegic multipath storage plugin and module for it to manipulate LUNs on Netapp storage. Tested on pretty old 7-mode FAS2040 on iSCSI, but shuld also work on newer clustered setups and over FC media. Plugin needs non-default for PVE packages: multipath-tools, scsitools. Also option ``uid_attribute ID_WWN_WITH_EXTENSION'' and to blacklist all in multipath.conf are required.

Dominik Csapak d.csapak at proxmox.com
Thu Jul 14 16:55:36 CEST 2016


  i just glanced over the patch, but here are a few comments.

first, i believe your commit message got merged in the title?
second, i believe your intention is good (support for netapp and so on), 
but your approach is backwards:

instead of a multipath plugin with multiple backends, why
not make a plugin for each backend itself, which maps the
disks to multipath devices.

this way the options for the plugins don't get confusing in the future
and you can still have the code they share factored out in a module

third, sadly we do not have a netapp here, so i don't see how we could 
test this. (and if we accept this, we would have to support it),
comments to this from anyone?

further comments inline


On 07/14/2016 03:26 PM, Dmitry Petuhov wrote:
> Signed-off-by: Dmitry Petuhov <mityapetuhov at gmail.com>
> ---
>  PVE/Storage.pm                |   2 +
>  PVE/Storage/LunCmd/NetApp.pm  | 459 ++++++++++++++++++++++++++++++++++++++++++
>  PVE/Storage/MPDirectPlugin.pm | 383 +++++++++++++++++++++++++++++++++++
>  3 files changed, 844 insertions(+)
>  create mode 100644 PVE/Storage/LunCmd/NetApp.pm
>  create mode 100644 PVE/Storage/MPDirectPlugin.pm
>
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index 991131a..a9260b6 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -32,6 +32,7 @@ use PVE::Storage::GlusterfsPlugin;
>  use PVE::Storage::ZFSPoolPlugin;
>  use PVE::Storage::ZFSPlugin;
>  use PVE::Storage::DRBDPlugin;
> +use PVE::Storage::MPDirectPlugin;
>
>  # load and initialize all plugins
>  PVE::Storage::DirPlugin->register();
> @@ -46,6 +47,7 @@ PVE::Storage::GlusterfsPlugin->register();
>  PVE::Storage::ZFSPoolPlugin->register();
>  PVE::Storage::ZFSPlugin->register();
>  PVE::Storage::DRBDPlugin->register();
> +PVE::Storage::MPDirectPlugin->register();
>  PVE::Storage::Plugin->init();
>
>  my $UDEVADM = '/sbin/udevadm';
> diff --git a/PVE/Storage/LunCmd/NetApp.pm b/PVE/Storage/LunCmd/NetApp.pm
> new file mode 100644
> index 0000000..45df688
> --- /dev/null
> +++ b/PVE/Storage/LunCmd/NetApp.pm
> @@ -0,0 +1,459 @@
> +package PVE::Storage::LunCmd::NetApp;
> +
> +use strict;
> +use warnings;
> +use LWP::UserAgent;
> +use HTTP::Request;
> +use XML::Simple;
> +use Switch;

this introduces two implicit dependencies

while libswitch-perl is installed by default on debian jessie, it could 
still happen that someone deinstalls it, so either add a depends in the 
package files, or do it like in the rest of the code and use
sequential if () elsif () ... codeblocks

the second dependency libxml-simple-perl is not installed by default
and has to be included in the depends section


> +
> +sub netapp_request {
> +    my ($scfg, $vserver, $params) = @_;
> +
> +	my $vfiler = $vserver ? "vfiler='$vserver'" : "";
> +
> +	my $content = "<?xml version='1.0' encoding='UTF-8' ?>\n";
> +	$content .= "<!DOCTYPE netapp SYSTEM 'file:/etc/netapp_filer.dtd'>\n";
> +	$content .= "<netapp $vfiler version='1.19' xmlns='http://www.netapp.com/filer/admin'>\n";
> +	$content .= $params;
> +	$content .= "</netapp>\n";
> +	my $url = "http://".$scfg->{adminserver}."/servlets/netapp.servlets.admin.XMLrequest_filer";
> +	my $request = HTTP::Request->new('POST',"$url");
> +	$request->authorization_basic($scfg->{login},$scfg->{password});
> +
> +	$request->content($content);
> +	$request->content_length(length($content));
> +	my $ua = LWP::UserAgent->new;
> +	my $response = $ua->request($request);
> +	my $xmlparser = XML::Simple->new( KeepRoot => 1 );
> +	my $xmlresponse = $xmlparser->XMLin($response->{_content});
> +
> +	if(is_array($xmlresponse->{netapp}->{results})){
> +	    foreach my $result (@{$xmlresponse->{netapp}->{results}}) {
> +		if($result->{status} ne 'passed'){
> +		    die "netapp api error : ".$result->{reason};
> +		}
> +	    }
> +	}
> +	elsif ($xmlresponse->{netapp}->{results}->{status} ne 'passed') {
> +	    die "netapp api error : ".$content.$xmlresponse->{netapp}->{results}->{reason};
> +	}
> +
> +	return $xmlresponse;
> +}
> +
> +sub netapp_build_params {
> +    my ($execute, %params) = @_;
> +
> +    my $xml = "<$execute>\n";
> +    while (my ($property, $value) = each(%params)){
> +	$xml.="<$property>$value</$property>\n";
> +    }
> +    $xml.="</$execute>\n";
> +
> +    return $xml;
> +
> +}
> +
> +
> +sub netapp_create_volume {
> +    my ($scfg, $volume, $size) = @_;
> +
> +    print "netapp create volume $volume\n";
> +    my $aggregate = $scfg->{array};
> +    my $xmlparams = ($scfg->{api} == 8 && $scfg->{vserver})?netapp_build_params("volume-create", "containing-aggr-name" => $aggregate, "volume" => $volume, "size" => "$size", "junction-path" => "/images/$volume", "space-reserve" => "none"):
> +		    netapp_build_params("volume-create", "containing-aggr-name" => $aggregate, "volume" => $volume, "size" => "$size", "space-reserve" => "none");
> +    netapp_request($scfg, $scfg->{vserver}, $xmlparams);
> +
> +}
> +
> +sub netapp_sisenable_volume {
> +    my ($scfg, $volume) = @_;
> +
> +    print "netapp enable sis on volume $volume\n";
> +    my $xmlparams = netapp_build_params("sis-enable", "path" => "/vol/$volume");
> +    netapp_request($scfg, $scfg->{vserver}, $xmlparams);
> +}
> +
> +sub netapp_sissetconfig_volume {
> +    my ($scfg, $volume) = @_;
> +
> +    print "netapp enable compression on volume $volume\n";
> +    my $xmlparams = netapp_build_params("sis-set-config", "enable-compression" => "true", "enable-inline-compression" => "true", "schedule" => "-", "path" => "/vol/$volume");
> +    netapp_request($scfg, $scfg->{vserver}, $xmlparams);
> +}
> +
> +sub netapp_autosize_volume {
> +    my ($scfg, $volume) = @_;
> +
> +    print "netapp enable autosize on volume $volume\n";
> +    my $xmlparams = ($scfg->{api} == 8)?
> +		netapp_build_params('volume-autosize-set', 'mode' => 'grow_shrink', 'volume' => $volume):
> +		netapp_build_params('volume-autosize-set', 'is-enabled' => 'true', 'volume' => $volume);
> +    netapp_request($scfg, $scfg->{vserver}, $xmlparams);
> +}
> +
> +sub netapp_snapshotsetreserve_volume {
> +    my ($scfg, $volume) = @_;
> +
> +    print "netapp set snapshotreserver 0% on volume $volume\n";
> +    my $xmlparams = netapp_build_params("snapshot-set-reserve", "volume" => $volume, "percentage" => "0");
> +    netapp_request($scfg, $scfg->{vserver}, $xmlparams);
> +}
> +
> +sub netapp_file_rename {
> +    my($scfg, $src, $dst) = @_;
> +    print "netapp rename file $src $dst\n";
> +    my $xmlparams ="<file-rename-file><from-path>$src</from-path><to-path>$dst</to-path></file-rename-file>";
> +    netapp_request($scfg, $scfg->{vserver}, $xmlparams);
> +}
> +
> +sub netapp_list_vols {
> +    my ($scfg, $vmid) = @_;
> +
> +    my $list = {};
> +    if ($scfg->{api} == 8) {
> +	my $xmlparams = '<volume-get-iter><desired-attributes><volume-attributes><volume-id-attributes><name></name></volume-id-attributes></volume-attributes></desired-attributes><max-records>5000</max-records></volume-get-iter>';
> +	my $xmlresponse = netapp_request($scfg, $scfg->{vserver}, $xmlparams);
> +
> +	foreach my $result (@{$xmlresponse->{netapp}->{results}->{"attributes-list"}->{"volume-attributes"}}) {
> +	    if ($result->{'volume-id-attributes'}->{name} =~  m/^(vm_.+)/) {
> +		my ($volume) = ($1);
> +		$list->{$volume} = $volume;
> +	    }
> +	}
> +    } elsif ($scfg->{api} == 7) {
> +	my $xmlresponse = netapp_request($scfg, $scfg->{vserver}, netapp_build_params("volume-list-info-iter-start"));
> +
> +	my $tag     = $xmlresponse->{netapp}->{results}->{tag};
> +	my $records = $xmlresponse->{netapp}->{results}->{records};
> +
> +	$xmlresponse = netapp_request($scfg, $scfg->{vserver}, netapp_build_params('volume-list-info-iter-next', 'tag' => $tag, 'maximum' => $records ));
> +
> +	my $list = {};
> +	foreach my $name (keys %{$xmlresponse->{netapp}->{results}->{"volumes"}->{"volume-info"}}) {
> +	    if ($name =~  m/^(vm_.+)/) {
> +		my ($volume) = ($1);
> +		$list->{$volume} = $volume;
> +	    }
> +	}
> +
> +	$xmlresponse = netapp_request($scfg, $scfg->{vserver}, netapp_build_params('volume-list-info-iter-end', 'tag' => $tag));
> +    }
> +    return $list;
> +}
> +
> +sub netapp_resize_volume {
> +    my ($scfg, $volume, $size) = @_;
> +
> +    netapp_request($scfg, $scfg->{vserver}, netapp_build_params("volume-size", "volume" => $volume, "new-size" => "$size" ));
> +}
> +
> +sub netapp_snapshot_create {
> +    my ($scfg, $volume, $snapname) = @_;
> +
> +    netapp_request($scfg, $scfg->{vserver}, netapp_build_params("snapshot-create", "volume" => $volume, "snapshot" => "$snapname" ));
> +}
> +
> +sub netapp_snapshot_exist {
> +    my ($scfg, $volume, $snap) = @_;
> +
> +    my $snapshotslist = netapp_request($scfg, $scfg->{vserver}, netapp_build_params("snapshot-list-info", "volume" => "$volume"));
> +    my $snapshotexist = undef;
> +    $snapshotexist = 1 if (defined($snapshotslist->{"netapp"}->{"results"}->{"snapshots"}->{"snapshot-info"}->{"$snap"}));
> +    $snapshotexist = 1 if (defined($snapshotslist->{netapp}->{results}->{"snapshots"}->{"snapshot-info"}->{name}) && $snapshotslist->{netapp}->{results}->{"snapshots"}->{"snapshot-info"}->{name} eq $snap);
> +    return $snapshotexist;
> +}
> +
> +sub netapp_snapshot_rollback {
> +    my ($scfg, $volume, $snapname) = @_;
> +
> +    netapp_request($scfg, $scfg->{vserver}, netapp_build_params("snapshot-restore-volume", "volume" => $volume, "snapshot" => "$snapname" ));
> +}
> +
> +sub netapp_snapshot_delete {
> +    my ($scfg, $volume, $snapname) = @_;
> +
> +    netapp_request($scfg, $scfg->{vserver}, netapp_build_params("snapshot-delete", "volume" => $volume, "snapshot" => "$snapname" ));
> +}
> +
> +sub netapp_unmount_volume {
> +    my ($scfg, $volume) = @_;
> +
> +	print "netapp umount volume $volume\n";
> +	my $xmlparams = netapp_build_params("volume-unmount", "volume-name" => "$volume", "force" => "true");
> +        netapp_request($scfg, $scfg->{vserver}, $xmlparams);
> +}
> +
> +sub netapp_offline_volume {
> +    my ($scfg, $volume) = @_;
> +
> +	print "netapp offline volume $volume\n";
> +	my $xmlparams = netapp_build_params("volume-offline", "name" => "$volume");
> +        netapp_request($scfg, $scfg->{vserver}, $xmlparams);
> +}
> +
> +sub netapp_destroy_volume {
> +    my ($scfg, $volume) = @_;
> +
> +	print "netapp destroy volume $volume\n";
> +	my $xmlparams = netapp_build_params("volume-destroy", "name" => "$volume");
> +        netapp_request($scfg, $scfg->{vserver}, $xmlparams);
> +}
> +
> +sub netapp_clone_volume {
> +    my ($scfg, $volumesrc, $snap, $volumedst) = @_;
> +
> +    netapp_request($scfg, $scfg->{vserver}, netapp_build_params("volume-clone-create",
> +								"parent-volume" => "$volumesrc",
> +								"parent-snapshot" => "$snap",
> +								"volume" => "$volumedst",
> +								"junction-path" => "/images/$volumedst",
> +								"junction-active" => "true",
> +								"space-reserve" => "none",
> +								));
> +}
> +
> +sub netapp_list_luns {
> +    my ($scfg, $vmid) = @_;
> +    my $list = {};
> +
> +    if ($scfg->{api} == 8) {
> +	my $xmlresponse = netapp_request($scfg, $scfg->{'vserver'},
> +		'<lun-get-iter><desired-attributes><lun-attributes><path></path><serial-number></serial-number>'.
> +		'<size></size><state></state><mapped></mapped></lun-attributes></desired-attributes>'.
> +		'<max-records>5000</max-records></lun-get-iter>');
> +
> +	foreach my $lun (@{$xmlresponse->{'netapp'}->{'results'}->{'attributes-list'}->{'lun-attributes'}}) {
> +	    next unless $lun->{'path'} =~  m!/vol/(\S+)/(vm-(\d+)-disk-\d+)$!;
> +	    next if defined($vmid) and $3 != $vmid;
> +
> +	    my $name = $2;
> +	    $list->{$name}->{'vol'} = $1;
> +	    # Get wwn from LUN's serial number
> +	    $list->{$name}->{'wwn'} = $lun->{'serial-number'};
> +	    $list->{$name}->{'wwn'} =~ s/(.)/sprintf("%x",ord($1))/eg; #convert to hex
> +	    $list->{$name}->{'wwn'} = '60a98000' . $list->{$name}->{'wwn'}; # Add netapp-specific prefix
> +	    $list->{$name}->{'path'} = $lun->{'path'};
> +	    $list->{$name}->{'size'} = $lun->{'size'};
> +	    $list->{$name}->{'onlnie'} = ($lun->{'state'} eq 'online')?'true':'false';

s/onlnie/online/

> +	    $list->{$name}->{'mapped'} = $lun->{'mapped'};
> +	}
> +    } elsif ($scfg->{api} == 7) {
> +
> +	my $xmlresponse = netapp_request($scfg, $scfg->{'vserver'}, netapp_build_params('lun-list-info'));
> +
> +	foreach my $lun (@{$xmlresponse->{'netapp'}->{'results'}->{'luns'}->{'lun-info'}}) {
> +	    next unless $lun->{'path'} =~  m!/vol/(\S+)/(vm-(\d+)-disk-\d+)$!;
> +	    next if defined($vmid) and $3 != $vmid;
> +
> +	    my $name = $2;
> +	    $list->{$name}->{'vol'} = $1;
> +	    # Get wwn from LUN's serial number
> +	    $list->{$name}->{'wwn'} = $lun->{'serial-number'};
> +	    $list->{$name}->{'wwn'} =~ s/(.)/sprintf("%x",ord($1))/eg; #convert to hex
> +	    $list->{$name}->{'wwn'} = '60a98000' . $list->{$name}->{'wwn'}; # Add netapp-specific prefix
> +	    $list->{$name}->{'path'} = $lun->{'path'};
> +	    $list->{$name}->{'size'} = $lun->{'size'};
> +	    $list->{$name}->{'online'} = $lun->{'online'};
> +	    $list->{$name}->{'mapped'} = $lun->{'mapped'};
> +	}
> +    }
> +
> +    return $list;
> +}
> +
> +sub netapp_create_lun {
> +    my ($scfg, $vol, $name, $size) = @_;
> +
> +    my $xmlparams = ($scfg->{api} == 8)?
> +		netapp_build_params('lun-create-by-size', 'ostype' => 'linux', 'path' => "/vol/$vol/$name" , 'size' => $size,
> +			'space-allocation-enabled' => 'true', 'space-reservation-enabled' => 'false'):
> +		netapp_build_params('lun-create-by-size', 'ostype' => 'linux', 'path' => "/vol/$vol/$name" , 'size' => $size,
> +			'space-reservation-enabled' => 'false');
> +
> +    my $xmlresponse = netapp_request($scfg, $scfg->{'vserver'}, $xmlparams);
> +}
> +
> +sub netapp_map_lun {
> +    my ($scfg, $vol, $name) = @_;
> +
> +    my $xmlparams = netapp_build_params('lun-map', 'initiator-group' => $scfg->{igroup}, 'path' => "/vol/$vol/$name");
> +
> +    my $xmlresponse = netapp_request($scfg, $scfg->{'vserver'}, $xmlparams);
> +}
> +
> +sub netapp_resize_lun {
> +    my ($scfg, $vol, $name, $newsize) = @_;
> +
> +    my $xmlparams = netapp_build_params('lun-resize', 'path' => "/vol/$vol/$name", 'size' => $newsize);
> +    my $xmlresponse = netapp_request($scfg, $scfg->{'vserver'}, $xmlparams);
> +}
> +
> +sub netapp_unmap_lun {
> +    my ($scfg, $vol, $name) = @_;
> +
> +    my $xmlparams = netapp_build_params('lun-unmap', 'initiator-group' => $scfg->{'igroup'}, 'path' => "/vol/$vol/$name");
> +
> +    my $xmlresponse = netapp_request($scfg, $scfg->{'vserver'}, $xmlparams);
> +}
> +
> +sub netapp_delete_lun {
> +    my ($scfg, $vol, $name) = @_;
> +
> +    # First, get lun offline
> +    my $xmlresponse = netapp_request($scfg, $scfg->{'vserver'}, netapp_build_params('lun-offline', 'path' => "/vol/$vol/$name"));
> +
> +    # Then delete it
> +    $xmlresponse = netapp_request($scfg, $scfg->{'vserver'}, netapp_build_params('lun-destroy', 'path' => "/vol/$vol/$name"));
> +}
> +
> +sub netapp_aggregate_size {
> +    my ($scfg) = @_;
> +
> +    if ($scfg->{api} == 8) {
> +	my $list = netapp_request($scfg, undef, netapp_build_params("aggr-get-iter", "desired-attributes" => "" ));
> +
> +        foreach my $aggregate (@{$list->{netapp}->{results}->{"attributes-list"}->{"aggr-attributes"}}) {
> +	    if($aggregate->{"aggregate-name"} eq $scfg->{array}){
> +	        my $used = $aggregate->{"aggr-space-attributes"}->{"size-used"};
> +	        my $total = $aggregate->{"aggr-space-attributes"}->{"size-total"};
> +	        my $free = $aggregate->{"aggr-space-attributes"}->{"size-available"};
> +	        return ($total, $free, $used, 1);
> +	    }
> +	}
> +    } elsif ($scfg->{api} == 7) {
> +	my $xmlresponse = netapp_request($scfg, undef, netapp_build_params("aggr-space-list-info"));
> +
> +        foreach my $aggregate (@{$xmlresponse->{netapp}->{results}->{"aggregates"}->{"aggr-space-info"}}) {
> +	    if($aggregate->{"aggregate-name"} eq $scfg->{array}){
> +		my $used = $aggregate->{"size-used"};
> +		my $total = $aggregate->{"size-nominal"};
> +		my $free = $aggregate->{"size-free"};
> +		return ($total, $free, $used, 1);
> +	    }
> +	}
> +    }
> +}
> +
> +sub is_array {
> +  my ($ref) = @_;
> +  # Firstly arrays need to be references, throw
> +  #  out non-references early.
> +  return 0 unless ref $ref;
> +
> +  if (ref $ref eq 'ARRAY'){
> +  return 1;
> +  }
> +  return 0;

nothing in this method after this gets executed in any way, so why have 
it? also the code is not very nice, why would
you try to convert to an array and parse the error message?

i dont think a check like this is necessary, but correct me if i'm wrong

> +
> +
> +  # Now try and eval a bit of code to treat the
> +  #  reference as an array.  If it complains
> +  #  in the 'Not an ARRAY reference' then we're
> +  #  sure it's not an array, otherwise it was.
> +  eval {
> +    my $a = @$ref;
> +  };
> +  if ($@=~/^Not an ARRAY reference/) {
> +    return 0;
> +  } elsif ($@) {
> +    die "Unexpected error in eval: $@\n";
> +  } else {
> +    return 1;
> +  }
> +
> +}
> +
> +sub run_lun_command {
> +    my ($scfg, $timeout, $method, @params) = @_;
> +
> +    my $msg;
> +
> +    switch($method) {
> +	case 'create_lu'   {
> +	    my ($name, $size) = @params;
> +	    my $vol = $name;
> +	    $vol =~ s/-/_/g;
> +	    $vol .= '_vol';
> +	    netapp_create_volume($scfg, $vol, int($size*1.05));

why $size*1.05 ? is there a reason for this?
it would be good to comment such things

> +	    netapp_sisenable_volume($scfg, $vol);
> +	    netapp_sissetconfig_volume($scfg, $vol);
> +	    netapp_autosize_volume($scfg, $vol);
> +	    netapp_snapshotsetreserve_volume($scfg, $vol);
> +	    netapp_create_lun($scfg, $vol, $name, $size);
> +	    $msg = 1;
> +	}
> +	case 'delete_lu'   {
> +	    my ($name) = @params;
> +	    my $vol = $name;
> +	    $vol =~ s/-/_/g;
> +	    $vol .= '_vol';
> +	    netapp_delete_lun($scfg, $vol, $name);
> +	    netapp_unmount_volume($scfg,$vol) if ($scfg->{api} == 8 && $scfg->{vserver});
> +	    netapp_offline_volume($scfg,$vol);
> +	    netapp_destroy_volume($scfg,$vol);
> +	    $msg = 1;
> +	}
> +	case 'resize_lu'   {
> +	    my ($name, $size) = @params;
> +	    my $vol = $name;
> +	    $vol =~ s/-/_/g;
> +	    $vol .= '_vol';
> +	    netapp_resize_volume($scfg, $vol, $size);
> +	    netapp_resize_lun($scfg, $vol, $name, $size);
> +	    $msg = 1;
> +	}
> +	case 'list_lu'     {
> +	    my ($vmid) = @params;
> +	    $msg = netapp_list_luns($scfg, $vmid);
> +	}
> +	case 'map_lu'      {
> +	    my ($name) = @params;
> +	    my $vol = $name;
> +	    $vol =~ s/-/_/g;
> +	    $vol .= '_vol';
> +	    netapp_map_lun($scfg, $vol, $name);
> +	    $msg = 1;
> +	}
> +	case 'unmap_lu'    {
> +	    my ($name) = @params;
> +	    my $vol = $name;
> +	    $vol =~ s/-/_/g;
> +	    $vol .= '_vol';
> +	    netapp_unmap_lun($scfg, $vol, $name);
> +	    $msg = 1;
> +	}
> +	case 'create_snap' {
> +	    my ($name, $snapname) = @params;
> +	    my $vol = $name;
> +	    $vol =~ s/-/_/g;
> +	    $vol .= '_vol';
> +	    netapp_snapshot_create($scfg, $vol, $snapname);
> +	    $msg = 1;
> +	}
> +	case 'delete_snap' {
> +	    my ($name, $snapname) = @params;
> +	    my $vol = $name;
> +	    $vol =~ s/-/_/g;
> +	    $vol .= '_vol';
> +	    netapp_snapshot_delete($scfg, $vol, $snapname);
> +	    $msg = 1;
> +	}
> +	case 'rollback_snap' {
> +	    my ($name, $snapname) = @params;
> +	    my $vol = $name;
> +	    $vol =~ s/-/_/g;
> +	    $vol .= '_vol';
> +	    netapp_snapshot_rollback($scfg, $vol, $snapname);
> +	    $msg = 1;
> +	}
> +	case 'space_status' {
> +	    $msg = netapp_aggregate_size($scfg);
> +	}
> +    }
> +    return $msg;
> +}
> +

the code

my $vol = $name;
$vol =~ s/-/_/g;
$vol .= '_vol';

is in nearly every case statement, i think it would be better to do this
once in an if/else statement at the beginning of the  method,
so if the code has to be changed, it only has to be changed once

> +
> +1;
> diff --git a/PVE/Storage/MPDirectPlugin.pm b/PVE/Storage/MPDirectPlugin.pm
> new file mode 100644
> index 0000000..327c278
> --- /dev/null
> +++ b/PVE/Storage/MPDirectPlugin.pm
> @@ -0,0 +1,383 @@
> +package PVE::Storage::MPDirectPlugin;
> +
> +use strict;
> +use warnings;
> +use Data::Dumper;
> +use IO::File;
> +use Switch;
> +use PVE::HA::NodeStatus;
> +use PVE::Tools qw(run_command trim file_read_firstline dir_glob_foreach);
> +use PVE::Storage::Plugin;
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::Cluster qw(cfs_read_file cfs_write_file cfs_lock_file);
> +use PVE::Storage::LunCmd::NetApp;
> +
> +use base qw(PVE::Storage::Plugin);
> +
> +# Scan storage bus for new paths. Linux SCSI subsystem is not automatic,
> +# so it doesn't know when new LUNS appear|disappear, and we need to kick it.
> +sub renew_media {
> +    my ($scfg) = @_;
> +    run_command(['/sbin/rescan-scsi-bus', '-r', '-i', '-a']);
> +#    switch($scfg->{media}) {
> +#	case 'fc'    { system('/bin/bash', '-c',
> +#	    'for host in `ls /sys/class/fc_host`; do '.
> +#	    'echo 1 > /sys/class/fc_host/$host/issue_lip & done'); }
> +#	case 'scsi'  { system('/bin/bash', '-c',
> +#	    'for host in `ls /sys/class/scsi_host`; do '.
> +#	    'echo - - -  > /sys/class/scsi_host/$host/scan & done'); }
> +#	case 'iscsi' { system('/usr/bin/iscsiadm', '-m', 'session', '--rescan'); }
> +#    }
> +}
> +

why have the commented code here? should it be only temporarily disabled?
if yes, please add a comment, otherwise why leave it here


> +sub multipath_enable {
> +    my ($scfg, $wwn) = @_;
> +    open my $mpd, '<', '/etc/multipath.conf';
> +    open my $mpdt, '>', '/etc/multipath.conf.new';
> +
> +    renew_media($scfg);
> +
> +    #Copy contents and insert line just afer exceptions block beginning
> +    while (my $line = <$mpd>) {
> +	print $mpdt $line;
> +	print $mpdt "\twwid \"0x$wwn\"\n" if $line =~ m/^blacklist_exceptions \{/;
> +    }
> +
> +    close $mpdt;
> +    close $mpd;
> +    unlink '/etc/multipath.conf';
> +    rename '/etc/multipath.conf.new','/etc/multipath.conf';
> +
> +    #force devmap reload to connect new device
> +    system('/sbin/multipath', '-r');
> +}
> +
> +sub multipath_disable {
> +    my ($scfg, $wwn) = @_;
> +
> +    #get paths to delete them later
> +#    my @paths;
> +#    run_command(['/sbin/multipath', '-l', "0x$wwn"], outfunc => sub {
> +#	my ($line) = @_;
> +#	return unless $line =~ m/\s+\S+\s+(\d+:\d+:\d+:\d+)\s+(sd[a-z]+)\s+/;
> +#	push @paths, $2;
> +#    });

same as above

> +
> +    open my $mpd, '<', '/etc/multipath.conf';
> +    open my $mpdt, '>', '/etc/multipath.conf.new';
> +
> +    #Just copy contents except requested wwn
> +    while (my $line = <$mpd>) {
> +	print $mpdt $line unless $line =~ m/wwid "0x$wwn"/;
> +    }
> +
> +    close $mpdt;
> +    close $mpd;
> +    unlink '/etc/multipath.conf';
> +    rename '/etc/multipath.conf.new','/etc/multipath.conf';
> +
> +    #disable selected wwn multipathing
> +    system('/sbin/multipath', '-f', $wwn);
> +}
> +
> +# Utility functions
> +sub mp_get_name {
> +    my ($class, $scfg, $wwn) = @_;
> +
> +    my $luns = $class->run_lun_command($scfg, 'list_lu', undef);
> +
> +    foreach my $name (keys %$luns) {
> +	return $name if $luns->{$name}->{'wwn'} eq $wwn;
> +    }
> +    die "cannot get name for wwn $wwn\n";
> +}
> +
> +sub mp_get_wwn {
> +    my ($class, $scfg, $name) = @_;
> +
> +    my $luns = $class->run_lun_command($scfg, 'list_lu', undef);
> +
> +    return $luns->{$name}->{'wwn'};
> +}
> +
> +
> +sub run_lun_command {
> +    my ($class, $scfg, $action, @params) = @_;
> +    switch($scfg->{backend}) {
> +	case 'netapp' { return PVE::Storage::LunCmd::NetApp::run_lun_command($scfg, undef, $action, @params); }
> +	else { die "Unsupportned backend '". $scfg->{backend} ."' for multipath storage"; }
> +    }
> +}
> +
> +# Configuration
> +
> +sub type {
> +    return 'mpdirect';
> +}
> +
> +sub plugindata {
> +    return {
> +	content => [ {images => 1, rootdir => 1, none => 1}, { images => 1 }],
> +    };
> +}
> +
> +sub properties {
> +    return {
> +        vserver => {
> +            description => "Vserver name. Netapp-spcific.",
> +            type => 'string',
> +        },
> +        array => {
> +            description => "Array/Pool/Aggregate name.",
> +            type => 'string',
> +        },
> +	adminserver => {
> +	    description => "Management IP or DNS name of storage.",
> +	    type => 'string', format => 'pve-storage-server',
> +	},
> +	login => {
> +	    description => "login",
> +	    type => 'string',
> +	},
> +	password => {
> +	    description => "password",
> +	    type => 'string',
> +	},
> +	igroup => {
> +	    description => "Initiator group name",
> +	    type => 'string',
> +	},
> +	api => {
> +	    description => "API version (backend-specific)",
> +	    type => 'string',
> +	},
> +	backend => {
> +	    description => "Backend storage type",
> +	    type => 'string',
> +	    enum => ['netapp'],
> +	},
> +	media => {
> +	    description => "Backing media",
> +	    type => 'string',
> +	    enum => ['scsi','iscsi','fc'],
> +	},
> +    };
> +}
> +
> +sub options {
> +    return {
> +	adminserver => { fixed => 1 },
> +	login => { fixed => 1 },
> +	password => { optional => 1 },
> +	vserver => { optional => 1 },
> +	array => { fixed => 1 },
> +        nodes => { optional => 1 },
> +	disable => { optional => 1 },
> +	content => { optional => 1 },
> +	igroup => { optional => 1 },
> +	api => { optional => 1 },
> +	backend => { fixed => 1 },
> +	media => { fixed => 1 },
> +    }
> +}
> +
> +# Storage implementation
> +
> +sub parse_volname {
> +    my ($class, $volname) = @_;
> +
> +    if ($volname =~ m/vm-(\d+)-disk-\S+/) {
> +	return ('images', $volname, $1, undef, undef, undef, 'raw');
> +    } else {
> +	die "Invalid volume $volname";
> +    }
> +}
> +
> +sub filesystem_path {
> +    my ($class, $scfg, $volname, $snapname) = @_;
> +
> +    die "Direct attached device snapshot is not implemented" if defined($snapname);
> +
> +    my ($vtype, $name, $vmid) = $class->parse_volname($volname);
> +    die "Cannot find WWN for volume $volname" unless my $wwn = $class->mp_get_wwn($scfg, $name);
> +
> +    my $path = "/dev/disk/by-id/dm-uuid-mpath-0x$wwn";
> +
> +    return wantarray ? ($path, $vmid, $vtype) : $path;
> +}
> +
> +sub create_base {
> +    my ($class, $storeid, $scfg, $volname) = @_;
> +
> +    die "Creating base image is currently unimplemented";
> +}
> +
> +sub clone_image {
> +    my ($class, $scfg, $storeid, $volname, $vmid, $snap) = @_;
> +
> +    die "Cloning image is currently unimplemented";
> +}
> +
> +# Seems like this method gets size in kilobytes somehow,
> +# while listing methost return bytes. That's strange.
> +sub alloc_image {
> +    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
> +
> +    my $luns = $class->run_lun_command($scfg, 'list_lu', $vmid);
> +    unless ($name) {
> +	for (my $i = 1; $i < 100; $i++) {
> +	    if (!$luns->{"vm-$vmid-disk-$i"}) {
> +		$name = "vm-$vmid-disk-$i";
> +		last;
> +	    }
> +	}
> +    }
> +
> +    $class->run_lun_command($scfg, 'create_lu', $name, $size*1024);
> +    $class->run_lun_command($scfg, 'map_lu', $name); #Merge it to create?
> +    return $name;
> +}
> +
> +sub free_image {
> +    my ($class, $storeid, $scfg, $volname, $isBase) = @_;
> +
> +    my $wwn = $class->mp_get_wwn($scfg, $volname);
> +
> +    $class->run_lun_command($scfg, 'unmap_lu', $volname); #Merge it to delete?
> +    $class->run_lun_command($scfg, 'delete_lu', $volname);
> +
> +    dir_glob_foreach('/etc/pve/nodes', '\w+', sub {
> +	my ($node) = @_;
> +	run_command(['/usr/bin/ssh', $node, '/sbin/rescan-scsi-bus -r -i']);
> +    });
> +
> +#	my @paths;
> +#	my $lastdev;
> +#	#get paths to delete them later
> +#	run_command(['/usr/bin/ssh', $node, 'bash -c \'for dev in /dev/sd*; do udevadm info -q property -n $dev | grep -e "^ID_WWN_WITH_EXTENSION" -e "^DEVNAME" ; done\''], outfunc => sub {
> +#	    my ($line) = @_;
> +#	    $lastdev = $1 if $line =~ m!^DEVNAME=/dev/(sd[a-z]+)!;
> +#	    push (@paths, $lasdev) if $line =~ m!^ID_WWN_WITH_EXTENSION=0x(\S+)! and $1 eq $wwn;
> +#	});
> +
> +#	return if scalar(@paths) == 0;
> +#	#delete patchs we lust got
> +#	run_command(['/usr/bin/ssh', $node, 'bash -c \'for dev in '. join(' ', @paths)
> +#		    .' ; do ; done']);
> +}

same as above

> +
> +sub list_images {
> +    my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;
> +
> +    my $res = [];
> +
> +    my $luns = $class->run_lun_command($scfg, 'list_lu', undef);
> +
> +    foreach my $name (keys %$luns) {
> +
> +	my $volid = "$storeid:$name";
> +
> +	if ($vollist) {
> +	    my $found = grep { $_ eq $volid } @$vollist;
> +	    next if !$found;
> +	} else {
> +	    next if defined($vmid);
> +	}
> +
> +	push @$res, {
> +	    'volid' => $volid, 'format' => 'raw', 'size' => $luns->{$name}->{'size'},
> +	};
> +
> +    }
> +
> +    return $res;
> +}
> +
> +sub status {
> +    my ($class, $storeid, $scfg, $cache) = @_;
> +
> +    return $class->run_lun_command($scfg, 'space_status');
> +}
> +
> +sub activate_storage {
> +    my ($class, $storeid, $scfg, $cache) = @_;
> +
> +    # Server's SCSI subsystem is always up, so there's nothing to do
> +    return 1;
> +}
> +
> +sub deactivate_storage {
> +    my ($class, $storeid, $scfg, $cache) = @_;
> +
> +    return 1;
> +}
> +
> +sub activate_volume {
> +    my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
> +
> +    die "volume snapshot is not possible on direct-attached storage device" if $snapname;
> +
> +    warn "Activating '$volname'\n";
> +    multipath_enable($scfg, $class->mp_get_wwn($scfg,$volname));
> +
> +    return 1;
> +}
> +
> +sub deactivate_volume {
> +    my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
> +
> +    die "volume snapshot is not possible on direct-attached storage  device" if $snapname;
> +
> +    warn "Deactivating '$volname'\n";
> +    multipath_disable($scfg, $class->mp_get_wwn($scfg,$volname));
> +
> +    return 1;
> +}
> +
> +sub volume_resize {
> +    my ($class, $scfg, $storeid, $volname, $size, $running) = @_;
> +
> +    $class->run_lun_command($scfg, 'delete_lu', $volname);
> +}
> +
> +sub volume_snapshot {
> +    my ($class, $scfg, $storeid, $volname, $snap) = @_;
> +
> +    $class->run_lun_command($scfg, 'create_snap', $volname, $snap);
> +}
> +
> +sub volume_snapshot_rollback {
> +    my ($class, $scfg, $storeid, $volname, $snap) = @_;
> +
> +    $class->run_lun_command($scfg, 'rollback_snap', $volname, $snap);
> +}
> +
> +sub volume_snapshot_delete {
> +    my ($class, $scfg, $storeid, $volname, $snap) = @_;
> +
> +    $class->run_lun_command($scfg, 'delete_snap', $volname, $snap);
> +}
> +
> +sub volume_has_feature {
> +    my ($class, $scfg, $feature, $storeid, $volname, $snapname, $running) = @_;
> +
> +    my $features = {
> +	snapshot => { current => 1, snap => 1 },
> +	sparseinit => { current => 1 },
> +    };
> +
> +    my ($vtype, $name, $vmid, $basename, $basevmid, $isBase) =
> +	$class->parse_volname($volname);
> +
> +    my $key = undef;
> +    if($snapname) {
> +	$key = 'snap';
> +    } else {
> +	$key =  $isBase ? 'base' : 'current';
> +    }
> +    return 1 if $features->{$feature}->{$key};
> +
> +    return undef;
> +}
> +
> +1;
>





More information about the pve-devel mailing list