From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <d.tschlatscher@proxmox.com>
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 87E7A71D7C
for <pve-devel@lists.proxmox.com>; Fri, 20 May 2022 15:29:19 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 860331BA19
for <pve-devel@lists.proxmox.com>; Fri, 20 May 2022 15:29:19 +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 id BA50D1B9FF
for <pve-devel@lists.proxmox.com>; Fri, 20 May 2022 15:29:17 +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 928D743735
for <pve-devel@lists.proxmox.com>; Fri, 20 May 2022 15:29:17 +0200 (CEST)
From: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Fri, 20 May 2022 15:28:56 +0200
Message-Id: <20220520132859.144041-1-d.tschlatscher@proxmox.com>
X-Mailer: git-send-email 2.30.2
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.082 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
T_SCC_BODY_TEXT_LINE -0.01 -
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [storage.pm, content.pm]
Subject: [pve-devel] [PATCH storage v3 1/4] fix #3972: Remove the .notes
file when a backup is deleted
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Fri, 20 May 2022 13:29:19 -0000
When a VM or Container backup was deleted, the .notes file was not
removed, therefore, over time the dump folder would get polluted with
notes for backups that no longer existed. As backup names contain a
timestamp and as the notes cannot be reused because of this, I think
it is safe to just delete them just like we do with the .log file.
Furthermore, I sourced the deletion of the log and notes file into a
new function called "archive_auxiliaries_remove". Additionally, the
archive_info object now returns one more field containing the name of
the notes file. The test cases have to be adapted to expect this new
value as the package will not compile otherwise.
Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
PVE/API2/Storage/Content.pm | 5 ++---
PVE/Storage.pm | 20 +++++++++++++++-----
test/archive_info_test.pm | 6 ++++++
3 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/PVE/API2/Storage/Content.pm b/PVE/API2/Storage/Content.pm
index 8ff858d..41b577c 100644
--- a/PVE/API2/Storage/Content.pm
+++ b/PVE/API2/Storage/Content.pm
@@ -456,9 +456,8 @@ __PACKAGE__->register_method ({
print "Removed volume '$volid'\n";
if ($vtype eq 'backup'
&& $path =~ /(.*\/vzdump-\w+-\d+-\d{4}_\d{2}_\d{2}-\d{2}_\d{2}_\d{2})[^\/]+$/) {
- my $logpath = "$1.log";
- # try to cleanup our backup log file too, if still existing, #318
- unlink($logpath) if -e $logpath;
+ # Remove log file #318 and notes file #3972 if they still exist
+ PVE::Storage::archive_auxiliaries_remove($path);
}
};
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 72458cf..93c3b4e 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -1566,6 +1566,7 @@ sub archive_info {
if ($volid =~ /^(vzdump-${type}-([1-9][0-9]{2,8})-(\d{4})_(\d{2})_(\d{2})-(\d{2})_(\d{2})_(\d{2}))\.${extension}$/) {
$info->{logfilename} = "$1.log";
+ $info->{notesfilename} = "$filename.notes";
$info->{vmid} = int($2);
$info->{ctime} = timelocal($8, $7, $6, $5, $4 - 1, $3);
$info->{is_std_name} = 1;
@@ -1585,16 +1586,25 @@ sub archive_remove {
die "cannot remove protected archive '$archive_path'\n"
if -e protection_file_path($archive_path);
+ unlink $archive_path or die "removing archive $archive_path failed: $!\n";
+
+ archive_auxiliaries_remove($archive_path);
+}
+
+sub archive_auxiliaries_remove {
+ my ($archive_path) = @_;
+
my $dirname = dirname($archive_path);
my $archive_info = eval { archive_info($archive_path) } // {};
my $logfn = $archive_info->{logfilename};
+ my $notesfn = $archive_info->{notesfilename};
- unlink $archive_path or die "removing archive $archive_path failed: $!\n";
+ for my $type (qw(log notes)) {
+ my $filename = $archive_info->{"${type}filename"} or next;
+ my $path = "$dirname/$filename";
- if (defined($logfn)) {
- my $logpath = "$dirname/$logfn";
- if (-e $logpath) {
- unlink $logpath or warn "removing log file $logpath failed: $!\n";
+ if (-e $path) {
+ unlink $path or warn "Removing $type file failed: $!\n";
}
}
}
diff --git a/test/archive_info_test.pm b/test/archive_info_test.pm
index 7e84b6a..bbc0e6f 100644
--- a/test/archive_info_test.pm
+++ b/test/archive_info_test.pm
@@ -24,6 +24,7 @@ my $tests = [
expected => {
'filename' => "vzdump-lxc-$vmid-3070_01_01-00_00_00.tgz",
'logfilename' => "vzdump-lxc-$vmid-3070_01_01-00_00_00.log",
+ 'notesfilename'=> "vzdump-lxc-$vmid-3070_01_01-00_00_00.tgz.notes",
'type' => 'lxc',
'format' => 'tar',
'decompressor' => ['tar', '-z'],
@@ -39,6 +40,7 @@ my $tests = [
expected => {
'filename' => "vzdump-lxc-$vmid-1970_01_01-02_00_30.tgz",
'logfilename' => "vzdump-lxc-$vmid-1970_01_01-02_00_30.log",
+ 'notesfilename'=> "vzdump-lxc-$vmid-1970_01_01-02_00_30.tgz.notes",
'type' => 'lxc',
'format' => 'tar',
'decompressor' => ['tar', '-z'],
@@ -54,6 +56,7 @@ my $tests = [
expected => {
'filename' => "vzdump-lxc-$vmid-2020_03_30-21_39_30.tgz",
'logfilename' => "vzdump-lxc-$vmid-2020_03_30-21_39_30.log",
+ 'notesfilename'=> "vzdump-lxc-$vmid-2020_03_30-21_39_30.tgz.notes",
'type' => 'lxc',
'format' => 'tar',
'decompressor' => ['tar', '-z'],
@@ -69,6 +72,7 @@ my $tests = [
expected => {
'filename' => "vzdump-openvz-$vmid-2020_03_30-21_39_30.tgz",
'logfilename' => "vzdump-openvz-$vmid-2020_03_30-21_39_30.log",
+ 'notesfilename'=> "vzdump-openvz-$vmid-2020_03_30-21_39_30.tgz.notes",
'type' => 'openvz',
'format' => 'tar',
'decompressor' => ['tar', '-z'],
@@ -84,6 +88,7 @@ my $tests = [
expected => {
'filename' => "vzdump-qemu-$vmid-2020_03_30-21_39_30.tgz",
'logfilename' => "vzdump-qemu-$vmid-2020_03_30-21_39_30.log",
+ 'notesfilename'=> "vzdump-qemu-$vmid-2020_03_30-21_39_30.tgz.notes",
'type' => 'qemu',
'format' => 'tar',
'decompressor' => ['tar', '-z'],
@@ -138,6 +143,7 @@ for my $virt (sort keys %$bkp_suffix) {
expected => {
'filename' => "vzdump-$virt-$vmid-2020_03_30-21_12_40.$format.$suffix",
'logfilename' => "vzdump-$virt-$vmid-2020_03_30-21_12_40.log",
+ 'notesfilename'=> "vzdump-$virt-$vmid-2020_03_30-21_12_40.$format.$suffix.notes",
'type' => "$virt",
'format' => "$format",
'decompressor' => $decomp->{$suffix},
--
2.30.2