Re: [pve-devel] [PATCH pve-container 1/1] Adding new mount point type named 'zfs' to let configure a ZFS dataset as mount point for LXC container

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

> As we know, ProxMox have only three possible "categories" of mount points: ProxMox storage provider supplied, block device and bind mount. I've prepared a little patch for pve-container package which adds a fourth "category" named "zfs"  - so with this patch it's possible to add such ZFS dataset into container config in a form "mpN: <ZFS pool name>/<dataset path>,<mount path>". This new type can be useful in some cases - for instance when we need to mount ZFS dataset in the container but need to keep this dataset not mounted on the host.

nit: for single patches, there is no need to add a coverletter. also, please include relevant information in the commit message!

introducing a new mountpoint type is definitely not the right approach. could you give a reason why you want to hide the container contents from the host?

this could be implemented in pve-container (e.g., by mounting the ZFS dataset corresponding to a PVE-managed volume like we mount block devices or raw images, instead of relying on the fact that they are already mounted and bind mounting them.. we already do the same for ZFS snapshots in __mountpoint_mount, for example) and in pve-storage (e.g., by having a flag there that controls mounting, or skipping mounting if mountpoint=none or legacy) without the need for any other special handling. careful checks to see whether we rely on ZFS-backed mountpoints already being mounted anywhere else would still be needed (move volume might be one place, for example).

> Konstantin Filippov via pve-devel <pve-devel@lists.proxmox.com> hat am 10.05.2023 02:08 CEST geschrieben:
> Signed-off-by: Konstantin Filippov <frank030366@hotmail.com>
> ---
>  src/PVE/LXC.pm        | 4 ++++
>  src/PVE/LXC/Config.pm | 3 ++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index d138161..30cf48d 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -1839,6 +1839,10 @@ sub __mountpoint_mount {
>  	my ($devpath) = (Cwd::realpath($volid) =~ /^(.*)$/s); # realpath() taints
>  	PVE::Tools::run_command(['mount', @extra_opts, $volid, $mount_path]) if $mount_path;
>  	return wantarray ? ($volid, 0, $devpath) : $volid;
> +    } elsif ($type eq 'zfs') {
> +	push @extra_opts, '-o', 'ro' if $readonly;
> +	PVE::Tools::run_command(['mount.zfs', @extra_opts, $volid, $mount_path]) if $mount_path;
> +	return wantarray ? ($volid, 0, undef) : $volid
>      } elsif ($type eq 'bind') {
>  	die "directory '$volid' does not exist\n" if ! -d $volid;
>  	bindmount($volid, $parentfd, $last_dir//$rootdir, $mount_path, $readonly, @extra_opts) if $mount_path;
> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
> index ac9db94..056ec98 100644
> --- a/src/PVE/LXC/Config.pm
> +++ b/src/PVE/LXC/Config.pm
> @@ -1557,7 +1557,8 @@ sub classify_mountpoint {
>  	return 'device' if $vol =~ m!^/dev/!;
>  	return 'bind';
>      }
> -    return 'volume';
> +    return 'volume' if $vol =~ m!:.*(vm|subvol)-[0-9]*-disk-[0-9]*!;
> +    return 'zfs';
>  }
>  
>  my $__is_volume_in_use = sub {
> -- 
> 2.30.2