[pbs-devel] [PATCH proxmox v2] sys: file: use renameat2() from `nix` crate

* [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