From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id 7EAFB1FF391
	for <inbox@lore.proxmox.com>; Wed, 12 Jun 2024 15:17:02 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 9796311610;
	Wed, 12 Jun 2024 15:17:36 +0200 (CEST)
From: Christian Ebner <c.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Wed, 12 Jun 2024 15:17:13 +0200
Message-Id: <20240612131713.133353-4-c.ebner@proxmox.com>
X-Mailer: git-send-email 2.39.2
In-Reply-To: <20240612131713.133353-1-c.ebner@proxmox.com>
References: <20240612131713.133353-1-c.ebner@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.027 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 T_SCC_BODY_TEXT_LINE    -0.01 -
Subject: [pbs-devel] [PATCH v4 proxmox-backup 3/3] client: pxar: fix fuse
 mount performance for split archives
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.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 3:
- adapt to dropped unsafe for `open_contents_at_range`

 pbs-client/src/pxar/extract.rs | 62 ++++++++++++++++------------------
 pbs-pxar-fuse/src/lib.rs       | 19 ++++++-----
 src/api2/tape/restore.rs       |  2 +-
 3 files changed, 41 insertions(+), 42 deletions(-)

diff --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..4c26de200 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;
@@ -20,7 +19,7 @@ use futures::sink::SinkExt;
 use futures::stream::{StreamExt, TryStreamExt};
 
 use proxmox_io::vec;
-use pxar::accessor::{self, EntryRangeInfo, ReadAt};
+use pxar::accessor::{self, ContentRange, EntryRangeInfo, ReadAt};
 
 use proxmox_fuse::requests::{self, FuseRequest};
 use proxmox_fuse::{EntryParam, Fuse, ReplyBufState, Request, ROOT_ID};
@@ -130,7 +129,7 @@ struct Lookup {
     inode: u64,
     parent: u64,
     entry_range_info: EntryRangeInfo,
-    content_range: Option<Range<u64>>,
+    content_range: Option<ContentRange>,
 }
 
 impl Lookup {
@@ -138,7 +137,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 +432,17 @@ 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) => self
+                .accessor
+                .open_contents_at_range(range)
+                .await
+                .map_err(|err| err.into()),
             None => io_return!(libc::EBADF),
         }
     }
@@ -581,7 +584,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