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.tschlatsc...@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'



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to