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 4F5B590912 for ; Mon, 12 Feb 2024 11:30:44 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 30F5716B5E for ; Mon, 12 Feb 2024 11:30:44 +0100 (CET) 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 for ; Mon, 12 Feb 2024 11:30:42 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 235DB4797C for ; Mon, 12 Feb 2024 11:30:42 +0100 (CET) From: Maximiliano Sandoval To: pbs-devel@lists.proxmox.com Date: Mon, 12 Feb 2024 11:30:40 +0100 Message-Id: <20240212103040.222591-1-m.sandoval@proxmox.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.002 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy 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 T_SCC_BODY_TEXT_LINE -0.01 - Subject: [pbs-devel] [PATCH pbs-client v4] fix #3699: client: prefer xdg cache directory for tmp files 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, 12 Feb 2024 10:30:44 -0000 Adds a helper to create temporal files in XDG_CACHE_HOME. If we cannot create a file there, we fallback to /tmp as before. Note that the temporary files stored by the client might grow arbitrarily in size, making XDG_RUNTIME_DIR a less desirable option. Citing the Arch wiki [1]: > Should not store large files as it may be mounted as a tmpfs. [1] https://wiki.archlinux.org/title/XDG_Base_Directory Signed-off-by: Maximiliano Sandoval --- Differences from v3: - Link create_tmp_file in docs - Improve commit message Differences from v2: - Files are created at XDG_CACHE_HOME directly - If opening fails, we try opening at /tmp - We check if the directory exists as part of the error handling of open() pbs-client/src/backup_reader.rs | 31 ++++++++-------------------- pbs-client/src/backup_writer.rs | 13 ++---------- pbs-client/src/tools/mod.rs | 26 +++++++++++++++++++++++ proxmox-backup-client/src/catalog.rs | 19 +++-------------- 4 files changed, 40 insertions(+), 49 deletions(-) diff --git a/pbs-client/src/backup_reader.rs b/pbs-client/src/backup_reader.rs index 36d8ebcf..4706abc7 100644 --- a/pbs-client/src/backup_reader.rs +++ b/pbs-client/src/backup_reader.rs @@ -1,7 +1,6 @@ use anyhow::{format_err, Error}; use std::fs::File; use std::io::{Seek, SeekFrom, Write}; -use std::os::unix::fs::OpenOptionsExt; use std::sync::Arc; use futures::future::AbortHandle; @@ -141,18 +140,14 @@ impl BackupReader { /// Download a .blob file /// - /// This creates a temporary file in /tmp (using O_TMPFILE). The data is verified using - /// the provided manifest. + /// This creates a temporary file (See [`crate::tools::create_tmp_file`] for + /// details). The data is verified using the provided manifest. pub async fn download_blob( &self, manifest: &BackupManifest, name: &str, ) -> Result, Error> { - let mut tmpfile = std::fs::OpenOptions::new() - .write(true) - .read(true) - .custom_flags(libc::O_TMPFILE) - .open("/tmp")?; + let mut tmpfile = crate::tools::create_tmp_file()?; self.download(name, &mut tmpfile).await?; @@ -167,18 +162,14 @@ impl BackupReader { /// Download dynamic index file /// - /// This creates a temporary file in /tmp (using O_TMPFILE). The index is verified using - /// the provided manifest. + /// This creates a temporary file (See [`crate::tools::create_tmp_file`] for + /// details). The index is verified using the provided manifest. pub async fn download_dynamic_index( &self, manifest: &BackupManifest, name: &str, ) -> Result { - let mut tmpfile = std::fs::OpenOptions::new() - .write(true) - .read(true) - .custom_flags(libc::O_TMPFILE) - .open("/tmp")?; + let mut tmpfile = crate::tools::create_tmp_file()?; self.download(name, &mut tmpfile).await?; @@ -194,18 +185,14 @@ impl BackupReader { /// Download fixed index file /// - /// This creates a temporary file in /tmp (using O_TMPFILE). The index is verified using - /// the provided manifest. + /// This creates a temporary file (See [`crate::tools::create_tmp_file`] for + /// details). The index is verified using the provided manifest. pub async fn download_fixed_index( &self, manifest: &BackupManifest, name: &str, ) -> Result { - let mut tmpfile = std::fs::OpenOptions::new() - .write(true) - .read(true) - .custom_flags(libc::O_TMPFILE) - .open("/tmp")?; + let mut tmpfile = crate::tools::create_tmp_file()?; self.download(name, &mut tmpfile).await?; diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs index 8a03d8ea..f184c750 100644 --- a/pbs-client/src/backup_writer.rs +++ b/pbs-client/src/backup_writer.rs @@ -1,6 +1,5 @@ use std::collections::HashSet; use std::future::Future; -use std::os::unix::fs::OpenOptionsExt; use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; @@ -523,11 +522,7 @@ impl BackupWriter { manifest: &BackupManifest, known_chunks: Arc>>, ) -> Result { - let mut tmpfile = std::fs::OpenOptions::new() - .write(true) - .read(true) - .custom_flags(libc::O_TMPFILE) - .open("/tmp")?; + let mut tmpfile = crate::tools::create_tmp_file()?; let param = json!({ "archive-name": archive_name }); self.h2 @@ -562,11 +557,7 @@ impl BackupWriter { manifest: &BackupManifest, known_chunks: Arc>>, ) -> Result { - let mut tmpfile = std::fs::OpenOptions::new() - .write(true) - .read(true) - .custom_flags(libc::O_TMPFILE) - .open("/tmp")?; + let mut tmpfile = crate::tools::create_tmp_file()?; let param = json!({ "archive-name": archive_name }); self.h2 diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs index 1b0123a3..eeb24114 100644 --- a/pbs-client/src/tools/mod.rs +++ b/pbs-client/src/tools/mod.rs @@ -3,8 +3,10 @@ use std::collections::HashMap; use std::env::VarError::{NotPresent, NotUnicode}; use std::fs::File; use std::io::{BufRead, BufReader}; +use std::os::unix::fs::OpenOptionsExt; use std::os::unix::io::FromRawFd; use std::process::Command; +use std::sync::OnceLock; use anyhow::{bail, format_err, Context, Error}; use serde_json::{json, Value}; @@ -526,3 +528,27 @@ pub fn place_xdg_file( .and_then(|base| base.place_config_file(file_name).map_err(Error::from)) .with_context(|| format!("failed to place {} in xdg home", description)) } + +/// Creates a temporary file (with O_TMPFILE) in XDG_CACHE_HOME. If we cannot +/// create the file there it will be created in /tmp instead. +pub fn create_tmp_file() -> std::io::Result { + static TMP_PATH: OnceLock = OnceLock::new(); + let tmp_path = TMP_PATH.get_or_init(|| { + xdg::BaseDirectories::new() + .map(|base| base.get_cache_home()) + .unwrap_or_else(|_| std::path::PathBuf::from("/tmp")) + }); + + let mut open_opts_binding = std::fs::OpenOptions::new(); + let builder = open_opts_binding + .write(true) + .read(true) + .custom_flags(libc::O_TMPFILE); + builder.open(tmp_path).or_else(|err| { + if tmp_path != std::path::Path::new("/tmp") { + builder.open("/tmp") + } else { + Err(err) + } + }) +} diff --git a/proxmox-backup-client/src/catalog.rs b/proxmox-backup-client/src/catalog.rs index 72b22e67..f3bc4ede 100644 --- a/proxmox-backup-client/src/catalog.rs +++ b/proxmox-backup-client/src/catalog.rs @@ -1,5 +1,4 @@ use std::io::{Seek, SeekFrom}; -use std::os::unix::fs::OpenOptionsExt; use std::sync::Arc; use anyhow::{bail, format_err, Error}; @@ -104,11 +103,7 @@ async fn dump_catalog(param: Value) -> Result { let mut reader = BufferedDynamicReader::new(index, chunk_reader); - let mut catalogfile = std::fs::OpenOptions::new() - .write(true) - .read(true) - .custom_flags(libc::O_TMPFILE) - .open("/tmp")?; + let mut catalogfile = pbs_client::tools::create_tmp_file()?; std::io::copy(&mut reader, &mut catalogfile) .map_err(|err| format_err!("unable to download catalog - {}", err))?; @@ -196,11 +191,7 @@ async fn catalog_shell(param: Value) -> Result<(), Error> { ) .await?; - let mut tmpfile = std::fs::OpenOptions::new() - .write(true) - .read(true) - .custom_flags(libc::O_TMPFILE) - .open("/tmp")?; + let mut tmpfile = pbs_client::tools::create_tmp_file()?; let (manifest, _) = client.download_manifest().await?; manifest.check_fingerprint(crypt_config.as_ref().map(Arc::as_ref))?; @@ -240,11 +231,7 @@ async fn catalog_shell(param: Value) -> Result<(), Error> { most_used, ); let mut reader = BufferedDynamicReader::new(index, chunk_reader); - let mut catalogfile = std::fs::OpenOptions::new() - .write(true) - .read(true) - .custom_flags(libc::O_TMPFILE) - .open("/tmp")?; + let mut catalogfile = pbs_client::tools::create_tmp_file()?; std::io::copy(&mut reader, &mut catalogfile) .map_err(|err| format_err!("unable to download catalog - {}", err))?; -- 2.39.2