Re: [pbs-devel] [PATCH v8 proxmox-backup 54/69] client: pxar: add helper to handle optional preludes

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On May 28, 2024 11:42 am, Christian Ebner wrote:
> Pxar archives with format version 2 allows to store optional
> information file format version and prelude entries.
> 
> Cover the case for these entries, the file format version entry being
> introduced to distinguish between different file formats used for
> encoding as well as the prelude entry used to store optional metadata
> such as the pxar cli exlude parameters.
> 
> Add the logic to accept and decode these prelude entries when
> accessing the archive via a decoder instance.
> 
> For now simply ignore them.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes since version 7:
> - no changes
> 
> changes since version 6:
> - Get rid of additional, but unused EntryKind::Version return value
> 
>  pbs-client/src/pxar/create.rs             |  2 +-
>  pbs-client/src/pxar/extract.rs            |  7 ++---
>  pbs-client/src/pxar/tools.rs              |  7 +++++
>  pbs-client/src/tools/mod.rs               | 31 +++++++++++++++++++++++
>  src/api2/tape/restore.rs                  | 17 +++++--------
>  src/tape/file_formats/snapshot_archive.rs |  1 +
>  6 files changed, 50 insertions(+), 15 deletions(-)
> 
> diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs
> index be86df356..9a9882fc7 100644
> --- a/pbs-client/src/pxar/create.rs
> +++ b/pbs-client/src/pxar/create.rs
> @@ -226,7 +226,7 @@ where
>      }
>  
>      let metadata_mode = options.previous_ref.is_some() && writers.archive.payload().is_some();
> -    let mut encoder = Encoder::new(writers.archive, &metadata).await?;
> +    let mut encoder = Encoder::new(writers.archive, &metadata, None).await?;
>  
>      let mut patterns = options.patterns;
>  
> diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
> index 5f5ac6188..e22390606 100644
> --- a/pbs-client/src/pxar/extract.rs
> +++ b/pbs-client/src/pxar/extract.rs
> @@ -29,6 +29,7 @@ use proxmox_compression::zip::{ZipEncoder, ZipEntry};
>  use crate::pxar::dir_stack::PxarDirStack;
>  use crate::pxar::metadata;
>  use crate::pxar::Flags;
> +use crate::tools::handle_root_with_optional_format_version_prelude;
>  
>  pub struct PxarExtractOptions<'a> {
>      pub match_list: &'a [MatchEntry],
> @@ -124,9 +125,7 @@ where
>          // we use this to keep track of our directory-traversal
>          decoder.enable_goodbye_entries(true);
>  
> -        let root = decoder
> -            .next()
> -            .context("found empty pxar archive")?
> +        let (root, _) = handle_root_with_optional_format_version_prelude(&mut decoder)
>              .context("error reading pxar archive")?;
>  
>          if !root.is_dir() {
> @@ -267,6 +266,8 @@ where
>          };
>  
>          let extract_res = match (did_match, entry.kind()) {
> +            (_, EntryKind::Version(_version)) => Ok(()),
> +            (_, EntryKind::Prelude(_prelude)) => Ok(()),
>              (_, EntryKind::Directory) => {
>                  self.callback(entry.path());
>  
> diff --git a/pbs-client/src/pxar/tools.rs b/pbs-client/src/pxar/tools.rs
> index 459951d50..27e5185a3 100644
> --- a/pbs-client/src/pxar/tools.rs
> +++ b/pbs-client/src/pxar/tools.rs
> @@ -172,6 +172,13 @@ pub fn format_multi_line_entry(entry: &Entry) -> String {
>      let meta = entry.metadata();
>  
>      let (size, link, type_name, payload_offset) = match entry.kind() {
> +        EntryKind::Version(version) => (format!("{version:?}"), String::new(), "version", None),
> +        EntryKind::Prelude(prelude) => (
> +            "0".to_string(),
> +            format!("raw data: {:?} bytes", prelude.data.len()),
> +            "prelude",
> +            None,
> +        ),
>          EntryKind::File {
>              size,
>              payload_offset,
> diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
> index f43058dbd..8d4fefaf3 100644
> --- a/pbs-client/src/tools/mod.rs
> +++ b/pbs-client/src/tools/mod.rs
> @@ -539,3 +539,34 @@ pub fn has_pxar_filename_extension(name: &str, with_didx_extension: bool) -> boo
>          name.ends_with(".pxar") || name.ends_with(".mpxar") || name.ends_with(".ppxar")
>      }
>  }
> +
> +/// Decode possible format version and prelude entries before getting the root directory
> +/// entry.
> +///
> +/// Returns the root directory entry and, if present, the prelude entry
> +pub fn handle_root_with_optional_format_version_prelude<R: pxar::decoder::SeqRead>(
> +    decoder: &mut pxar::decoder::sync::Decoder<R>,
> +) -> Result<(pxar::Entry, Option<pxar::Entry>), Error> {
> +    let first = decoder
> +        .next()
> +        .ok_or_else(|| format_err!("missing root entry"))??;
> +    match first.kind() {
> +        pxar::EntryKind::Directory => Ok((first, None)),

should we log the version here?

> +        pxar::EntryKind::Version(_version) => {

and here?

> +            let second = decoder
> +                .next()
> +                .ok_or_else(|| format_err!("missing root entry"))??;
> +            match second.kind() {
> +                pxar::EntryKind::Directory => Ok((second, None)),
> +                pxar::EntryKind::Prelude(_prelude) => {
> +                    let third = decoder
> +                        .next()
> +                        .ok_or_else(|| format_err!("missing root entry"))??;
> +                    Ok((third, Some(second)))
> +                }
> +                _ => bail!("unexpected entry kind {:?}", second.kind()),
> +            }
> +        }
> +        _ => bail!("unexpected entry kind {:?}", first.kind()),
> +    }
> +}
> diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
> index 9184ff934..382909647 100644
> --- a/src/api2/tape/restore.rs
> +++ b/src/api2/tape/restore.rs
> @@ -23,6 +23,7 @@ use pbs_api_types::{
>      PRIV_DATASTORE_MODIFY, PRIV_TAPE_READ, TAPE_RESTORE_NAMESPACE_SCHEMA,
>      TAPE_RESTORE_SNAPSHOT_SCHEMA, UPID_SCHEMA,
>  };
> +use pbs_client::tools::handle_root_with_optional_format_version_prelude;
>  use pbs_config::CachedUserInfo;
>  use pbs_datastore::dynamic_index::DynamicIndexReader;
>  use pbs_datastore::fixed_index::FixedIndexReader;
> @@ -1713,17 +1714,11 @@ fn try_restore_snapshot_archive<R: pxar::decoder::SeqRead>(
>      decoder: &mut pxar::decoder::sync::Decoder<R>,
>      snapshot_path: &Path,
>  ) -> Result<BackupManifest, Error> {
> -    let _root = match decoder.next() {
> -        None => bail!("missing root entry"),
> -        Some(root) => {
> -            let root = root?;
> -            match root.kind() {
> -                pxar::EntryKind::Directory => { /* Ok */ }
> -                _ => bail!("wrong root entry type"),
> -            }
> -            root
> -        }
> -    };
> +    let (root, _) = handle_root_with_optional_format_version_prelude(decoder)?;
> +    match root.kind() {
> +        pxar::EntryKind::Directory => { /* Ok */ }
> +        _ => bail!("wrong root entry type"),
> +    }
>  
>      let root_path = Path::new("/");
>      let manifest_file_name = OsStr::new(MANIFEST_BLOB_NAME);
> diff --git a/src/tape/file_formats/snapshot_archive.rs b/src/tape/file_formats/snapshot_archive.rs
> index 82f466980..f5a588f4e 100644
> --- a/src/tape/file_formats/snapshot_archive.rs
> +++ b/src/tape/file_formats/snapshot_archive.rs
> @@ -61,6 +61,7 @@ pub fn tape_write_snapshot_archive<'a>(
>          let mut encoder = pxar::encoder::sync::Encoder::new(
>              pxar::PxarVariant::Unified(PxarTapeWriter::new(writer)),
>              &root_metadata,
> +            None,
>          )?;
>  
>          for filename in file_list.iter() {
> -- 
> 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