On 11/7/19 9:34 AM, Fabian Grünbichler wrote:
On November 6, 2019 1:46 pm, Fabian Ebner wrote:
A new mountpoint property is added to the schema for ZFSPool storages.
When needed for the first time, the current mount point is determined and
written to the storage config.

Signed-off-by: Fabian Ebner <f.eb...@proxmox.com>
---
  PVE/Storage/ZFSPoolPlugin.pm | 25 +++++++++++++++++++++++--
  1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index 16fb0d6..44c8123 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 },
      };
  }
@@ -148,11 +153,27 @@ sub path {
      my ($vtype, $name, $vmid) = $class->parse_volname($volname);
my $path = '';
+    my $mountpoint = $scfg->{mountpoint};
+
+    # automatically write mountpoint to storage config if it is not present
+    if (!$mountpoint) {
+       $mountpoint = $class->zfs_request($scfg, undef, 'get', '-o', 'value',
+                                         'mountpoint', '-H', $scfg->{pool});
+       chomp($mountpoint);
+
+       eval {
+           PVE::Storage::lock_storage_config(sub {
+               my $cfg = PVE::Storage::config();
+               $cfg->{ids}->{$storeid}->{mountpoint} = $mountpoint;
+               PVE::Storage::write_config($cfg);
+           }, "update storage config failed");
+       };
+       warn $@ if $@;
+    }
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";
        }

it would be great if this "get mountpoint property of dataset" (or better:
even "get arbitrary properties of dataset") helper could be factored out.
we already have two calls to "zfs get -Hp '...' $dataset" that could
re-use such a helper.

then we could add a single check in activate_storage (maybe just for the
branch where we actually just imported the pool?) to verify that the
stored mountpoint value is correct, and if not update it (or warn about
the problem). that check could have a very low timeout, and errors could
be ignored since it is just a double-check.

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


I noticed that the $scfg parameter in zfs_request is never used. Should I clean that up? Otherwise I'd need to include the $scfg parameter in the helper function as well, where it also isn't needed.

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

Reply via email to