Re: [PATCH v1 proxmox 2/2] fix #7690: proxmox-sys: close fd before rename/unlink in replace_file

[Christian Ebner <c.ebner@proxmox.com>]

On 6/17/26 12:49 PM, Robert Obkircher wrote:
> Fix the rename operation on WORM file systems and allow FUSE file
> systems to unlink without creating temporary .fuse_hidden files.

question: what about other helpers using a similar pattern when removing 
a temp file in error case, e.g. unlink() after file content writing or 
fsync() failed while still holding an open file descriptor in 
atomic_open_or_create_file()? These will produce the .fuse_hidden files 
as well?

> Closing the fd earlier shouldn't have any noticeable effects on normal
> file systems, because the path-based rename and unlink operations are
> completely separate from the content.
> 
> Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7690
> Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
> ---
>   proxmox-sys/src/fs/file.rs | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/proxmox-sys/src/fs/file.rs b/proxmox-sys/src/fs/file.rs
> index 533e0896..03223e5a 100644
> --- a/proxmox-sys/src/fs/file.rs
> +++ b/proxmox-sys/src/fs/file.rs
> @@ -180,6 +180,7 @@ pub fn replace_file<P: AsRef<Path>>(
>       let (mut file, tmp_path) = make_tmp_file(&path, options)?;
>   
>       if let Err(err) = file.write_all(data) {
> +        drop(file);
>           let _ = unistd::unlink(&tmp_path);
>           bail!("write failed: {}", err);
>       }
> @@ -187,11 +188,16 @@ pub fn replace_file<P: AsRef<Path>>(
>       if fsync {
>           // make sure data is on disk
>           if let Err(err) = nix::unistd::fsync(file.as_raw_fd()) {
> +            drop(file);
>               let _ = unistd::unlink(&tmp_path);
>               bail!("fsync failed: {}", err);
>           }
>       }
>   
> +    // Allow WORM file systems to commit the contents before the rename
> +    // and prevent temporary .fuse_hidden* files created by unlink.
> +    drop(file);
> +
>       if let Err(err) = std::fs::rename(&tmp_path, &path) {
>           let _ = unistd::unlink(&tmp_path);
>           bail!(