From: Maximiliano Sandoval <m.sandoval@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH pbs-client v4] fix #3699: client: prefer xdg cache directory for tmp files
Date: Mon, 12 Feb 2024 11:30:40 +0100 [thread overview]
Message-ID: <20240212103040.222591-1-m.sandoval@proxmox.com> (raw)
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 <m.sandoval@proxmox.com>
---
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<DataBlobReader<'_, File>, 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<DynamicIndexReader, 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?;
@@ -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<FixedIndexReader, 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?;
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<Mutex<HashSet<[u8; 32]>>>,
) -> Result<FixedIndexReader, 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()?;
let param = json!({ "archive-name": archive_name });
self.h2
@@ -562,11 +557,7 @@ impl BackupWriter {
manifest: &BackupManifest,
known_chunks: Arc<Mutex<HashSet<[u8; 32]>>>,
) -> Result<DynamicIndexReader, 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()?;
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<std::fs::File> {
+ static TMP_PATH: OnceLock<std::path::PathBuf> = 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<Value, Error> {
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
next reply other threads:[~2024-02-12 10:30 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-12 10:30 Maximiliano Sandoval [this message]
2024-02-12 10:37 ` Maximiliano Sandoval
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240212103040.222591-1-m.sandoval@proxmox.com \
--to=m.sandoval@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal