[pve-devel] [PATCH storage v3 1/4] fix #3972: Remove the .notes file when a backup is deleted

[pve-devel] [PATCH storage v3 1/4] fix #3972: Remove the .notes file when a backup is deleted

[Daniel Tschlatscher]

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

[pve-devel] [PATCH storage v3 2/4] fix #3972: Adapted unlink calls for archive files in case of ENOENT

[Daniel Tschlatscher]

This improves handling when two archive remove calls are creating a
race condition where one would formerly encounter an error. Now both
finish successfully.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
 PVE/Storage.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 93c3b4e..6f2558f 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -1586,7 +1586,7 @@ 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";
+    unlink $archive_path or $! == ENOENT or die "removing archive $archive_path failed: $!\n";
 
     archive_auxiliaries_remove($archive_path);
 }
@@ -1604,7 +1604,7 @@ sub archive_auxiliaries_remove {
 	my $path = "$dirname/$filename";
 
 	if (-e $path) {
-	    unlink $path or warn "Removing $type file failed: $!\n";
+	    unlink $path or $! == ENOENT or warn "Removing $type file failed: $!\n";
 	}
     }
 }
-- 
2.30.2

Re: [pve-devel] [PATCH storage v3 2/4] fix #3972: Adapted unlink calls for archive files in case of ENOENT

[Fabian Ebner]

Am 20.05.22 um 15:28 schrieb Daniel Tschlatscher:
> This improves handling when two archive remove calls are creating a
> race condition where one would formerly encounter an error. Now both
> finish successfully.
> 
> Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>

Commit title shouldn't include "fix #3972". The first patch does that ;)

[pve-devel] [PATCH storage v3 3/4] fix #3972: Switched to using log_warn of PVE::RESTEnvironment

[Daniel Tschlatscher]

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
 PVE/Storage.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 6f2558f..98ecd60 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -1604,7 +1604,8 @@ sub archive_auxiliaries_remove {
 	my $path = "$dirname/$filename";
 
 	if (-e $path) {
-	    unlink $path or $! == ENOENT or warn "Removing $type file failed: $!\n";
+	    unlink $path or $! == ENOENT
+		or PVE::RESTEnvironment::log_warn("Removing $type file failed: $!");
 	}
     }
 }
-- 
2.30.2

Re: [pve-devel] [PATCH storage v3 3/4] fix #3972: Switched to using log_warn of PVE::RESTEnvironment

[Fabian Ebner]

Am 20.05.22 um 15:28 schrieb Daniel Tschlatscher:
> Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>

Commit title shouldn't include "fix #3972"

> ---
>  PVE/Storage.pm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index 6f2558f..98ecd60 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -1604,7 +1604,8 @@ sub archive_auxiliaries_remove {
>  	my $path = "$dirname/$filename";
>  
>  	if (-e $path) {
> -	    unlink $path or $! == ENOENT or warn "Removing $type file failed: $!\n";
> +	    unlink $path or $! == ENOENT
> +		or PVE::RESTEnvironment::log_warn("Removing $type file failed: $!");

Missing use/import for the module. And you could import log_warn
directly so that it can be used without the package prefix to make
everything fit on one line here.

>  	}
>      }
>  }

[pve-devel] [PATCH storage v3 4/4] Added a LOG_EXT constant as a counterpart to NOTES_EXT

[Daniel Tschlatscher]

and refactored usages for .log and .notes with them.
At some parts in the test case code I had to source new variables to
shorten the line length to not exceed the 100 column line limit.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
 PVE/Storage.pm            |  4 ++--
 PVE/Storage/Plugin.pm     |  1 +
 test/archive_info_test.pm | 32 ++++++++++++++++++--------------
 3 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 98ecd60..d2e1a5b 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -1565,8 +1565,8 @@ sub archive_info {
 	$info->{type} = $type;
 
 	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->{logfilename} = "$1".PVE::Storage::Plugin::LOG_EXT;
+	    $info->{notesfilename} = "$filename".PVE::Storage::Plugin::NOTES_EXT;
 	    $info->{vmid} = int($2);
 	    $info->{ctime} = timelocal($8, $7, $6, $5, $4 - 1, $3);
 	    $info->{is_std_name} = 1;
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 0a100b7..7e53189 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -21,6 +21,7 @@ use base qw(PVE::SectionConfig);
 use constant COMPRESSOR_RE => 'gz|lzo|zst';
 
 use constant NOTES_EXT => ".notes";
+use constant LOG_EXT => ".log";
 
 our @COMMON_TAR_FLAGS = qw(
     --one-file-system
diff --git a/test/archive_info_test.pm b/test/archive_info_test.pm
index bbc0e6f..cf03c3d 100644
--- a/test/archive_info_test.pm
+++ b/test/archive_info_test.pm
@@ -10,6 +10,9 @@ use Test::More;
 
 my $vmid = 16110;
 
+my $LOG_EXT = PVE::Storage::Plugin::LOG_EXT;
+my $NOTES_EXT = PVE::Storage::Plugin::NOTES_EXT;
+
 # an array of test cases, each test is comprised of the following keys:
 # description => to identify a single test
 # archive     => the input filename for archive_info
@@ -23,8 +26,8 @@ my $tests = [
 	archive     => "backup/vzdump-lxc-$vmid-3070_01_01-00_00_00.tgz",
 	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",
+	    'logfilename'  => "vzdump-lxc-$vmid-3070_01_01-00_00_00".$LOG_EXT,
+	    'notesfilename'=> "vzdump-lxc-$vmid-3070_01_01-00_00_00.tgz".$NOTES_EXT,
 	    'type'         => 'lxc',
 	    'format'       => 'tar',
 	    'decompressor' => ['tar', '-z'],
@@ -39,8 +42,8 @@ my $tests = [
 	archive     => "backup/vzdump-lxc-$vmid-1970_01_01-02_00_30.tgz",
 	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",
+	    'logfilename'  => "vzdump-lxc-$vmid-1970_01_01-02_00_30".$LOG_EXT,
+	    'notesfilename'=> "vzdump-lxc-$vmid-1970_01_01-02_00_30.tgz".$NOTES_EXT,
 	    'type'         => 'lxc',
 	    'format'       => 'tar',
 	    'decompressor' => ['tar', '-z'],
@@ -55,8 +58,8 @@ my $tests = [
 	archive     => "backup/vzdump-lxc-$vmid-2020_03_30-21_39_30.tgz",
 	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",
+	    'logfilename'  => "vzdump-lxc-$vmid-2020_03_30-21_39_30".$LOG_EXT,
+	    'notesfilename'=> "vzdump-lxc-$vmid-2020_03_30-21_39_30.tgz".$NOTES_EXT,
 	    'type'         => 'lxc',
 	    'format'       => 'tar',
 	    'decompressor' => ['tar', '-z'],
@@ -71,8 +74,8 @@ my $tests = [
 	archive     => "backup/vzdump-openvz-$vmid-2020_03_30-21_39_30.tgz",
 	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",
+	    'logfilename'  => "vzdump-openvz-$vmid-2020_03_30-21_39_30".$LOG_EXT,
+	    'notesfilename'=> "vzdump-openvz-$vmid-2020_03_30-21_39_30.tgz".$NOTES_EXT,
 	    'type'         => 'openvz',
 	    'format'       => 'tar',
 	    'decompressor' => ['tar', '-z'],
@@ -87,8 +90,8 @@ my $tests = [
 	archive     => "/here/be/Back-ups/vzdump-qemu-$vmid-2020_03_30-21_39_30.tgz",
 	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",
+	    'logfilename'  => "vzdump-qemu-$vmid-2020_03_30-21_39_30".$LOG_EXT,
+	    'notesfilename'=> "vzdump-qemu-$vmid-2020_03_30-21_39_30.tgz".$NOTES_EXT,
 	    'type'         => 'qemu',
 	    'format'       => 'tar',
 	    'decompressor' => ['tar', '-z'],
@@ -135,15 +138,16 @@ my $bkp_suffix = {
 # create more test cases for backup files matches
 for my $virt (sort keys %$bkp_suffix) {
     my ($format, $decomp) = $bkp_suffix->{$virt}->@*;
+    my $archive_name = "vzdump-$virt-$vmid-2020_03_30-21_12_40";
 
     for my $suffix (sort keys %$decomp) {
 	push @$tests, {
 	    description => "Backup archive, $virt, $format.$suffix",
-	    archive     => "backup/vzdump-$virt-$vmid-2020_03_30-21_12_40.$format.$suffix",
+	    archive     => "backup/$archive_name.$format.$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",
+		'filename'     => "$archive_name.$format.$suffix",
+		'logfilename'  => $archive_name.$LOG_EXT,
+		'notesfilename'=> "$archive_name.$format.$suffix".$NOTES_EXT,
 		'type'         => "$virt",
 		'format'       => "$format",
 		'decompressor' => $decomp->{$suffix},
-- 
2.30.2

Re: [pve-devel] [PATCH storage v3 4/4] Added a LOG_EXT constant as a counterpart to NOTES_EXT

[Fabian Ebner]

Am 20.05.22 um 15:28 schrieb Daniel Tschlatscher:
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 0a100b7..7e53189 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -21,6 +21,7 @@ use base qw(PVE::SectionConfig);
>  use constant COMPRESSOR_RE => 'gz|lzo|zst';
>  
>  use constant NOTES_EXT => ".notes";
> +use constant LOG_EXT => ".log";

Nit: not ordered alphabetically

Re: [pve-devel] [PATCH storage v3 1/4] fix #3972: Remove the .notes file when a backup is deleted

[Thomas Lamprecht]

On 20/05/2022 15:28, Daniel Tschlatscher wrote:
> 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>
> ---

the "changes since v2" sections are missing from all patches, making it
harder to check if all comments got addressed or what other changes there
were...

>  PVE/API2/Storage/Content.pm |  5 ++---
>  PVE/Storage.pm              | 20 +++++++++++++++-----
>  test/archive_info_test.pm   |  6 ++++++
>  3 files changed, 23 insertions(+), 8 deletions(-)
> 

Re: [pve-devel] [PATCH storage v3 1/4] fix #3972: Remove the .notes file when a backup is deleted

[Daniel Tschlatscher]

On 5/25/22 17:06, Thomas Lamprecht wrote:
> On 20/05/2022 15:28, Daniel Tschlatscher wrote:
>> 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>
>> ---
> the "changes since v2" sections are missing from all patches, making it
> harder to check if all comments got addressed or what other changes there
> were...
>
>>   PVE/API2/Storage/Content.pm |  5 ++---
>>   PVE/Storage.pm              | 20 +++++++++++++++-----
>>   test/archive_info_test.pm   |  6 ++++++
>>   3 files changed, 23 insertions(+), 8 deletions(-)
>>
Sorry for that, the changes from v2 are:

- Test cases are now part of the initial commit to make it compilable by 
itself.

- File 'unlink' now checks whether the file is already gone to avoid 
errors when removals are racing

- The code to 'unlink' the files was made a bit more concise and DRY by 
utilizing a loop

- Instead of plain 'warn' now use 'log_warn()'

- The file extensions for '.notes' and '.log' files now use constants 
declared in 'PVE::Storage::Plugin'

Re: [pve-devel] [PATCH storage v3 1/4] fix #3972: Remove the .notes file when a backup is deleted

[Fabian Ebner]

Am 20.05.22 um 15:28 schrieb Daniel Tschlatscher:
> 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>

Only found small nits, so consider the series:
Reviewed-by: Fabian Ebner <f.ebner@proxmox.com>

> @@ -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};

Both $logfn and $notesfn are unused.

>  
> -    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";
>  	}
>      }
>  }