[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

[Konstantin]

Hello,

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

I'm new here, so sorry - will follow rules in future.

/>//could you give a reason why you want to hide the container contents 
from the host?/

I'll try to explain my points. I'm using Proxmox as a base for my home 
NAS in addition with possibility to setup some test environments to play 
around. So one LXC container is playing NAS role - it has all required 
software installed (samba/ftp/etc) and have a big volume mounted for 
data storage (8-10TB). If it will be created and configured as proxmox 
builtin storage volume (using ZFS storage provider) I have at least 3 
points which I'm not comfortable with:

- this big dataset will be mounted to PVE host and will be visible and 
accessible from host so every (for example) file search operation will 
be affected by this dataset. I would like to narrow any such file 
operation only to host related stuff, not to my NAS data;

- in addition while operating on host I have a probability to 
accidentally affect or destroy my NAS data so I'd like to avoid this 
possibility anyway;

- simple "pct destroy" command will destroy all proxmox storage provided 
mount points as well. I'd like to avoid such possibilty anyway.

As I see in pve-container code - only bind mount and block device mount 
can be used as non-proxmox volume. But bind mount isn't acceptable for 
me according to points above. ZFS dataset isn't a block device - so it 
cannot be mounted using standard notation in LXC config. That's why I'm 
proposing this patch - it adds the capality to use ZFS filesystem as 
mount point for LXC container. With this functionality I can just add 
the following line (or configure with pct) to LXC container config:

mp1: tank/nas-data,mp=/data

And after that ZFS dataset "tank/nas-data" will be mounted inside 
container and will not be exposed to host (of course mountpoint=legacy 
should be set for this dataset). Maybe other more elegant ways possible 
to implement this but this the only way I've found.

Best regards,

Konstantin


On 11.05.2023 11:27, Fabian Grünbichler wrote:
>> 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 at lists.proxmox.com>  hat am 10.05.2023 02:08 CEST geschrieben:
>> Signed-off-by: Konstantin Filippov<frank030366 at 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