Re: [pbs-devel] [PATCH proxmox-backup 4/8] api: datastore: conditional lookup for catalog endpoint

["Fabian Grünbichler" <f.gruenbichler@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