[PATCH storage] fix #8076: zfs: activate volume: check for existence to fix regression with cloud-init disk

[PATCH storage] fix #8076: zfs: activate volume: check for existence to fix regression with cloud-init disk

[Fiona Ebner]

Explicitly checking for existence during activation has the advantage
to catch issues earlier with a clearer error message. Note that the
implementation in Plugin.pm already explicitly checks for existence,
so this also aligns the implementation for ZFS closer to that.

This also fixes #8076: For a zvol (i.e. 'raw' format),
zfs_wait_for_zvol_link() was called without first checking if the
volume actually exists. This means that for non-existent volumes,
activation would only fail after the timeout of 300 seconds in worker
context.

This change also means that non-existent snapshots are caught during
activation already, since previously, activate_volume() would return
early without doing anything for snapshots.

The error message "dataset does not exist" is matched against, because
the exit status from 'zfs' will always be 1 if an error occured; from
the man page:
> The zfs utility exits 0 on success, 1 if an error occurs, and 2 if
> invalid command line options were specified.
Note that zfs_delete_zvol() already does the same.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/Storage/ZFSPoolPlugin.pm | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/src/PVE/Storage/ZFSPoolPlugin.pm b/src/PVE/Storage/ZFSPoolPlugin.pm
index 61c49d9..023bd21 100644
--- a/src/PVE/Storage/ZFSPoolPlugin.pm
+++ b/src/PVE/Storage/ZFSPoolPlugin.pm
@@ -288,6 +288,21 @@ sub zfs_request {
     return $msg;
 }
 
+my sub zfs_volume_exists {
+    my ($class, $scfg, $volname, $snapname) = @_;
+
+    my $name = ($class->parse_volname($volname))[1];
+    my $dataset = "$scfg->{pool}/${name}";
+    $dataset .= "\@${snapname}" if defined($snapname);
+
+    eval { $class->zfs_request($scfg, 10, 'list', '-o', 'name', '-Hp', $dataset); };
+    if (my $err = $@) {
+        return 0 if $err =~ m/dataset does not exist/;
+        die $err;
+    }
+    return 1;
+}
+
 sub zfs_wait_for_zvol_link {
     my ($class, $scfg, $volname, $timeout) = @_;
 
@@ -739,6 +754,12 @@ sub deactivate_storage {
 sub activate_volume {
     my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
 
+    if (!zfs_volume_exists($class, $scfg, $volname, $snapname)) {
+        my $name = "$volname";
+        $name .= "\@${snapname}" if defined($snapname);
+        die "volume ${name} does not exist on storage ${storeid}\n";
+    }
+
     return 1 if defined($snapname);
 
     my (undef, $dataset, undef, undef, undef, undef, $format) = $class->parse_volname($volname);
-- 
2.47.3