From: Turkijan <turkijan083@gmail.com>
To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox] sys: file: use renameat2() from `nix` crate
Date: Thu, 11 Jul 2024 13:11:12 -0400 [thread overview]
Message-ID: <CAFtnzVeytCY22pYY96eXio54JvhdfUpZ8UQw7MH2YAVAcwTgdQ@mail.gmail.com> (raw)
In-Reply-To: <20240711170723.1115046-1-c.heiss@proxmox.com>
[-- Attachment #1.1: Type: text/plain, Size: 3828 bytes --]
Have you changed dev package?
On Thu, Jul 11, 2024 at 1:08 PM Christoph Heiss <c.heiss@proxmox.com> wrote:
> Saves us from converting the paths to raw C strings ourselves, thus
> simplifying it quite a bit.
>
> No functional changes.
>
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> proxmox-sys/src/fs/file.rs | 51 ++++++++++++++++----------------------
> 1 file changed, 22 insertions(+), 29 deletions(-)
>
> diff --git a/proxmox-sys/src/fs/file.rs b/proxmox-sys/src/fs/file.rs
> index ac513891..3b20c4ea 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`
> + 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 `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 result = nix::unistd::linkat(
> + None,
> + &temp_file_name,
> + None,
> + &path.to_path_buf(),
> + nix::unistd::LinkatFlags::NoSymlinkFollow,
> + );
> + 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
>
>
[-- Attachment #1.2: Type: text/html, Size: 5321 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
next prev parent reply other threads:[~2024-07-11 17:11 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-11 17:07 Christoph Heiss
2024-07-11 17:11 ` Turkijan [this message]
2024-07-11 19:39 ` Dietmar Maurer
2024-07-12 7:37 ` Christian Ebner
2024-07-12 7:44 ` Christoph Heiss
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=CAFtnzVeytCY22pYY96eXio54JvhdfUpZ8UQw7MH2YAVAcwTgdQ@mail.gmail.com \
--to=turkijan083@gmail.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.