From dcefd9dd282cd0a699ddb5bb99ccb98589ca52d4 Mon Sep 17 00:00:00 2001 From: Fabian Ebner Date: Mon, 18 Nov 2019 11:45:38 +0100 Subject: [PATCH] fix #2085: add mountpoint property for non-default ZFS pool MPs When adding a zfspool storage with 'pvesm add' the mount point is now added automatically to the storage configuration if it can be determined. path() does not assume the default mountpoint anymore, fixing 2085. Signed-off-by: Fabian Ebner --- PVE/Storage/ZFSPoolPlugin.pm | 31 +++++++++++++++++++++++++++++-- test/run_test_zfspoolplugin.pl | 26 ++++++++++++++------------ 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm index b8adf1c..507795b 100644 --- a/PVE/Storage/ZFSPoolPlugin.pm +++ b/PVE/Storage/ZFSPoolPlugin.pm @@ -32,6 +32,10 @@ sub properties { description => "use sparse volumes", type => 'boolean', }, + mountpoint => { + description => "mount point", + type => 'string', format => 'pve-storage-path', + }, }; } @@ -44,6 +48,7 @@ sub options { disable => { optional => 1 }, content => { optional => 1 }, bwlimit => { optional => 1 }, + mountpoint => { optional => 1 }, }; } @@ -142,17 +147,39 @@ sub parse_volname { # virtual zfs methods (subclass can overwrite them) +sub on_add_hook { + my ($class, $storeid, $scfg, %param) = @_; + + my $cfg_mountpoint = $scfg->{mountpoint}; + my $mountpoint; + + # ignore failure, pool might currently not be imported + eval { + $mountpoint = $class->zfs_get_properties($scfg, 'mountpoint', $scfg->{pool}, 1); + PVE::JSONSchema::check_format(properties()->{mountpoint}->{format}, $mountpoint); + }; + + if (defined($cfg_mountpoint)) { + if (defined($mountpoint) && !($cfg_mountpoint =~ m|^\Q$mountpoint\E/?$|)) { + warn "warning for $storeid - mountpoint: $cfg_mountpoint " . + "does not match current mount point: $mountpoint\n"; + } + } else { + $scfg->{mountpoint} = $mountpoint; + } +} + sub path { my ($class, $scfg, $volname, $storeid, $snapname) = @_; my ($vtype, $name, $vmid) = $class->parse_volname($volname); my $path = ''; + my $mountpoint = $scfg->{mountpoint} // "/$scfg->{pool}"; if ($vtype eq "images") { if ($name =~ m/^subvol-/ || $name =~ m/^basevol-/) { - # fixme: we currently assume standard mount point?! - $path = "/$scfg->{pool}/$name"; + $path = "$mountpoint/$name"; } else { $path = "/dev/zvol/$scfg->{pool}/$name"; } diff --git a/test/run_test_zfspoolplugin.pl b/test/run_test_zfspoolplugin.pl index 785d3b7..9c5e841 100755 --- a/test/run_test_zfspoolplugin.pl +++ b/test/run_test_zfspoolplugin.pl @@ -16,6 +16,7 @@ my $verbose = undef; my $storagename = "zfstank99"; my $subvol = 'regressiontest'; +my $mountpoint = "${subvol}_mnt"; #volsize in GB my $volsize = 1; @@ -142,10 +143,10 @@ my $test19 = sub { $fail = 0; eval { @res = PVE::Storage::path($cfg, "$storagename:$ctdisk"); - if ($res[0] ne "\/regressiontest\/$ctdisk") { + if ($res[0] ne "\/$mountpoint\/$ctdisk") { $count++; $fail = 1; - warn "Test 19 d: path is not correct: expected \'/regressiontest\/$ctdisk'\ get \'$res[0]\'"; + warn "Test 19 d: path is not correct: expected \'\/$mountpoint\/$ctdisk'\ get \'$res[0]\'"; } if ($res[1] ne "202") { if (!$fail) { @@ -171,10 +172,10 @@ my $test19 = sub { $fail = 0; eval { @res = PVE::Storage::path($cfg, "$storagename:$ctbase"); - if ($res[0] ne "\/regressiontest\/$ctbase") { + if ($res[0] ne "\/$mountpoint\/$ctbase") { $count++; $fail = 1; - warn "Test 19 e: path is not correct: expected \'\/regressiontest\/$ctbase'\ get \'$res[0]\'"; + warn "Test 19 e: path is not correct: expected \'\/$mountpoint\/$ctbase'\ get \'$res[0]\'"; } if ($res[1] ne "200") { if (!$fail) { @@ -200,10 +201,10 @@ my $test19 = sub { $fail = 0; eval { @res = PVE::Storage::path($cfg, "$storagename:$ctbase\/$ctlinked"); - if ($res[0] ne "\/regressiontest\/$ctlinked") { + if ($res[0] ne "\/$mountpoint\/$ctlinked") { $count++; $fail = 1; - warn "Test 19 f: path is not correct: expected \'\/regressiontest\/$ctlinked'\ get \'$res[0]\'"; + warn "Test 19 f: path is not correct: expected \'\/$mountpoint\/$ctlinked'\ get \'$res[0]\'"; } if ($res[1] ne "201") { if (!$fail) { @@ -1188,11 +1189,11 @@ my $test7 = sub { eval { PVE::Storage::volume_snapshot($cfg, "$storagename:$ctdisk", 'snap1'); - run_command("touch \/$zpath\/$ctdisk\/test.txt", outfunc => $parse_guid); + run_command("touch \/$mountpoint\/$ctdisk\/test.txt", outfunc => $parse_guid); eval { PVE::Storage::volume_snapshot_rollback($cfg, "$storagename:$ctdisk", 'snap1'); eval { - run_command("ls \/$zpath\/$ctdisk\/test.txt", errofunc => sub {}); + run_command("ls \/$mountpoint\/$ctdisk\/test.txt", errofunc => sub {}); }; if (!$@) { $count++; @@ -1212,11 +1213,11 @@ my $test7 = sub { eval { PVE::Storage::volume_snapshot($cfg, "$storagename:$ctbase", 'snap1'); - run_command("touch \/$zpath\/$ctbase\/test.txt", outfunc => $parse_guid); + run_command("touch \/$mountpoint\/$ctbase\/test.txt", outfunc => $parse_guid); eval { PVE::Storage::volume_snapshot_rollback($cfg, "$storagename:$ctbase", 'snap1'); eval { - run_command("ls \/$zpath\/$ctbase\/test.txt", errofunc => sub {}); + run_command("ls \/$mountpoint\/$ctbase\/test.txt", errofunc => sub {}); }; if (!$@) { $count++; @@ -1236,7 +1237,7 @@ my $test7 = sub { eval { PVE::Storage::volume_snapshot($cfg, "$storagename:$ctbase/$ctlinked", 'snap1'); - run_command("touch \/$zpath\/$ctlinked\/test.txt", outfunc => $parse_guid); + run_command("touch \/$mountpoint\/$ctlinked\/test.txt", outfunc => $parse_guid); eval { PVE::Storage::volume_snapshot_rollback($cfg, "$storagename:$ctbase/$ctlinked", 'snap1'); eval { @@ -2650,7 +2651,7 @@ sub setup_zpool { } my $pwd = cwd(); eval { - run_command("zpool create $subvol $pwd\/zpool.img"); + run_command("zpool create -m \/$mountpoint $subvol $pwd\/zpool.img"); }; if ($@) { clean_up_zpool(); @@ -2698,6 +2699,7 @@ $cfg = {'ids' => { 'rootdir' => 1 }, 'pool' => $subvol, + 'mountpoint' => "\/$mountpoint", 'type' => 'zfspool' }, },