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 A1D7B717B7 for ; Thu, 12 May 2022 15:20:35 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 892102790A for ; Thu, 12 May 2022 15:20:35 +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 86A4D27901 for ; Thu, 12 May 2022 15:20:34 +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 4302B434AA for ; Thu, 12 May 2022 15:20:34 +0200 (CEST) From: Daniel Tschlatscher To: pve-devel@lists.proxmox.com Date: Thu, 12 May 2022 15:17:52 +0200 Message-Id: <20220512131753.126854-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.051 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 v2 1/2] 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 May 2022 13:20:35 -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". This change makes it easier to remove these files through archive_remove(), but also other parts in the code while keeping it DRY. I saw this change necessary because archive_remove() does not invoke external plugins, and doing so might have unintended side effects. Signed-off-by: Daniel Tschlatscher --- PVE/API2/Storage/Content.pm | 5 ++--- PVE/Storage.pm | 19 +++++++++++++++++-- 2 files changed, 19 insertions(+), 5 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..5317043 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,11 +1586,18 @@ 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}; - - unlink $archive_path or die "removing archive $archive_path failed: $!\n"; + my $notesfn = $archive_info->{notesfilename}; if (defined($logfn)) { my $logpath = "$dirname/$logfn"; @@ -1597,6 +1605,13 @@ sub archive_remove { unlink $logpath or warn "removing log file $logpath failed: $!\n"; } } + + if (defined($notesfn)) { + my $notespath = "$dirname/$notesfn"; + if (-e $notespath) { + unlink $notespath or warn "removing notes file $notespath failed: $!\n"; + } + } } sub extract_vzdump_config_tar { -- 2.30.2