all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [PATCH 1/2] add tempfile helper without O_TMPFILE or memfd_create
Date: Thu, 23 Jul 2020 12:58:43 +0200	[thread overview]
Message-ID: <1595500927.o7a7kiv177.astroid@nora.none> (raw)
In-Reply-To: <20200723093824.20056-1-m.limbeck@proxmox.com>

On July 23, 2020 11:38 am, Mira Limbeck wrote:
> As WSL does not support O_TMPFILE nor memfd_create, we need a different
> way to create tempfiles if we want to support windows host backups
> through WSL (without ADS).
> The workaround is to use mkstemp in /tmp and unlinking the file
> right after creation.
> 
> Add FIXME comment to change it back to O_TMPFILE once we no longer require
> the mkstemp workaround for windows support.
> 
> Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
> ---
>  src/tools.rs          |  1 +
>  src/tools/tempfile.rs | 13 +++++++++++++
>  2 files changed, 14 insertions(+)
>  create mode 100644 src/tools/tempfile.rs
> 
> diff --git a/src/tools.rs b/src/tools.rs
> index 44db796d..5e892d41 100644
> --- a/src/tools.rs
> +++ b/src/tools.rs
> @@ -30,6 +30,7 @@ pub mod fs;
>  pub mod format;
>  pub mod lru_cache;
>  pub mod runtime;
> +pub mod tempfile;
>  pub mod ticket;
>  pub mod timer;
>  pub mod statistics;
> diff --git a/src/tools/tempfile.rs b/src/tools/tempfile.rs
> new file mode 100644
> index 00000000..a91ce806
> --- /dev/null
> +++ b/src/tools/tempfile.rs
> @@ -0,0 +1,13 @@
> +use nix::unistd::{mkstemp, unlink};
> +use std::fs::File;
> +use std::os::unix::io::FromRawFd;
> +use anyhow;
> +
> +// FIXME change to O_TMPFILE once a native windows backup client exists

shouldn't we try O_TMPFILE first, and only if it fails fall back to 
mkstemp? or guard this with a feature ("wsl")? also not sure whether all 
current (and future) callers are okay with the different semantics (with 
mkstemp the file is briefly available via a path even if you unlink it 
directly afterwards..).

it also might be better off in proxmox instead of proxmox-backup, it's a 
very generic function after all..

> +pub fn tempfile() -> anyhow::Result<File> {
> +    let (fd, path) = mkstemp("/tmp/proxmox-backup-client_XXXXXX")?;
> +    unlink(path.as_path())?;
> +    let file = unsafe { File::from_raw_fd(fd) };
> +
> +    Ok(file)
> +}
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 




      parent reply	other threads:[~2020-07-23 10:58 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-23  9:38 Mira Limbeck
2020-07-23  9:38 ` [pbs-devel] [PATCH 2/2] replace O_TMPFILE file creation with tempfile() Mira Limbeck
2020-07-23 10:58 ` Fabian Grünbichler [this message]

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=1595500927.o7a7kiv177.astroid@nora.none \
    --to=f.gruenbichler@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