From: Christian Ebner <c.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH v2 proxmox-backup 5/5] client: pxar: fix fuse mount performance for split archives
Date: Tue, 11 Jun 2024 15:29:46 +0200 [thread overview]
Message-ID: <20240611132946.332980-6-c.ebner@proxmox.com> (raw)
In-Reply-To: <20240611132946.332980-1-c.ebner@proxmox.com>
Adapt to the decoder/accessor 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 checks 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>
---
changes since version 1:
- adapt to cover missing check for access via accessor in FUSE mount
- include suggested code change to correctly provide error context
| 62 ++++++++++++++++------------------
pbs-pxar-fuse/src/lib.rs | 14 ++++----
src/api2/tape/restore.rs | 2 +-
3 files changed, 37 insertions(+), 41 deletions(-)
--git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
index 38d4ca89a..e1192c73f 100644
--- a/pbs-client/src/pxar/extract.rs
+++ b/pbs-client/src/pxar/extract.rs
@@ -373,26 +373,22 @@ where
Ok(())
}
}
- (true, EntryKind::File { size, .. }) => {
- let contents = self.decoder.contents();
-
- if let Some(mut contents) = contents {
- self.extractor.extract_file(
- &file_name,
- metadata,
- *size,
- &mut contents,
- self.extractor
- .overwrite_flags
- .contains(OverwriteFlags::FILE),
- )
- } else {
- Err(format_err!(
- "found regular file entry without contents in archive"
- ))
- }
- .context(PxarExtractContext::ExtractFile)
+ (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),
(false, _) => Ok(()), // skip this
};
@@ -872,7 +868,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 +891,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 +1030,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 +1049,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 +1273,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/pbs-pxar-fuse/src/lib.rs b/pbs-pxar-fuse/src/lib.rs
index 780a4ddbe..c75fba28c 100644
--- a/pbs-pxar-fuse/src/lib.rs
+++ b/pbs-pxar-fuse/src/lib.rs
@@ -5,7 +5,6 @@ use std::ffi::{OsStr, OsString};
use std::future::Future;
use std::io;
use std::mem;
-use std::ops::Range;
use std::os::unix::ffi::OsStrExt;
use std::path::Path;
use std::pin::Pin;
@@ -21,6 +20,7 @@ use futures::stream::{StreamExt, TryStreamExt};
use proxmox_io::vec;
use pxar::accessor::{self, EntryRangeInfo, ReadAt};
+use pxar::format::ContentRange;
use proxmox_fuse::requests::{self, FuseRequest};
use proxmox_fuse::{EntryParam, Fuse, ReplyBufState, Request, ROOT_ID};
@@ -130,7 +130,7 @@ struct Lookup {
inode: u64,
parent: u64,
entry_range_info: EntryRangeInfo,
- content_range: Option<Range<u64>>,
+ content_range: Option<ContentRange>,
}
impl Lookup {
@@ -138,7 +138,7 @@ impl Lookup {
inode: u64,
parent: u64,
entry_range_info: EntryRangeInfo,
- content_range: Option<Range<u64>>,
+ content_range: Option<ContentRange>,
) -> Box<Lookup> {
Box::new(Self {
refs: AtomicUsize::new(1),
@@ -433,13 +433,13 @@ impl SessionImpl {
}
}
- fn open_content(&self, lookup: &LookupRef) -> Result<FileContents, Error> {
+ async fn open_content<'a>(&'a self, lookup: &'a LookupRef<'a>) -> Result<FileContents, Error> {
if is_dir_inode(lookup.inode) {
io_return!(libc::EISDIR);
}
- match lookup.content_range.clone() {
- Some(range) => Ok(unsafe { self.accessor.open_contents_at_range(range) }),
+ match &lookup.content_range {
+ Some(range) => Ok(unsafe { self.accessor.open_contents_at_range(range).await? }),
None => io_return!(libc::EBADF),
}
}
@@ -581,7 +581,7 @@ impl SessionImpl {
async fn read(&self, inode: u64, len: usize, offset: u64) -> Result<Vec<u8>, Error> {
let file = self.get_lookup(inode)?;
- let content = self.open_content(&file)?;
+ let content = self.open_content(&file).await?;
let mut buf = vec::undefined(len);
let mut pos = 0;
// fuse' read is different from normal read - no short reads allowed except for EOF!
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
next prev parent reply other threads:[~2024-06-11 13:30 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-11 13:29 [pbs-devel] [PATCH v2 pxar proxmox-backp 0/5] " Christian Ebner
2024-06-11 13:29 ` [pbs-devel] [PATCH v2 pxar 1/5] format: add helper for payload header consistency checks Christian Ebner
2024-06-11 13:29 ` [pbs-devel] [PATCH v2 pxar 2/5] format: add helper type ContentRange Christian Ebner
2024-06-11 13:29 ` [pbs-devel] [PATCH v2 pxar 3/5] decoder: move payload header check for split input Christian Ebner
2024-06-11 13:29 ` [pbs-devel] [PATCH v2 pxar 4/5] accessor: add payload checks for split archives Christian Ebner
2024-06-11 13:29 ` Christian Ebner [this message]
2024-06-12 7:53 ` [pbs-devel] [PATCH v2 pxar proxmox-backp 0/5] fix fuse mount performance " 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=20240611132946.332980-6-c.ebner@proxmox.com \
--to=c.ebner@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