[pve-devel] [Patch container 1/2] Also rename MP when CT is converted to template.

[Thomas Lamprecht]

On 5/23/18 4:28 PM, Wolfgang Link wrote:
> Mountpoints should use the same names as rootfs when they converted to a template.

good catch!
but please try to keep commit message line length < 70 characters,
i.e., add a line break above, I'd suggest a commit message alike:

template create: also handle mountpoints besides rootfs

we only handled the special rootfs mount so creating a template
from a container with additional mountpoint did not work correctly.
Use foreach_mountpoint to create a base vdisk for all mount points
after checking if the storage supports it.

> So we also renamed MP.
> ---
>  src/PVE/LXC.pm | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index 4f9674d..928234a 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -869,17 +869,32 @@ sub template_create {
>  
>      my $storecfg = PVE::Storage::config();
>  
> -    my $rootinfo = PVE::LXC::Config->parse_ct_rootfs($conf->{rootfs});
> -    my $volid = $rootinfo->{volume};
> +    PVE::LXC::Config->foreach_mountpoint($conf, sub {
> +	my ($ms, $mountpoint) = @_;
>  
> -    die "Template feature is not available for '$volid'\n"
> -	if !PVE::Storage::volume_has_feature($storecfg, 'template', $volid);
> +	my $volume = $ms eq "rootfs" ?
> +	    PVE::LXC::Config->parse_ct_rootfs($conf->{$ms}) :
> +	    PVE::LXC::Config->parse_ct_mountpoint($conf->{$ms});
> +	my $volid = $volume->{volume};

a real if/else would be easier to read here, IMO.
*But* you actually also do not need that one here, if you look at
"PVE::LXC::Config->foreach_mountpoint" (or better it's real
implementation, foreach_mountpoint_full) you'll see that the second
parameter $mountpoint is already the correctly parsed one, so you can
just use:

my $volid = $mountpoint->{volume};

>  
> -    PVE::Storage::activate_volumes($storecfg, [$volid]);
> +	die "Template feature is not available for '$volid'\n"
> +	    if !PVE::Storage::volume_has_feature($storecfg, 'template', $volid);
> +    });
>  
> -    my $template_volid = PVE::Storage::vdisk_create_base($storecfg, $volid);
> -    $rootinfo->{volume} = $template_volid;
> -    $conf->{rootfs} = PVE::LXC::Config->print_ct_mountpoint($rootinfo, 1);
> +    PVE::LXC::Config->foreach_mountpoint($conf, sub {
> +	my ($ms, $mountpoint) = @_;
> +
> +	my $volume = $ms eq "rootfs" ?
> +	    PVE::LXC::Config->parse_ct_rootfs($conf->{$ms}) :
> +	    PVE::LXC::Config->parse_ct_mountpoint($conf->{$ms});

same as above

> +	my $volid = $volume->{volume};
> +
> +	PVE::Storage::activate_volumes($storecfg, [$volid]);
> +
> +	my $template_volid = PVE::Storage::vdisk_create_base($storecfg, $volid);
> +	$volume->{volume} = $template_volid;
> +	$conf->{$ms} = PVE::LXC::Config->print_ct_mountpoint($volume, $ms eq "rootfs");
> +    });
>  
>      PVE::LXC::Config->write_config($vmid, $conf);
>  }
>