From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 3420A1FF13F for ; Thu, 26 Feb 2026 15:40:28 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 77E5E33DB2; Thu, 26 Feb 2026 15:41:26 +0100 (CET) From: Robert Obkircher To: pbs-devel@lists.proxmox.com Subject: [PATCH v1 proxmox-backup 11/11] tape: media catalog: use Flock wrapper instead of deprecated function Date: Thu, 26 Feb 2026 15:40:25 +0100 Message-ID: <20260226144033.211039-12-r.obkircher@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260226144033.211039-1-r.obkircher@proxmox.com> References: <20260226144033.211039-1-r.obkircher@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1772116862348 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.057 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 Message-ID-Hash: O223QPNSGPCCHZXGOJDCPTI5DXJDZ7TA X-Message-ID-Hash: O223QPNSGPCCHZXGOJDCPTI5DXJDZ7TA X-MailFrom: r.obkircher@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Switch to the new wrapper type to fix the deprecation warnings. Panic if unlock fails, because there is no easy way to return the File that was temporarily moved out of the field. An alternative to moving out of the optional field would be to use `File::try_clone` but that isn't ideal either because duplicated file descriptors share the same lock status. That also doesn't solve the problem that propagating the error without leaking the locked file descriptor requires unsafe code or a change to nix. [1] https://lore.proxmox.com/pbs-devel/1763634535.5sqd1r4buu.astroid@yuna.none/ Signed-off-by: Robert Obkircher --- src/tape/media_catalog.rs | 49 +++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/src/tape/media_catalog.rs b/src/tape/media_catalog.rs index 63329a65..372bf59d 100644 --- a/src/tape/media_catalog.rs +++ b/src/tape/media_catalog.rs @@ -6,6 +6,7 @@ use std::path::{Path, PathBuf}; use anyhow::{bail, format_err, Error}; use endian_trait::Endian; +use nix::fcntl::{Flock, FlockArg}; use proxmox_sys::fs::read_subdir; @@ -194,7 +195,7 @@ impl MediaCatalog { let me = proxmox_lang::try_block!({ Self::create_basedir(base_path)?; - let mut file = std::fs::OpenOptions::new() + let file = std::fs::OpenOptions::new() .read(true) .write(write) .create(create) @@ -219,9 +220,10 @@ impl MediaCatalog { }; // Note: lock file, to get a consistent view with load_catalog - nix::fcntl::flock(file.as_raw_fd(), nix::fcntl::FlockArg::LockExclusive)?; + let mut file = Flock::lock(file, FlockArg::LockExclusive) + .map_err(|e| format_err!("flock failed: {e:?}"))?; let result = me.load_catalog(&mut file, media_id.media_set_label.as_ref()); - nix::fcntl::flock(file.as_raw_fd(), nix::fcntl::FlockArg::Unlock)?; + let file = file.unlock().expect("shouldn't fail for valid fd"); let (found_magic_number, _) = result?; @@ -367,27 +369,30 @@ impl MediaCatalog { return Ok(()); } - match self.file { - Some(ref mut file) => { - let pending = &self.pending; - // Note: lock file, to get a consistent view with load_catalog - nix::fcntl::flock(file.as_raw_fd(), nix::fcntl::FlockArg::LockExclusive)?; - let result: Result<(), Error> = proxmox_lang::try_block!({ - file.write_all(pending)?; - file.flush()?; - file.sync_data()?; - Ok(()) - }); - nix::fcntl::flock(file.as_raw_fd(), nix::fcntl::FlockArg::Unlock)?; - - result?; - } - None => bail!("media catalog not writable (opened read only)"), - } + let Some(file) = self.file.take() else { + bail!("media catalog not writable (opened read only)"); + }; + + let mut file = Flock::lock(file, FlockArg::LockExclusive).map_err(|(f, e)| { + self.file = Some(f); + format_err!("flock failed: {:?} - {e:?}", self.file) + })?; - self.pending = Vec::new(); + let pending = &self.pending; + // Note: lock file, to get a consistent view with load_catalog + let result: Result<(), Error> = proxmox_lang::try_block!({ + file.write_all(pending)?; + file.flush()?; + file.sync_data()?; + Ok(()) + }); - Ok(()) + self.file = Some(file.unlock().expect("shouldn't fail for valid fd")); + + if result.is_ok() { + self.pending = Vec::new(); + } + result } /// Conditionally commit if in pending data is large (> 1Mb) -- 2.47.3