all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox v2] sys: file: use renameat2() from `nix` crate
@ 2024-07-12  7:54 Christoph Heiss
  2024-07-12  8:16 ` Turkijan
  2024-07-18 10:29 ` Thomas Lamprecht
  0 siblings, 2 replies; 8+ messages in thread
From: Christoph Heiss @ 2024-07-12  7:54 UTC (permalink / raw)
  To: pbs-devel

Saves us from converting the paths to raw C strings ourselves, thus
simplifying it quite a bit.

Since the `nix` crate does not provide link() directly, only linkat(),
that is used instead - which behaves exactly the same as link() when
using AT_FDCWD and an empty flag set.

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * use `AtFlags` instead of deprecated `LinkatFlags` (thanks Christian!)
  * use an empty flag set, as AT_SYMLINK_NOFOLLOW is the default anyway
    for linkat()
  * adjust accompanying comment to mention `linkat` instead of `link`

 proxmox-sys/src/fs/file.rs | 53 +++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 30 deletions(-)

diff --git a/proxmox-sys/src/fs/file.rs b/proxmox-sys/src/fs/file.rs
index ac513891..b2d6ddf6 100644
--- a/proxmox-sys/src/fs/file.rs
+++ b/proxmox-sys/src/fs/file.rs
@@ -10,7 +10,6 @@ use nix::errno::Errno;
 use nix::fcntl::OFlag;
 use nix::sys::stat;
 use nix::unistd;
-use nix::NixPath;
 use serde_json::Value;
 
 use proxmox_lang::try_block;
@@ -266,30 +265,32 @@ pub fn atomic_open_or_create_file<P: AsRef<Path>>(
 
     // rotate the file into place, but use `RENAME_NOREPLACE`, so in case 2 processes race against
     // the initialization, the first one wins!
-    let rename_result = temp_file_name.with_nix_path(|c_file_name| {
-        path.with_nix_path(|new_path| unsafe {
-            // This also works on file systems which don't support hardlinks (eg. vfat)
-            match Errno::result(libc::renameat2(
-                libc::AT_FDCWD,
-                c_file_name.as_ptr(),
-                libc::AT_FDCWD,
-                new_path.as_ptr(),
-                libc::RENAME_NOREPLACE,
-            )) {
-                Err(Errno::EINVAL) => (), // dumb file system, try `link`+`unlink`
-                other => return other,
-            };
-            // but some file systems don't support `RENAME_NOREPLACE`
-            // so we just use `link` + `unlink` instead
-            let result = Errno::result(libc::link(c_file_name.as_ptr(), new_path.as_ptr()));
-            let _ = libc::unlink(c_file_name.as_ptr());
+    let rename_result = match nix::fcntl::renameat2(
+        None, // AT_FDCWD
+        &temp_file_name,
+        None, // AT_FDCWD
+        path,
+        nix::fcntl::RenameFlags::RENAME_NOREPLACE,
+    ) {
+        Err(Errno::EINVAL) => {
+            // some file systems don't support `RENAME_NOREPLACE`
+            // so we just use `linkat` + `unlink` instead
+            let result = nix::unistd::linkat(
+                None,
+                &temp_file_name,
+                None,
+                &path.to_path_buf(),
+                nix::unistd::AtFlags::empty(),
+            );
+            let _ = nix::unistd::unlink(&temp_file_name);
             result
-        })
-    });
+        }
+        other => other,
+    };
 
     match rename_result {
-        Ok(Ok(Ok(_))) => Ok(file),
-        Ok(Ok(Err(err))) => {
+        Ok(_) => Ok(file),
+        Err(err) => {
             // if another process has already raced ahead and created
             // the file, let's just open theirs instead:
             let _ = nix::unistd::unlink(&temp_file_name);
@@ -308,14 +309,6 @@ pub fn atomic_open_or_create_file<P: AsRef<Path>>(
                 );
             }
         }
-        Ok(Err(err)) => {
-            let _ = nix::unistd::unlink(&temp_file_name);
-            bail!("with_nix_path {:?} failed - {}", path, err);
-        }
-        Err(err) => {
-            let _ = nix::unistd::unlink(&temp_file_name);
-            bail!("with_nix_path {:?} failed - {}", temp_file_name, err);
-        }
     }
 }
 
-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-07-18 12:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-12  7:54 [pbs-devel] [PATCH proxmox v2] sys: file: use renameat2() from `nix` crate Christoph Heiss
2024-07-12  8:16 ` Turkijan
2024-07-12  9:39   ` Christian Ebner
2024-07-18 10:29 ` Thomas Lamprecht
2024-07-18 11:19   ` Christian Ebner
2024-07-18 12:03     ` Thomas Lamprecht
2024-07-18 12:13       ` Christian Ebner
2024-07-18 12:19         ` Thomas Lamprecht

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