From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 7711D76F91 for ; Mon, 19 Jul 2021 12:44:36 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6D587252BE for ; Mon, 19 Jul 2021 12:44:36 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 92A49252B3 for ; Mon, 19 Jul 2021 12:44:35 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 66E37415DF for ; Mon, 19 Jul 2021 12:44:35 +0200 (CEST) Date: Mon, 19 Jul 2021 12:44:27 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion References: <20210716082834.2354163-1-dietmar@proxmox.com> In-Reply-To: <<20210716082834.2354163-1-dietmar@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1626690094.9fpnyum3i1.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.489 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment 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 1/2] new helper atomic_open_or_create_file() 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: , X-List-Received-Date: Mon, 19 Jul 2021 10:44:36 -0000 one small nit, otherwise this looks okay (and DOES set the permissions). On July 16, 2021 10:28 am, Dietmar Maurer wrote: > --- > proxmox/src/tools/fs.rs | 86 ++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 85 insertions(+), 1 deletion(-) >=20 > diff --git a/proxmox/src/tools/fs.rs b/proxmox/src/tools/fs.rs > index 12e96bd..2a93b30 100644 > --- a/proxmox/src/tools/fs.rs > +++ b/proxmox/src/tools/fs.rs > @@ -12,9 +12,10 @@ use nix::errno::Errno; > use nix::fcntl::OFlag; > use nix::sys::stat; > use nix::unistd::{self, Gid, Uid}; > +use nix::NixPath; > use serde_json::Value; > =20 > -use crate::sys::error::SysResult; > +use crate::sys::error::{SysError, SysResult}; > use crate::sys::timer; > use crate::tools::fd::Fd; > use crate::try_block; > @@ -187,6 +188,89 @@ pub fn replace_file>( > Ok(()) > } > =20 > +/// Like open(2), but allows setting initial data, perm, owner and group > +/// > +/// Since we need to initialize the file, we also need a solid slow > +/// path where we create the file. In order to avoid races, we create > +/// it in a temporary location and rotate it in place. > +pub fn atomic_open_or_create_file>( > + path: P, > + mut oflag: OFlag, > + initial_data: &[u8], > + options: CreateOptions, > +) -> Result { > + let path =3D path.as_ref(); > + > + if oflag.contains(OFlag::O_TMPFILE) { > + bail!("open {:?} failed - unsupported OFlag O_TMPFILE", path); > + } > + > + oflag.remove(OFlag::O_CREAT); // we want to handle CREAT ourselfes > + > + // Note: 'mode' is ignored, because oflag does not contain O_CREAT o= r O_TMPFILE > + match nix::fcntl::open(path, oflag, stat::Mode::empty()) { > + Ok(fd) =3D> return Ok(unsafe { File::from_raw_fd(fd) }), > + Err(err) =3D> { > + if err.not_found() { > + // fall thrue - try to create the file > + } else { > + bail!("open {:?} failed - {}", path, err); > + } > + } > + } > + > + let (mut file, temp_file_name) =3D make_tmp_file(path, options)?; so after this point we have a temp file that requires cleanup > + > + if !initial_data.is_empty() { > + file.write_all(initial_data).map_err(|err| { > + let _ =3D nix::unistd::unlink(&temp_file_name); > + format_err!( > + "writing initial data to {:?} failed - {}", > + temp_file_name, > + err, > + ) > + })?; > + } > + > + // 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 { > + let rc =3D libc::renameat2( > + libc::AT_FDCWD, > + c_file_name.as_ptr(), > + libc::AT_FDCWD, > + new_path.as_ptr(), > + libc::RENAME_NOREPLACE, > + ); > + nix::errno::Errno::result(rc) > + })? > + })?; but here we bubble up the outer Result if it's an error, without any=20 cleanup. > + > + match rename_result { > + 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); > + > + if err.already_exists() { > + match nix::fcntl::open(path, oflag, stat::Mode::empty())= { > + Ok(fd) =3D> Ok(unsafe { File::from_raw_fd(fd) }), > + Err(err) =3D> bail!("open {:?} failed - {}", path, e= rr), > + } > + } else { > + bail!( > + "failed to move file at {:?} into place at {:?} - {}= ", > + temp_file_name, > + path, > + err > + ); > + } > + } > + } > +} > + > /// Change ownership of an open file handle > pub fn fchown(fd: RawFd, owner: Option, group: Option) -> Resu= lt<(), Error> { > // According to the POSIX specification, -1 is used to indicate that= owner and group > --=20 > 2.30.2 >=20 >=20 > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel >=20 >=20 >=20