From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id DB0BD1FF2D5 for ; Fri, 12 Jul 2024 10:15:57 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 679F93E381; Fri, 12 Jul 2024 10:16:22 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1720772172; x=1721376972; 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=m5GSFDXRpThvQ4sfctKM/mxfJ5EaooRbhHMXp85E+ls=; b=V37XNBoivjjLwoM8t5Inkz9hL5K2C/0AeJhUw6q4QPQ5ds038xYwO1/DuGXITinPqH 2SXdJYbKJH6Xfzzz/uyPNnTG+rrcND2zrwZ6ERXbghH59KUu6MjzjeVHTMWpm7z7CjQ1 4LXpUVQcOoSJJALmODvU3ug4ynW6H5/2aQIRqpy9w4idjsF5nHD24PDM7b6WwYQqE8Xi sWD59JuPcYTOOXTR8v9T41pxw4syqdCKk1bsf9gZ+p8DLPQrgQQm2inSnmb0dp1h5Di2 aDXsbzD3MQnkhHdNe88WkPwp3jANUFVxJSVZya5QxoD0iIQyVJifV1fQPP5ZrzPTwih0 UTEw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720772172; x=1721376972; 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=m5GSFDXRpThvQ4sfctKM/mxfJ5EaooRbhHMXp85E+ls=; b=RQOPt8awzAdkOuO4WNXtfzJrH8qYa2idBjwNxHrvWvwyXj1DcZm/t2MVEmnSkUCtOD t79YLGYBGFZlW7UYigLZgS+oUUHn0EDOGsVw7rRNSMIByvhIRLyNHChiaA9FZWvtC5Mi 8oP31c4vx805TPyd8jXH4QHnamFjG+xd/4HcighEtI/gCYFLX96nfGavVcTl3Rlj5iB9 EUsd/d3OzIE3wNGr0YkD/vBtXWoyOC98ZIS6zHX7/X0KMloQzsBrpOqqcm2sqmFwKPCo SEcJiqxSZexuIKodMLyO5eCBqPN80N1653EJz4HViUMWNNhFqCa892RComwvqW6V0Atn WxQg== X-Gm-Message-State: AOJu0YwKrXw+VpSY2Wb79yks9bsKnMDgcpwQylVsFFoAqk5Im6uBl6nK yYYwOq/0b8DgLq8PRAOU/XBXSnAzYPFdCMtkly3J5eFiMxOjHNu6FHEajLjHa1tTLhJXM/NK1OB nObqlWzF58AC8Lcc0h5Xa8deh79ZBg93tdm0YLw== X-Google-Smtp-Source: AGHT+IGRY3L7zio6WLGP2A2gRZ6AEcrSHGsViR4P31bSuiQxwlZa+VdJ2PMnI0qFFrkZopbJ9I67kJYXEtVkOM7x79M= X-Received: by 2002:a05:6a21:4d81:b0:1bd:1af0:385c with SMTP id adf61e73a8af0-1c29820c0a3mr11659037637.9.1720772172012; Fri, 12 Jul 2024 01:16:12 -0700 (PDT) MIME-Version: 1.0 References: <20240712075449.238010-1-c.heiss@proxmox.com> In-Reply-To: <20240712075449.238010-1-c.heiss@proxmox.com> From: Turkijan Date: Fri, 12 Jul 2024 04:16:29 -0400 Message-ID: To: Proxmox Backup Server development discussion X-SPAM-LEVEL: Spam detection results: 0 AWL 0.138 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [file.rs, proxmox.com] Subject: Re: [pbs-devel] [PATCH proxmox v2] 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="===============5342384192258006372==" Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" --===============5342384192258006372== Content-Type: multipart/alternative; boundary="0000000000000d65e5061d087e62" --0000000000000d65e5061d087e62 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable how to update this for my test proxmox backup server? On Fri, Jul 12, 2024 at 3:55=E2=80=AFAM Christoph Heiss 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 > --- > 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>( > > // 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` > - // 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 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 `linkat` + `unlink` instead > + let result =3D nix::unistd::linkat( > + None, > + &temp_file_name, > + None, > + &path.to_path_buf(), > + nix::unistd::AtFlags::empty(), > + ); > + 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 > > --0000000000000d65e5061d087e62 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
how to update this for my test proxmox backup server?
On Fr= i, Jul 12, 2024 at 3:55=E2=80=AFAM Christoph Heiss <c.heiss@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@proxmox.com>
---
Changes v1 -> v2:
=C2=A0 * use `AtFlags` instead of deprecated `LinkatFlags` (thanks Christia= n!)
=C2=A0 * use an empty flag set, as AT_SYMLINK_NOFOLLOW is the default anywa= y
=C2=A0 =C2=A0 for linkat()
=C2=A0 * adjust accompanying comment to mention `linkat` instead of `link`<= br>
=C2=A0proxmox-sys/src/fs/file.rs | 53 +++++++++++++++++---------------------
=C2=A01 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;
=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 =C2=A0 =C2=A0 =C2=A0 =C2=A0 // so we just use `link` + `unli= nk` 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 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 // so we just use `linkat` + `un= link` instead
+=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::AtFla= gs::empty(),
+=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

--0000000000000d65e5061d087e62-- --===============5342384192258006372== 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 --===============5342384192258006372==--