all lists on lists.proxmox.com
 help / color / mirror / Atom feed
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





  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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal