From c3e2ff806f1f558996e7654e502ea332227c13b0 Mon Sep 17 00:00:00 2001 From: Daniel Tschlatscher Date: Tue, 14 Jun 2022 11:00:10 +0200 Subject: [PATCH] fix #3972: Remove the .notes file when a backup is deleted 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 Reviewed-by: Fabian Ebner Signed-off-by: Wolfgang Bumiller --- PVE/API2/Storage/Content.pm | 5 ++--- PVE/Storage.pm | 24 ++++++++++++++++-------- test/archive_info_test.pm | 6 ++++++ 3 files changed, 24 insertions(+), 11 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..70dd663 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,23 @@ sub archive_remove { die "cannot remove protected archive '$archive_path'\n" if -e protection_file_path($archive_path); - my $dirname = dirname($archive_path); - my $archive_info = eval { archive_info($archive_path) } // {}; - my $logfn = $archive_info->{logfilename}; - unlink $archive_path or die "removing archive $archive_path failed: $!\n"; - if (defined($logfn)) { - my $logpath = "$dirname/$logfn"; - if (-e $logpath) { - unlink $logpath or warn "removing log file $logpath 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) } // {}; + + for my $type (qw(log notes)) { + my $filename = $archive_info->{"${type}filename"} or next; + my $path = "$dirname/$filename"; + + 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},