From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id BA9D29C0CD for ; Mon, 23 Oct 2023 13:19:26 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8DD3C12AC9 for ; Mon, 23 Oct 2023 13:18:56 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 23 Oct 2023 13:18:55 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 3A3BB4458F for ; Mon, 23 Oct 2023 13:18:55 +0200 (CEST) From: Christian Ebner To: pve-devel@lists.proxmox.com Date: Mon, 23 Oct 2023 13:18:33 +0200 Message-Id: <20231023111835.238407-2-c.ebner@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20231023111835.238407-1-c.ebner@proxmox.com> References: <20231023111835.238407-1-c.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.026 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment PROLO_LEO2 0.1 Meta Catches all Leo drug variations so far SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [RFC v2 pve-container pve-manager 1/3] backup: do not delete not backed-up mps on restore X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 23 Oct 2023 11:19:26 -0000 The current behaviour of the restore is to recreate all backed up mountpoints and remove all previous ones, causing potential data loss on restore when the mountpoint was not included in the backup and the user not aware of this behaviour. By checking the mountpoint configuration from the backup, only recreate the disks which are included in the backup and add them as mountpoints. Leave all other mountpoints untouched and attach them as unused disks as final step of the restore. To facilitate selective restore from PBS backups, split the currently single root pxar archive into a pxar archive for each individual mountpoint, while remaining backwards compatible. Signed-off-by: Christian Ebner --- changes since v1: - deep clone of orig_mp_param, otherwise the variable points to the same data. - refactor restore_archive params src/PVE/API2/LXC.pm | 34 +++++++++++++++----- src/PVE/LXC/Create.pm | 72 +++++++++++++++++++++++++++++++++++-------- src/PVE/VZDump/LXC.pm | 10 +++--- 3 files changed, 92 insertions(+), 24 deletions(-) diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm index 28d14de..090dddf 100644 --- a/src/PVE/API2/LXC.pm +++ b/src/PVE/API2/LXC.pm @@ -4,6 +4,7 @@ use strict; use warnings; use Socket qw(SOCK_STREAM); +use Storable qw(dclone); use PVE::SafeSyslog; use PVE::Tools qw(extract_param run_command); @@ -381,8 +382,10 @@ __PACKAGE__->register_method({ my $vollist = []; eval { my $orig_mp_param; # only used if $restore + my $clear_mps; if ($restore) { die "can't overwrite running container\n" if PVE::LXC::check_running($vmid); + if ($archive ne '-') { my $orig_conf; print "recovering backed-up configuration from '$archive'\n"; @@ -419,11 +422,19 @@ __PACKAGE__->register_method({ print "recovering backed-up configuration from '$archive'\n"; (undef, $orig_mp_param) = PVE::LXC::Create::recover_config($storage_cfg, $archive, $vmid); } - $mp_param = $orig_mp_param; + $mp_param = dclone $orig_mp_param; die "rootfs configuration could not be recovered, please check and specify manually!\n" if !defined($mp_param->{rootfs}); PVE::LXC::Config->foreach_volume($mp_param, sub { my ($ms, $mountpoint) = @_; + if ($ms eq 'rootfs' || $mountpoint->{backup}) { + # backup conf contains the mp, clear for retsore + $clear_mps->{$ms} = $mountpoint; + } else { + # do not add as mp, will be attach as unused at the end + delete $mp_param->{$ms}; + return; + } my $type = $mountpoint->{type}; if ($type eq 'volume') { die "unable to detect disk size - please specify $ms (size)\n" @@ -459,18 +470,24 @@ __PACKAGE__->register_method({ $vollist = PVE::LXC::create_disks($storage_cfg, $vmid, $mp_param, $conf); - # we always have the 'create' lock so check for more than 1 entry - if (scalar(keys %$old_conf) > 1) { - # destroy old container volumes - PVE::LXC::destroy_lxc_container($storage_cfg, $vmid, $old_conf, { lock => 'create' }); - } + # Delete old mountpoints which are restored from backup. + PVE::LXC::Config->foreach_volume($old_conf, sub { + my ($name, $mountpoint, undef) = @_; + return if !defined($clear_mps->{$name}); + PVE::LXC::delete_mountpoint_volume($storage_cfg, $vmid, $mountpoint->{volume}); + }); eval { my $rootdir = PVE::LXC::mount_all($vmid, $storage_cfg, $conf, 1); $bwlimit = PVE::Storage::get_bandwidth_limit('restore', [keys %used_storages], $bwlimit); print "restoring '$archive' now..\n" if $restore && $archive ne '-'; - PVE::LXC::Create::restore_archive($storage_cfg, $archive, $rootdir, $conf, $ignore_unpack_errors, $bwlimit); + + my $restore_opts = { + 'orig_mps' => $orig_mp_param, + }; + PVE::LXC::Create::restore_archive( + $storage_cfg, $archive, $rootdir, $conf, $ignore_unpack_errors, $bwlimit, $restore_opts); if ($restore) { print "merging backed-up and given configuration..\n"; @@ -501,6 +518,9 @@ __PACKAGE__->register_method({ $conf->{template} = 1; } PVE::LXC::Config->write_config($vmid, $conf); + + # Attach all additionally found mountpoints as unused disks. + PVE::LXC::rescan($vmid, 1, 0); }; 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 f4c3220..98ab4a4 100644 --- a/src/PVE/LXC/Create.pm +++ b/src/PVE/LXC/Create.pm @@ -83,27 +83,33 @@ sub detect_architecture { } sub restore_archive { - my ($storage_cfg, $archive, $rootdir, $conf, $no_unpack_error, $bwlimit) = @_; + my ($storage_cfg, $archive, $rootdir, $conf, $no_unpack_error, $bwlimit, $restore_opts) = @_; + + PVE::LXC::Config->foreach_volume($restore_opts->{orig_mps}, sub { + my ($name, $vol, undef) = @_; + $restore_opts->{orig_mps}->{$name} = $vol; + }); my ($storeid, $volname) = PVE::Storage::parse_volume_id($archive, 1); if (defined($storeid)) { my $scfg = PVE::Storage::storage_check_enabled($storage_cfg, $storeid); if ($scfg->{type} eq 'pbs') { - return restore_proxmox_backup_archive($storage_cfg, $archive, $rootdir, $conf, $no_unpack_error, $bwlimit); + return restore_proxmox_backup_archive( + $storage_cfg, $archive, $rootdir, $conf, $no_unpack_error, $bwlimit, $restore_opts); } } $archive = PVE::Storage::abs_filesystem_path($storage_cfg, $archive) if $archive ne '-'; - restore_tar_archive($archive, $rootdir, $conf, $no_unpack_error, $bwlimit); + restore_tar_archive($archive, $rootdir, $conf, $no_unpack_error, $bwlimit, $restore_opts); } sub restore_proxmox_backup_archive { - my ($storage_cfg, $archive, $rootdir, $conf, $no_unpack_error, $bwlimit) = @_; + my ($storage_cfg, $archive, $rootdir, $conf, $no_unpack_error, $bwlimit, $restore_opts) = @_; my ($storeid, $volname) = PVE::Storage::parse_volume_id($archive); my $scfg = PVE::Storage::storage_config($storage_cfg, $storeid); - my ($vtype, $name, undef, undef, undef, undef, $format) = + my ($vtype, $snapshot, undef, undef, undef, undef, $format) = PVE::Storage::parse_volname($storage_cfg, $archive); die "got unexpected vtype '$vtype'\n" if $vtype ne 'backup'; @@ -114,14 +120,47 @@ sub restore_proxmox_backup_archive { my $userns_cmd = PVE::LXC::userns_command($id_map); my $cmd = "restore"; - my $param = [$name, "root.pxar", $rootdir, '--allow-existing-dirs']; + PVE::LXC::Config->foreach_volume($conf, sub { + my ($name, $vol, undef) = @_; - if ($no_unpack_error) { - push(@$param, '--ignore-extract-device-errors'); - } + my $orig_mp = $restore_opts->{orig_mps}->{$name}; + if (!defined($orig_mp)) { + print "'$name': mp not present in original backup.\n"; + return; + } - PVE::Storage::PBSPlugin::run_raw_client_cmd( - $scfg, $storeid, $cmd, $param, userns_cmd => $userns_cmd); + if ($name ne 'rootfs' && (!defined($orig_mp->{backup}) || !$orig_mp->{backup})) { + print "'$name': mp not included in original backup.\n"; + return; + } + + $name = 'root' if $name eq 'rootfs'; + my $target = "$rootdir/.$vol->{mp}"; + if ($orig_mp->{mp} ne $vol->{mp}) { + print "'$name': path differs from backed-up path, restore to backed-up path.\n"; + print " If this is not intended, change the path after restore.\n"; + $target = "$rootdir/.$orig_mp->{mp}"; + } + + my $param = [$snapshot, "$name.pxar", $target, '--allow-existing-dirs']; + + if ($no_unpack_error) { + push(@$param, '--ignore-extract-device-errors'); + } + + eval { + # This will fail for backups created without mp archive splitting + PVE::Storage::PBSPlugin::run_raw_client_cmd( + $scfg, $storeid, $cmd, $param, userns_cmd => $userns_cmd); + }; + my $err = $@; + if ($err) { + # Only handle root restore failure as hard error + die $err if $name eq 'root'; + print "extracting moutpoint '$name' failed:\n$err"; + print "backup created with older version?\n"; + } + }); # if arch is set, we do not try to autodetect it return if defined($conf->{arch}); @@ -130,11 +169,20 @@ sub restore_proxmox_backup_archive { } sub restore_tar_archive { - my ($archive, $rootdir, $conf, $no_unpack_error, $bwlimit) = @_; + my ($archive, $rootdir, $conf, $no_unpack_error, $bwlimit, $restore_opts) = @_; my ($id_map, $rootuid, $rootgid) = PVE::LXC::parse_id_maps($conf); my $userns_cmd = PVE::LXC::userns_command($id_map); + PVE::LXC::Config->foreach_volume($conf, sub { + my ($name, $vol, undef) = @_; + my $orig_mp = $restore_opts->{orig_mps}->{$name}; + if (defined($orig_mp->{mp}) && $orig_mp->{mp} ne $vol->{mp}) { + print "'$name': path differs from backed-up path, restore to backed-up path.\n"; + print " If this is not intended, change the path after restore.\n"; + } + }); + my $archive_fh; my $tar_input = '<&STDIN'; my @compression_opt; diff --git a/src/PVE/VZDump/LXC.pm b/src/PVE/VZDump/LXC.pm index c68a06f..0d411b8 100644 --- a/src/PVE/VZDump/LXC.pm +++ b/src/PVE/VZDump/LXC.pm @@ -381,11 +381,11 @@ sub archive { push @$param, "fw.conf:$fw_conf"; } - my $rootdir = $snapdir; - push @$param, "root.pxar:$rootdir"; - - foreach my $disk (@sources) { - push @$param, '--include-dev', "$snapdir/$disk"; + foreach my $disk (@$disks) { + my $name = $disk->{name}; + # Needed for backwards compatibility with previous backups + $name = 'root' if $name eq 'rootfs'; + push @$param, "$name.pxar:$snapdir/.$disk->{mp}"; } push @$param, '--skip-lost-and-found' if $userns_cmd; -- 2.39.2