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 AF0151FF15E for <inbox@lore.proxmox.com>; Tue, 25 Mar 2025 12:33:12 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4F40C7CF9; Tue, 25 Mar 2025 12:33:08 +0100 (CET) Mime-Version: 1.0 Date: Tue, 25 Mar 2025 12:33:04 +0100 Message-Id: <D8PBBY0PUO0B.3NGT7MHN2AT3S@proxmox.com> From: "Shannon Sterz" <s.sterz@proxmox.com> To: "Wolfgang Bumiller" <w.bumiller@proxmox.com> X-Mailer: aerc 0.20.1-0-g2ecb8770224a-dirty References: <20250324125157.165976-1-s.sterz@proxmox.com> <q4sjofdggv4xtqgiw4b4td5l2fxona2pq4nq57rvgqivdhvvun@jyrc2cplnxcb> In-Reply-To: <q4sjofdggv4xtqgiw4b4td5l2fxona2pq4nq57rvgqivdhvvun@jyrc2cplnxcb> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.015 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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> On Tue Mar 25, 2025 at 12:26 PM CET, Wolfgang Bumiller wrote: > 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> alright, will add those changes and submit a v9. thanks! > > 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