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 2/2] client: pxar: fix fuse mount performance for split archives
Date: Tue, 11 Jun 2024 10:29:21 +0200 [thread overview]
Message-ID: <1718092294.tmjatog7d5.astroid@yuna.none> (raw)
In-Reply-To: <20240610151157.529760-3-c.ebner@proxmox.com>
On June 10, 2024 5:11 pm, Christian Ebner wrote:
> Adapt to the decoder method changes introduced in the pxar library,
> which were introduced in order to move the consistency check for
> metadata and payload data archives.
>
> The new location of the check allows to access the pxar archive via
> a `Split` variant reader instance, without penalization when just
> accessing the metadata, not reading any payload data.
>
> This greatly improves performance when accessing fuse mounted
> archives.
>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> pbs-client/src/pxar/extract.rs | 33 ++++++++++++++++++---------------
> src/api2/tape/restore.rs | 2 +-
> 2 files changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
> index 38d4ca89a..b3545650e 100644
> --- a/pbs-client/src/pxar/extract.rs
> +++ b/pbs-client/src/pxar/extract.rs
> @@ -374,7 +374,10 @@ where
> }
> }
> (true, EntryKind::File { size, .. }) => {
> - let contents = self.decoder.contents();
> + let contents = match self.decoder.contents() {
> + Ok(contents) => contents,
> + Err(err) => return Some(Err(err.into())),
> + };
the error handling below adds context which this part here is lacking.
I think the whole match arm would be easier to understand, while
preserving the context, if written as:
(true, EntryKind::File { size, .. }) => match self.decoder.contents() {
Ok(Some(mut contents)) => self.extractor.extract_file(
&file_name,
metadata,
*size,
&mut contents,
self.extractor
.overwrite_flags
.contains(OverwriteFlags::FILE),
),
Ok(None) => Err(format_err!(
"found regular file entry without contents in archive"
)),
Err(err) => Err(err.into()),
}
.context(PxarExtractContext::ExtractFile),
>
> if let Some(mut contents) = contents {
> self.extractor.extract_file(
> @@ -872,7 +875,8 @@ where
> match entry.kind() {
> EntryKind::File { .. } => {
> let size = decoder.content_size().unwrap_or(0);
> - tar_add_file(&mut tarencoder, decoder.contents(), size, metadata, path).await?
> + let contents = decoder.contents().await?;
> + tar_add_file(&mut tarencoder, contents, size, metadata, path).await?
> }
> EntryKind::Hardlink(link) => {
> if !link.data.is_empty() {
> @@ -894,14 +898,9 @@ where
> path
> } else {
> let size = decoder.content_size().unwrap_or(0);
> - tar_add_file(
> - &mut tarencoder,
> - decoder.contents(),
> - size,
> - metadata,
> - path,
> - )
> - .await?;
> + let contents = decoder.contents().await?;
> + tar_add_file(&mut tarencoder, contents, size, metadata, path)
> + .await?;
> hardlinks.insert(realpath.to_owned(), path.to_owned());
> continue;
> }
> @@ -1038,7 +1037,8 @@ where
> metadata.stat.mode as u16,
> true,
> );
> - zip.add_entry(entry, decoder.contents())
> + let contents = decoder.contents().await?;
> + zip.add_entry(entry, contents)
> .await
> .context("could not send file entry")?;
> }
> @@ -1056,7 +1056,8 @@ where
> metadata.stat.mode as u16,
> true,
> );
> - zip.add_entry(entry, decoder.contents())
> + let contents = decoder.contents().await?;
> + zip.add_entry(entry, contents)
> .await
> .context("could not send file entry")?;
> }
> @@ -1279,14 +1280,16 @@ where
> .with_context(|| format!("error at entry {file_name_os:?}"))?;
> }
> EntryKind::File { size, .. } => {
> + let mut contents = decoder
> + .contents()
> + .await?
> + .context("found regular file entry without contents in archive")?;
> extractor
> .async_extract_file(
> &file_name,
> metadata,
> *size,
> - &mut decoder
> - .contents()
> - .context("found regular file entry without contents in archive")?,
> + &mut contents,
> extractor.overwrite_flags.contains(OverwriteFlags::FILE),
> )
> .await?
> diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
> index 382909647..2b2025f6d 100644
> --- a/src/api2/tape/restore.rs
> +++ b/src/api2/tape/restore.rs
> @@ -1748,7 +1748,7 @@ fn try_restore_snapshot_archive<R: pxar::decoder::SeqRead>(
> }
>
> let filename = entry.file_name();
> - let mut contents = match decoder.contents() {
> + let mut contents = match decoder.contents()? {
> None => bail!("missing file content"),
> Some(contents) => contents,
> };
> --
> 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-11 8:29 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-10 15:11 [pbs-devel] [PATCH proxmox-backup pxar 0/2] " Christian Ebner
2024-06-10 15:11 ` [pbs-devel] [PATCH pxar 1/2] decoder: move payload header check for split input Christian Ebner
2024-06-11 8:29 ` Fabian Grünbichler
2024-06-10 15:11 ` [pbs-devel] [PATCH proxmox-backup 2/2] client: pxar: fix fuse mount performance for split archives Christian Ebner
2024-06-11 8:29 ` Fabian Grünbichler [this message]
2024-06-11 13:31 ` [pbs-devel] [PATCH proxmox-backup pxar 0/2] " Christian Ebner
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=1718092294.tmjatog7d5.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