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

* [pve-devel] [PATCH v2 storage 1/2] fix #5071: zfs over iscsi: add 'zfs-base-path' configuration option
@ 2025-05-27 13:34 Fiona Ebner
  2025-05-27 13:34 ` [pve-devel] [PATCH v2 storage 2/2] zfs over iscsi: on-add hook: dynamically determine base path Fiona Ebner
  2025-06-05 11:02 ` [pve-devel] [PATCH v2 storage 1/2] fix #5071: zfs over iscsi: add 'zfs-base-path' configuration option Christoph Heiss
  0 siblings, 2 replies; 4+ messages in thread
From: Fiona Ebner @ 2025-05-27 13:34 UTC (permalink / raw)
  To: pve-devel

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>
---

Changes in v2:
* Cleaner/more robust approach, add storage configuration setting.

 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..64f1eda 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 specificed. 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


^ permalink raw reply	[flat|nested] 4+ messages in thread