From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 2B2571FF2CF for ; Thu, 11 Jul 2024 19:11:12 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9C74D37A2D; Thu, 11 Jul 2024 19:11:35 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1720717855; x=1721322655; darn=lists.proxmox.com; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date:message-id:reply-to; bh=BJXEFhQ2l9gP0DnIKN6qtIiSl2wOBXV71tv6kFZkcVg=; b=FLB0u+3D7bRKJyWEqRXgnVRjuLx1tHrI+J75hO/f0VWNL6cOP5iIrJ0RCZ/uWL8UHq alFD6Pm7LWOf9GF+cRV1ASlFZk1a3dX4CI/p9JB2kx4v0NL+2V7TRM+An0+YtYx8nNVu cZ48LP287oho2xudOXFSUP0hsFDeqn2no5I7PF7rQiulvSJbNoMWqTXpIrifnMs2KQqS KwsSzCOdgrKrVCP4sHVvB9WInfspuMMaBjWjFo1afnWPvymti02KL6X/SVbtNdwkg1v4 jDb4SMBIPpEjiUuBc+mNDQGnLFjwqv/Z3UyLwvTUV722ByY3lH5O4dEmFsh43p6SDmLn EGkg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720717855; x=1721322655; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=BJXEFhQ2l9gP0DnIKN6qtIiSl2wOBXV71tv6kFZkcVg=; b=Iq5aeHMo/jxT1RTZ7Y9ifRvOnWkNfZHvciiWeQ9LJ6+FScT6X9cjedGtv7XVZNYIiY opCsV8L8QqSajc+f59oNHwNCVtM8y9L3kGMvPIGgD+sr0OtT44+3cb992576FcoDNKJm ofBPceplIHWYDY1kqHd/Y703ibg09wHC+hmfmImZtqafTaJgMX7wVBA0rBV1T9iklb3I c8iS6JDpM6m5MCuHhhNCEdaI34tdqNK3p66+sye5VAmR4wuSY/pRFheWqvs3LQIDWvMK yBWgHwao7lhWQZpC68/jUdhU+fnmNG5AV0yl7R47ONIoGiz5EqkYJzb2b+QIPT4Zi/cC /ASQ== X-Gm-Message-State: AOJu0YwFAUzFMrMUz3M/brUfPVb7REAi9ipXHPdYKGzhlN68SC9cSrx7 k7vEMbXD3gLeYR8KdAR+i+Vx1jC0EatGiF+6DBHT0SaHMMjELKuHeMkIA09SxnRY2/AZvS4YEwC Ka1lfJXVYeuzE/2jcys5g9eJmk3VDoA78NGPKng== X-Google-Smtp-Source: AGHT+IFXQCCU+B56f1rUQcYtY+J8qtiJrbRnf6Q6tPJaVb2LzKNb/uwcenr/u7A3rtqw9P+SBKzs4Y9lSN0H/HdxBNY= X-Received: by 2002:a17:90a:c386:b0:2c9:7aea:2188 with SMTP id 98e67ed59e1d1-2ca35be79a2mr7264819a91.2.1720717854806; Thu, 11 Jul 2024 10:10:54 -0700 (PDT) MIME-Version: 1.0 References: <20240711170723.1115046-1-c.heiss@proxmox.com> In-Reply-To: <20240711170723.1115046-1-c.heiss@proxmox.com> From: Turkijan Date: Thu, 11 Jul 2024 13:11:12 -0400 Message-ID: To: Proxmox Backup Server development discussion X-SPAM-LEVEL: Spam detection results: 0 AWL 0.147 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain DMARC_PASS -0.1 DMARC pass policy FREEMAIL_ENVFROM_END_DIGIT 0.25 Envelope-from freemail username ends in digit FREEMAIL_FROM 0.001 Sender email is commonly abused enduser mail provider GB_FREEMAIL_NUM 1 Freemail spammy address HTML_MESSAGE 0.001 HTML included in message POISEN_SPAM_PILL_4 0.1 random spam to be learned in bayes RCVD_IN_DNSWL_NONE -0.0001 Sender listed at https://www.dnswl.org/, no trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH proxmox] sys: file: use renameat2() from `nix` crate X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: multipart/mixed; boundary="===============0501817387647147305==" Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" --===============0501817387647147305== Content-Type: multipart/alternative; boundary="0000000000007e9c55061cfbd84e" --0000000000007e9c55061cfbd84e Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Have you changed dev package? On Thu, Jul 11, 2024 at 1:08=E2=80=AFPM Christoph Heiss 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 > --- > 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>( > > // rotate the file into place, but use `RENAME_NOREPLACE`, so in cas= e > 2 processes race against > // the initialization, the first one wins! > - let rename_result =3D 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) =3D> (), // dumb file system, try > `link`+`unlink` > - other =3D> return other, > - }; > - // but some file systems don't support `RENAME_NOREPLACE` > + let rename_result =3D match nix::fcntl::renameat2( > + None, // AT_FDCWD > + &temp_file_name, > + None, // AT_FDCWD > + path, > + nix::fcntl::RenameFlags::RENAME_NOREPLACE, > + ) { > + Err(Errno::EINVAL) =3D> { > + // some file systems don't support `RENAME_NOREPLACE` > // so we just use `link` + `unlink` instead > - let result =3D Errno::result(libc::link(c_file_name.as_ptr()= , > new_path.as_ptr())); > - let _ =3D libc::unlink(c_file_name.as_ptr()); > + let result =3D nix::unistd::linkat( > + None, > + &temp_file_name, > + None, > + &path.to_path_buf(), > + nix::unistd::LinkatFlags::NoSymlinkFollow, > + ); > + let _ =3D nix::unistd::unlink(&temp_file_name); > result > - }) > - }); > + } > + other =3D> other, > + }; > > match rename_result { > - Ok(Ok(Ok(_))) =3D> Ok(file), > - Ok(Ok(Err(err))) =3D> { > + Ok(_) =3D> Ok(file), > + Err(err) =3D> { > // if another process has already raced ahead and created > // the file, let's just open theirs instead: > let _ =3D nix::unistd::unlink(&temp_file_name); > @@ -308,14 +309,6 @@ pub fn atomic_open_or_create_file>( > ); > } > } > - Ok(Err(err)) =3D> { > - let _ =3D nix::unistd::unlink(&temp_file_name); > - bail!("with_nix_path {:?} failed - {}", path, err); > - } > - Err(err) =3D> { > - let _ =3D 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 > > --0000000000007e9c55061cfbd84e Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Have you changed dev package?


On Thu, Jul 11= , 2024 at 1:08=E2=80=AFPM Christoph Heiss <c.heiss@proxmox.com> wrote:
Saves us from converting the paths to raw C st= rings ourselves, thus
simplifying it quite a bit.

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
=C2=A0proxmox-sys/src/fs/file.rs | 51 ++++++++++++++++----------------------
=C2=A01 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;
=C2=A0use nix::fcntl::OFlag;
=C2=A0use nix::sys::stat;
=C2=A0use nix::unistd;
-use nix::NixPath;
=C2=A0use serde_json::Value;

=C2=A0use proxmox_lang::try_block;
@@ -266,30 +265,32 @@ pub fn atomic_open_or_create_file<P: AsRef<Path= >>(

=C2=A0 =C2=A0 =C2=A0// rotate the file into place, but use `RENAME_NOREPLAC= E`, so in case 2 processes race against
=C2=A0 =C2=A0 =C2=A0// the initialization, the first one wins!
-=C2=A0 =C2=A0 let rename_result =3D temp_file_name.with_nix_path(|c_file_n= ame| {
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 path.with_nix_path(|new_path| unsafe {
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 // This also works on file syste= ms which don't support hardlinks (eg. vfat)
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 match Errno::result(libc::rename= at2(
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 libc::AT_FDCWD, -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 c_file_name.as_ptr= (),
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 libc::AT_FDCWD, -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 new_path.as_ptr(),=
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 libc::RENAME_NOREP= LACE,
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 )) {
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Err(Errno::EINVAL)= =3D> (), // dumb file system, try `link`+`unlink`
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 other =3D> retu= rn other,
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 };
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 // but some file systems don'= ;t support `RENAME_NOREPLACE`
+=C2=A0 =C2=A0 let rename_result =3D match nix::fcntl::renameat2(
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 None, // AT_FDCWD
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 &temp_file_name,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 None, // AT_FDCWD
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 path,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 nix::fcntl::RenameFlags::RENAME_NOREPLACE,
+=C2=A0 =C2=A0 ) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 Err(Errno::EINVAL) =3D> {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 // some file systems don't s= upport `RENAME_NOREPLACE`
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0// so we just use `link` + = `unlink` instead
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 let result =3D Errno::result(lib= c::link(c_file_name.as_ptr(), new_path.as_ptr()));
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 let _ =3D libc::unlink(c_file_na= me.as_ptr());
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 let result =3D nix::unistd::link= at(
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 None,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 &temp_file_nam= e,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 None,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 &path.to_path_= buf(),
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 nix::unistd::Linka= tFlags::NoSymlinkFollow,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 );
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 let _ =3D nix::unistd::unlink(&a= mp;temp_file_name);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0result
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 })
-=C2=A0 =C2=A0 });
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 other =3D> other,
+=C2=A0 =C2=A0 };

=C2=A0 =C2=A0 =C2=A0match rename_result {
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 Ok(Ok(Ok(_))) =3D> Ok(file),
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 Ok(Ok(Err(err))) =3D> {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 Ok(_) =3D> Ok(file),
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 Err(err) =3D> {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0// if another process has a= lready raced ahead and created
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0// the file, let's just= open theirs instead:
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0let _ =3D nix::unistd::unli= nk(&temp_file_name);
@@ -308,14 +309,6 @@ pub fn atomic_open_or_create_file<P: AsRef<Path&= gt;>(
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 Ok(Err(err)) =3D> {
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 let _ =3D nix::unistd::unlink(&a= mp;temp_file_name);
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 bail!("with_nix_path {:?} f= ailed - {}", path, err);
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 Err(err) =3D> {
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 let _ =3D nix::unistd::unlink(&a= mp;temp_file_name);
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 bail!("with_nix_path {:?} f= ailed - {}", temp_file_name, err);
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
=C2=A0 =C2=A0 =C2=A0}
=C2=A0}

--
2.45.1



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

--0000000000007e9c55061cfbd84e-- --===============0501817387647147305== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel --===============0501817387647147305==--