* [pve-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve
@ 2022-05-31 11:17 ` Dominik Csapak
0 siblings, 0 replies; 23+ messages in thread
From: Dominik Csapak @ 2022-05-31 11:17 UTC (permalink / raw)
To: pve-devel, pbs-devel
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve
@ 2022-05-31 11:17 ` Dominik Csapak
0 siblings, 0 replies; 23+ messages in thread
From: Dominik Csapak @ 2022-05-31 11:17 UTC (permalink / raw)
To: pve-devel, pbs-devel
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pve-devel] [PATCH proxmox 1/2] proxmox-compression: make ZstdEncoder stream a bit more generic
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
@ 2022-05-31 11:17 ` Dominik Csapak
-1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [pbs-devel] [PATCH proxmox 1/2] proxmox-compression: make ZstdEncoder stream a bit more generic
@ 2022-05-31 11:17 ` Dominik Csapak
0 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [pve-devel] [PATCH proxmox 2/2] proxmox-compression: add 'tar_directory'
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
@ 2022-05-31 11:17 ` Dominik Csapak
-1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [pbs-devel] [PATCH proxmox 2/2] proxmox-compression: add 'tar_directory'
@ 2022-05-31 11:17 ` Dominik Csapak
0 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [pve-devel] [PATCH proxmox-backup 1/2] restore-daemon: add 'format' parameter to the 'extract' handler
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
@ 2022-05-31 11:17 ` Dominik Csapak
-1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 1/2] restore-daemon: add 'format' parameter to the 'extract' handler
@ 2022-05-31 11:17 ` Dominik Csapak
0 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [pve-devel] [PATCH proxmox-backup 2/2] file-restore: add 'tar' option to 'extract' command
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
@ 2022-05-31 11:17 ` Dominik Csapak
-1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/2] file-restore: add 'tar' option to 'extract' command
@ 2022-05-31 11:17 ` Dominik Csapak
0 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [pve-devel] [PATCH common 1/1] PBSClient: add 'tar' parameter to file_restore_extract
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
@ 2022-05-31 11:17 ` Dominik Csapak
-1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [pbs-devel] [PATCH common 1/1] PBSClient: add 'tar' parameter to file_restore_extract
@ 2022-05-31 11:17 ` Dominik Csapak
0 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [pve-devel] [PATCH storage 1/1] api/filerestore: add 'tar' parameter to 'download' api
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
@ 2022-05-31 11:17 ` Dominik Csapak
-1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [pbs-devel] [PATCH storage 1/1] api/filerestore: add 'tar' parameter to 'download' api
@ 2022-05-31 11:17 ` Dominik Csapak
0 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [pve-devel] [PATCH widget-toolkit 1/1] window/FileBrowser: enable tar button by default
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
@ 2022-05-31 11:17 ` Dominik Csapak
-1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [pbs-devel] [PATCH widget-toolkit 1/1] window/FileBrowser: enable tar button by default
@ 2022-05-31 11:17 ` Dominik Csapak
0 siblings, 0 replies; 23+ 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] 23+ messages in thread
* Re: [pve-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
` (7 preceding siblings ...)
(?)
@ 2022-07-01 12:12 ` Dominik Csapak
-1 siblings, 0 replies; 23+ messages in thread
From: Dominik Csapak @ 2022-07-01 12:12 UTC (permalink / raw)
To: pve-devel
any comment?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pve-devel] [pbs-devel] [PATCH proxmox-backup 1/2] restore-daemon: add 'format' parameter to the 'extract' handler
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
@ 2022-07-05 11:39 ` Wolfgang Bumiller
-1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 1/2] restore-daemon: add 'format' parameter to the 'extract' handler
@ 2022-07-05 11:39 ` Wolfgang Bumiller
0 siblings, 0 replies; 23+ 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] 23+ messages in thread
* Re: [pve-devel] [pbs-devel] [PATCH proxmox-backup 2/2] file-restore: add 'tar' option to 'extract' command
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
@ 2022-07-05 11:43 ` Wolfgang Bumiller
-1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/2] file-restore: add 'tar' option to 'extract' command
@ 2022-07-05 11:43 ` Wolfgang Bumiller
0 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [pve-devel] applied-both: [PATCH proxmox 1/2] proxmox-compression: make ZstdEncoder stream a bit more generic
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
@ 2022-07-05 11:47 ` Wolfgang Bumiller
-1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [pbs-devel] applied-both: [PATCH proxmox 1/2] proxmox-compression: make ZstdEncoder stream a bit more generic
@ 2022-07-05 11:47 ` Wolfgang Bumiller
0 siblings, 0 replies; 23+ 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] 23+ messages in thread
end of thread, other threads:[~2022-07-05 11:47 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-31 11:17 [pve-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve Dominik Csapak
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
2022-05-31 11:17 ` [pve-devel] [PATCH proxmox 1/2] proxmox-compression: make ZstdEncoder stream a bit more generic Dominik Csapak
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
2022-07-05 11:47 ` [pve-devel] applied-both: " Wolfgang Bumiller
2022-07-05 11:47 ` [pbs-devel] " Wolfgang Bumiller
2022-05-31 11:17 ` [pve-devel] [PATCH proxmox 2/2] proxmox-compression: add 'tar_directory' Dominik Csapak
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
2022-05-31 11:17 ` [pve-devel] [PATCH proxmox-backup 1/2] restore-daemon: add 'format' parameter to the 'extract' handler Dominik Csapak
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
2022-07-05 11:39 ` [pve-devel] " Wolfgang Bumiller
2022-07-05 11:39 ` Wolfgang Bumiller
2022-05-31 11:17 ` [pve-devel] [PATCH proxmox-backup 2/2] file-restore: add 'tar' option to 'extract' command Dominik Csapak
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
2022-07-05 11:43 ` [pve-devel] " Wolfgang Bumiller
2022-07-05 11:43 ` Wolfgang Bumiller
2022-05-31 11:17 ` [pve-devel] [PATCH common 1/1] PBSClient: add 'tar' parameter to file_restore_extract Dominik Csapak
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
2022-05-31 11:17 ` [pve-devel] [PATCH storage 1/1] api/filerestore: add 'tar' parameter to 'download' api Dominik Csapak
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
2022-05-31 11:17 ` [pve-devel] [PATCH widget-toolkit 1/1] window/FileBrowser: enable tar button by default Dominik Csapak
2022-05-31 11:17 ` [pbs-devel] " Dominik Csapak
2022-07-01 12:12 ` [pve-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve Dominik Csapak
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