[pve-devel] [PATCH storage 2/2] fix #2085: Handle non-default mount point in path() using storage property 'path' for mount point

Thomas Lamprecht t.lamprecht at proxmox.com
Thu Nov 14 12:57:36 CET 2019


On 11/14/19 11:33 AM, Fabian Ebner wrote:
> 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().

but those other storage plugins are all Filesystem based, while we do not
use ZFS as file system based, but with volumes.
So using 'path' as a property name allows then all the FS based functions
to happen, as it's often used as a check in base implementation for if the
storage is filesystem based or block based. Not sure if this is a good idea,
one would need to probably take a closer look. (Thanks @Dietmar for hinting me)

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

The onboot thing would be nice, but having it for new added storages
automatically and the possibility to change it manually for existing
one is a good starter.





More information about the pve-devel mailing list