[pbs-devel] [PATCH v2 proxmox-backup 0/9] drop catalog encoding for split pxar archives

[pbs-devel] [PATCH v2 proxmox-backup 0/9] drop catalog encoding for split pxar archives

[Christian Ebner]

The catalog stores metadata needed for fast lookups via e.g. the file
browser for single file restore.
Since it is now possible to create snapshots with split pxar archives,
the same metadata can be accessed in a reasonable performat way also via
the metadata archive, effectively making the catalog unneeded duplicate
data for this case.

This patch series therefore allows to lookup directory entries where
needed via the metadata archive instead of the catalog and conditionally
drops the catalog encoding for snapshots using split archive encoding.

The patches adapt not only the `catalog` api endpoint, used by the
Proxmox Backup WebUI to access the snapshots contents but also the
proxomox-file-restore, which is used by the Proxmox Virtual Environments
PBS storage plugin to access and restore contents.

Tested that regular backups can still be inspected and restored and that
the same functionality is still available for split archive backups,
without now having a catalog encoded.

Functionally was tested with the following path series applied to PVE:
https://lists.proxmox.com/pipermail/pve-devel/2024-May/064024.html

Please note: For directories with may entries a noticeable performance
drop is present, which might be improved upon by optimizing decoding.

Changes since version 1:
- Do not base64 encode archive name
- Split off file restore archive name listing

The rest of the comments will be addressed as followups as discussed
off-list.

Christian Ebner (9):
  api: datastore: factor out path decoding for catalog
  api: datastore: move reusable code out of thread
  client: tools: add helper to lookup `ArchiveEntry`s via pxar
  api: datastore: conditional lookup for catalog endpoint
  api: datastore: add optional archive-name to file-restore
  file-restore: never list ppxar as archive
  file-restore: fallback to mpxar if catalog not present
  www: content: lookup via metadata archive instead of catalog
  client: backup: conditionally write catalog for file level backups

 pbs-client/src/pxar_backup_stream.rs |  11 ++-
 pbs-client/src/tools/mod.rs          |  84 +++++++++++++++++++
 proxmox-backup-client/src/main.rs    |  21 +++--
 proxmox-file-restore/src/main.rs     |  72 ++++++++++++----
 src/api2/admin/datastore.rs          | 121 ++++++++++++++++++---------
 www/datastore/Content.js             |   3 +
 6 files changed, 243 insertions(+), 69 deletions(-)

-- 
2.39.2

From 8dffaab3ce2c20b0c376d6c0dc8b3d561499fa24 Mon Sep 17 00:00:00 2001
From: Christian Ebner <c.ebner@proxmox.com>
Date: Fri, 7 Jun 2024 11:25:09 +0200

Christian Ebner (8):
  api: datastore: factor out path decoding for catalog
  api: datastore: move reusable code out of thread
  client: tools: add helper to lookup `ArchiveEntry`s via pxar
  api: datastore: conditional lookup for catalog endpoint
  api: datastore: add optional archive-name to file-restore
  file-restore: fallback to mpxar if catalog not present
  www: content: lookup via metadata archive instead of catalog
  client: backup: conditionally write catalog for file level backups

 pbs-client/src/pxar_backup_stream.rs |  11 ++-
 pbs-client/src/tools/mod.rs          |  84 +++++++++++++++++
 proxmox-backup-client/src/main.rs    |  21 +++--
 proxmox-file-restore/src/main.rs     |  72 +++++++++++----
 src/api2/admin/datastore.rs          | 129 ++++++++++++++++++---------
 www/datastore/Content.js             |   3 +
 6 files changed, 251 insertions(+), 69 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/9] api: datastore: factor out path decoding for catalog

[Christian Ebner]

The file path passed to the catalog is base64 encoded, with an exception
for the root.
Factor this check and decoding step out into a helper function to make
it reusable when doing the same for lookups via the metadata archive
instead of the catalog.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/admin/datastore.rs | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 34a9105dd..854302999 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1636,6 +1636,14 @@ pub fn upload_backup_log(
     .boxed()
 }
 
