From: Gabriel Goller <g.goller@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH v2 proxmox 1/2] process_locker: use ofd locking
Date: Wed, 6 Dec 2023 14:28:33 +0100 [thread overview]
Message-ID: <20231206132834.240700-2-g.goller@proxmox.com> (raw)
In-Reply-To: <20231206132834.240700-1-g.goller@proxmox.com>
Instead of using normal fcntl locking, use the OFD (open file
descriptor) flags [0].
This blocks the file on the file-descriptor level, not on the
process-level.
This solves two problems:
- If a process closes any file descriptor referring to a file,
then all of the process's locks on that file are released,
regardless of the file descriptor(s) on which the locks were
obtained. This is bad: it means that a process can lose its
locks on a file such as /etc/passwd or /etc/mtab when for some
reason a library function decides to open, read, and close the
same file.
- The threads in a process share locks. In other words, a
multithreaded program can't use record locking to ensure that
threads don't simultaneously access the same region of a file.
(Copied from [0])
Added a `create_dir_all()` call to create the folders if they are not
existing.
[0]: https://man7.org/linux/man-pages/man2/fcntl.2.html
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
proxmox-sys/src/process_locker.rs | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/proxmox-sys/src/process_locker.rs b/proxmox-sys/src/process_locker.rs
index 4874da8..58deacb 100644
--- a/proxmox-sys/src/process_locker.rs
+++ b/proxmox-sys/src/process_locker.rs
@@ -1,7 +1,7 @@
//! Inter-process reader-writer lock builder.
//!
//! This implementation uses fcntl record locks with non-blocking
-//! F_SETLK command (never blocks).
+//! F_OFD_SETLK command (never blocks).
//!
//! We maintain a map of shared locks with time stamps, so you can get
//! the timestamp for the oldest open lock with
@@ -13,8 +13,6 @@ use std::sync::{Arc, Mutex};
use anyhow::{bail, Error};
-// fixme: use F_OFD_ locks when implemented with nix::fcntl
-
// Note: flock lock conversion is not atomic, so we need to use fcntl
/// Inter-process reader-writer lock
@@ -53,9 +51,10 @@ 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))
- {
+ if let Err(err) = nix::fcntl::fcntl(
+ data.file.as_raw_fd(),
+ nix::fcntl::FcntlArg::F_OFD_SETLKW(&op),
+ ) {
panic!("unable to drop writer lock - {}", err);
}
}
@@ -93,9 +92,10 @@ 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))
- {
+ if let Err(err) = nix::fcntl::fcntl(
+ data.file.as_raw_fd(),
+ nix::fcntl::FcntlArg::F_OFD_SETLKW(&op),
+ ) {
panic!("unable to drop exclusive lock - {}", err);
}
@@ -108,6 +108,12 @@ impl 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 parent = lockfile
+ .as_ref()
+ .parent()
+ .ok_or(anyhow::anyhow!("Failed getting parent dir of locking file"))?;
+ std::fs::create_dir_all(parent)?;
+
let file = std::fs::OpenOptions::new()
.create(true)
.read(true)
@@ -132,7 +138,7 @@ impl ProcessLocker {
l_pid: 0,
};
- nix::fcntl::fcntl(file.as_raw_fd(), nix::fcntl::FcntlArg::F_SETLK(&op))?;
+ nix::fcntl::fcntl(file.as_raw_fd(), nix::fcntl::FcntlArg::F_OFD_SETLK(&op))?;
Ok(())
}
--
2.39.2
next prev parent reply other threads:[~2023-12-06 13:29 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-06 13:28 [pbs-devel] [PATCH v2 proxmox{, -backup} 0/2] Move ProcessLocker to tmpfs Gabriel Goller
2023-12-06 13:28 ` Gabriel Goller [this message]
2023-12-06 13:28 ` [pbs-devel] [PATCH v2 proxmox-backup 2/2] datastore: store datastore lock on tmpfs Gabriel Goller
2023-12-06 13:41 ` [pbs-devel] [PATCH v2 proxmox{, -backup} 0/2] Move ProcessLocker to tmpfs Fabian Grünbichler
2023-12-06 13:56 ` Gabriel Goller
2023-12-06 14:14 ` Fabian Grünbichler
2023-12-06 14:21 ` Gabriel Goller
2023-12-06 14:33 ` Fabian Grünbichler
2023-12-06 14:36 ` Thomas Lamprecht
2023-12-06 14:46 ` Gabriel Goller
2023-12-06 14:58 ` Thomas Lamprecht
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231206132834.240700-2-g.goller@proxmox.com \
--to=g.goller@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.