From: "Shannon Sterz" <s.sterz@proxmox.com>
To: "Shannon Sterz" <s.sterz@proxmox.com>, <pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox-backup v7 0/4] refactor datastore locking to use tmpfs
Date: Mon, 24 Mar 2025 13:54:11 +0100 [thread overview]
Message-ID: <D8OIFI8VQSYY.1XXVNSNWBWJGL@proxmox.com> (raw)
In-Reply-To: <20250311135200.309896-1-s.sterz@proxmox.com>
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
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
prev parent reply other threads:[~2025-03-24 12:54 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-11 13:51 Shannon Sterz
2025-03-11 13:51 ` [pbs-devel] [PATCH proxmox-backup v7 1/4] datastore/api/backup: prepare for fix of #3935 by adding lock helpers Shannon Sterz
2025-03-14 8:30 ` Christian Ebner
2025-03-11 13:51 ` [pbs-devel] [PATCH proxmox-backup v7 2/4] fix #3935: datastore/api/backup: move datastore locking to '/run' Shannon Sterz
2025-03-11 13:51 ` [pbs-devel] [PATCH proxmox-backup v7 3/4] fix #3935: datastore: move manifest locking to new locking method Shannon Sterz
2025-03-11 13:52 ` [pbs-devel] [PATCH proxmox-backup v7 4/4] fix: api: avoid race condition in set_backup_owner Shannon Sterz
2025-03-24 12:54 ` Shannon Sterz [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=D8OIFI8VQSYY.1XXVNSNWBWJGL@proxmox.com \
--to=s.sterz@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal