[pve-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve

[pve-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve

[Dominik Csapak]

like we have the tar.zst download button for pbs itself, add it for
pve for both vms and container file-restore

pve-storage depends on pve-common, which depends on proxmox-backup,
which depends on proxmox
(we must bump proxmox-compression and add the new version as a
dependency; also for the restore-daemon)

widget-toolkit could be done differently (just set enableTar to true;
either by default or in proxmox-backup), but since we don't use
it anywhere else, this seemed wrong

also i am not completely happy with the interface: i added a 'tar' parameter
to the download path (for gui compatibility) and to the file-restore
binary, but moved to a 'format' parameter in the restore-daemon.

i'd really prefer to have a single parameter style for that, but i did not want
to pass through the 'tar' through all layers, and moving all to a 'format'
parameter would be more work (api compatibility etc.). so maybe someone else has
another take on, which way i should pursue in a v2?

also AFAICS, the restore-daemon and file-restore binary always go
together, so do we have to keep the api of the restore-daemon
compatible ? (theoretically users could invoke that manually?)

proxmox:

Dominik Csapak (2):
  proxmox-compression: make ZstdEncoder stream a bit more generic
  proxmox-compression: add 'tar_directory'

 proxmox-compression/Cargo.toml  |   1 +
 proxmox-compression/src/tar.rs  | 116 ++++++++++++++++++++++++++++++++
 proxmox-compression/src/zstd.rs |  15 +++--
 3 files changed, 127 insertions(+), 5 deletions(-)

proxmox-backup:

Dominik Csapak (2):
  restore-daemon: add 'format' parameter to the 'extract' handler
  file-restore: add 'tar' option to 'extract' command

 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 ++++++++++++++-----
 .../src/proxmox_restore_daemon/api.rs         | 49 +++++++++++++++---
 5 files changed, 88 insertions(+), 23 deletions(-)

pve-common:

Dominik Csapak (1):
  PBSClient: add 'tar' parameter to file_restore_extract

 src/PVE/PBSClient.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

pve-storage:

Dominik Csapak (1):
  api/filerestore: add 'tar' parameter to 'download' api

 PVE/API2/Storage/FileRestore.pm | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

proxmox-widget-toolkit:

Dominik Csapak (1):
  window/FileBrowser: enable tar button by default

 src/window/FileBrowser.js | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH proxmox 1/2] proxmox-compression: make ZstdEncoder stream a bit more generic

[Dominik Csapak]

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

[pve-devel] [PATCH proxmox 2/2] proxmox-compression: add 'tar_directory'

[Dominik Csapak]

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

[pve-devel] [PATCH proxmox-backup 1/2] restore-daemon: add 'format' parameter to the 'extract' handler

[Dominik Csapak]

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

[pve-devel] [PATCH proxmox-backup 2/2] file-restore: add 'tar' option to 'extract' command

[Dominik Csapak]

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(&param)?;
@@ -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

[pve-devel] [PATCH common 1/1] PBSClient: add 'tar' parameter to file_restore_extract

[Dominik Csapak]

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

[pve-devel] [PATCH storage 1/1] api/filerestore: add 'tar' parameter to 'download' api

[Dominik Csapak]

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

[pve-devel] [PATCH widget-toolkit 1/1] window/FileBrowser: enable tar button by default

[Dominik Csapak]

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

Re: [pve-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve

[Dominik Csapak]

any comment?

Re: [pve-devel] [pbs-devel] [PATCH proxmox-backup 1/2] restore-daemon: add 'format' parameter to the 'extract' handler

[Wolfgang Bumiller]

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();

Re: [pve-devel] [pbs-devel] [PATCH proxmox-backup 2/2] file-restore: add 'tar' option to 'extract' command

[Wolfgang Bumiller]

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(&param)?;
> @@ -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

[pve-devel] applied-both: [PATCH proxmox 1/2] proxmox-compression: make ZstdEncoder stream a bit more generic

[Wolfgang Bumiller]

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