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

[Turkijan]

how to update this for my test proxmox backup server?

On Fri, Jul 12, 2024 at 3:55 AM Christoph Heiss <c.heiss at proxmox.com> wrote:

> 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 at 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 at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.proxmox.com/pipermail/pbs-devel/attachments/20240712/41378480/attachment.htm>