[pve-devel] [PATCH storage 2/2] fix #2085: Handle non-default mount point in path() using storage property 'path' for mount point
Fabian Ebner
f.ebner at proxmox.com
Thu Nov 14 11:33:49 CET 2019
Since other storage plugins use a property called 'path' for the mount point,
it was used here as well; not to be confused with the subroutine path().
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 <f.ebner at proxmox.com>
---
Changes from previous versions:
* do the handling in the on_add_hook instead of path()
* change the property name from mountpoint to path
* modified the pool used by the tests to use a non-standard mount point
I decided against adding the check to the import branch of activate storage as
well, since the warnings there would often not be visible to the user.
We could still add something later that checks if the mount points in
storage.cfg are correct on bootup or initial mount as Thomas suggested.
PVE/Storage/ZFSPoolPlugin.pm | 26 ++++++++++++++++++++++++--
test/run_test_zfspoolplugin.pl | 26 ++++++++++++++------------
2 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index b8adf1c..b64589f 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -44,6 +44,7 @@ sub options {
disable => { optional => 1 },
content => { optional => 1 },
bwlimit => { optional => 1 },
+ path => { optional => 1 },
};
}
@@ -142,17 +143,38 @@ sub parse_volname {
# virtual zfs methods (subclass can overwrite them)
+sub on_add_hook {
+ my ($class, $storeid, $scfg, %param) = @_;
+
+ my $cfg_mountpoint = $scfg->{path};
+ 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('pve-storage-path', $mountpoint);
+ };
+
+ if (defined($cfg_mountpoint)) {
+ if (defined($mountpoint) && !($cfg_mountpoint =~ m|^\Q$mountpoint\E/?$|)) {
+ warn "warning for $storeid - path: $cfg_mountpoint does not match current mount point: $mountpoint\n";
+ }
+ } else {
+ $scfg->{path} = $mountpoint;
+ }
+}
+
sub path {
my ($class, $scfg, $volname, $storeid, $snapname) = @_;
my ($vtype, $name, $vmid) = $class->parse_volname($volname);
my $path = '';
+ my $mountpoint = $scfg->{path} // "/$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..2deb848 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,
+ 'path' => "\/$mountpoint",
'type' => 'zfspool'
},
},
--
2.20.1
More information about the pve-devel
mailing list