[pbs-devel] [PATCH proxmox-backup] datastore: ignore group locking errors when removing snapshots

[pbs-devel] [PATCH proxmox-backup] datastore: ignore group locking errors when removing snapshots

[Shannon Sterz]

this is only needed for removing the group if the last snapshot is
removed, ignore locking failures, as the user can't do anything to
rectify the situation anymore.

log the locking error for debugging purposes, though.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 pbs-datastore/src/backup_info.rs | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index 557bb196..f7805eaf 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -599,12 +599,25 @@ impl BackupDir {
         let _ = std::fs::remove_file(self.lock_path()); // ignore errors

         let group = BackupGroup::from(self);
-        let _guard = group.lock().with_context(|| {
+        let guard = group.lock().with_context(|| {
             format!("while checking if group '{group:?}' is empty during snapshot destruction")
-        })?;
+        });

-        if group.list_backups()?.is_empty() && !*OLD_LOCKING {
+        // only remove the group if:
+        //
+        // - we can lock it: if we can't lock the group, it is still in use (either by another backup process or a
+        // parent caller (who needs to take care that empty groups are removed themselves).
+        // - it is now empty: if the group isn't empty, removing it will fail (to avoid removing
+        // backups that might still be used).
+        // - the new locking mechanism is used: if the old mechanism is used, a group removal here
+        // could lead to a race condition.
+        //
+        // do not error out, as we have already removed the snapshot, there is nothing a user could
+        // do to rectify the situation.
+        if guard.is_ok() && group.list_backups()?.is_empty() && !*OLD_LOCKING {
             group.remove_group_dir()?;
+        } else if let Err(err) = guard {
+            log::debug!("{err:#}");
         }

         Ok(())
--
2.39.5



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

[pbs-devel] applied: [PATCH proxmox-backup] datastore: ignore group locking errors when removing snapshots

[Thomas Lamprecht]

Am 03.04.25 um 12:14 schrieb Shannon Sterz:
> this is only needed for removing the group if the last snapshot is
> removed, ignore locking failures, as the user can't do anything to
> rectify the situation anymore.
> 
> log the locking error for debugging purposes, though.
> 
> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
>  pbs-datastore/src/backup_info.rs | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
>

applied, with wrapping the comment at 100cc and adding indentation for
the bullet points squashed in, thanks!


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