From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox-backup 4/8] api: datastore: conditional lookup for catalog endpoint
Date: Fri, 07 Jun 2024 12:23:26 +0200 [thread overview]
Message-ID: <1717755543.csxq1estz9.astroid@yuna.none> (raw)
In-Reply-To: <20240607094313.178742-5-c.ebner@proxmox.com>
On June 7, 2024 11:43 am, Christian Ebner wrote:
> 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 | 73 ++++++++++++++++++++++++++++---------
> 1 file changed, 55 insertions(+), 18 deletions(-)
>
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index 117dab080..e25a78bca 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 lookup given filepath (base64 encoded)",
> + optional: true,
> + },
why is this base64 encoded? the archive name is a safe ID..
> },
> },
> access: {
> @@ -1674,8 +1679,18 @@ 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 = if let Some(ref archive_name) = archive_name {
> + String::from_utf8(
> + base64::decode(archive_name)
> + .map_err(|err| format_err!("base64 decode of 'archive-name' failed - {err}"))?,
> + )?
> + } else {
> + CATALOG_NAME.to_string()
> + };
without b64, this would just be an unwrap_or_else ;)
> +
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>
> let ns = ns.unwrap_or_default();
> @@ -1692,8 +1707,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 +1714,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 mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
next prev parent reply other threads:[~2024-06-07 10:23 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-07 9:43 [pbs-devel] [PATCH proxmox-backup 0/8] drop catalog encoding for split pxar archives Christian Ebner
2024-06-07 9:43 ` [pbs-devel] [PATCH proxmox-backup 1/8] api: datastore: factor out path decoding for catalog Christian Ebner
2024-06-07 9:43 ` [pbs-devel] [PATCH proxmox-backup 2/8] api: datastore: move reusable code out of thread Christian Ebner
2024-06-07 9:43 ` [pbs-devel] [PATCH proxmox-backup 3/8] client: tools: add helper to lookup `ArchiveEntry`s via pxar Christian Ebner
2024-06-07 9:43 ` [pbs-devel] [PATCH proxmox-backup 4/8] api: datastore: conditional lookup for catalog endpoint Christian Ebner
2024-06-07 10:23 ` Fabian Grünbichler [this message]
2024-06-07 10:34 ` Christian Ebner
2024-06-07 9:43 ` [pbs-devel] [PATCH proxmox-backup 5/8] api: datastore: add optional archive-name to file-restore Christian Ebner
2024-06-07 10:24 ` Fabian Grünbichler
2024-06-07 9:43 ` [pbs-devel] [PATCH proxmox-backup 6/8] file-restore: fallback to mpxar if catalog not present Christian Ebner
2024-06-07 10:32 ` Fabian Grünbichler
2024-06-07 10:43 ` Christian Ebner
2024-06-07 11:35 ` Fabian Grünbichler
2024-06-07 11:41 ` Christian Ebner
2024-06-07 9:43 ` [pbs-devel] [PATCH proxmox-backup 7/8] www: content: lookup via metadata archive instead of catalog Christian Ebner
2024-06-07 9:43 ` [pbs-devel] [PATCH proxmox-backup 8/8] client: backup: conditionally write catalog for file level backups Christian Ebner
2024-06-07 10:48 ` Fabian Grünbichler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1717755543.csxq1estz9.astroid@yuna.none \
--to=f.gruenbichler@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox