[pve-devel] applied: [PATCH container] Fix mounting ZFS snapshots whose dataset is not mounted below '/'

[Thomas Lamprecht]

On 2/18/20 12:31 PM, Fabian Ebner wrote:
> Trying to back up a container with a ZFS dataset with non-standard mount
> would fail, see [0].
> 
> This also removes the near-dead code
> $name .= "\@$snapname";
> when snapname is false-y, but defined and turns
> the check for $snapname into one for definedness.
> 
> [0]: https://forum.proxmox.com/threads/lxc-backup-fails-unable-to-open-the-dataset-vzdump.64944/
> 
> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
> ---
> 
> Hopefully there is nothing that relies on the old behavior
> with $snapname. Or was it intended to be able to reach
> the 'zfs set acl' call with $snapname set?
> 

Honestly, from top of my head I'm not to sure, but that would had been an
orthogonal issue to that what you're fixing here, so: applied, thanks!

Wolfgang, in commit 719129ea4ea10ce6677cfb2d92b1cec974ca649f you added the
acl set, but the snapshot handling there never triggered. Was this meant to
do in both cases and should be thus pulled out before of the "if snapshot"
if/else?

>  src/PVE/LXC.pm | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index 34ca2a3..21bc9c6 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -1647,19 +1647,17 @@ sub __mountpoint_mount {
>  
>  	if ($format eq 'subvol') {
>  	    if ($mount_path) {
> -		if ($snapname) {
> +		my (undef, $name) = PVE::Storage::parse_volname($storage_cfg, $volid);
> +		if (defined($snapname)) {
> +		    $name .= "\@$snapname";
>  		    if ($scfg->{type} eq 'zfspool') {
> -			my $path_arg = $path;
> -			$path_arg =~ s!^/+!!;
> -			PVE::Tools::run_command(['mount', '-o', 'ro', @extra_opts, '-t', 'zfs', $path_arg, $mount_path]);
> +			PVE::Tools::run_command(['mount', '-o', 'ro', @extra_opts, '-t', 'zfs', "$scfg->{pool}/$name", $mount_path]);
>  		    } else {
>  			die "cannot mount subvol snapshots for storage type '$scfg->{type}'\n";
>  		    }
>  		} else {
>  		    if (defined($acl) && $scfg->{type} eq 'zfspool') {
>  			my $acltype = ($acl ? 'acltype=posixacl' : 'acltype=noacl');
> -			my (undef, $name) = PVE::Storage::parse_volname($storage_cfg, $volid);
> -			$name .= "\@$snapname" if defined($snapname);
>  			PVE::Tools::run_command(['zfs', 'set', $acltype, "$scfg->{pool}/$name"]);
>  		    }
>  		    bindmount($path, $parentfd, $last_dir//$rootdir, $mount_path, $readonly, @extra_opts);
>