* [pbs-devel] [PATCH proxmox 1/2] proxmox-compression: make ZstdEncoder stream a bit more generic
2022-05-31 11:17 [pbs-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve Dominik Csapak
@ 2022-05-31 11:17 ` Dominik Csapak
2022-07-05 11:47 ` [pbs-devel] applied-both: " Wolfgang Bumiller
2022-05-31 11:17 ` [pbs-devel] [PATCH proxmox 2/2] proxmox-compression: add 'tar_directory' Dominik Csapak
` (5 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Dominik Csapak @ 2022-05-31 11:17 UTC (permalink / raw)
To: pve-devel, pbs-devel
by not requiring the 'anyhow::Error' type from the source, but only
that it implements 'Into<Error>'. This way, we can also accept a stream
that produces e.g. an io::Error
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
proxmox-compression/src/zstd.rs | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/proxmox-compression/src/zstd.rs b/proxmox-compression/src/zstd.rs
index d9e5826..3d4b2ab 100644
--- a/proxmox-compression/src/zstd.rs
+++ b/proxmox-compression/src/zstd.rs
@@ -32,10 +32,11 @@ pub struct ZstdEncoder<'a, T> {
state: EncoderState,
}
-impl<'a, T, O> ZstdEncoder<'a, T>
+impl<'a, T, O, E> ZstdEncoder<'a, T>
where
- T: Stream<Item = Result<O, Error>> + Unpin,
+ T: Stream<Item = Result<O, E>> + Unpin,
O: Into<Bytes>,
+ E: Into<Error>,
{
/// Returns a new [ZstdEncoder] with default level 3
pub fn new(inner: T) -> Result<Self, io::Error> {
@@ -79,10 +80,11 @@ impl<'a, T> ZstdEncoder<'a, T> {
}
}
-impl<'a, T, O> Stream for ZstdEncoder<'a, T>
+impl<'a, T, O, E> Stream for ZstdEncoder<'a, T>
where
- T: Stream<Item = Result<O, Error>> + Unpin,
+ T: Stream<Item = Result<O, E>> + Unpin,
O: Into<Bytes>,
+ E: Into<Error>,
{
type Item = Result<Bytes, Error>;
@@ -93,7 +95,10 @@ where
match this.state {
EncoderState::Reading => {
if let Some(res) = ready!(Pin::new(&mut this.inner).poll_next(cx)) {
- let buf = res?;
+ let buf = match res {
+ Ok(buf) => buf,
+ Err(err) => return Poll::Ready(Some(Err(err.into()))),
+ };
this.input_buffer = buf.into();
this.state = EncoderState::Writing;
} else {
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [pbs-devel] applied-both: [PATCH proxmox 1/2] proxmox-compression: make ZstdEncoder stream a bit more generic
2022-05-31 11:17 ` [pbs-devel] [PATCH proxmox 1/2] proxmox-compression: make ZstdEncoder stream a bit more generic Dominik Csapak
@ 2022-07-05 11:47 ` Wolfgang Bumiller
0 siblings, 0 replies; 11+ messages in thread
From: Wolfgang Bumiller @ 2022-07-05 11:47 UTC (permalink / raw)
To: Dominik Csapak; +Cc: pve-devel, pbs-devel
applied both patches for proxmox-compression & bumped
On Tue, May 31, 2022 at 01:17:20PM +0200, Dominik Csapak wrote:
> by not requiring the 'anyhow::Error' type from the source, but only
> that it implements 'Into<Error>'. This way, we can also accept a stream
> that produces e.g. an io::Error
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> proxmox-compression/src/zstd.rs | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/proxmox-compression/src/zstd.rs b/proxmox-compression/src/zstd.rs
> index d9e5826..3d4b2ab 100644
> --- a/proxmox-compression/src/zstd.rs
> +++ b/proxmox-compression/src/zstd.rs
> @@ -32,10 +32,11 @@ pub struct ZstdEncoder<'a, T> {
> state: EncoderState,
> }
>
> -impl<'a, T, O> ZstdEncoder<'a, T>
> +impl<'a, T, O, E> ZstdEncoder<'a, T>
> where
> - T: Stream<Item = Result<O, Error>> + Unpin,
> + T: Stream<Item = Result<O, E>> + Unpin,
> O: Into<Bytes>,
> + E: Into<Error>,
> {
> /// Returns a new [ZstdEncoder] with default level 3
> pub fn new(inner: T) -> Result<Self, io::Error> {
> @@ -79,10 +80,11 @@ impl<'a, T> ZstdEncoder<'a, T> {
> }
> }
>
> -impl<'a, T, O> Stream for ZstdEncoder<'a, T>
> +impl<'a, T, O, E> Stream for ZstdEncoder<'a, T>
> where
> - T: Stream<Item = Result<O, Error>> + Unpin,
> + T: Stream<Item = Result<O, E>> + Unpin,
> O: Into<Bytes>,
> + E: Into<Error>,
> {
> type Item = Result<Bytes, Error>;
>
> @@ -93,7 +95,10 @@ where
> match this.state {
> EncoderState::Reading => {
> if let Some(res) = ready!(Pin::new(&mut this.inner).poll_next(cx)) {
> - let buf = res?;
> + let buf = match res {
> + Ok(buf) => buf,
> + Err(err) => return Poll::Ready(Some(Err(err.into()))),
> + };
> this.input_buffer = buf.into();
> this.state = EncoderState::Writing;
> } else {
> --
> 2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox 2/2] proxmox-compression: add 'tar_directory'
2022-05-31 11:17 [pbs-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve Dominik Csapak
2022-05-31 11:17 ` [pbs-devel] [PATCH proxmox 1/2] proxmox-compression: make ZstdEncoder stream a bit more generic Dominik Csapak
@ 2022-05-31 11:17 ` Dominik Csapak
2022-05-31 11:17 ` [pbs-devel] [PATCH proxmox-backup 1/2] restore-daemon: add 'format' parameter to the 'extract' handler Dominik Csapak
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Dominik Csapak @ 2022-05-31 11:17 UTC (permalink / raw)
To: pve-devel, pbs-devel
similar to 'zip_directory', this is intended to tar a local directory,
e.g. when we're in a restore vm.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
proxmox-compression/Cargo.toml | 1 +
proxmox-compression/src/tar.rs | 116 +++++++++++++++++++++++++++++++++
2 files changed, 117 insertions(+)
diff --git a/proxmox-compression/Cargo.toml b/proxmox-compression/Cargo.toml
index c5ad3fd..0618465 100644
--- a/proxmox-compression/Cargo.toml
+++ b/proxmox-compression/Cargo.toml
@@ -15,6 +15,7 @@ crc32fast = "1"
endian_trait = { version = "0.6" }
flate2 = "1.0"
futures = "0.3"
+libc = "0.2"
tokio = { version = "1.6", features = [ "fs", "io-util"] }
walkdir = "2"
tar = "0.4"
diff --git a/proxmox-compression/src/tar.rs b/proxmox-compression/src/tar.rs
index 7489e43..5aa4167 100644
--- a/proxmox-compression/src/tar.rs
+++ b/proxmox-compression/src/tar.rs
@@ -1,9 +1,12 @@
//! tar helper
+use std::collections::HashMap;
use std::io;
use std::os::unix::ffi::OsStrExt;
use std::path::{Component, Path, PathBuf};
use std::str;
+use anyhow::Error;
+
use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
use tar::{EntryType, Header};
@@ -162,3 +165,116 @@ where
}
Ok(())
}
+
+pub async fn tar_directory<W>(target: W, source: &Path) -> Result<(), Error>
+where
+ W: AsyncWrite + Unpin + Send,
+{
+ use std::os::unix::fs::{FileTypeExt, MetadataExt};
+ use walkdir::WalkDir;
+
+ let base_path = source.parent().unwrap_or_else(|| Path::new("/"));
+ let mut encoder = Builder::new(target);
+ let mut hardlinks: HashMap<u64, HashMap<u64, PathBuf>> = HashMap::new(); // dev -> inode -> first path
+
+ for entry in WalkDir::new(&source).into_iter() {
+ match entry {
+ Ok(entry) => {
+ let entry_path = entry.path().to_owned();
+ let encoder = &mut encoder;
+ let hardlinks = &mut hardlinks;
+
+ if let Err(err) = async move {
+ let entry_path_no_base = entry.path().strip_prefix(base_path)?;
+ let metadata = entry.metadata()?;
+ let mut header = Header::new_gnu();
+ header.set_mode(metadata.mode());
+ header.set_mtime(metadata.mtime() as u64);
+ header.set_uid(metadata.uid() as u64);
+ header.set_gid(metadata.gid() as u64);
+ header.set_size(0);
+ let dev = metadata.dev();
+
+ let file_type = entry.file_type();
+
+ if file_type.is_file() {
+ if metadata.nlink() > 1 {
+ let ino = metadata.ino();
+ if let Some(map) = hardlinks.get_mut(&dev) {
+ if let Some(target) = map.get(&ino) {
+ header.set_entry_type(tar::EntryType::Link);
+ encoder
+ .add_link(&mut header, entry_path_no_base, target)
+ .await?;
+ return Ok(());
+ } else {
+ map.insert(ino, entry_path_no_base.to_path_buf());
+ }
+ } else {
+ let mut map = HashMap::new();
+ map.insert(ino, entry_path_no_base.to_path_buf());
+ hardlinks.insert(dev, map);
+ }
+ }
+ let file = tokio::fs::File::open(entry.path()).await?;
+ header.set_size(metadata.size());
+ header.set_cksum();
+ encoder
+ .add_entry(&mut header, entry_path_no_base, file)
+ .await?;
+ } else if file_type.is_dir() {
+ header.set_entry_type(EntryType::Directory);
+ header.set_cksum();
+ encoder
+ .add_entry(&mut header, entry_path_no_base, tokio::io::empty())
+ .await?;
+ } else if file_type.is_symlink() {
+ let target = std::fs::read_link(entry.path())?;
+ header.set_entry_type(EntryType::Symlink);
+ encoder
+ .add_link(&mut header, entry_path_no_base, target)
+ .await?;
+ } else if file_type.is_block_device() {
+ header.set_entry_type(EntryType::Block);
+ header.set_device_major(unsafe { libc::major(dev) })?;
+ header.set_device_minor(unsafe { libc::minor(dev) })?;
+ header.set_cksum();
+ encoder
+ .add_entry(&mut header, entry_path_no_base, tokio::io::empty())
+ .await?;
+ } else if file_type.is_char_device() {
+ header.set_entry_type(EntryType::Char);
+ header.set_device_major(unsafe { libc::major(dev) })?;
+ header.set_device_minor(unsafe { libc::minor(dev) })?;
+ header.set_cksum();
+ encoder
+ .add_entry(&mut header, entry_path_no_base, tokio::io::empty())
+ .await?;
+ } else if file_type.is_fifo() {
+ header.set_entry_type(EntryType::Fifo);
+ header.set_device_major(0)?;
+ header.set_device_minor(0)?;
+ header.set_cksum();
+ encoder
+ .add_entry(&mut header, entry_path_no_base, tokio::io::empty())
+ .await?;
+ }
+ // ignore other file_types
+ Ok::<_, Error>(())
+ }
+ .await
+ {
+ eprintln!(
+ "zip: error encoding file or directory '{}': {}",
+ entry_path.display(),
+ err
+ );
+ }
+ }
+ Err(err) => {
+ eprintln!("zip: error reading directory entry: {}", err);
+ }
+ }
+ }
+ Ok(())
+}
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [pbs-devel] [PATCH proxmox-backup 1/2] restore-daemon: add 'format' parameter to the 'extract' handler
2022-05-31 11:17 [pbs-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve Dominik Csapak
2022-05-31 11:17 ` [pbs-devel] [PATCH proxmox 1/2] proxmox-compression: make ZstdEncoder stream a bit more generic Dominik Csapak
2022-05-31 11:17 ` [pbs-devel] [PATCH proxmox 2/2] proxmox-compression: add 'tar_directory' Dominik Csapak
@ 2022-05-31 11:17 ` Dominik Csapak
2022-07-05 11:39 ` Wolfgang Bumiller
2022-05-31 11:17 ` [pbs-devel] [PATCH proxmox-backup 2/2] file-restore: add 'tar' option to 'extract' command Dominik Csapak
` (3 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Dominik Csapak @ 2022-05-31 11:17 UTC (permalink / raw)
To: pve-devel, pbs-devel
this can be 'plain', 'pxar', 'zip' or 'tar.zst', and it returns the
content in the given format (with fallback to the old behaviour if not
given)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
note: needs a bumped 'proxmox-compression' in the Cargo.toml to build
.../src/proxmox_restore_daemon/api.rs | 49 ++++++++++++++++---
1 file changed, 43 insertions(+), 6 deletions(-)
diff --git a/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs b/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs
index aeb5a71d..c4977ce6 100644
--- a/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs
+++ b/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs
@@ -13,7 +13,7 @@ use serde_json::Value;
use tokio::sync::Semaphore;
use pathpatterns::{MatchEntry, MatchPattern, MatchType, Pattern};
-use proxmox_compression::zip::zip_directory;
+use proxmox_compression::{tar::tar_directory, zip::zip_directory, zstd::ZstdEncoder};
use proxmox_router::{
list_subdirs_api_method, ApiHandler, ApiMethod, ApiResponseFuture, Permission, Router,
RpcEnvironment, SubdirMap,
@@ -236,11 +236,24 @@ pub const API_METHOD_EXTRACT: ApiMethod = ApiMethod::new(
true,
&BooleanSchema::new(concat!(
"if true, return a pxar archive, otherwise either the ",
- "file content or the directory as a zip file"
+ "file content or the directory as a zip file. DEPRECATED: use 'format' instead."
))
.default(true)
.schema()
- )
+ ),
+ (
+ "format",
+ true,
+ &StringSchema::new("The desired format of the result.")
+ .format(&ApiStringFormat::Enum(&[
+ EnumEntry::new("plain", "Plain file (only works for single files)"),
+ EnumEntry::new("pxar", "PXAR archive"),
+ EnumEntry::new("zip", "ZIP archive"),
+ EnumEntry::new("tar.zst", "Zstd compressed TAR archive"),
+ ]))
+ .default("pxar")
+ .schema()
+ ),
]),
),
)
@@ -271,6 +284,11 @@ fn extract(
let path = Path::new(OsStr::from_bytes(&path[..]));
let pxar = param["pxar"].as_bool().unwrap_or(true);
+ let format = match param["format"].as_str() {
+ Some(format) => format.to_string(),
+ // FIXME: old default was plain or zip, remove that with 3.0
+ None => if pxar { "pxar".to_string() } else { String::new() }
+ };
let query_result = proxmox_async::runtime::block_in_place(move || {
let mut disk_state = crate::DISK_STATE.lock().unwrap();
@@ -290,7 +308,9 @@ fn extract(
let (mut writer, reader) = tokio::io::duplex(1024 * 64);
- if pxar {
+ let mut zstd = false;
+
+ if format == "pxar" {
tokio::spawn(async move {
let _inhibitor = _inhibitor;
let _permit = _permit;
@@ -349,12 +369,24 @@ fn extract(
error!("pxar streaming task failed - {}", err);
}
});
+ } else if format == "tar.zst" {
+ zstd = true;
+ tokio::spawn(async move {
+ let _inhibitor = _inhibitor;
+ let _permit = _permit;
+ if let Err(err) = tar_directory(&mut writer, &vm_path).await {
+ error!("file or dir streaming task failed - {}", err);
+ }
+ });
} else {
+ if format == "plain" && vm_path.is_dir() {
+ bail!("cannot stream dir with format 'plain'");
+ }
tokio::spawn(async move {
let _inhibitor = _inhibitor;
let _permit = _permit;
let result = async move {
- if vm_path.is_dir() {
+ if vm_path.is_dir() || format == "zip" {
zip_directory(&mut writer, &vm_path).await?;
Ok(())
} else if vm_path.is_file() {
@@ -377,7 +409,12 @@ fn extract(
let stream = tokio_util::io::ReaderStream::new(reader);
- let body = Body::wrap_stream(stream);
+ let body = if zstd {
+ let stream = ZstdEncoder::new(stream)?;
+ Body::wrap_stream(stream)
+ } else {
+ Body::wrap_stream(stream)
+ };
Ok(Response::builder()
.status(StatusCode::OK)
.header(header::CONTENT_TYPE, "application/octet-stream")
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup 1/2] restore-daemon: add 'format' parameter to the 'extract' handler
2022-05-31 11:17 ` [pbs-devel] [PATCH proxmox-backup 1/2] restore-daemon: add 'format' parameter to the 'extract' handler Dominik Csapak
@ 2022-07-05 11:39 ` Wolfgang Bumiller
0 siblings, 0 replies; 11+ messages in thread
From: Wolfgang Bumiller @ 2022-07-05 11:39 UTC (permalink / raw)
To: Dominik Csapak; +Cc: pve-devel, pbs-devel
On Tue, May 31, 2022 at 01:17:22PM +0200, Dominik Csapak wrote:
> this can be 'plain', 'pxar', 'zip' or 'tar.zst', and it returns the
> content in the given format (with fallback to the old behaviour if not
> given)
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> note: needs a bumped 'proxmox-compression' in the Cargo.toml to build
>
> .../src/proxmox_restore_daemon/api.rs | 49 ++++++++++++++++---
> 1 file changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs b/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs
> index aeb5a71d..c4977ce6 100644
> --- a/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs
> +++ b/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs
> @@ -13,7 +13,7 @@ use serde_json::Value;
> use tokio::sync::Semaphore;
>
> use pathpatterns::{MatchEntry, MatchPattern, MatchType, Pattern};
> -use proxmox_compression::zip::zip_directory;
> +use proxmox_compression::{tar::tar_directory, zip::zip_directory, zstd::ZstdEncoder};
> use proxmox_router::{
> list_subdirs_api_method, ApiHandler, ApiMethod, ApiResponseFuture, Permission, Router,
> RpcEnvironment, SubdirMap,
> @@ -236,11 +236,24 @@ pub const API_METHOD_EXTRACT: ApiMethod = ApiMethod::new(
> true,
> &BooleanSchema::new(concat!(
> "if true, return a pxar archive, otherwise either the ",
> - "file content or the directory as a zip file"
> + "file content or the directory as a zip file. DEPRECATED: use 'format' instead."
> ))
> .default(true)
> .schema()
> - )
> + ),
> + (
> + "format",
> + true,
> + &StringSchema::new("The desired format of the result.")
> + .format(&ApiStringFormat::Enum(&[
> + EnumEntry::new("plain", "Plain file (only works for single files)"),
> + EnumEntry::new("pxar", "PXAR archive"),
> + EnumEntry::new("zip", "ZIP archive"),
> + EnumEntry::new("tar.zst", "Zstd compressed TAR archive"),
> + ]))
> + .default("pxar")
> + .schema()
If you make an `#[api] enum RestoreFormat {}` and `use proxmox_schema::ApiType` you
can pass the enum's `RestoreFormat::API_SCHEMA` in here.
Would be nicer than using strings... (you can use `#[serde(rename)]` for
the "tar.zst" value).
On the other hand... We actually use a *tar* encoder, not a *tar.zstd*
encoder (obviously, given the nature of how this works), which makes me
wonder, shouldn't `zstd` maybe be a separate boolean?
I do think for a large single file "plain+zstd" makes sense, also
zstd-compressing a pxar might be worthwhile...
(And while zip+zstd doesn't make *that* much sense... I wouldn't really
care much either ;-) )
> + ),
> ]),
> ),
> )
> @@ -271,6 +284,11 @@ fn extract(
> let path = Path::new(OsStr::from_bytes(&path[..]));
>
> let pxar = param["pxar"].as_bool().unwrap_or(true);
> + let format = match param["format"].as_str() {
> + Some(format) => format.to_string(),
Maybe consider erroring if both `pxar` and `format` are set here?
Otherwise move the pxar binding into the None scope so it's gone
afterwards, it's a long function ;-)
> + // FIXME: old default was plain or zip, remove that with 3.0
> + None => if pxar { "pxar".to_string() } else { String::new() }
> + };
>
> let query_result = proxmox_async::runtime::block_in_place(move || {
> let mut disk_state = crate::DISK_STATE.lock().unwrap();
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/2] file-restore: add 'tar' option to 'extract' command
2022-05-31 11:17 [pbs-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve Dominik Csapak
` (2 preceding siblings ...)
2022-05-31 11:17 ` [pbs-devel] [PATCH proxmox-backup 1/2] restore-daemon: add 'format' parameter to the 'extract' handler Dominik Csapak
@ 2022-05-31 11:17 ` Dominik Csapak
2022-07-05 11:43 ` Wolfgang Bumiller
2022-05-31 11:17 ` [pbs-devel] [PATCH common 1/1] PBSClient: add 'tar' parameter to file_restore_extract Dominik Csapak
` (2 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Dominik Csapak @ 2022-05-31 11:17 UTC (permalink / raw)
To: pve-devel, pbs-devel
if the target ist stdout, we can now either have a zip (default) or a
tar.zst (with --tar 1) by making use of the new 'format' parameter
of the restore daemons 'extract' api
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
proxmox-file-restore/Cargo.toml | 1 +
proxmox-file-restore/src/block_driver.rs | 6 +--
proxmox-file-restore/src/block_driver_qemu.rs | 4 +-
proxmox-file-restore/src/main.rs | 51 ++++++++++++++-----
4 files changed, 45 insertions(+), 17 deletions(-)
diff --git a/proxmox-file-restore/Cargo.toml b/proxmox-file-restore/Cargo.toml
index 81eb7299..222a244f 100644
--- a/proxmox-file-restore/Cargo.toml
+++ b/proxmox-file-restore/Cargo.toml
@@ -13,6 +13,7 @@ nix = "0.19.1"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
tokio = { version = "1.6", features = [ "io-std", "rt", "rt-multi-thread", "time" ] }
+tokio-util = { version = "0.6", features = ["io"] }
pxar = { version = "0.10.1", features = [ "tokio-io" ] }
diff --git a/proxmox-file-restore/src/block_driver.rs b/proxmox-file-restore/src/block_driver.rs
index 0b5face9..ed8a19d0 100644
--- a/proxmox-file-restore/src/block_driver.rs
+++ b/proxmox-file-restore/src/block_driver.rs
@@ -55,7 +55,7 @@ pub trait BlockRestoreDriver {
details: SnapRestoreDetails,
img_file: String,
path: Vec<u8>,
- pxar: bool,
+ format: String,
) -> Async<Result<Box<dyn tokio::io::AsyncRead + Unpin + Send>, Error>>;
/// Return status of all running/mapped images, result value is (id, extra data), where id must
@@ -101,10 +101,10 @@ pub async fn data_extract(
details: SnapRestoreDetails,
img_file: String,
path: Vec<u8>,
- pxar: bool,
+ format: String,
) -> Result<Box<dyn tokio::io::AsyncRead + Send + Unpin>, Error> {
let driver = driver.unwrap_or(DEFAULT_DRIVER).resolve();
- driver.data_extract(details, img_file, path, pxar).await
+ driver.data_extract(details, img_file, path, format).await
}
#[api(
diff --git a/proxmox-file-restore/src/block_driver_qemu.rs b/proxmox-file-restore/src/block_driver_qemu.rs
index 362fff0d..dca5681e 100644
--- a/proxmox-file-restore/src/block_driver_qemu.rs
+++ b/proxmox-file-restore/src/block_driver_qemu.rs
@@ -215,7 +215,7 @@ impl BlockRestoreDriver for QemuBlockDriver {
details: SnapRestoreDetails,
img_file: String,
mut path: Vec<u8>,
- pxar: bool,
+ format: String,
) -> Async<Result<Box<dyn tokio::io::AsyncRead + Unpin + Send>, Error>> {
async move {
let client = ensure_running(&details).await?;
@@ -228,7 +228,7 @@ impl BlockRestoreDriver for QemuBlockDriver {
if let Err(err) = client
.download(
"api2/json/extract",
- Some(json!({ "path": path, "pxar": pxar })),
+ Some(json!({ "path": path, "format": format })),
&mut tx,
)
.await
diff --git a/proxmox-file-restore/src/main.rs b/proxmox-file-restore/src/main.rs
index 3420ea8e..693da091 100644
--- a/proxmox-file-restore/src/main.rs
+++ b/proxmox-file-restore/src/main.rs
@@ -4,8 +4,11 @@ use std::path::PathBuf;
use std::sync::Arc;
use anyhow::{bail, format_err, Error};
+use futures::StreamExt;
use serde_json::{json, Value};
+use tokio::io::AsyncWriteExt;
+use proxmox_compression::zstd::ZstdEncoder;
use proxmox_router::cli::{
complete_file_name, default_table_format_options, format_and_print_result_full,
get_output_format, run_cli_command, CliCommand, CliCommandMap, CliEnvironment, ColumnConfig,
@@ -18,7 +21,7 @@ use pxar::accessor::aio::Accessor;
use pxar::decoder::aio::Decoder;
use pbs_api_types::{BackupDir, BackupNamespace, CryptMode};
-use pbs_client::pxar::{create_zip, extract_sub_dir, extract_sub_dir_seq};
+use pbs_client::pxar::{create_tar, create_zip, extract_sub_dir, extract_sub_dir_seq};
use pbs_client::tools::{
complete_group_or_snapshot, complete_repository, connect, extract_repository_from_value,
key_source::{
@@ -346,9 +349,15 @@ async fn list(
description: "Group/Snapshot path.",
},
"path": {
- description: "Path to restore. Directories will be restored as .zip files if extracted to stdout.",
+ description: "Path to restore. Directories will be restored as archive files if extracted to stdout.",
type: String,
},
+ "tar": {
+ description: "If true, the resulting archive is a 'tar.zst' else it will be 'zip.",
+ optional: true,
+ default: false,
+ type: bool,
+ },
"base64": {
type: Boolean,
description: "If set, 'path' will be interpreted as base64 encoded.",
@@ -393,6 +402,7 @@ async fn extract(
base64: bool,
target: Option<String>,
verbose: bool,
+ tar: bool,
param: Value,
) -> Result<(), Error> {
let repo = extract_repository_from_value(¶m)?;
@@ -451,7 +461,7 @@ async fn extract(
let archive_size = reader.archive_size();
let reader = LocalDynamicReadAt::new(reader);
let decoder = Accessor::new(reader, archive_size).await?;
- extract_to_target(decoder, &path, target, verbose).await?;
+ extract_to_target(decoder, &path, target, verbose, tar).await?;
}
ExtractPath::VM(file, path) => {
let details = SnapRestoreDetails {
@@ -467,7 +477,8 @@ async fn extract(
};
if let Some(mut target) = target {
- let reader = data_extract(driver, details, file, path.clone(), true).await?;
+ let reader =
+ data_extract(driver, details, file, path.clone(), "pxar".to_string()).await?;
let decoder = Decoder::from_tokio(reader).await?;
extract_sub_dir_seq(&target, decoder, verbose).await?;
@@ -478,7 +489,9 @@ async fn extract(
format_err!("unable to remove temporary .pxarexclude-cli file - {}", e)
})?;
} else {
- let mut reader = data_extract(driver, details, file, path.clone(), false).await?;
+ let format = if tar { "tar.zst" } else { "zip" };
+ let mut reader =
+ data_extract(driver, details, file, path.clone(), format.to_owned()).await?;
tokio::io::copy(&mut reader, &mut tokio::io::stdout()).await?;
}
}
@@ -495,6 +508,7 @@ async fn extract_to_target<T>(
path: &[u8],
target: Option<PathBuf>,
verbose: bool,
+ tar: bool,
) -> Result<(), Error>
where
T: pxar::accessor::ReadAt + Clone + Send + Sync + Unpin + 'static,
@@ -515,13 +529,26 @@ where
tokio::io::copy(&mut file.contents().await?, &mut tokio::io::stdout()).await?;
}
_ => {
- create_zip(
- tokio::io::stdout(),
- decoder,
- OsStr::from_bytes(path),
- verbose,
- )
- .await?;
+ if tar {
+ let (writer, reader) = tokio::io::duplex(1024 * 1024);
+ let path = OsStr::from_bytes(path).to_owned();
+ tokio::spawn(async move { create_tar(writer, decoder, &path, verbose).await });
+ let mut zstdstream =
+ ZstdEncoder::new(tokio_util::io::ReaderStream::new(reader))?;
+ let mut stdout = tokio::io::stdout();
+ while let Some(buf) = zstdstream.next().await {
+ let buf = buf?;
+ stdout.write_all(&buf).await?;
+ }
+ } else {
+ create_zip(
+ tokio::io::stdout(),
+ decoder,
+ OsStr::from_bytes(path),
+ verbose,
+ )
+ .await?;
+ }
}
}
}
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup 2/2] file-restore: add 'tar' option to 'extract' command
2022-05-31 11:17 ` [pbs-devel] [PATCH proxmox-backup 2/2] file-restore: add 'tar' option to 'extract' command Dominik Csapak
@ 2022-07-05 11:43 ` Wolfgang Bumiller
0 siblings, 0 replies; 11+ messages in thread
From: Wolfgang Bumiller @ 2022-07-05 11:43 UTC (permalink / raw)
To: Dominik Csapak; +Cc: pve-devel, pbs-devel
needs a rebase ;-)
On Tue, May 31, 2022 at 01:17:23PM +0200, Dominik Csapak wrote:
> if the target ist stdout, we can now either have a zip (default) or a
> tar.zst (with --tar 1) by making use of the new 'format' parameter
> of the restore daemons 'extract' api
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> proxmox-file-restore/Cargo.toml | 1 +
> proxmox-file-restore/src/block_driver.rs | 6 +--
> proxmox-file-restore/src/block_driver_qemu.rs | 4 +-
> proxmox-file-restore/src/main.rs | 51 ++++++++++++++-----
> 4 files changed, 45 insertions(+), 17 deletions(-)
>
> diff --git a/proxmox-file-restore/Cargo.toml b/proxmox-file-restore/Cargo.toml
> index 81eb7299..222a244f 100644
> --- a/proxmox-file-restore/Cargo.toml
> +++ b/proxmox-file-restore/Cargo.toml
> @@ -13,6 +13,7 @@ nix = "0.19.1"
> serde = { version = "1.0", features = ["derive"] }
> serde_json = "1.0"
> tokio = { version = "1.6", features = [ "io-std", "rt", "rt-multi-thread", "time" ] }
> +tokio-util = { version = "0.6", features = ["io"] }
>
> pxar = { version = "0.10.1", features = [ "tokio-io" ] }
>
> diff --git a/proxmox-file-restore/src/block_driver.rs b/proxmox-file-restore/src/block_driver.rs
> index 0b5face9..ed8a19d0 100644
> --- a/proxmox-file-restore/src/block_driver.rs
> +++ b/proxmox-file-restore/src/block_driver.rs
> @@ -55,7 +55,7 @@ pub trait BlockRestoreDriver {
> details: SnapRestoreDetails,
> img_file: String,
> path: Vec<u8>,
> - pxar: bool,
> + format: String,
Similarly, an enum (and possibly zstd bool) would make more sense here
IMO.
> ) -> Async<Result<Box<dyn tokio::io::AsyncRead + Unpin + Send>, Error>>;
>
> /// Return status of all running/mapped images, result value is (id, extra data), where id must
> @@ -101,10 +101,10 @@ pub async fn data_extract(
> details: SnapRestoreDetails,
> img_file: String,
> path: Vec<u8>,
> - pxar: bool,
> + format: String,
> ) -> Result<Box<dyn tokio::io::AsyncRead + Send + Unpin>, Error> {
> let driver = driver.unwrap_or(DEFAULT_DRIVER).resolve();
> - driver.data_extract(details, img_file, path, pxar).await
> + driver.data_extract(details, img_file, path, format).await
> }
>
> #[api(
> diff --git a/proxmox-file-restore/src/block_driver_qemu.rs b/proxmox-file-restore/src/block_driver_qemu.rs
> index 362fff0d..dca5681e 100644
> --- a/proxmox-file-restore/src/block_driver_qemu.rs
> +++ b/proxmox-file-restore/src/block_driver_qemu.rs
> @@ -215,7 +215,7 @@ impl BlockRestoreDriver for QemuBlockDriver {
> details: SnapRestoreDetails,
> img_file: String,
> mut path: Vec<u8>,
> - pxar: bool,
> + format: String,
> ) -> Async<Result<Box<dyn tokio::io::AsyncRead + Unpin + Send>, Error>> {
> async move {
> let client = ensure_running(&details).await?;
> @@ -228,7 +228,7 @@ impl BlockRestoreDriver for QemuBlockDriver {
> if let Err(err) = client
> .download(
> "api2/json/extract",
> - Some(json!({ "path": path, "pxar": pxar })),
> + Some(json!({ "path": path, "format": format })),
> &mut tx,
> )
> .await
> diff --git a/proxmox-file-restore/src/main.rs b/proxmox-file-restore/src/main.rs
> index 3420ea8e..693da091 100644
> --- a/proxmox-file-restore/src/main.rs
> +++ b/proxmox-file-restore/src/main.rs
> @@ -4,8 +4,11 @@ use std::path::PathBuf;
> use std::sync::Arc;
>
> use anyhow::{bail, format_err, Error};
> +use futures::StreamExt;
> use serde_json::{json, Value};
> +use tokio::io::AsyncWriteExt;
>
> +use proxmox_compression::zstd::ZstdEncoder;
> use proxmox_router::cli::{
> complete_file_name, default_table_format_options, format_and_print_result_full,
> get_output_format, run_cli_command, CliCommand, CliCommandMap, CliEnvironment, ColumnConfig,
> @@ -18,7 +21,7 @@ use pxar::accessor::aio::Accessor;
> use pxar::decoder::aio::Decoder;
>
> use pbs_api_types::{BackupDir, BackupNamespace, CryptMode};
> -use pbs_client::pxar::{create_zip, extract_sub_dir, extract_sub_dir_seq};
> +use pbs_client::pxar::{create_tar, create_zip, extract_sub_dir, extract_sub_dir_seq};
> use pbs_client::tools::{
> complete_group_or_snapshot, complete_repository, connect, extract_repository_from_value,
> key_source::{
> @@ -346,9 +349,15 @@ async fn list(
> description: "Group/Snapshot path.",
> },
> "path": {
> - description: "Path to restore. Directories will be restored as .zip files if extracted to stdout.",
> + description: "Path to restore. Directories will be restored as archive files if extracted to stdout.",
> type: String,
> },
> + "tar": {
Why not stick with `format` here as well? (and the remaining hunks)
> + description: "If true, the resulting archive is a 'tar.zst' else it will be 'zip.",
> + optional: true,
> + default: false,
> + type: bool,
> + },
> "base64": {
> type: Boolean,
> description: "If set, 'path' will be interpreted as base64 encoded.",
> @@ -393,6 +402,7 @@ async fn extract(
> base64: bool,
> target: Option<String>,
> verbose: bool,
> + tar: bool,
> param: Value,
> ) -> Result<(), Error> {
> let repo = extract_repository_from_value(¶m)?;
> @@ -451,7 +461,7 @@ async fn extract(
> let archive_size = reader.archive_size();
> let reader = LocalDynamicReadAt::new(reader);
> let decoder = Accessor::new(reader, archive_size).await?;
> - extract_to_target(decoder, &path, target, verbose).await?;
> + extract_to_target(decoder, &path, target, verbose, tar).await?;
> }
> ExtractPath::VM(file, path) => {
> let details = SnapRestoreDetails {
> @@ -467,7 +477,8 @@ async fn extract(
> };
>
> if let Some(mut target) = target {
> - let reader = data_extract(driver, details, file, path.clone(), true).await?;
> + let reader =
> + data_extract(driver, details, file, path.clone(), "pxar".to_string()).await?;
> let decoder = Decoder::from_tokio(reader).await?;
> extract_sub_dir_seq(&target, decoder, verbose).await?;
>
> @@ -478,7 +489,9 @@ async fn extract(
> format_err!("unable to remove temporary .pxarexclude-cli file - {}", e)
> })?;
> } else {
> - let mut reader = data_extract(driver, details, file, path.clone(), false).await?;
> + let format = if tar { "tar.zst" } else { "zip" };
> + let mut reader =
> + data_extract(driver, details, file, path.clone(), format.to_owned()).await?;
> tokio::io::copy(&mut reader, &mut tokio::io::stdout()).await?;
> }
> }
> @@ -495,6 +508,7 @@ async fn extract_to_target<T>(
> path: &[u8],
> target: Option<PathBuf>,
> verbose: bool,
> + tar: bool,
> ) -> Result<(), Error>
> where
> T: pxar::accessor::ReadAt + Clone + Send + Sync + Unpin + 'static,
> @@ -515,13 +529,26 @@ where
> tokio::io::copy(&mut file.contents().await?, &mut tokio::io::stdout()).await?;
> }
> _ => {
> - create_zip(
> - tokio::io::stdout(),
> - decoder,
> - OsStr::from_bytes(path),
> - verbose,
> - )
> - .await?;
> + if tar {
> + let (writer, reader) = tokio::io::duplex(1024 * 1024);
> + let path = OsStr::from_bytes(path).to_owned();
> + tokio::spawn(async move { create_tar(writer, decoder, &path, verbose).await });
> + let mut zstdstream =
> + ZstdEncoder::new(tokio_util::io::ReaderStream::new(reader))?;
> + let mut stdout = tokio::io::stdout();
> + while let Some(buf) = zstdstream.next().await {
> + let buf = buf?;
> + stdout.write_all(&buf).await?;
> + }
> + } else {
> + create_zip(
> + tokio::io::stdout(),
> + decoder,
> + OsStr::from_bytes(path),
> + verbose,
> + )
> + .await?;
> + }
> }
> }
> }
> --
> 2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH common 1/1] PBSClient: add 'tar' parameter to file_restore_extract
2022-05-31 11:17 [pbs-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve Dominik Csapak
` (3 preceding siblings ...)
2022-05-31 11:17 ` [pbs-devel] [PATCH proxmox-backup 2/2] file-restore: add 'tar' option to 'extract' command Dominik Csapak
@ 2022-05-31 11:17 ` Dominik Csapak
2022-05-31 11:17 ` [pbs-devel] [PATCH storage 1/1] api/filerestore: add 'tar' parameter to 'download' api Dominik Csapak
2022-05-31 11:17 ` [pbs-devel] [PATCH widget-toolkit 1/1] window/FileBrowser: enable tar button by default Dominik Csapak
6 siblings, 0 replies; 11+ messages in thread
From: Dominik Csapak @ 2022-05-31 11:17 UTC (permalink / raw)
To: pve-devel, pbs-devel
so that we can get a 'tar.zst' from proxmox-file-restore
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/PVE/PBSClient.pm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index 37385d7..d671ea1 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -407,7 +407,7 @@ sub file_restore_extract_prepare {
# this blocks while data is transfered, call this from a background worker
sub file_restore_extract {
- my ($self, $output_file, $snapshot, $filepath, $base64) = @_;
+ my ($self, $output_file, $snapshot, $filepath, $base64, $tar) = @_;
(my $namespace, $snapshot) = split_namespaced_parameter($snapshot);
@@ -424,7 +424,7 @@ sub file_restore_extract {
return run_raw_client_cmd(
$self,
"extract",
- [ $snapshot, $filepath, "-", "--base64", $base64 ? 1 : 0 ],
+ [ $snapshot, $filepath, "-", "--base64", $base64 ? 1 : 0, '--tar', $tar ? 1 : 0 ],
binary => "proxmox-file-restore",
namespace => $namespace,
errfunc => $errfunc,
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [pbs-devel] [PATCH storage 1/1] api/filerestore: add 'tar' parameter to 'download' api
2022-05-31 11:17 [pbs-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve Dominik Csapak
` (4 preceding siblings ...)
2022-05-31 11:17 ` [pbs-devel] [PATCH common 1/1] PBSClient: add 'tar' parameter to file_restore_extract Dominik Csapak
@ 2022-05-31 11:17 ` Dominik Csapak
2022-05-31 11:17 ` [pbs-devel] [PATCH widget-toolkit 1/1] window/FileBrowser: enable tar button by default Dominik Csapak
6 siblings, 0 replies; 11+ messages in thread
From: Dominik Csapak @ 2022-05-31 11:17 UTC (permalink / raw)
To: pve-devel, pbs-devel
to be able to download 'tar.zst' archives
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
PVE/API2/Storage/FileRestore.pm | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/PVE/API2/Storage/FileRestore.pm b/PVE/API2/Storage/FileRestore.pm
index 5630f52..1c44b3c 100644
--- a/PVE/API2/Storage/FileRestore.pm
+++ b/PVE/API2/Storage/FileRestore.pm
@@ -156,6 +156,12 @@ __PACKAGE__->register_method ({
description => 'base64-path to the directory or file to download.',
type => 'string',
},
+ tar => {
+ description => "Download dirs as 'tar.zst' instead of 'zip'.",
+ type => 'boolean',
+ optional => 1,
+ default => 0,
+ },
},
},
returns => {
@@ -171,6 +177,7 @@ __PACKAGE__->register_method ({
my $path = extract_param($param, 'filepath');
my $storeid = extract_param($param, 'storage');
my $volid = $parse_volname_or_id->($storeid, $param->{volume});
+ my $tar = extract_param($param, 'tar') // 0;
my $cfg = PVE::Storage::config();
my $scfg = PVE::Storage::storage_config($cfg, $storeid);
@@ -188,7 +195,7 @@ __PACKAGE__->register_method ({
$rpcenv->fork_worker('pbs-download', undef, $user, sub {
my $name = decode_base64($path);
print "Starting download of file: $name\n";
- $client->file_restore_extract($fifo, [$scfg->{namespace}, $snap], $path, 1);
+ $client->file_restore_extract($fifo, [$scfg->{namespace}, $snap], $path, 1, $tar);
});
my $ret = {
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [pbs-devel] [PATCH widget-toolkit 1/1] window/FileBrowser: enable tar button by default
2022-05-31 11:17 [pbs-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve Dominik Csapak
` (5 preceding siblings ...)
2022-05-31 11:17 ` [pbs-devel] [PATCH storage 1/1] api/filerestore: add 'tar' parameter to 'download' api Dominik Csapak
@ 2022-05-31 11:17 ` Dominik Csapak
6 siblings, 0 replies; 11+ messages in thread
From: Dominik Csapak @ 2022-05-31 11:17 UTC (permalink / raw)
To: pve-devel, pbs-devel
all endpoints now can handle the 'tar' parameter, so add it for all
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/window/FileBrowser.js | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/window/FileBrowser.js b/src/window/FileBrowser.js
index a519d6b..c2f485a 100644
--- a/src/window/FileBrowser.js
+++ b/src/window/FileBrowser.js
@@ -61,10 +61,6 @@ Ext.define("Proxmox.window.FileBrowser", {
'd': true, // directories
},
- // enable tar download, this will add a menu to the "Download" button when the selection
- // can be downloaded as `.tar` files
- enableTar: false,
-
// prefix to prepend to downloaded file names
downloadPrefix: '',
},
@@ -127,7 +123,7 @@ Ext.define("Proxmox.window.FileBrowser", {
view.lookup('selectText').setText(st);
let canDownload = view.downloadURL && view.downloadableFileTypes[data.type];
- let enableMenu = view.enableTar && data.type === 'd';
+ let enableMenu = data.type === 'd';
let downloadBtn = view.lookup('downloadBtn');
downloadBtn.setDisabled(!canDownload || enableMenu);
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread