ZFS: refactor waiting for zvol symlinks

and actually do that not just for creating zvols, but also when
activating them. this should fix a range of issues/races that sometimes
occured on bootup, snapshot rollback or similar operations.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index 2e9c4c75953bbf9fa93fc6a90660c453f34cdeb6..f66b2771d6227cc5f5b47b8710fce7b67ea3a818 100644 (file)
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -189,6 +189,23 @@ sub zfs_request {
     return $msg;
 }
 
+sub zfs_wait_for_zvol_link {
+    my ($class, $scfg, $volname, $timeout) = @_;
+
+    my $default_timeout = PVE::RPCEnvironment->is_worker() ? 60*5 : 10;
+    $timeout = $default_timeout if !defined($timeout);
+
+    my ($devname, undef, undef) = $class->path($scfg, $volname);
+
+    for (my $i = 1; $i <= $timeout; $i++) {
+       last if -b $devname;
+       die "timeout: no zvol device link for '$volname' found after $timeout sec found.\n"
+           if $i == $timeout;
+
+       sleep(1);
+    }
+}
+
 sub alloc_image {
     my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
 
@@ -202,16 +219,8 @@ sub alloc_image {
            if !$volname;
 
        $class->zfs_create_zvol($scfg, $volname, $size);
-       my $devname = "/dev/zvol/$scfg->{pool}/$volname";
+       $class->zfs_wait_for_zvol_link($scfg, $volname);
 
-       my $timeout = PVE::RPCEnvironment->is_worker() ? 60*5 : 10;
-       for (my $i = 1; $i <= $timeout; $i++) {
-           last if -b $devname;
-           die "Timeout: no zvol after $timeout sec found.\n"
-               if $i == $timeout;
-
-           sleep(1);
-       }
     } elsif ( $fmt eq 'subvol') {
 
        die "illegal name '$volname' - should be 'subvol-$vmid-*'\n"
@@ -545,6 +554,15 @@ sub deactivate_storage {
 
 sub activate_volume {
     my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
+
+    return 1 if defined($snapname);
+
+    my (undef, undef, undef, undef, undef, undef, $format) = $class->parse_volname($volname);
+
+    return 1 if $format ne 'raw';
+
+    $class->zfs_wait_for_zvol_link($scfg, $volname);
+
     return 1;
 }
 

diff --git a/test/run_test_zfspoolplugin.pl b/test/run_test_zfspoolplugin.pl
index 63b1456403dafc888cc9acf1500f3005c703b9c3..785d3b7de5a627836dec9125d5dde21afd964bde 100755 (executable)
--- a/test/run_test_zfspoolplugin.pl
+++ b/test/run_test_zfspoolplugin.pl
@@ -1096,6 +1096,7 @@ my $test7 = sub {
     };
 
     eval {
+       PVE::Storage::activate_volumes($cfg, ["$storagename:$vmdisk"]);
        run_command("sgdisk --randomize-guids \/dev\/zvol\/$zpath\/$vmdisk", outfunc => $parse_guid);
        run_command("sgdisk -p \/dev\/zvol\/$zpath\/$vmdisk", outfunc => $parse_guid);
 
@@ -1105,6 +1106,7 @@ my $test7 = sub {
        run_command("sgdisk --randomize-guids \/dev\/zvol\/$zpath\/$vmdisk", outfunc => $parse_guid);
        eval {
            PVE::Storage::volume_snapshot_rollback($cfg, "$storagename:$vmdisk", 'snap1');
+           PVE::Storage::activate_volumes($cfg, ["$storagename:$vmdisk"]);
            $tmp_guid = undef;
            run_command("sgdisk -p \/dev\/zvol\/$zpath\/$vmdisk", outfunc => $parse_guid);
            if ($old_guid ne $tmp_guid) {
@@ -1124,6 +1126,7 @@ my $test7 = sub {
     $tmp_guid = undef;
 
     eval {
+       PVE::Storage::activate_volumes($cfg, ["$storagename:$vmbase"]);
        run_command("sgdisk --randomize-guids \/dev\/zvol\/$zpath\/$vmbase", outfunc => $parse_guid);
        run_command("sgdisk -p \/dev\/zvol\/$zpath\/$vmbase", outfunc => $parse_guid);
 
@@ -1133,6 +1136,7 @@ my $test7 = sub {
        run_command("sgdisk --randomize-guids \/dev\/zvol\/$zpath\/$vmbase", outfunc => $parse_guid);
        eval {
            PVE::Storage::volume_snapshot_rollback($cfg, "$storagename:$vmbase", 'snap1');
+           PVE::Storage::activate_volumes($cfg, ["$storagename:$vmbase"]);
            $tmp_guid = undef;
            run_command("sgdisk -p \/dev\/zvol\/$zpath\/$vmbase", outfunc => $parse_guid);
            if ($old_guid ne $tmp_guid) {
@@ -1152,6 +1156,7 @@ my $test7 = sub {
     $tmp_guid = undef;
 
     eval {
+       PVE::Storage::activate_volumes($cfg, ["$storagename:$vmbase/$vmlinked"]);
        run_command("sgdisk --randomize-guids \/dev\/zvol\/$zpath\/$vmlinked", outfunc => $parse_guid);
        run_command("sgdisk -p \/dev\/zvol\/$zpath\/$vmlinked", outfunc => $parse_guid);
 
@@ -1161,6 +1166,7 @@ my $test7 = sub {
        run_command("sgdisk --randomize-guids \/dev\/zvol\/$zpath\/$vmlinked", outfunc => $parse_guid);
        eval {
            PVE::Storage::volume_snapshot_rollback($cfg, "$storagename:$vmbase\/$vmlinked", 'snap1');
+           PVE::Storage::activate_volumes($cfg, ["$storagename:$vmbase/$vmlinked"]);
            $tmp_guid = undef;
            run_command("sgdisk -p \/dev\/zvol\/$zpath\/$vmlinked", outfunc => $parse_guid);
            if ($old_guid ne $tmp_guid) {
@@ -2585,7 +2591,6 @@ my $test1 = sub {
 $tests->{1} = $test1;
 
 sub setup_zfs {
-
     #create VM zvol
     print "create zvol $vmdisk\n" if $verbose;
     run_command("zfs create -V${volsize}G $zpath\/$vmdisk");
@@ -2601,14 +2606,23 @@ sub setup_zfs {
     print "create subvol $ctdisk\n" if $verbose;
     run_command("zfs create -o refquota=${volsize}G $zpath\/$ctdisk");
 
-    print "create subvol $vmbase\n" if $verbose;
+    print "create subvol $ctbase\n" if $verbose;
     run_command("zfs create -o refquota=${volsize}G $zpath\/$ctbase");
     run_command("zfs snapshot $zpath\/$ctbase$basesnap");
 
-    print "create linked clone $vmlinked\n" if $verbose;
+    print "create linked clone $ctlinked\n" if $verbose;
     run_command("zfs clone $zpath\/$ctbase$basesnap $zpath\/$ctlinked -o refquota=${volsize}G");
-    run_command("udevadm trigger --subsystem-match block");
-    run_command("udevadm settle --timeout 10 --exit-if-exists=/dev/zvol/$zpath\/$ctlinked");
+
+    my $vollist = [
+       "$storagename:$vmdisk",
+       "$storagename:$vmbase",
+       "$storagename:$vmbase/$vmlinked",
+       "$storagename:$ctdisk",
+       "$storagename:$ctbase",
+       "$storagename:$ctbase/$ctlinked",
+    ];
+
+    PVE::Storage::activate_volumes($cfg, $vollist);
 }
 
 sub cleanup_zfs {