On 4/16/19 5:37 PM, Christian Ebner wrote:
> 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.
a not-tested-just-looked-at review below ;-)
>
> Signed-off-by: Christian Ebner <c.eb...@proxmox.com>
> ---
> src/PVE/API2/LXC.pm | 28 ++++++++++++++++++++++++++++
> src/PVE/LXC/Create.pm | 2 +-
> 2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 5a8a9c9..42e11fb 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,10 @@ __PACKAGE__->register_method({
> if ($is_root && $archive ne '-') {
> my $orig_conf;
> ($orig_conf, $orig_mp_param) =
> PVE::LXC::Create::recover_config($archive);
> + if ($orig_conf->{template}) {
> + $was_template = $orig_conf->{template};
> + delete $orig_conf->{template};
> + }
whole block can be reduced to a single line, like:
$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
> @@ -424,6 +429,29 @@ __PACKAGE__->register_method({
> $conf->{$mp} = $delayed_mp_param->{$mp};
> }
> PVE::LXC::Config->write_config($vmid, $conf);
> +
> + # 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";
from here -
> + 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!\nLeave restored backup as container instead\n"
> + if $scfg->{ids}->{$sid}->{path};
> + });
> + };
do here it's a straight copy from the POST template call below, we could
either factor this out in a method which gets only the conf (scfg can be
obtained inside) or a module wide closure, as both calls are located here,
not sure what feels better, but I'd like to not have this twice here.
> + if (my $err = $@) {
> + warn $err;
> + } else {
hmm this is also a straight copy from the template create API call..
> + PVE::LXC::template_create($vmid, $conf);
> + $conf->{template} = 1;
> + PVE::LXC::Config->write_config($vmid, $conf);
this write could be removed if we move this all a bit up, as there a write is
already in place
> + PVE::LXC::update_lxc_config($vmid, $conf);
a comment for above could be nice, for templates it just removes the
local generated LXC config and early returns.
> + }
> + }
> };
> if (my $err = $@) {
> PVE::LXC::destroy_disks($storage_cfg, $vollist);
> diff --git a/src/PVE/LXC/Create.pm b/src/PVE/LXC/Create.pm
> index c0ef1d7..4b527ff 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,7 @@ 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
> + next if $key =~ /^template$/; # restored CT is never a template by
> default
maybe a comment more like:
# we know if it was a template in the restore API call and check if the target
# storage supports creating a template there
> if ($restricted && $key eq 'lxc') {
> warn "skipping custom lxc options, restore manually as root:\n";
> warn "--------------------------------\n";
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
