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 7608D9885B for ; Wed, 15 Nov 2023 15:32:33 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4F9B78622 for ; Wed, 15 Nov 2023 15:32:03 +0100 (CET) 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 ; Wed, 15 Nov 2023 15:32:02 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 63251431BD for ; Wed, 15 Nov 2023 15:32:02 +0100 (CET) From: Gabriel Goller To: pbs-devel@lists.proxmox.com Date: Wed, 15 Nov 2023 15:31:58 +0100 Message-Id: <20231115143158.217714-1-g.goller@proxmox.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.251 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 T_SCC_BODY_TEXT_LINE -0.01 - Subject: [pbs-devel] [PATCH proxmox v2] sys: open process_locker lockfile lazy 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: Wed, 15 Nov 2023 14:32:33 -0000 When setting a datastore in maintenance mode (offline or read-only) we should be able to unmount it. This isn't possible because the `ChunkReader` has a `ProcessLocker` instance that holds an open file descriptor to (f.e.) `/mnt/datastore/test1/.lock`. The `ChunkReader` is created at startup, so if the datastore is not set to a maintenance mode at startup, we always have the lockfile open. Now we create/open the lockfile lazy, when a shared lock or a exclusive lock is wanted. Like this, we can set a datastore to 'offline' and unmount it without restarting the proxmox-backup service. Signed-off-by: Gabriel Goller --- changes v2: * removed debug prints proxmox-sys/src/process_locker.rs | 77 ++++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 21 deletions(-) diff --git a/proxmox-sys/src/process_locker.rs b/proxmox-sys/src/process_locker.rs index 4874da8..88c74dd 100644 --- a/proxmox-sys/src/process_locker.rs +++ b/proxmox-sys/src/process_locker.rs @@ -8,7 +8,9 @@ //! `oldest_shared_lock()`. use std::collections::HashMap; +use std::fs::File; use std::os::unix::io::AsRawFd; +use std::path::PathBuf; use std::sync::{Arc, Mutex}; use anyhow::{bail, Error}; @@ -19,7 +21,8 @@ use anyhow::{bail, Error}; /// Inter-process reader-writer lock pub struct ProcessLocker { - file: std::fs::File, + path: PathBuf, + file_descriptor: Option, exclusive: bool, writers: usize, next_guard_id: u64, @@ -53,11 +56,16 @@ impl Drop for ProcessLockSharedGuard { l_pid: 0, }; - if let Err(err) = - nix::fcntl::fcntl(data.file.as_raw_fd(), nix::fcntl::FcntlArg::F_SETLKW(&op)) - { - panic!("unable to drop writer lock - {}", err); + if let Some(file) = &data.file_descriptor { + if let Err(err) = + nix::fcntl::fcntl(file.as_raw_fd(), nix::fcntl::FcntlArg::F_SETLKW(&op)) + { + panic!("unable to drop writer lock - {}", err); + } + } else { + panic!("file descriptor has been closed while locking"); } + data.file_descriptor = None; } if data.writers > 0 { data.writers -= 1; @@ -93,29 +101,30 @@ impl Drop for ProcessLockExclusiveGuard { l_pid: 0, }; - if let Err(err) = - nix::fcntl::fcntl(data.file.as_raw_fd(), nix::fcntl::FcntlArg::F_SETLKW(&op)) - { - panic!("unable to drop exclusive lock - {}", err); + if let Some(file) = &data.file_descriptor { + if let Err(err) = + nix::fcntl::fcntl(file.as_raw_fd(), nix::fcntl::FcntlArg::F_SETLKW(&op)) + { + panic!("unable to drop exclusive lock - {}", err); + } + } else { + panic!("file descriptor has been closed while locking"); } + data.file_descriptor = None; data.exclusive = false; } } impl ProcessLocker { - /// Create a new instance for the specified file. + /// Create a new instance of the ProcessLocker. /// - /// This simply creates the file if it does not exist. - pub fn new>(lockfile: P) -> Result>, Error> { - let file = std::fs::OpenOptions::new() - .create(true) - .read(true) - .write(true) - .open(lockfile)?; - + /// Does not check if the lockfile exists. The lockfile gets opened/created + /// when using [Self::try_shared_lock()] or [Self::try_exclusive_lock()]. + pub fn new(lockfile: PathBuf) -> Result>, Error> { Ok(Arc::new(Mutex::new(Self { - file, + path: lockfile, + file_descriptor: None, exclusive: false, writers: 0, next_guard_id: 0, @@ -144,7 +153,20 @@ impl ProcessLocker { let mut data = locker.lock().unwrap(); if data.writers == 0 && !data.exclusive { - if let Err(err) = Self::try_lock(&data.file, libc::F_RDLCK) { + if data.file_descriptor.is_none() { + let file = std::fs::OpenOptions::new() + .create(true) + .read(true) + .write(true) + .open(&data.path)?; + data.file_descriptor = Some(file); + } + if let Err(err) = Self::try_lock( + data.file_descriptor + .as_ref() + .expect("unable to open lockfile"), + libc::F_RDLCK, + ) { bail!("unable to get shared lock - {}", err); } } @@ -187,7 +209,20 @@ impl ProcessLocker { bail!("already locked exclusively"); } - if let Err(err) = Self::try_lock(&data.file, libc::F_WRLCK) { + if data.file_descriptor.is_none() { + let file = std::fs::OpenOptions::new() + .create(true) + .read(true) + .write(true) + .open(&data.path)?; + data.file_descriptor = Some(file); + } + if let Err(err) = Self::try_lock( + data.file_descriptor + .as_ref() + .expect("unable to open lockfile"), + libc::F_WRLCK, + ) { bail!("unable to get exclusive lock - {}", err); } -- 2.39.2