From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <pbs-devel-bounces@lists.proxmox.com> Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 993DC1FF15E for <inbox@lore.proxmox.com>; Tue, 25 Mar 2025 12:26:41 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id AF1987C45; Tue, 25 Mar 2025 12:26:37 +0100 (CET) Date: Tue, 25 Mar 2025 12:26:34 +0100 From: Wolfgang Bumiller <w.bumiller@proxmox.com> To: Shannon Sterz <s.sterz@proxmox.com> Message-ID: <q4sjofdggv4xtqgiw4b4td5l2fxona2pq4nq57rvgqivdhvvun@jyrc2cplnxcb> References: <20250324125157.165976-1-s.sterz@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20250324125157.165976-1-s.sterz@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.080 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH proxmox-backup v8 0/4] refactor datastore locking to use tmpfs X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion <pbs-devel.lists.proxmox.com> List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe> List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/> List-Post: <mailto:pbs-devel@lists.proxmox.com> List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help> List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe> Reply-To: Proxmox Backup Server development discussion <pbs-devel@lists.proxmox.com> Cc: pbs-devel@lists.proxmox.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com> The suggested anyhow::Context changes make sense, and maybe the d/rules change I proposed in 2/4. With those this is: Acked-by: Wolfgang Bumiller <w.bumiller@proxmox.com> On Mon, Mar 24, 2025 at 01:51:54PM +0100, 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 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. > > ---- > changes from v7 (thanks @ Christian Ebner): > * use anyhow's `Context` to provide more context on the call site of a > locking helper call > * rebase on top of current master to apply cleanly again > > 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 [1]). > > [1]: 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 | 236 ++++++++++++++++++++++++--- > pbs-datastore/src/datastore.rs | 86 +++++----- > pbs-datastore/src/snapshot_reader.rs | 20 ++- > src/api2/admin/datastore.rs | 13 +- > src/api2/backup/environment.rs | 21 +-- > src/api2/backup/mod.rs | 13 +- > src/api2/reader/mod.rs | 11 +- > src/backup/verify.rs | 12 +- > src/server/sync.rs | 13 +- > 13 files changed, 342 insertions(+), 123 deletions(-) > > -- > 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel