[pbs-devel] [PATCH proxmox 1/2] new helper atomic_open_or_create_file()
Fabian Grünbichler
f.gruenbichler at proxmox.com
Mon Jul 19 12:44:27 CEST 2021
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(-)
>
> 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;
>
> -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<P: AsRef<Path>>(
> Ok(())
> }
>
> +/// 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<P: AsRef<Path>>(
> + path: P,
> + mut oflag: OFlag,
> + initial_data: &[u8],
> + options: CreateOptions,
> +) -> Result<File, Error> {
> + let path = 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 or O_TMPFILE
> + match nix::fcntl::open(path, oflag, stat::Mode::empty()) {
> + Ok(fd) => return Ok(unsafe { File::from_raw_fd(fd) }),
> + Err(err) => {
> + if err.not_found() {
> + // fall thrue - try to create the file
> + } else {
> + bail!("open {:?} failed - {}", path, err);
> + }
> + }
> + }
> +
> + let (mut file, temp_file_name) = 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 _ = 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 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 {
> + let rc = 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
cleanup.
> +
> + match rename_result {
> + 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);
> +
> + if err.already_exists() {
> + match nix::fcntl::open(path, oflag, stat::Mode::empty()) {
> + Ok(fd) => Ok(unsafe { File::from_raw_fd(fd) }),
> + Err(err) => bail!("open {:?} failed - {}", path, err),
> + }
> + } 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<Uid>, group: Option<Gid>) -> Result<(), Error> {
> // According to the POSIX specification, -1 is used to indicate that owner and group
> --
> 2.30.2
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
>
More information about the pbs-devel
mailing list