From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 141BB7ABBB for ; Tue, 5 Jul 2022 16:54:54 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0B0C524C3 for ; Tue, 5 Jul 2022 16:54:24 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 5 Jul 2022 16:54:22 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id E2D2C40471 for ; Tue, 5 Jul 2022 16:54:21 +0200 (CEST) From: Stefan Sterz To: pbs-devel@lists.proxmox.com Date: Tue, 5 Jul 2022 16:54:17 +0200 Message-Id: <20220705145418.388066-5-s.sterz@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220705145418.388066-1-s.sterz@proxmox.com> References: <20220705145418.388066-1-s.sterz@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.051 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% 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 T_SCC_BODY_TEXT_LINE -0.01 - Subject: [pbs-devel] [PATCH proxmox-backup v4 4/5] fix #3935: datastore: move manifest locking to new locking method X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Jul 2022 14:54:54 -0000 adds double stat'ing and removes directory hierarchy to bring manifest locking in-line with other locks used by the BackupDir trait. Signed-off-by: Stefan Sterz --- pbs-datastore/src/backup_info.rs | 35 ++++++++++++++++---------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs index 046ed6e9..43712700 100644 --- a/pbs-datastore/src/backup_info.rs +++ b/pbs-datastore/src/backup_info.rs @@ -443,25 +443,29 @@ impl BackupDir { /// Returns the filename to lock a manifest /// /// Also creates the basedir. The lockfile is located in - /// '/run/proxmox-backup/locks/{datastore}/[ns/{ns}/]+{type}/{id}/{timestamp}.index.json.lck' - fn manifest_lock_path(&self) -> Result { - let mut path = PathBuf::from(&format!("/run/proxmox-backup/locks/{}", self.store.name())); - path.push(self.relative_path()); + /// `${DATASTORE_LOCKS_DIR}/${datastore name}/${lock_file_name_helper(rpath)}.index.json.lck` + /// where rpath is the relative path of the snapshot. + /// + /// If the snapshot's relative path contains non-Unicode sequences they will be replaced via + /// [std::ffi::OsStr::to_string_lossy()]. + fn manifest_lock_path(&self) -> PathBuf { + let path = Path::new(DATASTORE_LOCKS_DIR).join(self.store.name()); - std::fs::create_dir_all(&path)?; - let ts = self.backup_time_string(); - path.push(&format!("{ts}{MANIFEST_LOCK_NAME}")); + let rpath = self.relative_path().join(&format!("{MANIFEST_LOCK_NAME}")); + let rpath = rpath.as_os_str().to_string_lossy(); - Ok(path) + path.join(lock_file_name_helper(&rpath)) } /// Locks the manifest of a snapshot, for example, to update or delete it. pub(crate) fn lock_manifest(&self) -> Result { - let path = self.manifest_lock_path()?; - - // actions locking the manifest should be relatively short, only wait a few seconds - open_backup_lockfile(&path, Some(std::time::Duration::from_secs(5)), true) - .map_err(|err| format_err!("unable to acquire manifest lock {:?} - {}", &path, err)) + lock_helper(self.store.name(), &self.manifest_lock_path(), |p| { + // update_manifest should never take a long time, so if + // someone else has the lock we can simply block a bit + // and should get it soon + open_backup_lockfile(&p, Some(Duration::from_secs(5)), true) + .map_err(|err| format_err!("unable to acquire manifest lock {p:?} - {err}")) + }) } /// Returns a file name for locking a snapshot. @@ -518,10 +522,7 @@ impl BackupDir { })?; // remove no longer needed lock files - if let Ok(path) = self.manifest_lock_path() { - let _ = std::fs::remove_file(path); // ignore errors - } - + let _ = std::fs::remove_file(self.manifest_lock_path()); // ignore errors let _ = std::fs::remove_file(self.lock_path()); // ignore errors Ok(()) -- 2.30.2