iscsi: code cleanup

[pve-storage.git] / PVE / Storage / ZFSPoolPlugin.pm

diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index 3808ab7e9ec0e3896c1068ed2b26b007393a228d..2e2abe35ac092a2f60ea1959ab97a0d68799551d 100644 (file)
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -2,12 +2,15 @@ package PVE::Storage::ZFSPoolPlugin;
 
 use strict;
 use warnings;
+
 use IO::File;
+use Net::IP;
 use POSIX;
-use PVE::Tools qw(run_command);
-use PVE::Storage::Plugin;
+
+use PVE::ProcFSTools;
 use PVE::RPCEnvironment;
-use Net::IP;
+use PVE::Storage::Plugin;
+use PVE::Tools qw(run_command);
 
 use base qw(PVE::Storage::Plugin);
 
@@ -131,6 +134,8 @@ sub on_add_hook {
     } else {
        $scfg->{mountpoint} = $mountpoint;
     }
+
+    return;
 }
 
 sub path {
@@ -329,9 +334,10 @@ sub zfs_create_subvol {
     my ($class, $scfg, $volname, $size) = @_;
 
     my $dataset = "$scfg->{pool}/$volname";
+    my $quota = $size ? "${size}k" : "none";
 
     my $cmd = ['create', '-o', 'acltype=posixacl', '-o', 'xattr=sa',
-              '-o', "refquota=${size}k", $dataset];
+              '-o', "refquota=${quota}", $dataset];
 
     $class->zfs_request($scfg, undef, @$cmd);
 }
@@ -463,18 +469,30 @@ sub volume_snapshot_delete {
 sub volume_snapshot_rollback {
     my ($class, $scfg, $storeid, $volname, $snap) = @_;
 
-    my $vname = ($class->parse_volname($volname))[1];
+    my (undef, $vname, undef, undef, undef, undef, $format) = $class->parse_volname($volname);
+
+    my $msg = $class->zfs_request($scfg, undef, 'rollback', "$scfg->{pool}/$vname\@$snap");
 
-    $class->zfs_request($scfg, undef, 'rollback', "$scfg->{pool}/$vname\@$snap");
+    # we have to unmount rollbacked subvols, to invalidate wrong kernel
+    # caches, they get mounted in activate volume again
+    # see zfs bug #10931 https://github.com/openzfs/zfs/issues/10931
+    if ($format eq 'subvol') {
+       $class->zfs_request($scfg, undef, 'unmount', "$scfg->{pool}/$vname");
+    }
+
+    return $msg;
 }
 
 sub volume_rollback_is_possible {
     my ($class, $scfg, $storeid, $volname, $snap) = @_;
 
     my $recentsnap = $class->zfs_get_latest_snapshot($scfg, $volname);
-    if ($snap ne $recentsnap) {
-       die "can't rollback, more recent snapshots exist\n";
-    }
+
+    die "can't rollback, no snapshots exist at all\n"
+       if !defined($recentsnap);
+
+    die "can't rollback, '$snap' is not most recent snapshot\n"
+       if $snap ne $recentsnap;
 
     return 1;
 }
@@ -505,32 +523,49 @@ sub volume_snapshot_list {
     return $snaps;
 }
 
+my sub dataset_mounted_heuristic {
+    my ($dataset) = @_;
+
+    my $mounts = PVE::ProcFSTools::parse_proc_mounts();
+    for my $mp (@$mounts) {
+       my ($what, $dir, $fs) = $mp->@*;
+       next if $fs ne 'zfs';
+       # check for root-dataset or any child-dataset (root-dataset could have 'canmount=off')
+       # If any child is mounted heuristically assume that `zfs mount -a` was successful
+       next if $what !~ m!^$dataset(?:/|$)!;
+       return 1;
+    }
+    return 0;
+}
+
 sub activate_storage {
     my ($class, $storeid, $scfg, $cache) = @_;
 
     # Note: $scfg->{pool} can include dataset <pool>/<dataset>
-    my $pool = $scfg->{pool};
-    $pool =~ s!/.*$!!;
+    my $dataset = $scfg->{pool};
+    my $pool = ($dataset =~ s!/.*$!!r);
+
+    return 1 if dataset_mounted_heuristic($dataset); # early return
 
     my $pool_imported = sub {
-       my @param = ('-o', 'name', '-H', "$pool");
+       my @param = ('-o', 'name', '-H', $pool);
        my $res = eval { $class->zfs_request($scfg, undef, 'zpool_list', @param) };
-       if ($@) {
-           warn "$@\n";
-           return undef;
-       }
+       warn "$@\n" if $@;
+
        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");
+       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->();
+           die "could not activate storage '$storeid', $err\n" if !$pool_imported->();
        }
     }
+    eval { $class->zfs_request($scfg, undef, 'mount', '-a') };
+    die "could not activate storage '$storeid', $@\n" if $@;
     return 1;
 }
 
@@ -544,11 +579,16 @@ sub activate_volume {
 
     return 1 if defined($snapname);
 
-    my (undef, undef, undef, undef, undef, undef, $format) = $class->parse_volname($volname);
-
-    return 1 if $format ne 'raw';
+    my (undef, $dataset, undef, undef, undef, undef, $format) = $class->parse_volname($volname);
 
-    $class->zfs_wait_for_zvol_link($scfg, $volname);
+    if ($format eq 'raw') {
+       $class->zfs_wait_for_zvol_link($scfg, $volname);
+    } elsif ($format eq 'subvol') {
+       my $mounted = $class->zfs_get_properties($scfg, 'mounted', "$scfg->{pool}/$dataset");
+       if ($mounted !~ m/^yes$/) {
+           $class->zfs_request($scfg, undef, 'mount', "$scfg->{pool}/$dataset");
+       }
+    }
 
     return 1;
 }
@@ -571,7 +611,7 @@ sub clone_image {
     my $name = $class->find_free_diskname($storeid, $scfg, $vmid, $format);
 
     if ($format eq 'subvol') {
-       my $size = $class->zfs_request($scfg, undef, 'list', '-H', '-o', 'refquota', "$scfg->{pool}/$basename");
+       my $size = $class->zfs_request($scfg, undef, 'list', '-Hp', '-o', 'refquota', "$scfg->{pool}/$basename");
        chomp($size);
        $class->zfs_request($scfg, undef, 'clone', "$scfg->{pool}/$basename\@$snap", "$scfg->{pool}/$name", '-o', "refquota=$size");
     } else {
@@ -720,7 +760,7 @@ sub volume_import {
     my $zfspath = "$scfg->{pool}/$dataset";
     my $suffix = defined($base_snapshot) ? "\@$base_snapshot" : '';
     my $exists = 0 == run_command(['zfs', 'get', '-H', 'name', $zfspath.$suffix],
-                            noerr => 1, errfunc => sub {});
+                                 noerr => 1, quiet => 1);
     if (defined($base_snapshot)) {
        die "base snapshot '$zfspath\@$base_snapshot' doesn't exist\n" if !$exists;
     } elsif ($exists) {