+fn decode_path(path: &str) -> Result<Vec<u8>, Error> {
+    if path != "root" && path != "/" {
+        base64::decode(path).map_err(|err| format_err!("base64 decoding of path failed - {err}"))
+    } else {
+        Ok(vec![b'/'])
+    }
+}
+
 #[api(
     input: {
         properties: {
@@ -1709,12 +1717,7 @@ pub async fn catalog(
 
         let mut catalog_reader = CatalogReader::new(reader);
 
-        let path = if filepath != "root" && filepath != "/" {
-            base64::decode(filepath)?
-        } else {
-            vec![b'/']
-        };
-
+        let path = decode_path(&filepath)?;
         catalog_reader.list_dir_contents(&path)
     })
     .await?
-- 
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/9] api: datastore: move reusable code out of thread

[Christian Ebner]

Move code that can be reused when having to  perform a lookup via the
pxar metadata archive instead of the catalog out of the thread.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/admin/datastore.rs | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 854302999..117dab080 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1678,30 +1678,30 @@ pub async fn catalog(
 ) -> Result<Vec<ArchiveEntry>, Error> {
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
 
-    tokio::task::spawn_blocking(move || {
-        let ns = ns.unwrap_or_default();
+    let ns = ns.unwrap_or_default();
 
-        let datastore = check_privs_and_load_store(
-            &store,
-            &ns,
-            &auth_id,
-            PRIV_DATASTORE_READ,
-            PRIV_DATASTORE_BACKUP,
-            Some(Operation::Read),
-            &backup_dir.group,
-        )?;
+    let datastore = check_privs_and_load_store(
+        &store,
+        &ns,
+        &auth_id,
+        PRIV_DATASTORE_READ,
+        PRIV_DATASTORE_BACKUP,
+        Some(Operation::Read),
+        &backup_dir.group,
+    )?;
 
-        let backup_dir = datastore.backup_dir(ns, backup_dir)?;
+    let backup_dir = datastore.backup_dir(ns, backup_dir)?;
 
-        let file_name = CATALOG_NAME;
+    let file_name = CATALOG_NAME;
 
-        let (manifest, files) = read_backup_index(&backup_dir)?;
-        for file in files {
-            if file.filename == file_name && file.crypt_mode == Some(CryptMode::Encrypt) {
-                bail!("cannot decode '{}' - is encrypted", file_name);
-            }
+    let (manifest, files) = read_backup_index(&backup_dir)?;
+    for file in files {
+        if file.filename == file_name && file.crypt_mode == Some(CryptMode::Encrypt) {
+            bail!("cannot decode '{file_name}' - is encrypted");
         }
+    }
 
+    tokio::task::spawn_blocking(move || {
         let mut path = datastore.base_path();
         path.push(backup_dir.relative_path());
         path.push(file_name);
-- 
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/9] client: tools: add helper to lookup `ArchiveEntry`s via pxar

[Christian Ebner]

In preparation to lookup entries via the pxar metadata archive
instead of the catalog, in order to drop encoding the catalog
for snapshots using split pxar archives altogehter.

This helper allows to lookup the directory entries via the provided
accessor instance and formats them to be compatible with the output
as produced by lookups via the catalog.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-client/src/tools/mod.rs | 84 +++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index 29cd33df1..436475738 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -1,9 +1,12 @@
 //! Shared tools useful for common CLI clients.
 use std::collections::HashMap;
 use std::env::VarError::{NotPresent, NotUnicode};
+use std::ffi::OsStr;
 use std::fs::File;
 use std::io::{BufRead, BufReader};
+use std::os::unix::ffi::OsStrExt;
 use std::os::unix::io::FromRawFd;
+use std::path::PathBuf;
 use std::process::Command;
 
 use anyhow::{bail, format_err, Context, Error};
@@ -16,7 +19,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::BackupManifest;
+use pxar::accessor::aio::Accessor;
+use pxar::accessor::ReadAt;
+use pxar::format::SignedDuration;
+use pxar::{mode, EntryKind};
 
 use crate::{BackupRepository, HttpClient, HttpClientOptions};
 
@@ -635,3 +643,79 @@ pub fn raise_nofile_limit() -> Result<libc::rlimit64, Error> {
 
     Ok(old)
 }
+
+/// Look up the directory entries of the given directory `path` in a pxar archive via it's given
+/// `accessor` and return the entries formatted as [`ArchiveEntry`]'s, compatible with reading
+/// entries from the catalog.
+///
+/// If the optional `path_prefix` is given, all returned entry paths will be prefixed with it.
+pub async fn pxar_metadata_catalog_lookup<T: Clone + ReadAt>(
+    accessor: Accessor<T>,
+    path: &OsStr,
+    path_prefix: Option<&str>,
+) -> Result<Vec<ArchiveEntry>, Error> {
+    let root = accessor.open_root().await?;
+    let dir_entry = root
+        .lookup(&path)
+        .await
+        .map_err(|err| format_err!("lookup failed - {err}"))?
+        .ok_or_else(|| format_err!("lookup failed - error opening '{path:?}'"))?;
+
+    let mut entries = Vec::new();
+    if let EntryKind::Directory = dir_entry.kind() {
+        let dir_entry = dir_entry
+            .enter_directory()
+            .await
+            .map_err(|err| format_err!("failed to enter directory - {err}"))?;
+
+        let mut entries_iter = dir_entry.read_dir();
+        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) => -1 * 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_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())
+            };
+            entries.push(ArchiveEntry::new(
+                entry_path.as_os_str().as_bytes(),
+                Some(&entry_attr),
+            ));
+        }
+    } else {
+        bail!(format!(
+            "expected directory entry, got entry kind '{:?}'",
+            dir_entry.kind()
+        ));
+    }
+
+    Ok(entries)
+}
-- 
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/9] api: datastore: conditional lookup for catalog endpoint

[Christian Ebner]

Add an optional `archive-name` parameter, indicating the metadata
archive to be used for directory content lookups instead of the
catalog. If provided, instead of the catalog reader, a pxar Accessor
instance is created to perform the lookup.

This is in preparation for dropping catalog encoding for snapshots
with split pxar archive encoding.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/admin/datastore.rs | 66 +++++++++++++++++++++++++++----------
 1 file changed, 48 insertions(+), 18 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 117dab080..b90302966 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1659,7 +1659,12 @@ fn decode_path(path: &str) -> Result<Vec<u8>, Error> {
             "filepath": {
                 description: "Base64 encoded path.",
                 type: String,
-            }
+            },
+            "archive-name": {
+                type: String,
+                description: "Name of the archive to use for lookup",
+                optional: true,
+            },
         },
     },
     access: {
@@ -1674,8 +1679,11 @@ pub async fn catalog(
     ns: Option<BackupNamespace>,
     backup_dir: pbs_api_types::BackupDir,
     filepath: String,
+    archive_name: Option<String>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Vec<ArchiveEntry>, Error> {
+    let file_name = archive_name.clone().unwrap_or_else(|| CATALOG_NAME.to_string());
+
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
 
     let ns = ns.unwrap_or_default();
@@ -1692,8 +1700,6 @@ pub async fn catalog(
 
     let backup_dir = datastore.backup_dir(ns, backup_dir)?;
 
-    let file_name = CATALOG_NAME;
-
     let (manifest, files) = read_backup_index(&backup_dir)?;
     for file in files {
         if file.filename == file_name && file.crypt_mode == Some(CryptMode::Encrypt) {
@@ -1701,26 +1707,50 @@ pub async fn catalog(
         }
     }
 
-    tokio::task::spawn_blocking(move || {
-        let mut path = datastore.base_path();
-        path.push(backup_dir.relative_path());
-        path.push(file_name);
+    if archive_name.is_none() {
+        tokio::task::spawn_blocking(move || {
+            let mut path = datastore.base_path();
+            path.push(backup_dir.relative_path());
+            path.push(&file_name);
 
-        let index = DynamicIndexReader::open(&path)
-            .map_err(|err| format_err!("unable to read dynamic index '{:?}' - {}", &path, err))?;
+            let index = DynamicIndexReader::open(&path)
+                .map_err(|err| format_err!("unable to read dynamic index '{path:?}' - {err}"))?;
 
-        let (csum, size) = index.compute_csum();
-        manifest.verify_file(file_name, &csum, size)?;
+            let (csum, size) = index.compute_csum();
+            manifest.verify_file(&file_name, &csum, size)?;
 
-        let chunk_reader = LocalChunkReader::new(datastore, None, CryptMode::None);
-        let reader = BufferedDynamicReader::new(index, chunk_reader);
+            let chunk_reader = LocalChunkReader::new(datastore, None, CryptMode::None);
+            let reader = BufferedDynamicReader::new(index, chunk_reader);
 
-        let mut catalog_reader = CatalogReader::new(reader);
+            let mut catalog_reader = CatalogReader::new(reader);
 
-        let path = decode_path(&filepath)?;
-        catalog_reader.list_dir_contents(&path)
-    })
-    .await?
+            let path = decode_path(&filepath)?;
+            catalog_reader.list_dir_contents(&path)
+        })
+        .await?
+    } else {
+        let (archive_name, payload_archive_name) =
+            pbs_client::tools::get_pxar_archive_names(&file_name, &manifest)?;
+        let (reader, archive_size) =
+            get_local_pxar_reader(datastore.clone(), &manifest, &backup_dir, &archive_name)?;
+
+        let reader = if let Some(payload_archive_name) = payload_archive_name {
+            let payload_input =
+                get_local_pxar_reader(datastore, &manifest, &backup_dir, &payload_archive_name)?;
+            pxar::PxarVariant::Split(reader, payload_input)
+        } else {
+            pxar::PxarVariant::Unified(reader)
+        };
+        let accessor = Accessor::new(reader, archive_size).await?;
+
+        let file_path = decode_path(&filepath)?;
+        pbs_client::tools::pxar_metadata_catalog_lookup(
+            accessor,
+            OsStr::from_bytes(&file_path),
+            None,
+        )
+        .await
+    }
 }
 
 #[sortable]
-- 
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/9] api: datastore: add optional archive-name to file-restore

[Christian Ebner]

Allow to pass the archive name as optional api call parameter instead
of having it as prefix to the path.
If this parameter is given, instead of splitting of the archive name
from the path, the parameter itself is used, leaving the path
untouched.

This allows to restore single files from the archive, without having
to artificially construct the path in case of file restores for split
pxar archives, where the response path of the listing does not
include the archive, as opposed to the response provided by lookup
via the catalog.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/admin/datastore.rs | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index b90302966..225ff6fec 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1766,6 +1766,7 @@ pub const API_METHOD_PXAR_FILE_DOWNLOAD: ApiMethod = ApiMethod::new(
             ("backup-time", false, &BACKUP_TIME_SCHEMA),
             ("filepath", false, &StringSchema::new("Base64 encoded path").schema()),
             ("tar", true, &BooleanSchema::new("Download as .tar.zst").schema()),
+            ("archive-name", true, &StringSchema::new("Archive name").schema()),
         ]),
     )
 ).access(
@@ -1833,9 +1834,16 @@ pub fn pxar_file_download(
             components.remove(0);
         }
 
-        let mut split = components.splitn(2, |c| *c == b'/');
-        let pxar_name = std::str::from_utf8(split.next().unwrap())?;
-        let file_path = split.next().unwrap_or(b"/");
+        let (pxar_name, file_path) = if let Some(archive_name) = param["archive-name"].as_str() {
+            let archive_name = archive_name.as_bytes().to_owned();
+            (archive_name, base64::decode(&filepath)?)
+        } else {
+            let mut split = components.splitn(2, |c| *c == b'/');
+            let pxar_name = split.next().unwrap();
+            let file_path = split.next().unwrap_or(b"/");
+            (pxar_name.to_owned(), file_path.to_owned())
+        };
+        let pxar_name = std::str::from_utf8(&pxar_name)?;
         let (manifest, files) = read_backup_index(&backup_dir)?;
         for file in files {
             if file.filename == pxar_name && file.crypt_mode == Some(CryptMode::Encrypt) {
@@ -1858,7 +1866,7 @@ pub fn pxar_file_download(
         let decoder = Accessor::new(reader, archive_size).await?;
 
         let root = decoder.open_root().await?;
-        let path = OsStr::from_bytes(file_path).to_os_string();
+        let path = OsStr::from_bytes(&file_path).to_os_string();
         let file = root
             .lookup(&path)
             .await?
-- 
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/9] file-restore: never list ppxar as archive

[Christian Ebner]

Payload data archives cannot be used to navigate the content, so
exclude them from the archive listing, as this is used by
Proxmox VE to list in the file browser.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 proxmox-file-restore/src/main.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/proxmox-file-restore/src/main.rs b/proxmox-file-restore/src/main.rs
index 38cc1ce85..0f16f3d42 100644
--- a/proxmox-file-restore/src/main.rs
+++ b/proxmox-file-restore/src/main.rs
@@ -124,7 +124,8 @@ async fn list_files(
         ExtractPath::ListArchives => {
             let mut entries = vec![];
             for file in manifest.files() {
-                if !has_pxar_filename_extension(&file.filename, true)
+                if !file.filename.ends_with(".pxar.didx")
+                    && !file.filename.ends_with(".mpxar.didx")
                     && !file.filename.ends_with(".img.fidx")
                 {
                     continue;
-- 
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/9] file-restore: fallback to mpxar if catalog not present

[Christian Ebner]

The `proxmox-file-restore list` command will uses the provided path to
lookup and list directory entries via the catalog. Fallback to using
the metadata archive if the catalog is not present for fast lookups in
a backup snapshot.

This is in preparation for dropping encoding of the catalog for
snapshots using split archive encoding. Proxmox VE's storage plugin
uses this to allow single file restore for LXCs.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 proxmox-file-restore/src/main.rs | 69 +++++++++++++++++++++++++-------
 1 file changed, 54 insertions(+), 15 deletions(-)

diff --git a/proxmox-file-restore/src/main.rs b/proxmox-file-restore/src/main.rs
index 0f16f3d42..a09873467 100644
--- a/proxmox-file-restore/src/main.rs
+++ b/proxmox-file-restore/src/main.rs
@@ -147,24 +147,63 @@ async fn list_files(
             Ok(entries)
         }
         ExtractPath::Pxar(file, mut path) => {
-            let index = client
-                .download_dynamic_index(&manifest, CATALOG_NAME)
+            if let Ok(file_info) = manifest.lookup_file_info(CATALOG_NAME) {
+                let index = client
+                    .download_dynamic_index(&manifest, CATALOG_NAME)
+                    .await?;
+                let most_used = index.find_most_used_chunks(8);
+                let chunk_reader = RemoteChunkReader::new(
+                    client.clone(),
+                    crypt_config,
+                    file_info.chunk_crypt_mode(),
+                    most_used,
+                );
+                let reader = BufferedDynamicReader::new(index, chunk_reader);
+                let mut catalog_reader = CatalogReader::new(reader);
+
+                let mut fullpath = file.into_bytes();
+                fullpath.append(&mut path);
+
+                catalog_reader.list_dir_contents(&fullpath)
+            } else {
+                if path.is_empty() {
+                    path = vec![b'/'];
+                }
+
+                let (archive_name, payload_archive_name) =
+                    pbs_client::tools::get_pxar_archive_names(&file, &manifest)?;
+
+                let (reader, archive_size) = get_remote_pxar_reader(
+                    &archive_name,
+                    client.clone(),
+                    &manifest,
+                    crypt_config.clone(),
+                )
                 .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,
-                file_info.chunk_crypt_mode(),
-                most_used,
-            );
-            let reader = BufferedDynamicReader::new(index, chunk_reader);
-            let mut catalog_reader = CatalogReader::new(reader);
 
-            let mut fullpath = file.into_bytes();
-            fullpath.append(&mut path);
+                let reader = if let Some(payload_archive_name) = payload_archive_name {
+                    let (payload_reader, payload_size) = get_remote_pxar_reader(
+                        &payload_archive_name,
+                        client,
+                        &manifest,
+                        crypt_config,
+                    )
+                    .await?;
+                    pxar::PxarVariant::Split(reader, (payload_reader, payload_size))
+                } else {
+                    pxar::PxarVariant::Unified(reader)
+                };
+
+                let accessor = Accessor::new(reader, archive_size).await?;
+                let path = OsStr::from_bytes(&path);
 
-            catalog_reader.list_dir_contents(&fullpath)
+                pbs_client::tools::pxar_metadata_catalog_lookup(
+                    accessor,
+                    &path,
+                    Some(&archive_name),
+                )
+                .await
+            }
         }
         ExtractPath::VM(file, path) => {
             let details = SnapRestoreDetails {
-- 
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 8/9] www: content: lookup via metadata archive instead of catalog

[Christian Ebner]

In case of pxar archives with split metadata and payload data, the
metadata archive has to be used to lookup entries for navigation
before performing a single file restore.

Decide based on the archive filename extension whether to use the
`catalog` or the `pxar-lookup` api endpoint.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 www/datastore/Content.js | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/www/datastore/Content.js b/www/datastore/Content.js
index 6dd1ab319..e11b14b54 100644
--- a/www/datastore/Content.js
+++ b/www/datastore/Content.js
@@ -786,6 +786,9 @@ Ext.define('PBS.DataStoreContent', {
 		'backup-time': (time.getTime()/1000).toFixed(0),
 		'backup-type': type,
 	    };
+	    if (rec.data.filename.endsWith(".mpxar.didx")) {
+		extraParams['archive-name'] = rec.data.filename;
+	    }
 	    if (view.namespace && view.namespace !== '') {
 		extraParams.ns = view.namespace;
 	    }
-- 
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 9/9] client: backup: conditionally write catalog for file level backups

[Christian Ebner]

Only write the catalog when using the regular backup mode, do not write
it when using the split archive mode.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-client/src/pxar_backup_stream.rs | 11 +++++++----
 proxmox-backup-client/src/main.rs    | 21 ++++++++++++---------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/pbs-client/src/pxar_backup_stream.rs b/pbs-client/src/pxar_backup_stream.rs
index f322566f0..cdbcdfec2 100644
--- a/pbs-client/src/pxar_backup_stream.rs
+++ b/pbs-client/src/pxar_backup_stream.rs
@@ -15,7 +15,7 @@ use nix::sys::stat::Mode;
 use proxmox_async::blocking::TokioWriterAdapter;
 use proxmox_io::StdChannelWriter;
 
-use pbs_datastore::catalog::CatalogWriter;
+use pbs_datastore::catalog::{BackupCatalogWriter, CatalogWriter};
 
 use crate::inject_reused_chunks::InjectChunks;
 use crate::pxar::create::PxarWriters;
@@ -42,7 +42,7 @@ impl Drop for PxarBackupStream {
 impl PxarBackupStream {
     pub fn new<W: Write + Send + 'static>(
         dir: Dir,
-        catalog: Arc<Mutex<CatalogWriter<W>>>,
+        catalog: Option<Arc<Mutex<CatalogWriter<W>>>>,
         options: crate::pxar::PxarCreateOptions,
         boundaries: Option<mpsc::Sender<InjectChunks>>,
         separate_payload_stream: bool,
@@ -82,7 +82,10 @@ impl PxarBackupStream {
         let handler = async move {
             if let Err(err) = crate::pxar::create_archive(
                 dir,
-                PxarWriters::new(writer, Some(catalog)),
+                PxarWriters::new(
+                    writer,
+                    catalog.map(|c| c as Arc<Mutex<dyn BackupCatalogWriter + Send>>),
+                ),
                 crate::pxar::Flags::DEFAULT,
                 move |path| {
                     log::debug!("{:?}", path);
@@ -122,7 +125,7 @@ impl PxarBackupStream {
 
     pub fn open<W: Write + Send + 'static>(
         dirname: &Path,
-        catalog: Arc<Mutex<CatalogWriter<W>>>,
+        catalog: Option<Arc<Mutex<CatalogWriter<W>>>>,
         options: crate::pxar::PxarCreateOptions,
         boundaries: Option<mpsc::Sender<InjectChunks>>,
         separate_payload_stream: bool,
diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index 09af912df..f1c7fbf93 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -192,7 +192,7 @@ async fn backup_directory<P: AsRef<Path>>(
     archive_name: &str,
     payload_target: Option<&str>,
     chunk_size: Option<usize>,
-    catalog: Arc<Mutex<CatalogWriter<TokioWriterAdapter<StdChannelWriter<Error>>>>>,
+    catalog: Option<Arc<Mutex<CatalogWriter<TokioWriterAdapter<StdChannelWriter<Error>>>>>>,
     pxar_create_options: pbs_client::pxar::PxarCreateOptions,
     upload_options: UploadOptions,
 ) -> Result<(BackupStats, Option<BackupStats>), Error> {
@@ -1059,19 +1059,20 @@ async fn create_backup(
                     };
 
                 // start catalog upload on first use
-                if catalog.is_none() {
+                if catalog.is_none() && !detection_mode.is_data() && !detection_mode.is_metadata() {
                     let catalog_upload_res =
                         spawn_catalog_upload(client.clone(), crypto.mode == CryptMode::Encrypt)?;
                     catalog = Some(catalog_upload_res.catalog_writer);
                     catalog_result_rx = Some(catalog_upload_res.result);
                 }
-                let catalog = catalog.as_ref().unwrap();
 
                 log_file("directory", &filename, &target);
-                catalog
-                    .lock()
-                    .unwrap()
-                    .start_directory(std::ffi::CString::new(target.as_str())?.as_c_str())?;
+                if let Some(catalog) = catalog.as_ref() {
+                    catalog
+                        .lock()
+                        .unwrap()
+                        .start_directory(std::ffi::CString::new(target.as_str())?.as_c_str())?;
+                }
 
                 let mut previous_ref = None;
                 let max_cache_size = if detection_mode.is_metadata() {
@@ -1139,7 +1140,7 @@ async fn create_backup(
                     &target,
                     payload_target.as_deref(),
                     chunk_size_opt,
-                    catalog.clone(),
+                    catalog.as_ref().cloned(),
                     pxar_options,
                     upload_options,
                 )
@@ -1155,7 +1156,9 @@ async fn create_backup(
                     )?;
                 }
                 manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
-                catalog.lock().unwrap().end_directory()?;
+                if let Some(catalog) = catalog.as_ref() {
+                    catalog.lock().unwrap().end_directory()?;
+                }
             }
             (BackupSpecificationType::IMAGE, false) => {
                 log_file("image", &filename, &target);
-- 
2.39.2



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

[pbs-devel] applied-series: [PATCH v2 proxmox-backup 0/9] drop catalog encoding for split pxar archives

[Fabian Grünbichler]

with two minor follow-ups (using schema for achive-name, and cargo fmt)

On June 7, 2024 1:37 pm, Christian Ebner wrote:
> The catalog stores metadata needed for fast lookups via e.g. the file
> browser for single file restore.
> Since it is now possible to create snapshots with split pxar archives,
> the same metadata can be accessed in a reasonable performat way also via
> the metadata archive, effectively making the catalog unneeded duplicate
> data for this case.
> 
> This patch series therefore allows to lookup directory entries where
> needed via the metadata archive instead of the catalog and conditionally
> drops the catalog encoding for snapshots using split archive encoding.
> 
> The patches adapt not only the `catalog` api endpoint, used by the
> Proxmox Backup WebUI to access the snapshots contents but also the
> proxomox-file-restore, which is used by the Proxmox Virtual Environments
> PBS storage plugin to access and restore contents.
> 
> Tested that regular backups can still be inspected and restored and that
> the same functionality is still available for split archive backups,
> without now having a catalog encoded.
> 
> Functionally was tested with the following path series applied to PVE:
> https://lists.proxmox.com/pipermail/pve-devel/2024-May/064024.html
> 
> Please note: For directories with may entries a noticeable performance
> drop is present, which might be improved upon by optimizing decoding.
> 
> Changes since version 1:
> - Do not base64 encode archive name
> - Split off file restore archive name listing
> 
> The rest of the comments will be addressed as followups as discussed
> off-list.
> 
> Christian Ebner (9):
>   api: datastore: factor out path decoding for catalog
>   api: datastore: move reusable code out of thread
>   client: tools: add helper to lookup `ArchiveEntry`s via pxar
>   api: datastore: conditional lookup for catalog endpoint
>   api: datastore: add optional archive-name to file-restore
>   file-restore: never list ppxar as archive
>   file-restore: fallback to mpxar if catalog not present
>   www: content: lookup via metadata archive instead of catalog
>   client: backup: conditionally write catalog for file level backups
> 
>  pbs-client/src/pxar_backup_stream.rs |  11 ++-
>  pbs-client/src/tools/mod.rs          |  84 +++++++++++++++++++
>  proxmox-backup-client/src/main.rs    |  21 +++--
>  proxmox-file-restore/src/main.rs     |  72 ++++++++++++----
>  src/api2/admin/datastore.rs          | 121 ++++++++++++++++++---------
>  www/datastore/Content.js             |   3 +
>  6 files changed, 243 insertions(+), 69 deletions(-)
> 
> -- 
> 2.39.2
> 
> From 8dffaab3ce2c20b0c376d6c0dc8b3d561499fa24 Mon Sep 17 00:00:00 2001
> From: Christian Ebner <c.ebner@proxmox.com>
> Date: Fri, 7 Jun 2024 11:25:09 +0200
> 
> Christian Ebner (8):
>   api: datastore: factor out path decoding for catalog
>   api: datastore: move reusable code out of thread
>   client: tools: add helper to lookup `ArchiveEntry`s via pxar
>   api: datastore: conditional lookup for catalog endpoint
>   api: datastore: add optional archive-name to file-restore
>   file-restore: fallback to mpxar if catalog not present
>   www: content: lookup via metadata archive instead of catalog
>   client: backup: conditionally write catalog for file level backups
> 
>  pbs-client/src/pxar_backup_stream.rs |  11 ++-
>  pbs-client/src/tools/mod.rs          |  84 +++++++++++++++++
>  proxmox-backup-client/src/main.rs    |  21 +++--
>  proxmox-file-restore/src/main.rs     |  72 +++++++++++----
>  src/api2/admin/datastore.rs          | 129 ++++++++++++++++++---------
>  www/datastore/Content.js             |   3 +
>  6 files changed, 251 insertions(+), 69 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