On November 7, 2019 12:52 pm, Fabian Ebner wrote:
> 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.
it's used by ZFS over iSCSI, where the plugin is derived from
ZFSPoolPlugin, so it's not safe to remove ;)
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
