* [pbs-devel] [PATCH proxmox] sys: open process_locker lockfile lazy
@ 2023-11-15 11:01 Gabriel Goller
2023-11-15 14:32 ` Gabriel Goller
0 siblings, 1 reply; 2+ messages in thread
From: Gabriel Goller @ 2023-11-15 11:01 UTC (permalink / raw)
To: pbs-devel
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 <g.goller@proxmox.com>
---
proxmox-sys/src/process_locker.rs | 79 +++++++++++++++++++++++--------
1 file changed, 58 insertions(+), 21 deletions(-)
diff --git a/proxmox-sys/src/process_locker.rs b/proxmox-sys/src/process_locker.rs
index 4874da8..b612317 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<File>,
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,31 @@ 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");
}
+ println!("closing exclusive file lock");
+ 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<P: AsRef<std::path::Path>>(lockfile: P) -> Result<Arc<Mutex<Self>>, 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<Arc<Mutex<Self>>, Error> {
Ok(Arc::new(Mutex::new(Self {
- file,
+ path: lockfile,
+ file_descriptor: None,
exclusive: false,
writers: 0,
next_guard_id: 0,
@@ -144,7 +154,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 +210,21 @@ 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);
+ }
+ println!("locking exclusive lock: {:?}", &data.file_descriptor);
+ 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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [pbs-devel] [PATCH proxmox] sys: open process_locker lockfile lazy
2023-11-15 11:01 [pbs-devel] [PATCH proxmox] sys: open process_locker lockfile lazy Gabriel Goller
@ 2023-11-15 14:32 ` Gabriel Goller
0 siblings, 0 replies; 2+ messages in thread
From: Gabriel Goller @ 2023-11-15 14:32 UTC (permalink / raw)
To: pbs-devel
Forgot some debug prints :)
Submitted a new patch.
On 11/15/23 12:01, Gabriel Goller wrote:
> 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 <g.goller@proxmox.com>
> ---
> proxmox-sys/src/process_locker.rs | 79 +++++++++++++++++++++++--------
> 1 file changed, 58 insertions(+), 21 deletions(-)
>
> diff --git a/proxmox-sys/src/process_locker.rs b/proxmox-sys/src/process_locker.rs
> index 4874da8..b612317 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<File>,
> 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,31 @@ 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");
> }
>
> + println!("closing exclusive file lock");
> + 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<P: AsRef<std::path::Path>>(lockfile: P) -> Result<Arc<Mutex<Self>>, 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<Arc<Mutex<Self>>, Error> {
> Ok(Arc::new(Mutex::new(Self {
> - file,
> + path: lockfile,
> + file_descriptor: None,
> exclusive: false,
> writers: 0,
> next_guard_id: 0,
> @@ -144,7 +154,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 +210,21 @@ 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);
> + }
> + println!("locking exclusive lock: {:?}", &data.file_descriptor);
> + 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);
> }
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-11-15 14:32 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-15 11:01 [pbs-devel] [PATCH proxmox] sys: open process_locker lockfile lazy Gabriel Goller
2023-11-15 14:32 ` Gabriel Goller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal