[pve-devel] [PATCH storage] zpool: handle race with other zpool imports

[Thomas Lamprecht]

The underlying issue is that a zpool can get imported only once, so
we first check if it's in `zpool list`, and thus imported, and only
if it does not shows up there we try to import it.

But, this can race with either:
* parallel running activate_storage call, through CLI/API/daemon
* a zpool import from an admin (a bit unlikely, but hey that's the
  thing with race conditions ;))

So refactor the "is pool imported" check into a closure, and call it
addditionally if the import failed, and silent the error if the pool
is now listed, and thus imported. This makes it a little bit nicer to
read too, IMO.

Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
---
 PVE/Storage/ZFSPoolPlugin.pm | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index 047a06e..2e9c4c7 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -516,20 +516,23 @@ sub activate_storage {
     my $pool = $scfg->{pool};
     $pool =~ s!/.*$!!;
 
-    my @param = ('-o', 'name', '-H', "$pool");
-    my $res;
-    eval {
-	$res = $class->zfs_request($scfg, undef, 'zpool_list', @param);
-    };
-    my $err = $@;
-    if ($err || !defined($res) || $res !~ $pool) {
-	eval {
-	    @param = ('-d', '/dev/disk/by-id/', '-o', 'cachefile=none', "$pool");
-	    $class->zfs_request($scfg, undef, 'zpool_import', @param);
-	};
+    my $pool_imported = sub {
+	my @param = ('-o', 'name', '-H', "$pool");
+	my $res = eval { $class->zfs_request($scfg, undef, 'zpool_list', @param) };
 	if ($@) {
-	    warn "$err\n";
-	    die "could not activate storage '$storeid', $@\n";
+	    warn "$@\n";
+	    return undef;
+	}
+	return defined($res) && $res =~ m/$pool/;
+    };
+
+    if (!$pool_imported->()) {
+	# import can only be done if not yet imported!
+	my @param = ('-d', '/dev/disk/by-id/', '-o', 'cachefile=none', "$pool");
+	eval { $class->zfs_request($scfg, undef, 'zpool_import', @param) };
+	if (my $err = $@) {
+	    # just could've raced with another import, so recheck if it is imported
+	    die "could not activate storage '$storeid', $@\n" if !$pool_imported->();
 	}
     }
     return 1;
-- 
2.20.1