[pve-devel] [PATCH v3 storage 1/2] fix #5071: zfs over iscsi: add 'zfs-base-path' configuration option

[pve-devel] [PATCH v3 storage 1/2] fix #5071: zfs over iscsi: add 'zfs-base-path' configuration option

[Fiona Ebner]

Use '/dev/zvol' as a base path for new storages for providers 'iet'
and 'LIO', because that is what modern distributions use.

This is a breaking change regarding the addition of new storages on
older distributions, but it's enough to specify the base path '/dev'
explicitly for setups that require it.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
---

Changes in v3:
* Fix a typo in the description in the schema.

 src/PVE/Storage/LunCmd/Comstar.pm |  3 ++-
 src/PVE/Storage/LunCmd/Iet.pm     |  5 ++--
 src/PVE/Storage/LunCmd/Istgt.pm   |  5 ++--
 src/PVE/Storage/LunCmd/LIO.pm     |  5 ++--
 src/PVE/Storage/ZFSPlugin.pm      | 39 +++++++++++++++++++++++++++----
 5 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/src/PVE/Storage/LunCmd/Comstar.pm b/src/PVE/Storage/LunCmd/Comstar.pm
index 527e4ba..5a1d2e6 100644
--- a/src/PVE/Storage/LunCmd/Comstar.pm
+++ b/src/PVE/Storage/LunCmd/Comstar.pm
@@ -32,7 +32,8 @@ my $get_lun_cmd_map = sub {
 };
 
 sub get_base {
-    return '/dev/zvol/rdsk';
+    my ($scfg) = @_;
+    return $scfg->{'zfs-base-path'} || '/dev/zvol/rdsk';
 }
 
 sub run_lun_command {
diff --git a/src/PVE/Storage/LunCmd/Iet.pm b/src/PVE/Storage/LunCmd/Iet.pm
index 5b09b88..7482d66 100644
--- a/src/PVE/Storage/LunCmd/Iet.pm
+++ b/src/PVE/Storage/LunCmd/Iet.pm
@@ -133,7 +133,7 @@ my $parser = sub {
 
     my $line = 0;
 
-    my $base = get_base;
+    my $base = get_base($scfg);
     my $config = $get_config->($scfg);
     my @cfgfile = split "\n", $config;
 
@@ -471,7 +471,8 @@ sub run_lun_command {
 }
 
 sub get_base {
-    return '/dev';
+    my ($scfg) = @_;
+    return $scfg->{'zfs-base-path'} || '/dev';
 }
 
 1;
diff --git a/src/PVE/Storage/LunCmd/Istgt.pm b/src/PVE/Storage/LunCmd/Istgt.pm
index 2f758f9..a53e25e 100644
--- a/src/PVE/Storage/LunCmd/Istgt.pm
+++ b/src/PVE/Storage/LunCmd/Istgt.pm
@@ -304,7 +304,7 @@ my $parser = sub {
     $CONFIG =~ s/\n$//;
     die "$scfg->{target}: Target not found" unless $SETTINGS->{targets};
     my $max = $SETTINGS->{targets};
-    my $base = get_base;
+    my $base = get_base($scfg);
 
     for (my $i = 1; $i <= $max; $i++) {
         my $target = $SETTINGS->{nodebase}.':'.$SETTINGS->{"LogicalUnit$i"}->{TargetName};
@@ -595,7 +595,8 @@ sub run_lun_command {
 }
 
 sub get_base {
-    return '/dev/zvol';
+    my ($scfg) = @_;
+    return $scfg->{'zfs-base-path'} || '/dev/zvol';
 }
 
 1;
diff --git a/src/PVE/Storage/LunCmd/LIO.pm b/src/PVE/Storage/LunCmd/LIO.pm
index 9264e46..7207c05 100644
--- a/src/PVE/Storage/LunCmd/LIO.pm
+++ b/src/PVE/Storage/LunCmd/LIO.pm
@@ -223,7 +223,7 @@ my $extract_volname = sub {
     my ($scfg, $lunpath) = @_;
     my $volname = undef;
 
-    my $base = get_base;
+    my $base = get_base($scfg);
     if ($lunpath =~ /^$base\/$scfg->{pool}\/([\w\-]+)$/) {
 	$volname = $1;
 	my $prefix = $get_backstore_prefix->($scfg);
@@ -414,7 +414,8 @@ sub run_lun_command {
 }
 
 sub get_base {
-    return '/dev';
+    my ($scfg) = @_;
+    return $scfg->{'zfs-base-path'} || '/dev';
 }
 
 1;
diff --git a/src/PVE/Storage/ZFSPlugin.pm b/src/PVE/Storage/ZFSPlugin.pm
index 94cb11f..d0d330c 100644
--- a/src/PVE/Storage/ZFSPlugin.pm
+++ b/src/PVE/Storage/ZFSPlugin.pm
@@ -39,13 +39,13 @@ my $zfs_get_base = sub {
     my ($scfg) = @_;
 
     if ($scfg->{iscsiprovider} eq 'comstar') {
-        return PVE::Storage::LunCmd::Comstar::get_base;
+        return PVE::Storage::LunCmd::Comstar::get_base($scfg);
     } elsif ($scfg->{iscsiprovider} eq 'istgt') {
-        return PVE::Storage::LunCmd::Istgt::get_base;
+        return PVE::Storage::LunCmd::Istgt::get_base($scfg);
     } elsif ($scfg->{iscsiprovider} eq 'iet') {
-        return PVE::Storage::LunCmd::Iet::get_base;
+        return PVE::Storage::LunCmd::Iet::get_base($scfg);
     } elsif ($scfg->{iscsiprovider} eq 'LIO') {
-        return PVE::Storage::LunCmd::LIO::get_base;
+        return PVE::Storage::LunCmd::LIO::get_base($scfg);
     } else {
         $zfs_unknown_scsi_provider->($scfg->{iscsiprovider});
     }
@@ -204,6 +204,12 @@ sub properties {
 	    description => "target portal group for Linux LIO targets",
 	    type => 'string',
 	},
+	'zfs-base-path' => {
+	    description => "Base path where to look for the created ZFS block devices. Set"
+		." automatically during creation if not specified. Usually '/dev/zvol'.",
+	    type => 'string',
+	    format => 'pve-storage-path',
+	},
     };
 }
 
@@ -223,11 +229,36 @@ sub options {
 	lio_tpg => { optional => 1 },
 	content => { optional => 1 },
 	bwlimit => { optional => 1 },
+	'zfs-base-path' => { optional => 1 },
     };
 }
 
 # Storage implementation
 
+sub on_add_hook {
+    my ($class, $storeid, $scfg, %param) = @_;
+
+    if (!$scfg->{'zfs-base-path'}) {
+	my $base_path;
+	if ($scfg->{iscsiprovider} eq 'comstar') {
+	    $base_path = PVE::Storage::LunCmd::Comstar::get_base($scfg);
+	} elsif ($scfg->{iscsiprovider} eq 'istgt') {
+	    $base_path = PVE::Storage::LunCmd::Istgt::get_base($scfg);
+	} elsif ($scfg->{iscsiprovider} eq 'iet' || $scfg->{iscsiprovider} eq 'LIO') {
+	    # Provider implementations hard-code '/dev/', which does not work for distributions like
+	    # Debian 12. Keep that implementation as-is for backwards compatibility, but use
+	    # '/dev/zvol' here.
+	    $base_path = '/dev/zvol';
+	} else {
+	    $zfs_unknown_scsi_provider->($scfg->{iscsiprovider});
+	}
+
+	$scfg->{'zfs-base-path'} = $base_path;
+    }
+
+    return;
+}
+
 sub path {
     my ($class, $scfg, $volname, $storeid, $snapname) = @_;
 
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH v3 storage 2/2] zfs over iscsi: on-add hook: dynamically determine base path

[Fiona Ebner]

This reduces the potential breakage from commit "fix #5071: zfs over
iscsi: add 'zfs-base-path' configuration option". Only setups where
'/dev/zvol' exists, but is not a valid base, will still be affected.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
---

No changes in v3.

 src/PVE/Storage/ZFSPlugin.pm | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/src/PVE/Storage/ZFSPlugin.pm b/src/PVE/Storage/ZFSPlugin.pm
index d0d330c..ed7adad 100644
--- a/src/PVE/Storage/ZFSPlugin.pm
+++ b/src/PVE/Storage/ZFSPlugin.pm
@@ -3,9 +3,10 @@ package PVE::Storage::ZFSPlugin;
 use strict;
 use warnings;
 use IO::File;
-use POSIX;
+use POSIX qw(ENOENT);
 use PVE::Tools qw(run_command);
 use PVE::Storage::ZFSPoolPlugin;
+use PVE::RESTEnvironment qw(log_warn);
 use PVE::RPCEnvironment;
 
 use base qw(PVE::Storage::ZFSPoolPlugin);
@@ -246,9 +247,26 @@ sub on_add_hook {
 	    $base_path = PVE::Storage::LunCmd::Istgt::get_base($scfg);
 	} elsif ($scfg->{iscsiprovider} eq 'iet' || $scfg->{iscsiprovider} eq 'LIO') {
 	    # Provider implementations hard-code '/dev/', which does not work for distributions like
-	    # Debian 12. Keep that implementation as-is for backwards compatibility, but use
-	    # '/dev/zvol' here.
-	    $base_path = '/dev/zvol';
+	    # Debian 12. Keep that implementation as-is for backwards compatibility, but use custom
+	    # logic here.
+	    my $target = 'root@' . $scfg->{portal};
+	    my $cmd = [@ssh_cmd, '-i', "$id_rsa_path/$scfg->{portal}_id_rsa", $target];
+	    push $cmd->@*, 'ls', '/dev/zvol';
+
+	    my $rc = eval { run_command($cmd, timeout => 10, noerr => 1, quiet => 1) };
+	    my $err = $@;
+	    if (defined($rc) && $rc == 0) {
+		$base_path = '/dev/zvol';
+	    } elsif (defined($rc) && $rc == ENOENT) {
+		$base_path = '/dev';
+	    } else {
+		my $message = $err ? $err : "remote command failed";
+		chomp($message);
+		$message .= " ($rc)" if defined($rc);
+		$message .= " - check 'zfs-base-path' setting manually!";
+		log_warn($message);
+		$base_path = '/dev/zvol';
+	    }
 	} else {
 	    $zfs_unknown_scsi_provider->($scfg->{iscsiprovider});
 	}
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel