[pbs-devel] [PATCH 0/4] Smaller fixes/changes for file-restore

[pbs-devel] [PATCH 0/4] Smaller fixes/changes for file-restore

[Stefan Reiter]

Some stuff necessary for or discovered during development of the corresponding
GUI for PVE [0].

First patch is a resend, for easier application.

[0] https://lists.proxmox.com/pipermail/pve-devel/2021-April/047760.html


proxmox-backup: Stefan Reiter (4):
  file-restore: don't list non-pxar/-img *idx archives
  file-restore: print warnings on stderr
  file-restore: Add 'v' (Virtual) ArchiveEntry type
  file-restore: allow extracting a full pxar archive

 src/api2/helpers.rs                           |  2 +-
 src/api2/types/mod.rs                         | 13 ++++++----
 src/bin/proxmox-file-restore.rs               | 25 +++++++++++--------
 .../proxmox_file_restore/block_driver_qemu.rs |  4 +--
 src/bin/proxmox_restore_daemon/api.rs         |  9 ++++---
 5 files changed, 31 insertions(+), 22 deletions(-)

-- 
2.20.1

[pbs-devel] [PATCH RESEND proxmox-backup 1/4] file-restore: don't list non-pxar/-img *idx archives

[Stefan Reiter]

These can't be entered or restored anyway, and cause issues with catalog
files for example.

Also a clippy fix.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/bin/proxmox-file-restore.rs | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/bin/proxmox-file-restore.rs b/src/bin/proxmox-file-restore.rs
index c9ef3912..36fdd391 100644
--- a/src/bin/proxmox-file-restore.rs
+++ b/src/bin/proxmox-file-restore.rs
@@ -56,7 +56,7 @@ fn parse_path(path: String, base64: bool) -> Result<ExtractPath, Error> {
         return Ok(ExtractPath::ListArchives);
     }
 
-    while bytes.len() > 0 && bytes[0] == b'/' {
+    while !bytes.is_empty() && bytes[0] == b'/' {
         bytes.remove(0);
     }
 
@@ -170,10 +170,8 @@ async fn list(
         ExtractPath::ListArchives => {
             let mut entries = vec![];
             for file in manifest.files() {
-                match file.filename.rsplitn(2, '.').next().unwrap() {
-                    "didx" => {}
-                    "fidx" => {}
-                    _ => continue, // ignore all non fidx/didx
+                if !file.filename.ends_with(".pxar.didx") && !file.filename.ends_with(".img.fidx") {
+                    continue;
                 }
                 let path = format!("/{}", file.filename);
                 let attr = DirEntryAttribute::Directory { start: 0 };
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 2/4] file-restore: print warnings on stderr

[Stefan Reiter]

as we print JSON on stdout to be parsed

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/bin/proxmox_file_restore/block_driver_qemu.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/bin/proxmox_file_restore/block_driver_qemu.rs b/src/bin/proxmox_file_restore/block_driver_qemu.rs
index 607d7d8e..c9142129 100644
--- a/src/bin/proxmox_file_restore/block_driver_qemu.rs
+++ b/src/bin/proxmox_file_restore/block_driver_qemu.rs
@@ -94,7 +94,7 @@ async fn cleanup_map(map: &mut HashMap<String, VMState>) -> bool {
         if res.is_err() {
             // VM is not reachable, remove from map and inform user
             to_remove.push(name.clone());
-            println!(
+            eprintln!(
                 "VM '{}' (pid: {}, cid: {}) was not reachable, removing from map",
                 name, state.pid, state.cid
             );
@@ -129,7 +129,7 @@ async fn ensure_running(details: &SnapRestoreDetails) -> Result<VsockClient, Err
                     return Ok(client);
                 }
                 Err(err) => {
-                    println!("stale VM detected, restarting ({})", err);
+                    eprintln!("stale VM detected, restarting ({})", err);
                     // VM is dead, restart
                     let vms = start_vm(vm.cid, details).await?;
                     new_cid = vms.cid;
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 3/4] file-restore: Add 'v' (Virtual) ArchiveEntry type

[Stefan Reiter]

Encoded as "None", to avoid cluttering DirEntryAttribute, where it
wouldn't make any sense to have.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/api2/helpers.rs                   |  2 +-
 src/api2/types/mod.rs                 | 13 ++++++++-----
 src/bin/proxmox-file-restore.rs       |  9 +++++++--
 src/bin/proxmox_restore_daemon/api.rs |  9 +++++----
 4 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/src/api2/helpers.rs b/src/api2/helpers.rs
index 41391b77..52f80bad 100644
--- a/src/api2/helpers.rs
+++ b/src/api2/helpers.rs
@@ -48,7 +48,7 @@ pub fn list_dir_content<R: Read + Seek>(
         let mut components = path.clone();
         components.push(b'/');
         components.extend(&direntry.name);
-        let mut entry = ArchiveEntry::new(&components, &direntry.attr);
+        let mut entry = ArchiveEntry::new(&components, Some(&direntry.attr));
         if let DirEntryAttribute::File { size, mtime } = direntry.attr {
             entry.size = size.into();
             entry.mtime = mtime.into();
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index 19186ea2..9d1bd301 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -1354,19 +1354,22 @@ pub struct ArchiveEntry {
 }
 
 impl ArchiveEntry {
-    pub fn new(filepath: &[u8], entry_type: &DirEntryAttribute) -> Self {
+    pub fn new(filepath: &[u8], entry_type: Option<&DirEntryAttribute>) -> Self {
         Self {
             filepath: base64::encode(filepath),
             text: String::from_utf8_lossy(filepath.split(|x| *x == b'/').last().unwrap())
                 .to_string(),
-            entry_type: CatalogEntryType::from(entry_type).to_string(),
-            leaf: !matches!(entry_type, DirEntryAttribute::Directory { .. }),
+            entry_type: match entry_type {
+                Some(entry_type) => CatalogEntryType::from(entry_type).to_string(),
+                None => "v".to_owned(),
+            },
+            leaf: !matches!(entry_type, None | Some(DirEntryAttribute::Directory { .. })),
             size: match entry_type {
-                DirEntryAttribute::File { size, .. } => Some(*size),
+                Some(DirEntryAttribute::File { size, .. }) => Some(*size),
                 _ => None
             },
             mtime: match entry_type {
-                DirEntryAttribute::File { mtime, .. } => Some(*mtime),
+                Some(DirEntryAttribute::File { mtime, .. }) => Some(*mtime),
                 _ => None
             },
         }
diff --git a/src/bin/proxmox-file-restore.rs b/src/bin/proxmox-file-restore.rs
index 36fdd391..7799a76d 100644
--- a/src/bin/proxmox-file-restore.rs
+++ b/src/bin/proxmox-file-restore.rs
@@ -174,8 +174,13 @@ async fn list(
                     continue;
                 }
                 let path = format!("/{}", file.filename);
-                let attr = DirEntryAttribute::Directory { start: 0 };
-                entries.push(ArchiveEntry::new(path.as_bytes(), &attr));
+                let attr = if file.filename.ends_with(".pxar.didx") {
+                    // a pxar file is a file archive, so it's root is also a directory root
+                    Some(&DirEntryAttribute::Directory { start: 0 })
+                } else {
+                    None
+                };
+                entries.push(ArchiveEntry::new(path.as_bytes(), attr));
             }
 
             Ok(entries)
diff --git a/src/bin/proxmox_restore_daemon/api.rs b/src/bin/proxmox_restore_daemon/api.rs
index 7ac70278..82ead647 100644
--- a/src/bin/proxmox_restore_daemon/api.rs
+++ b/src/bin/proxmox_restore_daemon/api.rs
@@ -148,7 +148,7 @@ fn list(
             match root_entry {
                 DirEntryAttribute::File { .. } => {
                     // list on file, return details
-                    res.push(ArchiveEntry::new(&param_path, &root_entry));
+                    res.push(ArchiveEntry::new(&param_path, Some(&root_entry)));
                 }
                 DirEntryAttribute::Directory { .. } => {
                     // list on directory, return all contained files/dirs
@@ -176,7 +176,7 @@ fn list(
                             if let Ok(entry) = entry {
                                 res.push(ArchiveEntry::new(
                                     full_path.as_os_str().as_bytes(),
-                                    &entry,
+                                    Some(&entry),
                                 ));
                             }
                         }
@@ -192,7 +192,7 @@ fn list(
                 t_path.extend(t.as_bytes());
                 res.push(ArchiveEntry::new(
                     &t_path[..],
-                    &DirEntryAttribute::Directory { start: 0 },
+                    None,
                 ));
             }
         }
@@ -203,7 +203,8 @@ fn list(
                 c_path.extend(c.as_bytes());
                 res.push(ArchiveEntry::new(
                     &c_path[..],
-                    &DirEntryAttribute::Directory { start: 0 },
+                    // this marks the beginning of a filesystem, i.e. '/', so this is a Directory
+                    Some(&DirEntryAttribute::Directory { start: 0 }),
                 ));
             }
         }
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 4/4] file-restore: allow extracting a full pxar archive

[Stefan Reiter]

If the path for within the archive is empty, assume "/" to extract all
of it.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/bin/proxmox-file-restore.rs | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/bin/proxmox-file-restore.rs b/src/bin/proxmox-file-restore.rs
index 7799a76d..2726eeb7 100644
--- a/src/bin/proxmox-file-restore.rs
+++ b/src/bin/proxmox-file-restore.rs
@@ -399,14 +399,16 @@ async fn extract_to_target<T>(
 where
     T: pxar::accessor::ReadAt + Clone + Send + Sync + Unpin + 'static,
 {
+    let path = if path.is_empty() { b"/" } else { path };
+
     let root = decoder.open_root().await?;
     let file = root
-        .lookup(OsStr::from_bytes(&path))
+        .lookup(OsStr::from_bytes(path))
         .await?
         .ok_or_else(|| format_err!("error opening '{:?}'", path))?;
 
     if let Some(target) = target {
-        extract_sub_dir(target, decoder, OsStr::from_bytes(&path), verbose).await?;
+        extract_sub_dir(target, decoder, OsStr::from_bytes(path), verbose).await?;
     } else {
         match file.kind() {
             pxar::EntryKind::File { .. } => {
@@ -416,7 +418,7 @@ where
                 create_zip(
                     tokio::io::stdout(),
                     decoder,
-                    OsStr::from_bytes(&path),
+                    OsStr::from_bytes(path),
                     verbose,
                 )
                 .await?;
-- 
2.20.1

[pbs-devel] applied: [PATCH 0/4] Smaller fixes/changes for file-restore

[Thomas Lamprecht]

On 21.04.21 15:18, Stefan Reiter wrote:
> Some stuff necessary for or discovered during development of the corresponding
> GUI for PVE [0].
> 
> First patch is a resend, for easier application.
> 
> [0] https://lists.proxmox.com/pipermail/pve-devel/2021-April/047760.html
> 
> 
> proxmox-backup: Stefan Reiter (4):
>   file-restore: don't list non-pxar/-img *idx archives
>   file-restore: print warnings on stderr
>   file-restore: Add 'v' (Virtual) ArchiveEntry type
>   file-restore: allow extracting a full pxar archive
> 
>  src/api2/helpers.rs                           |  2 +-
>  src/api2/types/mod.rs                         | 13 ++++++----
>  src/bin/proxmox-file-restore.rs               | 25 +++++++++++--------
>  .../proxmox_file_restore/block_driver_qemu.rs |  4 +--
>  src/bin/proxmox_restore_daemon/api.rs         |  9 ++++---
>  5 files changed, 31 insertions(+), 22 deletions(-)
> 



applied, thanks!

Adapted the commit message of "file-restore: Add 'v' (Virtual) ArchiveEntry type"
slightly, to give some context for what this is actually used.