[pve-devel] [Patch container] fix #1778: check storage support templates

[Thomas Lamprecht]

Looks OK in general! A few style/typo nits still inline

On 6/6/18 12:00 PM, Wolfgang Link wrote:
> LXC can only create templates on storages which support linked clones.
> To prevent this, we will check before we convert to a template if the
> storage support this.
> ---
>  src/PVE/API2/LXC.pm | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index f555b84..896e8dc 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -1166,6 +1166,16 @@ __PACKAGE__->register_method({
>  	    die "you can't convert a CT to template if the CT is running\n"
>  		if PVE::LXC::check_running($vmid);
>  
> +	    my $cfg = PVE::Storage::config();

we normally use $scfg for the storage config, would be nice to keep that
consistency, especially as we have already $conf for the CT config in
this scope.

> +	    PVE::LXC::Config->foreach_mountpoint($conf, sub {
> +		my ($ms, $mountpoint) = @_;
> +
> +		my ($sid) =
> +		    PVE::Storage::parse_volume_id($mountpoint->{volume}, 0);

I'd put above in a single line, even if we go a few characters over 80,
optionally you could use $mp instead of $mountpoint if you dislike the
longer line.

> +		die "Storage \'$sid\' do not support templates!\n"

s/do not/does not/
also escaping the single quotes isn't needed as the string is in double
quotes.
If it was just one of those three little "issues" I would just fix it up,
but here it's maybe nicer if you could send a v2. :)

> +		    if $cfg->{ids}->{$sid}->{path};
> +	    });
> +
>  	    my $realcmd = sub {
>  		PVE::LXC::template_create($vmid, $conf);
>  
>