[pve-devel] [PATCH storage 1/2] improve zpool activate_storage

[Fabian Grünbichler]

the old code was way too broad here, this fixes at least the
following issues:
- importing of other/unconfigured zpools by "import -a"
- possible false positives if a pool name is a substring of
  another pool name because of "list" without pool name,
  potentially skipping activation for such pools
- not noticing failure to activate in activate_storage
  because the success of "zpool import -a" does not tell us
  anything about the pool we actually wanted to import

checking specifically for the pool to be activated when
calling "zpool list" gets rid of the second issue, and
trying to import only that pool fixes the other two.
---
 PVE/Storage/ZFSPoolPlugin.pm | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index 77ed72c..10b10c4 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -172,6 +172,8 @@ sub zfs_request {
 
     if ($method eq 'zpool_list') {
 	push @$cmd, 'zpool', 'list';
+    } elsif ($method eq 'zpool_import') {
+	push @$cmd, 'zpool', 'import';
     } else {
 	push @$cmd, 'zfs', $method;
     }
@@ -492,16 +494,22 @@ sub volume_rollback_is_possible {
 sub activate_storage {
     my ($class, $storeid, $scfg, $cache) = @_;
 
-    my @param = ('-o', 'name', '-H');
-
-    my $text = $class->zfs_request($scfg, undef, 'zpool_list', @param);
-
     # Note: $scfg->{pool} can include dataset <pool>/<dataset>
     my $pool = $scfg->{pool};
     $pool =~ s!/.*$!!;
 
-    if ($text !~ $pool) {
-	run_command("zpool import -d /dev/disk/by-id/ -a");
+    my @param = ('-o', 'name', '-H', "$pool");
+    my $res;
+    eval {
+	$res = $class->zfs_request($scfg, undef, 'zpool_list', @param);
+    };
+
+    if ($@ || !defined($res) || $res !~ $pool) {
+	eval {
+	    @param = ('-d', '/dev/disk/by-id/', "$pool");
+	    $class->zfs_request($scfg, undef, 'zpool_import', @param);
+	};
+	die "could not activate storage '$storeid', $@\n" if $@;
     }
     return 1;
 }
-- 
2.1.4