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

[Daniel Tschlatscher <d.tschlatscher@proxmox.com>]

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'