[pve-devel] [PATCH 09/09] This newest version fixes a serious bug cause my asynchronious AJAX calls from web gui.

[mir at datanom.net]

From: Michael Rasmussen <mir at datanom.net>

The problem displays its ugly face when the content tab is invoked in
the gui. This
request calls to plugin method 'list_images' which will produce a list
of all images
available on the storage server. After the gui receives this list it
will call the
plugins method 'path' on every image in the list, each call requires two
ssh requests
to the storage server and since each path call is made asynchronious a
race condition
is likely to happening if only a few images is found on the storage
causing the receive
buffer on the node being mangled with responses from numerous ssh calls.
This means that
the receive buffer growing faster than the the gui is able to consume
causing AJAX calls
to be terminated by the default timeout - eg. the gui gives up and shows
'communication error'
to the user.

To prevent this I have refactored the code handling 'list_images' so
that all needed information
is collected by the first call and at the same time this information is
cached in the plugin to
elimate further calls to the storage. Apart from solving the nasty bug
the refactored code also
has improved performance of 'list_images' with a factor 10;-)

Signed-off-by: Michael Rasmussen <mir at datanom.net>
---
 PVE/Storage/ZFSPlugin.pm | 79 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 53 insertions(+), 26 deletions(-)

diff --git a/PVE/Storage/ZFSPlugin.pm b/PVE/Storage/ZFSPlugin.pm
index c5bf8c4..e2404e4 100644
--- a/PVE/Storage/ZFSPlugin.pm
+++ b/PVE/Storage/ZFSPlugin.pm
@@ -12,6 +12,7 @@ use base qw(PVE::Storage::Plugin);
 
 my @ssh_opts = ('-o', 'BatchMode=yes');
 my @ssh_cmd = ('/usr/bin/ssh', @ssh_opts);
+my $images = ();
 
 sub zfs_request {
     my ($scfg, $method, @params) = @_;
@@ -274,14 +275,49 @@ sub zfs_delete_zvol {
     zfs_request($scfg, 'destroy', '-r', "$scfg->{pool}/$zvol");
 }
 
+sub zfs_get_lun_number {
+	my ($scfg, $name) = @_;
+	my $lunnum = undef;
+	
+	my $text = zfs_request($scfg, 'list_view', '-l', $name);
+	my @lines = split /\n/, $text;
+
+    foreach my $line (@lines) {
+        if ($line =~ /^\s*LUN\s*:\s*(\d+)$/) {
+			$lunnum = $1;
+			last;
+		}
+	}
+	
+	return $lunnum;
+}
+
+sub zfs_get_lun_number_from_name {
+	my ($scfg, $list, $name) = @_;
+	my $lun;
+
+	my $path = "/dev/zvol/rdsk/$scfg->{pool}/$name";
+    	foreach my $info (@$list) {
+    		if ($info->{source} =~ /^$path$/) {
+			$lun = $info->{lun};
+			last;
+		}
+    	}
+	return zfs_get_lun_number($scfg, $lun);
+}
+
 sub zfs_list_zvol {
     my ($scfg) = @_;
+    my $text;
 
-    my $text = zfs_request($scfg, 'list', '-o', 'name,volsize,origin', '-Hr');
+    $text = zfs_request($scfg, 'list', '-o', 'name,volsize,origin', '-Hr');
     my $zvols = zfs_parse_zvol_list($text);
     return undef if !$zvols;
 
-    my $list = {};
+    $text = zfs_request($scfg, 'list_lu', '-v');
+    my $luns = zfs_parse_lu_list($text);
+
+    my $list = ();
     foreach my $zvol (@$zvols) {
 		my @values = split('/', $zvol->{name});
 
@@ -300,33 +336,20 @@ sub zfs_list_zvol {
 			$parent = $1;
 		}
 
+		my $lun = zfs_get_lun_number_from_name($scfg, $luns, $image);
 		$list->{$pool}->{$image} = {
 			name => $image,
 			size => $zvol->{size},
 			parent => $parent,
 			format => 'raw',
-			vmid => $owner
+			vmid => $owner,
+			lun => $lun
 		};
     }
 
-    return $list;
-}
-
-sub zfs_get_lun_number {
-	my ($scfg, $name) = @_;
-	my $lunnum = undef;
-	
-	my $text = zfs_request($scfg, 'list_view', '-l', $name);
-	my @lines = split /\n/, $text;
+    $images = $list;
 
-    foreach my $line (@lines) {
-        if ($line =~ /^\s*LUN\s*:\s*(\d+)$/) {
-			$lunnum = $1;
-			last;
-		}
-	}
-	
-	return $lunnum;
+    return $list;
 }
 
 # Configuration
@@ -396,17 +419,22 @@ sub parse_volname {
 
 sub path {
     my ($class, $scfg, $volname) = @_;
+    my $lun;
 
     my ($vtype, $name, $vmid) = $class->parse_volname($volname);
     
     my $target = $scfg->{target};
     my $portal = $scfg->{portal};
 
-    my $map = zfs_list_lun_mapping_entries($scfg, $name);
-    die "could not find lun number" if !$map;
-    
-    my $lun = zfs_get_lun_number($scfg, @$map[0]->{lun});
-    die "lun is not OK\n" if (! defined($lun));   
+    if ($images && $images->{$scfg->{pool}}->{$name}) {
+		$lun = $images->{$scfg->{pool}}->{$name}->{lun};
+    } else {
+		my $map = zfs_list_lun_mapping_entries($scfg, $name);
+		die "could not find lun number" if !$map;
+		$lun = zfs_get_lun_number($scfg, @$map[0]->{lun});
+    }
+    die "lun is not OK\n" if (! defined($lun));
+
     my $path = "iscsi://$portal/$target/$lun";
 
     return ($path, $vmid, $vtype);
@@ -670,4 +698,3 @@ sub volume_has_feature {
 }
 
 1;
-
-- 
1.8.4.rc2