Re: [pbs-devel] applied-series: [PATCH proxmox-backup v9 0/4] refactor datastore locking to use tmpfs

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Thu, Mar 27, 2025 at 11:02:35AM +0100, Shannon Sterz wrote:
> On Wed Mar 26, 2025 at 4:36 PM CET, Thomas Lamprecht wrote:
> > Am 26.03.25 um 12:44 schrieb Shannon Sterz:> 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 through a post-install
> >> hook 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.
> >
> >> 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
> >>
> >
> > applied series, added Wolfgang's Acked-by – feel encouraged to pick such
> > things up already for a new revision, especially an A-b is less problematic
> > with minor changes as long as the overall design stays the same – and also
> > fixed a minor compile warning about an unused import that stemmed from
> > intra-revision changes, thanks!
> 
> thanks a bunch! should i always add all trailers mentioned on list?
> wasn't sure about the `Acked-by` tbh ^^'

At the very least any commits with no changes can accumulate all the
trailers.

For any changed commits you'll need to gauge whether or not another look
is warranted.
Eg. obviously typo or small rustfmt fixes are fine.
In the previous series I also specifically stated that my Ack includes
the proposed changes as I figured they'd be small enough to not require
another complete review, so as a review I would've just done a git-diff
between the 2 completely-applied versions rather than looked at the
individual patches first. (I don't necessarily do this between versions,
depending on how the changes are described in the emails, since with
regular read-throughs I'll usually pick up more things which I might
have missed in earlier versions.)


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