* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox