[pbs-devel] [PATCH v2 proxmox-backup 0/7] fix catalog dump and shell for split pxar archives

[pbs-devel] [PATCH v2 proxmox-backup 0/7] fix catalog dump and shell for split pxar archives

[Christian Ebner]

This patch series fixes the `catalog dump` and `catalog shell`
commands for snapshots created by setting the `change-detection-mode`
to either `data` or `metadata`, therefore using split pxar archive
encoding without encoding the dedicated catalog.

If no catalog file can be found in the snaphsots manifest, the fallback
behaviour is now to check if there are metadata archives present in the
manifest, and if so use the metadata archive accessor for either
dumping the contents in a format compatible to the catalog dump or to
interactively navigate and restore contents via the catalog shell.

Changes since version 1:
- Implement also the `catalog shell` command for split pxar archives
- Factor out common functionality into dedicated helpers

Christian Ebner (7):
  client: make helper to get remote pxar reader reusable
  client: tools: factor out entry path prefix helper
  client: tools: factor out pxar entry to dir entry mapping
  client: add helper to dump catalog from metadata archive
  client: catalog: fallback to metadata archives for catalog dump
  client: helper to mimic catalog find using metadata archive
  client: catalog shell: fallback to accessor for navigation

 pbs-client/src/catalog_shell.rs      | 303 ++++++++++++++++++++++-----
 pbs-client/src/tools/mod.rs          | 214 +++++++++++++++----
 proxmox-backup-client/src/catalog.rs |  65 +++++-
 proxmox-file-restore/src/main.rs     |  31 +--
 4 files changed, 488 insertions(+), 125 deletions(-)

-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox-backup 1/7] client: make helper to get remote pxar reader reusable

[Christian Ebner]

Move the get_remote_pxar_reader helper function so it can be reused
also for getting the metadata archive reader instance for the catalog
dump.

No functional changes.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- no functional changes, only rebased onto origin/master

 pbs-client/src/tools/mod.rs      | 32 ++++++++++++++++++++++++++++++--
 proxmox-file-restore/src/main.rs | 31 +++----------------------------
 2 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index 772cc1263..8c634ba76 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -9,7 +9,7 @@ use std::os::unix::fs::OpenOptionsExt;
 use std::os::unix::io::FromRawFd;
 use std::path::PathBuf;
 use std::process::Command;
-use std::sync::OnceLock;
+use std::sync::{Arc, OnceLock};
 
 use anyhow::{bail, format_err, Context, Error};
 use serde_json::{json, Value};
@@ -22,13 +22,16 @@ use proxmox_sys::fs::file_get_json;
 
 use pbs_api_types::{Authid, BackupNamespace, RateLimitConfig, UserWithTokens, BACKUP_REPO_URL};
 use pbs_datastore::catalog::{ArchiveEntry, DirEntryAttribute};
+use pbs_datastore::dynamic_index::{BufferedDynamicReader, LocalDynamicReadAt};
+use pbs_datastore::index::IndexFile;
 use pbs_datastore::BackupManifest;
+use pbs_tools::crypt_config::CryptConfig;
 use pxar::accessor::aio::Accessor;
 use pxar::accessor::ReadAt;
 use pxar::format::SignedDuration;
 use pxar::{mode, EntryKind};
 
-use crate::{BackupRepository, HttpClient, HttpClientOptions};
+use crate::{BackupReader, BackupRepository, HttpClient, HttpClientOptions, RemoteChunkReader};
 
 pub mod key_source;
 
@@ -555,6 +558,31 @@ pub fn place_xdg_file(
         .with_context(|| format!("failed to place {} in xdg home", description))
 }
 
+pub async fn get_remote_pxar_reader(
+    archive_name: &str,
+    client: Arc<BackupReader>,
+    manifest: &BackupManifest,
+    crypt_config: Option<Arc<CryptConfig>>,
+) -> Result<(LocalDynamicReadAt<RemoteChunkReader>, u64), Error> {
+    let index = client
+        .download_dynamic_index(manifest, archive_name)
+        .await?;
+    let most_used = index.find_most_used_chunks(8);
+
+    let file_info = manifest.lookup_file_info(archive_name)?;
+    let chunk_reader = RemoteChunkReader::new(
+        client.clone(),
+        crypt_config,
+        file_info.chunk_crypt_mode(),
+        most_used,
+    );
+
+    let reader = BufferedDynamicReader::new(index, chunk_reader);
+    let archive_size = reader.archive_size();
+
+    Ok((LocalDynamicReadAt::new(reader), archive_size))
+}
+
 pub fn get_pxar_archive_names(
     archive_name: &str,
     manifest: &BackupManifest,
diff --git a/proxmox-file-restore/src/main.rs b/proxmox-file-restore/src/main.rs
index 69d811fc1..db4ccae08 100644
--- a/proxmox-file-restore/src/main.rs
+++ b/proxmox-file-restore/src/main.rs
@@ -24,7 +24,7 @@ use pbs_api_types::{file_restore::FileRestoreFormat, BackupDir, BackupNamespace,
 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,
-    has_pxar_filename_extension,
+    get_remote_pxar_reader, has_pxar_filename_extension,
     key_source::{
         crypto_parameters_keep_fd, format_key_source, get_encryption_key_password, KEYFD_SCHEMA,
         KEYFILE_SCHEMA,
@@ -33,9 +33,9 @@ use pbs_client::tools::{
 };
 use pbs_client::{BackupReader, BackupRepository, RemoteChunkReader};
 use pbs_datastore::catalog::{ArchiveEntry, CatalogReader, DirEntryAttribute};
-use pbs_datastore::dynamic_index::{BufferedDynamicReader, LocalDynamicReadAt};
+use pbs_datastore::dynamic_index::BufferedDynamicReader;
 use pbs_datastore::index::IndexFile;
-use pbs_datastore::{BackupManifest, CATALOG_NAME};
+use pbs_datastore::CATALOG_NAME;
 use pbs_key_config::decrypt_key;
 use pbs_tools::crypt_config::CryptConfig;
 
@@ -353,31 +353,6 @@ async fn list(
     Ok(())
 }
 
-async fn get_remote_pxar_reader(
-    archive_name: &str,
-    client: Arc<BackupReader>,
-    manifest: &BackupManifest,
-    crypt_config: Option<Arc<CryptConfig>>,
-) -> Result<(LocalDynamicReadAt<RemoteChunkReader>, u64), Error> {
-    let index = client
-        .download_dynamic_index(manifest, archive_name)
-        .await?;
-    let most_used = index.find_most_used_chunks(8);
-
-    let file_info = manifest.lookup_file_info(archive_name)?;
-    let chunk_reader = RemoteChunkReader::new(
-        client.clone(),
-        crypt_config,
-        file_info.chunk_crypt_mode(),
-        most_used,
-    );
-
-    let reader = BufferedDynamicReader::new(index, chunk_reader);
-    let archive_size = reader.archive_size();
-
-    Ok((LocalDynamicReadAt::new(reader), archive_size))
-}
-
 #[api(
     input: {
         properties: {
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox-backup 2/7] client: tools: factor out entry path prefix helper

[Christian Ebner]

Move the logic to generate `FileEntry` paths with a given prefix to
its own helper function for it to be reusable for the catalog shell
implementation of split pxar archives.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- not present in pervious version

 pbs-client/src/tools/mod.rs | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index 8c634ba76..e6e548403 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -1,4 +1,5 @@
 //! Shared tools useful for common CLI clients.
+use pxar::accessor::aio::FileEntry;
 use std::collections::HashMap;
 use std::env::VarError::{NotPresent, NotUnicode};
 use std::ffi::OsStr;
@@ -742,16 +743,7 @@ pub async fn pxar_metadata_catalog_lookup<T: Clone + ReadAt>(
                 EntryKind::Socket => DirEntryAttribute::Socket,
             };
 
-            let entry_path = if let Some(prefix) = path_prefix {
-                let mut entry_path = PathBuf::from(prefix);
-                match entry.path().strip_prefix("/") {
-                    Ok(path) => entry_path.push(path),
-                    Err(_) => entry_path.push(entry.path()),
-                }
-                entry_path
-            } else {
-                PathBuf::from(entry.path())
-            };
+            let entry_path = entry_path_with_prefix(&entry, path_prefix.unwrap_or_default());
             entries.push(ArchiveEntry::new(
                 entry_path.as_os_str().as_bytes(),
                 Some(&entry_attr),
@@ -790,3 +782,13 @@ pub fn create_tmp_file() -> std::io::Result<std::fs::File> {
         }
     })
 }
+
+/// Generate entry path for given [`FileEntry`], prefixed by given `path_prefix` component(s).
+fn entry_path_with_prefix<T: Clone + ReadAt>(entry: &FileEntry<T>, path_prefix: &str) -> PathBuf {
+    let mut entry_path = PathBuf::from(&path_prefix);
+    match entry.path().strip_prefix("/") {
+        Ok(path) => entry_path.push(path),
+        Err(_) => entry_path.push(entry.path()),
+    }
+    entry_path
+}
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox-backup 3/7] client: tools: factor out pxar entry to dir entry mapping

[Christian Ebner]

Create a dedicated helper for performing the mapping of pxar
`FileEntry`s to `DirEntryAttribute`s.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- not present in previous version

 pbs-client/src/tools/mod.rs | 56 +++++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index e6e548403..97f71f3f1 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -720,27 +720,9 @@ pub async fn pxar_metadata_catalog_lookup<T: Clone + ReadAt>(
         while let Some(entry) = entries_iter.next().await {
             let entry = entry?.decode_entry().await?;
 
-            let entry_attr = match entry.kind() {
-                EntryKind::Version(_) | EntryKind::Prelude(_) | EntryKind::GoodbyeTable => continue,
-                EntryKind::Directory => DirEntryAttribute::Directory {
-                    start: entry.entry_range_info().entry_range.start,
-                },
-                EntryKind::File { size, .. } => {
-                    let mtime = match entry.metadata().mtime_as_duration() {
-                        SignedDuration::Positive(val) => i64::try_from(val.as_secs())?,
-                        SignedDuration::Negative(val) => -i64::try_from(val.as_secs())?,
-                    };
-                    DirEntryAttribute::File { size: *size, mtime }
-                }
-                EntryKind::Device(_) => match entry.metadata().file_type() {
-                    mode::IFBLK => DirEntryAttribute::BlockDevice,
-                    mode::IFCHR => DirEntryAttribute::CharDevice,
-                    _ => bail!("encountered unknown device type"),
-                },
-                EntryKind::Symlink(_) => DirEntryAttribute::Symlink,
-                EntryKind::Hardlink(_) => DirEntryAttribute::Hardlink,
-                EntryKind::Fifo => DirEntryAttribute::Fifo,
-                EntryKind::Socket => DirEntryAttribute::Socket,
+            let entry_attr = match map_to_dir_entry_attr(&entry)? {
+                Some(attr) => attr,
+                None => continue,
             };
 
             let entry_path = entry_path_with_prefix(&entry, path_prefix.unwrap_or_default());
@@ -792,3 +774,35 @@ fn entry_path_with_prefix<T: Clone + ReadAt>(entry: &FileEntry<T>, path_prefix:
     }
     entry_path
 }
+
+/// Map a pxar [`FileEntry`] to the corresponding [`DirEntryAttribute`].
+///
+/// Returns `Ok(None)` if there is no corresponding variant, `Err` if an unknown device type is
+/// encountered.
+pub(crate) fn map_to_dir_entry_attr<T: Clone + ReadAt>(
+    entry: &FileEntry<T>,
+) -> Result<Option<DirEntryAttribute>, Error> {
+    let attr = match entry.kind() {
+        EntryKind::Version(_) | EntryKind::Prelude(_) | EntryKind::GoodbyeTable => return Ok(None),
+        EntryKind::Directory => DirEntryAttribute::Directory {
+            start: entry.entry_range_info().entry_range.start,
+        },
+        EntryKind::File { size, .. } => {
+            let mtime = match entry.metadata().mtime_as_duration() {
+                SignedDuration::Positive(val) => i64::try_from(val.as_secs())?,
+                SignedDuration::Negative(val) => -i64::try_from(val.as_secs())?,
+            };
+            DirEntryAttribute::File { size: *size, mtime }
+        }
+        EntryKind::Device(_) => match entry.metadata().file_type() {
+            mode::IFBLK => DirEntryAttribute::BlockDevice,
+            mode::IFCHR => DirEntryAttribute::CharDevice,
+            _ => bail!("encountered unknown device type"),
+        },
+        EntryKind::Symlink(_) => DirEntryAttribute::Symlink,
+        EntryKind::Hardlink(_) => DirEntryAttribute::Hardlink,
+        EntryKind::Fifo => DirEntryAttribute::Fifo,
+        EntryKind::Socket => DirEntryAttribute::Socket,
+    };
+    Ok(Some(attr))
+}
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox-backup 4/7] client: add helper to dump catalog from metadata archive

[Christian Ebner]

Implements the methods to dump the contents of a metadata pxar
archive using the same output format as used by the catalog dump.

The helper function has been split into 2 for async recursion to
work.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- use attribute mapping helper
- get rid of unneeded `pxar_metadata_catalog_dump` and use
  `pxar_metadata_catalog_dump_dir` directly
- factor out `pxar_metadata_read_dir` helper, as that will be used by
  the catalog shell as well

 pbs-client/src/tools/mod.rs | 64 +++++++++++++++++++++++++++++++++++--
 1 file changed, 62 insertions(+), 2 deletions(-)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index 97f71f3f1..10a368fd2 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -4,11 +4,13 @@ use std::collections::HashMap;
 use std::env::VarError::{NotPresent, NotUnicode};
 use std::ffi::OsStr;
 use std::fs::File;
+use std::future::Future;
 use std::io::{BufRead, BufReader};
 use std::os::unix::ffi::OsStrExt;
 use std::os::unix::fs::OpenOptionsExt;
 use std::os::unix::io::FromRawFd;
 use std::path::PathBuf;
+use std::pin::Pin;
 use std::process::Command;
 use std::sync::{Arc, OnceLock};
 
@@ -22,12 +24,12 @@ use proxmox_schema::*;
 use proxmox_sys::fs::file_get_json;
 
 use pbs_api_types::{Authid, BackupNamespace, RateLimitConfig, UserWithTokens, BACKUP_REPO_URL};
-use pbs_datastore::catalog::{ArchiveEntry, DirEntryAttribute};
+use pbs_datastore::catalog::{ArchiveEntry, CatalogEntryType, DirEntryAttribute};
 use pbs_datastore::dynamic_index::{BufferedDynamicReader, LocalDynamicReadAt};
 use pbs_datastore::index::IndexFile;
 use pbs_datastore::BackupManifest;
 use pbs_tools::crypt_config::CryptConfig;
-use pxar::accessor::aio::Accessor;
+use pxar::accessor::aio::{Accessor, Directory};
 use pxar::accessor::ReadAt;
 use pxar::format::SignedDuration;
 use pxar::{mode, EntryKind};
@@ -806,3 +808,61 @@ pub(crate) fn map_to_dir_entry_attr<T: Clone + ReadAt>(
     };
     Ok(Some(attr))
 }
+
+/// Read a sorted list of pxar archive entries from given parent entry via the pxar accessor.
+pub(crate) async fn pxar_metadata_read_dir<T: Clone + Send + Sync + ReadAt>(
+    parent_dir: Directory<T>,
+) -> Result<Vec<FileEntry<T>>, Error> {
+    let mut entries_iter = parent_dir.read_dir();
+    let mut entries = Vec::new();
+    while let Some(entry) = entries_iter.next().await {
+        let entry = entry?.decode_entry().await?;
+        entries.push(entry);
+    }
+    entries.sort_unstable_by(|a, b| a.path().cmp(b.path()));
+    Ok(entries)
+}
+
+/// Dump pxar archive entry by using the same format used to dump entries from a catalog.
+fn pxar_metadata_catalog_dump_entry<'future, T: Clone + Send + Sync + ReadAt + 'future>(
+    entry: FileEntry<T>,
+    path_prefix: &'future str,
+) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'future>> {
+    let entry_path = entry_path_with_prefix(&entry, path_prefix);
+
+    Box::pin(async move {
+        if let Some(attr) = map_to_dir_entry_attr(&entry)? {
+            let etype = CatalogEntryType::from(&attr);
+            match attr {
+                DirEntryAttribute::File { size, mtime } => {
+                    let mut mtime_string = mtime.to_string();
+                    if let Ok(s) = proxmox_time::strftime_local("%FT%TZ", mtime) {
+                        mtime_string = s;
+                    }
+                    log::info!("{etype} {entry_path:?} {size} {mtime_string}");
+                }
+                DirEntryAttribute::Directory { .. } => {
+                    log::info!("{etype} {entry_path:?}");
+                    let dir = entry.enter_directory().await?;
+                    pxar_metadata_catalog_dump_dir(dir, path_prefix).await?;
+                }
+                _ => log::info!("{etype} {entry_path:?}"),
+            }
+        }
+
+        Ok(())
+    })
+}
+
+/// Recursively iterate over pxar archive entries and dump them using the same format used to dump
+/// entries from a catalog.
+pub async fn pxar_metadata_catalog_dump_dir<T: Clone + Send + Sync + ReadAt>(
+    parent_dir: Directory<T>,
+    path_prefix: &str,
+) -> Result<(), Error> {
+    let entries = pxar_metadata_read_dir(parent_dir).await?;
+    for entry in entries {
+        pxar_metadata_catalog_dump_entry(entry, path_prefix).await?;
+    }
+    Ok(())
+}
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox-backup 5/7] client: catalog: fallback to metadata archives for catalog dump

