[pbs-devel] [PATCH proxmox-backup v7 0/4] refactor datastore locking to use tmpfs

[Shannon Sterz]

Superseeded-by: https://lore.proxmox.com/pbs-devel/20250324125157.165976-1-s.sterz@proxmox.com/T/#t

On Tue Mar 11, 2025 at 2:51 PM CET, Shannon Sterz wrote:
> The goal of this series is to make it safer to remove backup groups &
> snapshots by separating the corresponding directories from their lock
> files. By moving the lock files to the tmpfs-backed '/run' directory,
> we also make sure that the lock files get cleaned up when the system
> reboots.
>
> This series refactors the locking mechanism inside the `DataStore`,
> `BackupDir` and `BackupGroup` traits. In a first step locking methods
> are added and the existing code is refactored to use them. Commit two
> derives a lock file name under '/run' for each group/snapshot. It also
> adds double stat'ing. To avoid issues when upgrading, the file
> `/run/proxmox-backup/old-locking` is created which is used to determine
> whether the system has been rebooted and we can safely use the new
> locking mechanism.
>
> The third commit refactors locking for manifests and brings it in-line
> with the group/snapshot locks. Finally, the last commit fixes a race
> condition when changing the owner of a datastore.
>
> This version is based on a fairly old series from 2022 [1].
>
> [1]: https://lore.proxmox.com/pbs-devel/20220824124829.392189-1-s.sterz@proxmox.com/
>
> ----
> changes from v6:
> * add old locking safe guards to avoid different versions of the locking
>   mechanism being used at the same time (see discussion here [2]).
>
> [2]: https://lore.proxmox.com/pbs-devel/20250306120810.361035-1-m.sandoval@proxmox.com/T/#u
>
> changes from v5:
> * re-phrase commit messages to make it clear which commit actually
>   fixes the issue and what the commit implies in-terms of semantic
>   changes for error messages (thanks @ Thomas Lamprecht)
> * make it so the series applies cleanly again and clean up a newly
>   added usage of `lock_dir_noblock`
>
> changes from v4 (thanks @ Wolfgang Bumiller):
> * stop using `to_string_lossy()`
> * switch funtion signature of `create_locked_backup_group()` and
>   `create_locked_backup_dir` to use the `Arc` version of a datastore.
> * smaller clippy fixes
> * rebased on current master
>
> changes from v3:
> * moved patch 2 to the front so it can be applied separatelly more
>   easily
> * rebased on current master
>
> changes from v2:
> * different encoding scheme for lock file names
> * refactored locking methods to be used by the new BackupDir and
>   BackupGroup traits
> * adapted lock file names to include namespaces
>
> changes from v1 (thanks @ Wolfgang Bumiller & Thomas Lamprecht):
> * split adding locking helpers and move '/run' into two commits
> * instead of stat'ing the path of lock file twice, only use the file
>   descriptor for one of the stat'ing procedures instead
> * several improvements to helper functions and documentation
>
> Shannon Sterz (4):
>   datastore/api/backup: prepare for fix of #3935 by adding lock helpers
>   fix #3935: datastore/api/backup: move datastore locking to '/run'
>   fix #3935: datastore: move manifest locking to new locking method
>   fix: api: avoid race condition in set_backup_owner
>
>  Cargo.toml                           |   2 +-
>  debian/postinst                      |   5 +
>  pbs-config/src/lib.rs                |  32 +++-
>  pbs-datastore/Cargo.toml             |   1 +
>  pbs-datastore/src/backup_info.rs     | 230 ++++++++++++++++++++++++---
>  pbs-datastore/src/datastore.rs       |  70 +++-----
>  pbs-datastore/src/snapshot_reader.rs |  20 ++-
>  src/api2/admin/datastore.rs          |   9 +-
>  src/api2/backup/environment.rs       |  14 +-
>  src/api2/backup/mod.rs               |   8 +-
>  src/api2/reader/mod.rs               |   7 +-
>  src/backup/verify.rs                 |  12 +-
>  src/server/sync.rs                   |  10 +-
>  13 files changed, 302 insertions(+), 118 deletions(-)
>
> --
> 2.39.5