[pve-devel] applied: [PATCH v2 container] fix: #1075: Correctly restore CT templates form backup

[Thomas Lamprecht]

Am 4/17/19 um 4:38 PM schrieb Christian Ebner:
> Restoring a backup from a CT template wrongly resulted in a CT with the template
> flag set in the config.
> This makes sure the CT template backup gets restored to a CT and only if the
> storage supports templates, the resulting CT is converted to a template.
> Otherwise the backup restores simply to a CT.

applied this one already, less concerns here as the CT logic was broken
anyway, i.e., it wrongly tried to filter out the template flag from the
config but failed to do, so it seemed like it "successfully" restored a
template, but that wasn't actually the case...
So we can make things not really worse with this here, IMO :) Thanks!

> 
> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
> ---
> 
> Version 2:
>     * Minor changes to improve code readability as suggested
>     * Refactor check if storage supports templates
>     * Omit unneeded call of PVE::LXC::update_lxc_config as the config does not
>       exist after a restore anyway
> 
>  src/PVE/API2/LXC.pm   | 40 ++++++++++++++++++++++++++++++++--------
>  src/PVE/LXC/Create.pm |  4 +++-
>  2 files changed, 35 insertions(+), 9 deletions(-)
> 
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 5a8a9c9..cf14d75 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -335,6 +335,7 @@ __PACKAGE__->register_method({
>  
>  	my $code = sub {
>  	    my $old_conf = PVE::LXC::Config->load_config($vmid);
> +	    my $was_template;
>  
>  	    my $vollist = [];
>  	    eval {
> @@ -344,6 +345,7 @@ __PACKAGE__->register_method({
>  		    if ($is_root && $archive ne '-') {
>  			my $orig_conf;
>  			($orig_conf, $orig_mp_param) = PVE::LXC::Create::recover_config($archive);
> +			$was_template = delete $orig_conf->{template};
>  			# When we're root call 'restore_configuration' with ristricted=0,
>  			# causing it to restore the raw lxc entries, among which there may be
>  			# 'lxc.idmap' entries. We need to make sure that the extracted contents
> @@ -423,6 +425,17 @@ __PACKAGE__->register_method({
>  		foreach my $mp (keys %$delayed_mp_param) {
>  		    $conf->{$mp} = $delayed_mp_param->{$mp};
>  		}
> +		# If the template flag was set, we try to convert again to template after restore
> +		if ($was_template) {
> +		    print STDERR "Convert restored container to template...\n";
> +		    if (my $err = check_storage_supports_templates($conf)) {
> +			warn $err;
> +			warn "Leave restored backup as container instead of converting to template.\n"
> +		    } else {
> +			PVE::LXC::template_create($vmid, $conf);
> +			$conf->{template} = 1;
> +		    }
> +		}
>  		PVE::LXC::Config->write_config($vmid, $conf);
>  	    };
>  	    if (my $err = $@) {
> @@ -443,6 +456,22 @@ __PACKAGE__->register_method({
>  	return $rpcenv->fork_worker($workername, $vmid, $authuser, $realcmd);
>      }});
>  
> +sub check_storage_supports_templates {
> +    my ($conf) = @_;
> +
> +    my $scfg = PVE::Storage::config();
> +    eval {
> +	PVE::LXC::Config->foreach_mountpoint($conf, sub {
> +	    my ($ms, $mp) = @_;
> +
> +	    my ($sid) = PVE::Storage::parse_volume_id($mp->{volume}, 0);
> +	    die "Warning: Directory storage '$sid' does not support container templates!\n"
> +		if $scfg->{ids}->{$sid}->{path};
> +	});
> +    };
> +    return $@
> +}
> +
>  __PACKAGE__->register_method({
>      name => 'vmdiridx',
>      path => '{vmid}',
> @@ -1177,14 +1206,9 @@ __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 $scfg = PVE::Storage::config();
> -	    PVE::LXC::Config->foreach_mountpoint($conf, sub {
> -		my ($ms, $mp) = @_;
> -
> -		my ($sid) =PVE::Storage::parse_volume_id($mp->{volume}, 0);
> -		die "Directory storage '$sid' does not support container templates!\n"
> -		    if $scfg->{ids}->{$sid}->{path};
> -	    });
> +	    if (my $err = check_storage_supports_templates($conf)) {
> +		die $err;
> +	    }
>  
>  	    my $realcmd = sub {
>  		PVE::LXC::template_create($vmid, $conf);
> diff --git a/src/PVE/LXC/Create.pm b/src/PVE/LXC/Create.pm
> index c0ef1d7..ed79611 100644
> --- a/src/PVE/LXC/Create.pm
> +++ b/src/PVE/LXC/Create.pm
> @@ -139,7 +139,6 @@ sub recover_config {
>  	$conf = PVE::LXC::Config::parse_pct_config("/lxc/0.conf" , $raw);
>  
>  	delete $conf->{snapshots};
> -	delete $conf->{template}; # restored CT is never a template
>  
>  	PVE::LXC::Config->foreach_mountpoint($conf, sub {
>  	    my ($ms, $mountpoint) = @_;
> @@ -174,6 +173,9 @@ sub restore_configuration {
>  	    next if $key eq 'digest' || $key eq 'rootfs' || $key eq 'snapshots' || $key eq 'unprivileged' || $key eq 'parent';
>  	    next if $key =~ /^mp\d+$/; # don't recover mountpoints
>  	    next if $key =~ /^unused\d+$/; # don't recover unused disks
> +	    # we know if it was a template in the restore API call and check if the target
> +	    # storage supports creating a template there
> +	    next if $key =~ /^template$/;
>  	    if ($restricted && $key eq 'lxc') {
>  		warn "skipping custom lxc options, restore manually as root:\n";
>  		warn "--------------------------------\n";
>