[Christian Ebner]

Commit c0302805c "client: backup: conditionally write catalog for
file level backups" drops encoding of the dedicated catalog when
archives are encoded as split metadata/data archives with the
`change-detection-mode` set to `data` or `metadata`.

Since the catalog is not present anymore, fallback to use the pxar
metadata archives in the manifest (if present) for generating the
listing of contents in a compatible manner.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- use `pxar_metadata_catalog_dump_dir` over the previous
  `pxar_metadata_catalog_dump` helper, by passing the archives
  root dir directly

 proxmox-backup-client/src/catalog.rs | 41 ++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/proxmox-backup-client/src/catalog.rs b/proxmox-backup-client/src/catalog.rs
index 0a374c011..8cb1eb414 100644
--- a/proxmox-backup-client/src/catalog.rs
+++ b/proxmox-backup-client/src/catalog.rs
@@ -8,11 +8,12 @@ use proxmox_router::cli::*;
 use proxmox_schema::api;
 
 use pbs_api_types::BackupNamespace;
-use pbs_client::tools::has_pxar_filename_extension;
 use pbs_client::tools::key_source::get_encryption_key_password;
+use pbs_client::tools::{get_remote_pxar_reader, has_pxar_filename_extension};
 use pbs_client::{BackupReader, RemoteChunkReader};
 use pbs_tools::crypt_config::CryptConfig;
 use pbs_tools::json::required_string_param;
+use pxar::accessor::aio::Accessor;
 
 use crate::helper;
 use crate::{
@@ -88,14 +89,48 @@ async fn dump_catalog(param: Value) -> Result<Value, Error> {
     let (manifest, _) = client.download_manifest().await?;
     manifest.check_fingerprint(crypt_config.as_ref().map(Arc::as_ref))?;
 
+    let file_info = match manifest.lookup_file_info(CATALOG_NAME) {
+        Ok(file_info) => file_info,
+        Err(err) => {
+            let mut metadata_archives = Vec::new();
+            // No catalog, fallback to metadata archives if present
+            for archive in manifest.files() {
+                if archive.filename.ends_with(".mpxar.didx") {
+                    metadata_archives.push(archive.filename.clone());
+                }
+            }
+            metadata_archives.sort_unstable_by(|a, b| a.cmp(b));
+
+            for archive in &metadata_archives {
+                let (reader, archive_size) = get_remote_pxar_reader(
+                    &archive,
+                    client.clone(),
+                    &manifest,
+                    crypt_config.clone(),
+                )
+                .await?;
+                // only care about the metadata, don't attach a payload reader
+                let reader = pxar::PxarVariant::Unified(reader);
+                let accessor = Accessor::new(reader, archive_size).await?;
+                let root_dir = accessor.open_root().await?;
+                let prefix = format!("./{archive}");
+                pbs_client::tools::pxar_metadata_catalog_dump_dir(root_dir, &prefix).await?;
+            }
+
+            if !metadata_archives.is_empty() {
+                return Ok(Value::Null);
+            }
+
+            bail!(err);
+        }
+    };
+
     let index = client
         .download_dynamic_index(&manifest, CATALOG_NAME)
         .await?;
 
     let most_used = index.find_most_used_chunks(8);
 
-    let file_info = manifest.lookup_file_info(CATALOG_NAME)?;
-
     let chunk_reader = RemoteChunkReader::new(
         client.clone(),
         crypt_config,
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox-backup 6/7] client: helper to mimic catalog find using metadata archive

[Christian Ebner]

Adds helper functions to reimplement the catalog shell functionality
for snapshots being encoded as split pxar archives.

Just as the `CatalogReader`s find method, recursively iterate entries
and call the given callback on all entries matched by the match
patterns, starting from the given parent entry.

The helper has been split into 2 functions for the async recursion to
work.
---
changes since version 1:
- not present in previous version

 pbs-client/src/tools/mod.rs | 40 +++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index 10a368fd2..2426c724c 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -23,6 +23,7 @@ use proxmox_router::cli::{complete_file_name, shellword_split};
 use proxmox_schema::*;
 use proxmox_sys::fs::file_get_json;
 
+use pathpatterns::{MatchList, MatchType};
 use pbs_api_types::{Authid, BackupNamespace, RateLimitConfig, UserWithTokens, BACKUP_REPO_URL};
 use pbs_datastore::catalog::{ArchiveEntry, CatalogEntryType, DirEntryAttribute};
 use pbs_datastore::dynamic_index::{BufferedDynamicReader, LocalDynamicReadAt};
@@ -866,3 +867,42 @@ pub async fn pxar_metadata_catalog_dump_dir<T: Clone + Send + Sync + ReadAt>(
     }
     Ok(())
 }
+
+/// Call the callback on given entry if matched by the match patterns.
+fn pxar_metadata_catalog_find_entry<'future, T: Clone + Send + Sync + ReadAt + 'future>(
+    entry: FileEntry<T>,
+    match_list: &'future (impl MatchList<'future> + Sync),
+    callback: &'future (dyn Fn(&[u8]) -> Result<(), Error> + Send + Sync),
+) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'future>> {
+    Box::pin(async move {
+        let file_mode = entry.metadata().file_type() as u32;
+        let entry_path = entry_path_with_prefix(&entry, "/").as_os_str().to_owned();
+
+        match match_list.matches(entry_path.as_bytes(), file_mode) {
+            Ok(Some(MatchType::Exclude)) => return Ok(()),
+            Ok(Some(MatchType::Include)) => callback(entry_path.as_bytes())?,
+            _ => (),
+        }
+
+        if let EntryKind::Directory = entry.kind() {
+            let dir_entry = entry.enter_directory().await?;
+            pxar_metadata_catalog_find(dir_entry, match_list, callback).await?;
+        }
+
+        Ok(())
+    })
+}
+
+/// Recursively iterate over pxar archive entries and call the callback on entries matching the
+/// match patterns.
+pub async fn pxar_metadata_catalog_find<'future, T: Clone + Send + Sync + ReadAt + 'future>(
+    parent_dir: Directory<T>,
+    match_list: &'future (impl pathpatterns::MatchList<'future> + Sync),
+    callback: &'future (dyn Fn(&[u8]) -> Result<(), Error> + Send + Sync),
+) -> Result<(), Error> {
+    let entries = pxar_metadata_read_dir(parent_dir).await?;
+    for entry in entries {
+        pxar_metadata_catalog_find_entry(entry, match_list, callback).await?;
+    }
+    Ok(())
+}
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox-backup 7/7] client: catalog shell: fallback to accessor for navigation

[Christian Ebner]

Make the catalog optional and use the pxar accessor for navigation if
the catalog is not provided.
This allows to use the metadata archive for navigraion, as for split
pxar archives no dedicated catalog is encoded.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- not present in previous version

 pbs-client/src/catalog_shell.rs      | 303 ++++++++++++++++++++++-----
 proxmox-backup-client/src/catalog.rs |  24 ++-
 2 files changed, 268 insertions(+), 59 deletions(-)

diff --git a/pbs-client/src/catalog_shell.rs b/pbs-client/src/catalog_shell.rs
index 349bb7cbc..eb734ad74 100644
--- a/pbs-client/src/catalog_shell.rs
+++ b/pbs-client/src/catalog_shell.rs
@@ -21,7 +21,7 @@ use pxar::accessor::ReadAt;
 use pxar::{EntryKind, Metadata};
 
 use pbs_datastore::catalog::{self, DirEntryAttribute};
-use proxmox_async::runtime::block_in_place;
+use proxmox_async::runtime::{block_in_place, block_on};
 
 use crate::pxar::Flags;
 
@@ -312,8 +312,9 @@ pub struct Shell {
     /// Interactive prompt.
     prompt: String,
 
-    /// Catalog reader instance to navigate
-    catalog: CatalogReader,
+    /// Optional catalog reader instance to navigate, if not present the Accessor is used for
+    /// navigation
+    catalog: Option<CatalogReader>,
 
     /// List of selected paths for restore
     selected: HashMap<OsString, MatchEntry>,
@@ -347,7 +348,7 @@ impl PathStackEntry {
 impl Shell {
     /// Create a new shell for the given catalog and pxar archive.
     pub async fn new(
-        mut catalog: CatalogReader,
+        mut catalog: Option<CatalogReader>,
         archive_name: &str,
         archive: Accessor,
     ) -> Result<Self, Error> {
@@ -355,11 +356,31 @@ impl Shell {
         let mut rl = rustyline::Editor::<CliHelper>::new();
         rl.set_helper(Some(cli_helper));
 
-        let catalog_root = catalog.root()?;
-        let archive_root = catalog
-            .lookup(&catalog_root, archive_name.as_bytes())?
-            .ok_or_else(|| format_err!("archive not found in catalog"))?;
-        let position = vec![PathStackEntry::new(archive_root)];
+        let mut position = Vec::new();
+        if let Some(catalog) = catalog.as_mut() {
+            let catalog_root = catalog.root()?;
+            let archive_root = catalog
+                .lookup(&catalog_root, archive_name.as_bytes())?
+                .ok_or_else(|| format_err!("archive not found in catalog"))?;
+            position.push(PathStackEntry::new(archive_root));
+        } else {
+            let root = archive.open_root().await?;
+            let root_entry = root.lookup_self().await?;
+            if let EntryKind::Directory = root_entry.kind() {
+                let entry_attr = DirEntryAttribute::Directory {
+                    start: root_entry.entry_range_info().entry_range.start,
+                };
+                position.push(PathStackEntry {
+                    catalog: catalog::DirEntry {
+                        name: archive_name.into(),
+                        attr: entry_attr,
+                    },
+                    pxar: Some(root_entry),
+                });
+            } else {
+                bail!("unexpected root entry type");
+            }
+        }
 
         let mut this = Self {
             rl,
@@ -450,7 +471,7 @@ impl Shell {
 
     async fn resolve_symlink(
         stack: &mut Vec<PathStackEntry>,
-        catalog: &mut CatalogReader,
+        catalog: &mut Option<CatalogReader>,
         accessor: &Accessor,
         follow_symlinks: &mut Option<usize>,
     ) -> Result<(), Error> {
@@ -468,7 +489,7 @@ impl Shell {
             };
 
             let new_stack =
-                Self::lookup(stack, &mut *catalog, accessor, Some(path), follow_symlinks).await?;
+                Self::lookup(stack, catalog, accessor, Some(path), follow_symlinks).await?;
 
             *stack = new_stack;
 
@@ -484,7 +505,7 @@ impl Shell {
     /// out.
     async fn step(
         stack: &mut Vec<PathStackEntry>,
-        catalog: &mut CatalogReader,
+        catalog: &mut Option<CatalogReader>,
         accessor: &Accessor,
         component: std::path::Component<'_>,
         follow_symlinks: &mut Option<usize>,
@@ -503,9 +524,37 @@ impl Shell {
                 if stack.last().unwrap().catalog.is_symlink() {
                     Self::resolve_symlink(stack, catalog, accessor, follow_symlinks).await?;
                 }
-                match catalog.lookup(&stack.last().unwrap().catalog, entry.as_bytes())? {
-                    Some(dir) => stack.push(PathStackEntry::new(dir)),
-                    None => bail!("no such file or directory: {:?}", entry),
+                if let Some(catalog) = catalog {
+                    match catalog.lookup(&stack.last().unwrap().catalog, entry.as_bytes())? {
+                        Some(dir) => stack.push(PathStackEntry::new(dir)),
+                        None => bail!("no such file or directory: {entry:?}"),
+                    }
+                } else {
+                    let parent_dir = if let Some(parent) = stack.last().as_ref() {
+                        if let Some(parent) = parent.pxar.as_ref() {
+                            parent.enter_directory().await?
+                        } else {
+                            bail!("missing pxar entry on stack");
+                        }
+                    } else {
+                        bail!("missing parent entry on stack");
+                    };
+                    match parent_dir.lookup(entry).await? {
+                        Some(entry) => {
+                            let entry_attr = match crate::tools::map_to_dir_entry_attr(&entry)? {
+                                Some(attr) => attr,
+                                None => bail!("entry without attributes"),
+                            };
+                            stack.push(PathStackEntry {
+                                catalog: catalog::DirEntry {
+                                    name: entry.entry().file_name().as_bytes().into(),
+                                    attr: entry_attr,
+                                },
+                                pxar: Some(entry),
+                            })
+                        }
+                        None => bail!("no such file or directory: {entry:?}"),
+                    }
                 }
             }
         }
@@ -515,7 +564,7 @@ impl Shell {
 
     fn step_nofollow(
         stack: &mut Vec<PathStackEntry>,
-        catalog: &mut CatalogReader,
+        catalog: &mut Option<CatalogReader>,
         component: std::path::Component<'_>,
     ) -> Result<(), Error> {
         use std::path::Component;
@@ -531,11 +580,33 @@ impl Shell {
             Component::Normal(entry) => {
                 if stack.last().unwrap().catalog.is_symlink() {
                     bail!("target is a symlink");
-                } else {
+                } else if let Some(catalog) = catalog.as_mut() {
                     match catalog.lookup(&stack.last().unwrap().catalog, entry.as_bytes())? {
                         Some(dir) => stack.push(PathStackEntry::new(dir)),
                         None => bail!("no such file or directory: {:?}", entry),
                     }
+                } else {
+                    let parent_dir = if let Some(parent) = stack.last().as_ref() {
+                        block_on(parent.pxar.as_ref().unwrap().enter_directory())?
+                    } else {
+                        bail!("missing pxar entry on stack");
+                    };
+                    match block_on(parent_dir.lookup(entry))? {
+                        Some(entry) => {
+                            let entry_attr = match crate::tools::map_to_dir_entry_attr(&entry)? {
+                                Some(attr) => attr,
+                                None => bail!("entry without attributes"),
+                            };
+                            stack.push(PathStackEntry {
+                                catalog: catalog::DirEntry {
+                                    name: entry.entry().file_name().as_bytes().into(),
+                                    attr: entry_attr,
+                                },
+                                pxar: Some(entry),
+                            })
+                        }
+                        None => bail!("no such file or directory: {entry:?}"),
+                    }
                 }
             }
         }
@@ -545,7 +616,7 @@ impl Shell {
     /// The pxar accessor is required to resolve symbolic links
     async fn walk_catalog(
         stack: &mut Vec<PathStackEntry>,
-        catalog: &mut CatalogReader,
+        catalog: &mut Option<CatalogReader>,
         accessor: &Accessor,
         path: &Path,
         follow_symlinks: &mut Option<usize>,
@@ -559,7 +630,7 @@ impl Shell {
     /// Non-async version cannot follow symlinks.
     fn walk_catalog_nofollow(
         stack: &mut Vec<PathStackEntry>,
-        catalog: &mut CatalogReader,
+        catalog: &mut Option<CatalogReader>,
         path: &Path,
     ) -> Result<(), Error> {
         for c in path.components() {
@@ -612,12 +683,34 @@ impl Shell {
                     tmp_stack = self.position.clone();
                 }
                 Self::walk_catalog_nofollow(&mut tmp_stack, &mut self.catalog, &path)?;
-                (&tmp_stack.last().unwrap().catalog, base, part)
+                (&tmp_stack.last().unwrap(), base, part)
             }
-            None => (&self.position.last().unwrap().catalog, "", input),
+            None => (&self.position.last().unwrap(), "", input),
         };
 
-        let entries = self.catalog.read_dir(parent)?;
+        let entries = if let Some(catalog) = self.catalog.as_mut() {
+            catalog.read_dir(&parent.catalog)?
+        } else {
+            let dir = if let Some(entry) = parent.pxar.as_ref() {
+                block_on(entry.enter_directory())?
+            } else {
+                bail!("missing pxar entry on stack");
+            };
+            let mut out = Vec::new();
+            let entries = block_on(crate::tools::pxar_metadata_read_dir(dir))?;
+            for entry in entries {
+                let mut name = base.to_string();
+                let file_name = entry.file_name().as_bytes();
+                if file_name.starts_with(part.as_bytes()) {
+                    name.push_str(std::str::from_utf8(file_name)?);
+                    if entry.is_dir() {
+                        name.push('/');
+                    }
+                    out.push(name);
+                }
+            }
+            return Ok(out);
+        };
 
         let mut out = Vec::new();
         for entry in entries {
@@ -637,7 +730,7 @@ impl Shell {
     // Break async recursion here: lookup -> walk_catalog -> step -> lookup
     fn lookup<'future, 's, 'c, 'a, 'p, 'y>(
         stack: &'s [PathStackEntry],
-        catalog: &'c mut CatalogReader,
+        catalog: &'c mut Option<CatalogReader>,
         accessor: &'a Accessor,
         path: Option<&'p Path>,
         follow_symlinks: &'y mut Option<usize>,
@@ -678,7 +771,23 @@ impl Shell {
 
         let last = stack.last().unwrap();
         if last.catalog.is_directory() {
-            let items = self.catalog.read_dir(&stack.last().unwrap().catalog)?;
+            let items = if let Some(catalog) = self.catalog.as_mut() {
+                catalog.read_dir(&stack.last().unwrap().catalog)?
+            } else {
+                let dir = if let Some(entry) = last.pxar.as_ref() {
+                    entry.enter_directory().await?
+                } else {
+                    bail!("missing pxar entry on stack");
+                };
+
+                let mut out = std::io::stdout();
+                let items = crate::tools::pxar_metadata_read_dir(dir).await?;
+                for item in items {
+                    out.write_all(&item.file_name().as_bytes())?;
+                    out.write_all(b"\n")?;
+                }
+                return Ok(());
+            };
             let mut out = std::io::stdout();
             // FIXME: columnize
             for item in items {
@@ -820,17 +929,37 @@ impl Shell {
     async fn list_matching_files(&mut self) -> Result<(), Error> {
         let matches = self.build_match_list();
 
-        self.catalog.find(
-            &self.position[0].catalog,
-            &mut Vec::new(),
-            &matches,
-            &mut |path: &[u8]| -> Result<(), Error> {
-                let mut out = std::io::stdout();
-                out.write_all(path)?;
-                out.write_all(b"\n")?;
-                Ok(())
-            },
-        )?;
+        if let Some(catalog) = self.catalog.as_mut() {
+            catalog.find(
+                &self.position[0].catalog,
+                &mut Vec::new(),
+                &matches,
+                &mut |path: &[u8]| -> Result<(), Error> {
+                    let mut out = std::io::stdout();
+                    out.write_all(path)?;
+                    out.write_all(b"\n")?;
+                    Ok(())
+                },
+            )?;
+        } else {
+            let parent_dir = self.position[0]
+                .pxar
+                .as_ref()
+                .unwrap()
+                .enter_directory()
+                .await?;
+            crate::tools::pxar_metadata_catalog_find(
+                parent_dir,
+                &matches,
+                &|path: &[u8]| -> Result<(), Error> {
+                    let mut out = std::io::stdout();
+                    out.write_all(path)?;
+                    out.write_all(b"\n")?;
+                    Ok(())
+                },
+            )
+            .await?;
+        }
 
         Ok(())
     }
@@ -841,18 +970,38 @@ impl Shell {
             MatchEntry::parse_pattern(pattern, PatternFlag::PATH_NAME, MatchType::Include)?;
 
         let mut found_some = false;
-        self.catalog.find(
-            &self.position[0].catalog,
-            &mut Vec::new(),
-            &[&pattern_entry],
-            &mut |path: &[u8]| -> Result<(), Error> {
-                found_some = true;
-                let mut out = std::io::stdout();
-                out.write_all(path)?;
-                out.write_all(b"\n")?;
-                Ok(())
-            },
-        )?;
+        if let Some(catalog) = self.catalog.as_mut() {
+            catalog.find(
+                &self.position[0].catalog,
+                &mut Vec::new(),
+                &[&pattern_entry],
+                &mut |path: &[u8]| -> Result<(), Error> {
+                    found_some = true;
+                    let mut out = std::io::stdout();
+                    out.write_all(path)?;
+                    out.write_all(b"\n")?;
+                    Ok(())
+                },
+            )?;
+        } else {
+            let parent_dir = self.position[0]
+                .pxar
+                .as_ref()
+                .unwrap()
+                .enter_directory()
+                .await?;
+            crate::tools::pxar_metadata_catalog_find(
+                parent_dir,
+                &[&pattern_entry],
+                &|path: &[u8]| -> Result<(), Error> {
+                    let mut out = std::io::stdout();
+                    out.write_all(path)?;
+                    out.write_all(b"\n")?;
+                    Ok(())
+                },
+            )
+            .await?;
+        }
 
         if found_some && select {
             self.selected.insert(pattern_os, pattern_entry);
@@ -960,22 +1109,44 @@ struct ExtractorState<'a> {
 
     extractor: crate::pxar::extract::Extractor,
 
-    catalog: &'a mut CatalogReader,
+    catalog: &'a mut Option<CatalogReader>,
     match_list: &'a [MatchEntry],
     accessor: &'a Accessor,
 }
 
 impl<'a> ExtractorState<'a> {
     pub fn new(
-        catalog: &'a mut CatalogReader,
+        catalog: &'a mut Option<CatalogReader>,
         dir_stack: Vec<PathStackEntry>,
         extractor: crate::pxar::extract::Extractor,
         match_list: &'a [MatchEntry],
         accessor: &'a Accessor,
     ) -> Result<Self, Error> {
-        let read_dir = catalog
-            .read_dir(&dir_stack.last().unwrap().catalog)?
-            .into_iter();
+        let read_dir = if let Some(catalog) = catalog.as_mut() {
+            catalog
+                .read_dir(&dir_stack.last().unwrap().catalog)?
+                .into_iter()
+        } else {
+            let dir = if let Some(entry) = dir_stack.last().unwrap().pxar.as_ref() {
+                block_on(entry.enter_directory())?
+            } else {
+                bail!("missing pxar entry on stack");
+            };
+            let entries = block_on(crate::tools::pxar_metadata_read_dir(dir))?;
+            entries
+                .into_iter()
+                .map(|entry| {
+                    let entry_attr = crate::tools::map_to_dir_entry_attr(&entry)
+                        .unwrap()
+                        .unwrap();
+                    catalog::DirEntry {
+                        name: entry.entry().file_name().as_bytes().into(),
+                        attr: entry_attr,
+                    }
+                })
+                .collect::<Vec<catalog::DirEntry>>()
+                .into_iter()
+        };
         Ok(Self {
             path: Vec::new(),
             path_len: 0,
@@ -1053,11 +1224,31 @@ impl<'a> ExtractorState<'a> {
         entry: catalog::DirEntry,
         match_result: Option<MatchType>,
     ) -> Result<(), Error> {
+        let entry_iter = if let Some(catalog) = self.catalog.as_mut() {
+            catalog.read_dir(&entry)?.into_iter()
+        } else {
+            self.dir_stack.push(PathStackEntry::new(entry.clone()));
+            let dir = Shell::walk_pxar_archive(self.accessor, &mut self.dir_stack).await?;
+            self.dir_stack.pop();
+            let dir = dir.enter_directory().await?;
+            let entries = block_on(crate::tools::pxar_metadata_read_dir(dir))?;
+            entries
+                .into_iter()
+                .map(|entry| {
+                    let entry_attr = crate::tools::map_to_dir_entry_attr(&entry)
+                        .unwrap()
+                        .unwrap();
+                    catalog::DirEntry {
+                        name: entry.entry().file_name().as_bytes().into(),
+                        attr: entry_attr,
+                    }
+                })
+                .collect::<Vec<catalog::DirEntry>>()
+                .into_iter()
+        };
         // enter a new directory:
-        self.read_dir_stack.push(mem::replace(
-            &mut self.read_dir,
-            self.catalog.read_dir(&entry)?.into_iter(),
-        ));
+        self.read_dir_stack
+            .push(mem::replace(&mut self.read_dir, entry_iter));
         self.matches_stack.push(self.matches);
         self.dir_stack.push(PathStackEntry::new(entry));
         self.path_len_stack.push(self.path_len);
diff --git a/proxmox-backup-client/src/catalog.rs b/proxmox-backup-client/src/catalog.rs
index 8cb1eb414..0a61db1a0 100644
--- a/proxmox-backup-client/src/catalog.rs
+++ b/proxmox-backup-client/src/catalog.rs
@@ -228,11 +228,29 @@ async fn catalog_shell(param: Value) -> Result<(), Error> {
     )
     .await?;
 
-    let mut tmpfile = pbs_client::tools::create_tmp_file()?;
-
     let (manifest, _) = client.download_manifest().await?;
     manifest.check_fingerprint(crypt_config.as_ref().map(Arc::as_ref))?;
 
+    if let Err(_err) = manifest.lookup_file_info(CATALOG_NAME) {
+        // No catalog, fallback to pxar archive accessor if present
+        let accessor = helper::get_pxar_fuse_accessor(
+            &server_archive_name,
+            client.clone(),
+            &manifest,
+            crypt_config.clone(),
+        )
+        .await?;
+
+        let state = Shell::new(None, &server_archive_name, accessor).await?;
+        log::info!("Starting interactive shell");
+        state.shell().await?;
+        record_repository(&repo);
+
+        return Ok(());
+    }
+
+    let mut tmpfile = pbs_client::tools::create_tmp_file()?;
+
     let decoder = helper::get_pxar_fuse_accessor(
         &server_archive_name,
         client.clone(),
@@ -266,7 +284,7 @@ async fn catalog_shell(param: Value) -> Result<(), Error> {
 
     catalogfile.seek(SeekFrom::Start(0))?;
     let catalog_reader = CatalogReader::new(catalogfile);
-    let state = Shell::new(catalog_reader, &server_archive_name, decoder).await?;
+    let state = Shell::new(Some(catalog_reader), &server_archive_name, decoder).await?;
 
     log::info!("Starting interactive shell");
     state.shell().await?;
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup 2/7] client: tools: factor out entry path prefix helper

[Fabian Grünbichler]

Quoting Christian Ebner (2024-07-22 12:30:29)
> Move the logic to generate `FileEntry` paths with a given prefix to
> its own helper function for it to be reusable for the catalog shell
> implementation of split pxar archives.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes since version 1:
> - not present in pervious version
> 
>  pbs-client/src/tools/mod.rs | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
> index 8c634ba76..e6e548403 100644
> --- a/pbs-client/src/tools/mod.rs
> +++ b/pbs-client/src/tools/mod.rs
> @@ -1,4 +1,5 @@
>  //! Shared tools useful for common CLI clients.
> +use pxar::accessor::aio::FileEntry;
>  use std::collections::HashMap;
>  use std::env::VarError::{NotPresent, NotUnicode};
>  use std::ffi::OsStr;
> @@ -742,16 +743,7 @@ pub async fn pxar_metadata_catalog_lookup<T: Clone + ReadAt>(
>                  EntryKind::Socket => DirEntryAttribute::Socket,
>              };
>  
> -            let entry_path = if let Some(prefix) = path_prefix {
> -                let mut entry_path = PathBuf::from(prefix);
> -                match entry.path().strip_prefix("/") {
> -                    Ok(path) => entry_path.push(path),
> -                    Err(_) => entry_path.push(entry.path()),
> -                }
> -                entry_path
> -            } else {
> -                PathBuf::from(entry.path())
> -            };
> +            let entry_path = entry_path_with_prefix(&entry, path_prefix.unwrap_or_default());
>              entries.push(ArchiveEntry::new(
>                  entry_path.as_os_str().as_bytes(),
>                  Some(&entry_attr),
> @@ -790,3 +782,13 @@ pub fn create_tmp_file() -> std::io::Result<std::fs::File> {
>          }
>      })
>  }
> +
> +/// Generate entry path for given [`FileEntry`], prefixed by given `path_prefix` component(s).
> +fn entry_path_with_prefix<T: Clone + ReadAt>(entry: &FileEntry<T>, path_prefix: &str) -> PathBuf {
> +    let mut entry_path = PathBuf::from(&path_prefix);
> +    match entry.path().strip_prefix("/") {
> +        Ok(path) => entry_path.push(path),
> +        Err(_) => entry_path.push(entry.path()),
> +    }
> +    entry_path
> +}

the factored out version is not equivalent to the inline one - if path_prefix
is None (unwrapped to the empty string), then this now converts an absolute
path to a relative one?


> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
>


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup 3/7] client: tools: factor out pxar entry to dir entry mapping

[Fabian Grünbichler]

Quoting Christian Ebner (2024-07-22 12:30:30)
> Create a dedicated helper for performing the mapping of pxar
> `FileEntry`s to `DirEntryAttribute`s.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes since version 1:
> - not present in previous version
> 
>  pbs-client/src/tools/mod.rs | 56 +++++++++++++++++++++++--------------
>  1 file changed, 35 insertions(+), 21 deletions(-)
> 
> diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
> index e6e548403..97f71f3f1 100644
> --- a/pbs-client/src/tools/mod.rs
> +++ b/pbs-client/src/tools/mod.rs
> @@ -720,27 +720,9 @@ pub async fn pxar_metadata_catalog_lookup<T: Clone + ReadAt>(
>          while let Some(entry) = entries_iter.next().await {
>              let entry = entry?.decode_entry().await?;
>  
> -            let entry_attr = match entry.kind() {
> -                EntryKind::Version(_) | EntryKind::Prelude(_) | EntryKind::GoodbyeTable => continue,
> -                EntryKind::Directory => DirEntryAttribute::Directory {
> -                    start: entry.entry_range_info().entry_range.start,
> -                },
> -                EntryKind::File { size, .. } => {
> -                    let mtime = match entry.metadata().mtime_as_duration() {
> -                        SignedDuration::Positive(val) => i64::try_from(val.as_secs())?,
> -                        SignedDuration::Negative(val) => -i64::try_from(val.as_secs())?,
> -                    };
> -                    DirEntryAttribute::File { size: *size, mtime }
> -                }
> -                EntryKind::Device(_) => match entry.metadata().file_type() {
> -                    mode::IFBLK => DirEntryAttribute::BlockDevice,
> -                    mode::IFCHR => DirEntryAttribute::CharDevice,
> -                    _ => bail!("encountered unknown device type"),
> -                },
> -                EntryKind::Symlink(_) => DirEntryAttribute::Symlink,
> -                EntryKind::Hardlink(_) => DirEntryAttribute::Hardlink,
> -                EntryKind::Fifo => DirEntryAttribute::Fifo,
> -                EntryKind::Socket => DirEntryAttribute::Socket,
> +            let entry_attr = match map_to_dir_entry_attr(&entry)? {
> +                Some(attr) => attr,
> +                None => continue,
>              };
>  
>              let entry_path = entry_path_with_prefix(&entry, path_prefix.unwrap_or_default());
> @@ -792,3 +774,35 @@ fn entry_path_with_prefix<T: Clone + ReadAt>(entry: &FileEntry<T>, path_prefix:
>      }
>      entry_path
>  }
> +
> +/// Map a pxar [`FileEntry`] to the corresponding [`DirEntryAttribute`].
> +///
> +/// Returns `Ok(None)` if there is no corresponding variant, `Err` if an unknown device type is
> +/// encountered.
> +pub(crate) fn map_to_dir_entry_attr<T: Clone + ReadAt>(

this could live as a TryFrom<pxar::Kind> in DirEntryAttribute?

> +    entry: &FileEntry<T>,
> +) -> Result<Option<DirEntryAttribute>, Error> {
> +    let attr = match entry.kind() {
> +        EntryKind::Version(_) | EntryKind::Prelude(_) | EntryKind::GoodbyeTable => return Ok(None),

provided this can be turned into an error as well, since mapping these entries
to a DirEntryAttribute doesn't make any sense?

AFAICT the existing call sites:
- bail if this returns None
- unwrap the Result and Option(!)
- continue if None (but that one is when iterating over a directory's entries,
  so shouldn't ever hit a Version or Prelude at least)
- don't handle None (when dumping an entry), that should be fine as well

> +        EntryKind::Directory => DirEntryAttribute::Directory {
> +            start: entry.entry_range_info().entry_range.start,
> +        },
> +        EntryKind::File { size, .. } => {
> +            let mtime = match entry.metadata().mtime_as_duration() {
> +                SignedDuration::Positive(val) => i64::try_from(val.as_secs())?,
> +                SignedDuration::Negative(val) => -i64::try_from(val.as_secs())?,
> +            };
> +            DirEntryAttribute::File { size: *size, mtime }
> +        }
> +        EntryKind::Device(_) => match entry.metadata().file_type() {
> +            mode::IFBLK => DirEntryAttribute::BlockDevice,
> +            mode::IFCHR => DirEntryAttribute::CharDevice,
> +            _ => bail!("encountered unknown device type"),
> +        },
> +        EntryKind::Symlink(_) => DirEntryAttribute::Symlink,
> +        EntryKind::Hardlink(_) => DirEntryAttribute::Hardlink,
> +        EntryKind::Fifo => DirEntryAttribute::Fifo,
> +        EntryKind::Socket => DirEntryAttribute::Socket,
> +    };
> +    Ok(Some(attr))
> +}
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
>


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup 0/7] fix catalog dump and shell for split pxar archives

[Fabian Grünbichler]

Quoting Christian Ebner (2024-07-22 12:30:27)
> This patch series fixes the `catalog dump` and `catalog shell`
> commands for snapshots created by setting the `change-detection-mode`
> to either `data` or `metadata`, therefore using split pxar archive
> encoding without encoding the dedicated catalog.
> 
> If no catalog file can be found in the snaphsots manifest, the fallback
> behaviour is now to check if there are metadata archives present in the
> manifest, and if so use the metadata archive accessor for either
> dumping the contents in a format compatible to the catalog dump or to
> interactively navigate and restore contents via the catalog shell.

some comments on individual patches, but one higher level one here:

the new helpers are mostly used by the catalog shell and by eachother. I think
it would make sense to find or create a more specific place than the catch-all
pbs-client tools. the 'handle_root_with_optional_format_version_prelude' also
seems a bit misplaced there and could maybe move closer to pxar related code as
well?

> 
> Changes since version 1:
> - Implement also the `catalog shell` command for split pxar archives
> - Factor out common functionality into dedicated helpers
> 
> Christian Ebner (7):
>   client: make helper to get remote pxar reader reusable
>   client: tools: factor out entry path prefix helper
>   client: tools: factor out pxar entry to dir entry mapping
>   client: add helper to dump catalog from metadata archive
>   client: catalog: fallback to metadata archives for catalog dump
>   client: helper to mimic catalog find using metadata archive
>   client: catalog shell: fallback to accessor for navigation
> 
>  pbs-client/src/catalog_shell.rs      | 303 ++++++++++++++++++++++-----
>  pbs-client/src/tools/mod.rs          | 214 +++++++++++++++----
>  proxmox-backup-client/src/catalog.rs |  65 +++++-
>  proxmox-file-restore/src/main.rs     |  31 +--
>  4 files changed, 488 insertions(+), 125 deletions(-)
> 
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
>


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup 3/7] client: tools: factor out pxar entry to dir entry mapping

[Christian Ebner]

On 8/7/24 11:22, Fabian Grünbichler wrote:
> Quoting Christian Ebner (2024-07-22 12:30:30)
>> Create a dedicated helper for performing the mapping of pxar
>> `FileEntry`s to `DirEntryAttribute`s.
>>
>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>> ---
>> changes since version 1:
>> - not present in previous version
>>
>>   pbs-client/src/tools/mod.rs | 56 +++++++++++++++++++++++--------------
>>   1 file changed, 35 insertions(+), 21 deletions(-)
>>
>> diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
>> index e6e548403..97f71f3f1 100644
>> --- a/pbs-client/src/tools/mod.rs
>> +++ b/pbs-client/src/tools/mod.rs
>> @@ -720,27 +720,9 @@ pub async fn pxar_metadata_catalog_lookup<T: Clone + ReadAt>(
>>           while let Some(entry) = entries_iter.next().await {
>>               let entry = entry?.decode_entry().await?;
>>   
>> -            let entry_attr = match entry.kind() {
>> -                EntryKind::Version(_) | EntryKind::Prelude(_) | EntryKind::GoodbyeTable => continue,
>> -                EntryKind::Directory => DirEntryAttribute::Directory {
>> -                    start: entry.entry_range_info().entry_range.start,
>> -                },
>> -                EntryKind::File { size, .. } => {
>> -                    let mtime = match entry.metadata().mtime_as_duration() {
>> -                        SignedDuration::Positive(val) => i64::try_from(val.as_secs())?,
>> -                        SignedDuration::Negative(val) => -i64::try_from(val.as_secs())?,
>> -                    };
>> -                    DirEntryAttribute::File { size: *size, mtime }
>> -                }
>> -                EntryKind::Device(_) => match entry.metadata().file_type() {
>> -                    mode::IFBLK => DirEntryAttribute::BlockDevice,
>> -                    mode::IFCHR => DirEntryAttribute::CharDevice,
>> -                    _ => bail!("encountered unknown device type"),
>> -                },
>> -                EntryKind::Symlink(_) => DirEntryAttribute::Symlink,
>> -                EntryKind::Hardlink(_) => DirEntryAttribute::Hardlink,
>> -                EntryKind::Fifo => DirEntryAttribute::Fifo,
>> -                EntryKind::Socket => DirEntryAttribute::Socket,
>> +            let entry_attr = match map_to_dir_entry_attr(&entry)? {
>> +                Some(attr) => attr,
>> +                None => continue,
>>               };
>>   
>>               let entry_path = entry_path_with_prefix(&entry, path_prefix.unwrap_or_default());
>> @@ -792,3 +774,35 @@ fn entry_path_with_prefix<T: Clone + ReadAt>(entry: &FileEntry<T>, path_prefix:
>>       }
>>       entry_path
>>   }
>> +
>> +/// Map a pxar [`FileEntry`] to the corresponding [`DirEntryAttribute`].
>> +///
>> +/// Returns `Ok(None)` if there is no corresponding variant, `Err` if an unknown device type is
>> +/// encountered.
>> +pub(crate) fn map_to_dir_entry_attr<T: Clone + ReadAt>(
> 
> this could live as a TryFrom<pxar::Kind> in DirEntryAttribute?
> 

That will not work, as to convert the entries, not just the kind but 
also their metadata is required.
But will go the suggested route and add this as `TryFrom<&FileEntry<T>>` 
instead, which contains all of what is needed.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup 0/7] fix catalog dump and shell for split pxar archives

[Christian Ebner]

superseded-by version 3:
https://lists.proxmox.com/pipermail/pbs-devel/2024-August/010504.html


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel