all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Max Carrara <m.carrara@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH v3 proxmox-backup 1/5] pbs-client: pxar: preserve error context
Date: Mon, 17 Jul 2023 10:04:06 +0200	[thread overview]
Message-ID: <20230717080410.429362-2-m.carrara@proxmox.com> (raw)
In-Reply-To: <20230717080410.429362-1-m.carrara@proxmox.com>

In order to preserve the source(s) of errors, `anyhow::Context` is
used instead of propagating errors via `Result::map_err()` and / or
`anyhow::format_err!()`.

This makes it possible to access e.g. an underlying `io::Error` or
`nix::Errno` etc. that caused an execution path to fail.

Certain usages of `anyhow::bail!()` are also changed / replaced
in order to preserve context.

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
 Changes v1 --> v2:
  * Replace instances of `.context(format!(...))` with
    `.with_context(|| format!(...))` where applicable to avoid unnecessarily
    calling `format!()`

 Changes v2 --> v3:
  * None

 pbs-client/src/pxar/create.rs    |  22 ++---
 pbs-client/src/pxar/dir_stack.rs |   8 +-
 pbs-client/src/pxar/extract.rs   | 141 ++++++++++++++-----------------
 pbs-client/src/pxar/metadata.rs  |  44 ++++------
 pbs-client/src/pxar/tools.rs     |  11 ++-
 5 files changed, 102 insertions(+), 124 deletions(-)

diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs
index a9a956c2..2577cf98 100644
--- a/pbs-client/src/pxar/create.rs
+++ b/pbs-client/src/pxar/create.rs
@@ -7,7 +7,7 @@ use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, OwnedFd, RawFd};
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex};
 
-use anyhow::{bail, format_err, Error};
+use anyhow::{bail, Context, Error};
 use futures::future::BoxFuture;
 use futures::FutureExt;
 use nix::dir::Dir;
@@ -159,7 +159,7 @@ where
         fs_magic,
         &mut fs_feature_flags,
     )
-    .map_err(|err| format_err!("failed to get metadata for source directory: {}", err))?;
+    .context("failed to get metadata for source directory")?;
 
     let mut device_set = options.device_set.clone();
     if let Some(ref mut set) = device_set {
@@ -441,7 +441,7 @@ impl Archiver {
             ) {
                 Ok(stat) => stat,
                 Err(ref err) if err.not_found() => continue,
-                Err(err) => bail!("stat failed on {:?}: {}", full_path, err),
+                Err(err) => return Err(err).context(format!("stat failed on {:?}", full_path)),
             };
 
             let match_path = PathBuf::from("/").join(full_path.clone());
@@ -796,7 +796,7 @@ fn get_fcaps(
             Ok(())
         }
         Err(Errno::EBADF) => Ok(()), // symlinks
-        Err(err) => bail!("failed to read file capabilities: {}", err),
+        Err(err) => Err(err).context("failed to read file capabilities"),
     }
 }
 
@@ -818,7 +818,7 @@ fn get_xattr_fcaps_acl(
             return Ok(());
         }
         Err(Errno::EBADF) => return Ok(()), // symlinks
-        Err(err) => bail!("failed to read xattrs: {}", err),
+        Err(err) => return Err(err).context("failed to read xattrs"),
     };
 
     for attr in &xattrs {
@@ -843,7 +843,9 @@ fn get_xattr_fcaps_acl(
             Err(Errno::ENODATA) => (), // it got removed while we were iterating...
             Err(Errno::EOPNOTSUPP) => (), // shouldn't be possible so just ignore this
             Err(Errno::EBADF) => (),   // symlinks, shouldn't be able to reach this either
-            Err(err) => bail!("error reading extended attribute {:?}: {}", attr, err),
+            Err(err) => {
+                return Err(err).context(format!("error reading extended attribute {attr:?}"))
+            }
         }
     }
 
@@ -858,7 +860,7 @@ fn get_chattr(metadata: &mut Metadata, fd: RawFd) -> Result<(), Error> {
         Err(errno) if errno_is_unsupported(errno) => {
             return Ok(());
         }
-        Err(err) => bail!("failed to read file attributes: {}", err),
+        Err(err) => return Err(err).context("failed to read file attributes"),
     }
 
     metadata.stat.flags |= Flags::from_chattr(attr).bits();
@@ -880,7 +882,7 @@ fn get_fat_attr(metadata: &mut Metadata, fd: RawFd, fs_magic: i64) -> Result<(),
         Err(errno) if errno_is_unsupported(errno) => {
             return Ok(());
         }
-        Err(err) => bail!("failed to read fat attributes: {}", err),
+        Err(err) => return Err(err).context("failed to read fat attributes"),
     }
 
     metadata.stat.flags |= Flags::from_fat_attr(attr).bits();
@@ -919,7 +921,7 @@ fn get_quota_project_id(
         if errno_is_unsupported(errno) {
             return Ok(());
         } else {
-            bail!("error while reading quota project id ({})", errno);
+            return Err(errno).context("error while reading quota project id");
         }
     }
 
@@ -973,7 +975,7 @@ fn get_acl_do(
         Err(Errno::EBADF) => return Ok(()),
         // Don't bail if there is no data
         Err(Errno::ENODATA) => return Ok(()),
-        Err(err) => bail!("error while reading ACL - {}", err),
+        Err(err) => return Err(err).context("error while reading ACL"),
     };
 
     process_acl(metadata, acl, acl_type)
diff --git a/pbs-client/src/pxar/dir_stack.rs b/pbs-client/src/pxar/dir_stack.rs
index 0cc4e6a5..43cbee1d 100644
--- a/pbs-client/src/pxar/dir_stack.rs
+++ b/pbs-client/src/pxar/dir_stack.rs
@@ -2,7 +2,7 @@ use std::ffi::OsString;
 use std::os::unix::io::{AsRawFd, BorrowedFd, RawFd};
 use std::path::{Path, PathBuf};
 
-use anyhow::{bail, format_err, Error};
+use anyhow::{bail, Context, Error};
 use nix::dir::Dir;
 use nix::fcntl::OFlag;
 use nix::sys::stat::{mkdirat, Mode};
@@ -130,7 +130,7 @@ impl PxarDirStack {
         let dirs_len = self.dirs.len();
         let mut fd = self.dirs[self.created - 1]
             .try_as_borrowed_fd()
-            .ok_or_else(|| format_err!("lost track of directory file descriptors"))?
+            .context("lost track of directory file descriptors")?
             .as_raw_fd();
 
         while self.created < dirs_len {
@@ -142,7 +142,7 @@ impl PxarDirStack {
 
         self.dirs[self.created - 1]
             .try_as_borrowed_fd()
-            .ok_or_else(|| format_err!("lost track of directory file descriptors"))
+            .context("lost track of directory file descriptors")
     }
 
     pub fn create_last_dir(&mut self, allow_existing_dirs: bool) -> Result<(), Error> {
@@ -156,7 +156,7 @@ impl PxarDirStack {
 
         self.dirs[0]
             .try_as_borrowed_fd()
-            .ok_or_else(|| format_err!("lost track of directory file descriptors"))
+            .context("lost track of directory file descriptors")
     }
 
     pub fn path(&self) -> &Path {
diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
index f6c1991f..7015dc36 100644
--- a/pbs-client/src/pxar/extract.rs
+++ b/pbs-client/src/pxar/extract.rs
@@ -8,7 +8,7 @@ use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex};
 
-use anyhow::{bail, format_err, Error};
+use anyhow::{bail, Context, Error};
 use nix::dir::Dir;
 use nix::fcntl::OFlag;
 use nix::sys::stat::Mode;
@@ -55,8 +55,8 @@ where
 
     let root = decoder
         .next()
-        .ok_or_else(|| format_err!("found empty pxar archive"))?
-        .map_err(|err| format_err!("error reading pxar archive: {}", err))?;
+        .context("found empty pxar archive")?
+        .context("error reading pxar archive")?;
 
     if !root.is_dir() {
         bail!("pxar archive does not start with a directory entry!");
@@ -67,14 +67,14 @@ where
         None,
         Some(CreateOptions::new().perm(Mode::from_bits_truncate(0o700))),
     )
-    .map_err(|err| format_err!("error creating directory {:?}: {}", destination, err))?;
+    .with_context(|| format!("error creating directory {:?}", destination))?;
 
     let dir = Dir::open(
         destination,
         OFlag::O_DIRECTORY | OFlag::O_CLOEXEC,
         Mode::empty(),
     )
-    .map_err(|err| format_err!("unable to open target directory {:?}: {}", destination, err,))?;
+    .with_context(|| format!("unable to open target directory {:?}", destination))?;
 
     let mut extractor = Extractor::new(
         dir,
@@ -92,7 +92,7 @@ where
     let mut err_path_stack = vec![OsString::from("/")];
     let mut current_match = options.extract_match_default;
     while let Some(entry) = decoder.next() {
-        let entry = entry.map_err(|err| format_err!("error reading pxar archive: {}", err))?;
+        let entry = entry.context("error reading pxar archive")?;
 
         let file_name_os = entry.file_name();
 
@@ -102,7 +102,7 @@ where
         }
 
         let file_name = CString::new(file_name_os.as_bytes())
-            .map_err(|_| format_err!("encountered file name with null-bytes"))?;
+            .context("encountered file name with null-bytes")?;
 
         let metadata = entry.metadata();
 
@@ -125,7 +125,7 @@ where
                 let create = current_match && match_result != Some(MatchType::Exclude);
                 extractor
                     .enter_directory(file_name_os.to_owned(), metadata.clone(), create)
-                    .map_err(|err| format_err!("error at entry {:?}: {}", file_name_os, err))?;
+                    .with_context(|| format!("error at entry {:?}", file_name_os))?;
 
                 // We're starting a new directory, push our old matching state and replace it with
                 // our new one:
@@ -142,16 +142,16 @@ where
             (_, EntryKind::GoodbyeTable) => {
                 // go up a directory
 
-                extractor.set_path(err_path_stack.pop().ok_or_else(|| {
-                    format_err!(
-                        "error at entry {:?}: unexpected end of directory",
+                extractor.set_path(err_path_stack.pop().with_context(|| {
+                    format!(
+                        "error at entry {:?} - unexpected end of directory",
                         file_name_os
                     )
                 })?);
 
                 extractor
                     .leave_directory()
-                    .map_err(|err| format_err!("error at entry {:?}: {}", file_name_os, err))?;
+                    .with_context(|| format!("error at entry {:?}", file_name_os))?;
 
                 // We left a directory, also get back our previous matching state. This is in sync
                 // with `dir_stack` so this should never be empty except for the final goodbye
@@ -196,14 +196,14 @@ where
                 &file_name,
                 metadata,
                 *size,
-                &mut decoder.contents().ok_or_else(|| {
-                    format_err!("found regular file entry without contents in archive")
-                })?,
+                &mut decoder
+                    .contents()
+                    .context("found regular file entry without contents in archive")?,
                 extractor.overwrite,
             ),
             (false, _) => Ok(()), // skip this
         }
-        .map_err(|err| format_err!("error at entry {:?}: {}", file_name_os, err))?;
+        .with_context(|| format!("error at entry {:?}", file_name_os))?;
     }
 
     if !extractor.dir_stack.is_empty() {
@@ -254,7 +254,7 @@ impl Extractor {
     pub fn on_error(&mut self, mut on_error: Box<dyn FnMut(Error) -> Result<(), Error> + Send>) {
         let path = Arc::clone(&self.current_path);
         self.on_error = Box::new(move |err: Error| -> Result<(), Error> {
-            on_error(format_err!("error at {:?}: {}", path.lock().unwrap(), err))
+            on_error(err.context(format!("error at {:?}", path.lock().unwrap())))
         });
     }
 
@@ -291,8 +291,8 @@ impl Extractor {
         let dir = self
             .dir_stack
             .pop()
-            .map_err(|err| format_err!("unexpected end of directory entry: {}", err))?
-            .ok_or_else(|| format_err!("broken pxar archive (directory stack underrun)"))?;
+            .context("unexpected end of directory entry")?
+            .context("broken pxar archive (directory stack underrun)")?;
 
         if let Some(fd) = dir.try_as_borrowed_fd() {
             metadata::apply(
@@ -302,7 +302,7 @@ impl Extractor {
                 &path_info,
                 &mut self.on_error,
             )
-            .map_err(|err| format_err!("failed to apply directory metadata: {}", err))?;
+            .context("failed to apply directory metadata")?;
         }
 
         Ok(())
@@ -316,7 +316,7 @@ impl Extractor {
         self.dir_stack
             .last_dir_fd(self.allow_existing_dirs)
             .map(|d| d.as_raw_fd())
-            .map_err(|err| format_err!("failed to get parent directory file descriptor: {}", err))
+            .context("failed to get parent directory file descriptor")
     }
 
     pub fn extract_symlink(
@@ -370,16 +370,12 @@ impl Extractor {
         device: libc::dev_t,
     ) -> Result<(), Error> {
         let mode = metadata.stat.mode;
-        let mode = u32::try_from(mode).map_err(|_| {
-            format_err!(
-                "device node's mode contains illegal bits: 0x{:x} (0o{:o})",
-                mode,
-                mode,
-            )
+        let mode = u32::try_from(mode).with_context(|| {
+            format!("device node's mode contains illegal bits: 0x{mode:x} (0o{mode:o})")
         })?;
         let parent = self.parent_fd()?;
         unsafe { c_result!(libc::mknodat(parent, file_name.as_ptr(), mode, device)) }
-            .map_err(|err| format_err!("failed to create device node: {}", err))?;
+            .context("failed to create device node")?;
 
         metadata::apply_at(
             self.feature_flags,
@@ -409,7 +405,7 @@ impl Extractor {
         let mut file = unsafe {
             std::fs::File::from_raw_fd(
                 nix::fcntl::openat(parent, file_name, oflags, Mode::from_bits(0o600).unwrap())
-                    .map_err(|err| format_err!("failed to create file {:?}: {}", file_name, err))?,
+                    .with_context(|| format!("failed to create file {file_name:?}"))?,
             )
         };
 
@@ -419,10 +415,10 @@ impl Extractor {
             file.as_raw_fd(),
             &mut self.on_error,
         )
-        .map_err(|err| format_err!("failed to apply initial flags: {}", err))?;
+        .context("failed to apply initial flags")?;
 
-        let result = sparse_copy(&mut *contents, &mut file)
-            .map_err(|err| format_err!("failed to copy file contents: {}", err))?;
+        let result =
+            sparse_copy(&mut *contents, &mut file).context("failed to copy file contents")?;
 
         if size != result.written {
             bail!(
@@ -436,7 +432,7 @@ impl Extractor {
             while match nix::unistd::ftruncate(file.as_raw_fd(), size as i64) {
                 Ok(_) => false,
                 Err(errno) if errno == nix::errno::Errno::EINTR => true,
-                Err(err) => bail!("error setting file size: {}", err),
+                Err(err) => return Err(err).context("error setting file size"),
             } {}
         }
 
@@ -467,7 +463,7 @@ impl Extractor {
         let mut file = tokio::fs::File::from_std(unsafe {
             std::fs::File::from_raw_fd(
                 nix::fcntl::openat(parent, file_name, oflags, Mode::from_bits(0o600).unwrap())
-                    .map_err(|err| format_err!("failed to create file {:?}: {}", file_name, err))?,
+                    .with_context(|| format!("failed to create file {file_name:?}"))?,
             )
         });
 
@@ -477,11 +473,11 @@ impl Extractor {
             file.as_raw_fd(),
             &mut self.on_error,
         )
-        .map_err(|err| format_err!("failed to apply initial flags: {}", err))?;
+        .context("failed to apply initial flags")?;
 
         let result = sparse_copy_async(&mut *contents, &mut file)
             .await
-            .map_err(|err| format_err!("failed to copy file contents: {}", err))?;
+            .context("failed to copy file contents")?;
 
         if size != result.written {
             bail!(
@@ -495,7 +491,7 @@ impl Extractor {
             while match nix::unistd::ftruncate(file.as_raw_fd(), size as i64) {
                 Ok(_) => false,
                 Err(errno) if errno == nix::errno::Errno::EINTR => true,
-                Err(err) => bail!("error setting file size: {}", err),
+                Err(err) => return Err(err).context("error setting file size"),
             } {}
         }
 
@@ -532,12 +528,12 @@ where
     header.set_size(size);
     add_metadata_to_header(&mut header, metadata);
     header.set_cksum();
+
     match contents {
         Some(content) => tar.add_entry(&mut header, path, content).await,
         None => tar.add_entry(&mut header, path, tokio::io::empty()).await,
     }
-    .map_err(|err| format_err!("could not send file entry: {}", err))?;
-    Ok(())
+    .context("could not send file entry")
 }
 
 /// Creates a tar file from `path` and writes it into `output`
@@ -551,7 +547,7 @@ where
     let file = root
         .lookup(&path)
         .await?
-        .ok_or_else(|| format_err!("error opening '{:?}'", path.as_ref()))?;
+        .with_context(|| format!("error opening {:?}", path.as_ref()))?;
 
     let mut components = file.entry().path().components();
     components.next_back(); // discard last
@@ -574,13 +570,13 @@ where
             tarencoder
                 .add_entry(&mut header, path, tokio::io::empty())
                 .await
-                .map_err(|err| format_err!("could not send dir entry: {}", err))?;
+                .context("could not send dir entry")?;
         }
 
         let mut decoder = dir.decode_full().await?;
         decoder.enable_goodbye_entries(false);
         while let Some(entry) = decoder.next().await {
-            let entry = entry.map_err(|err| format_err!("cannot decode entry: {}", err))?;
+            let entry = entry.context("cannot decode entry")?;
 
             let metadata = entry.metadata();
             let path = entry.path().strip_prefix(prefix)?;
@@ -595,7 +591,7 @@ where
                         let entry = root
                             .lookup(&path)
                             .await?
-                            .ok_or_else(|| format_err!("error looking up '{:?}'", path))?;
+                            .with_context(|| format!("error looking up {path:?}"))?;
                         let realfile = accessor.follow_hardlink(&entry).await?;
                         let metadata = realfile.entry().metadata();
                         let realpath = Path::new(link);
@@ -630,7 +626,7 @@ where
                         tarencoder
                             .add_link(&mut header, path, stripped_path)
                             .await
-                            .map_err(|err| format_err!("could not send hardlink entry: {}", err))?;
+                            .context("could not send hardlink entry")?;
                     }
                 }
                 EntryKind::Symlink(link) if !link.data.is_empty() => {
@@ -643,7 +639,7 @@ where
                     tarencoder
                         .add_link(&mut header, path, realpath)
                         .await
-                        .map_err(|err| format_err!("could not send symlink entry: {}", err))?;
+                        .context("could not send symlink entry")?;
                 }
                 EntryKind::Fifo => {
                     log::debug!("adding '{}' to tar", path.display());
@@ -657,7 +653,7 @@ where
                     tarencoder
                         .add_entry(&mut header, path, tokio::io::empty())
                         .await
-                        .map_err(|err| format_err!("could not send fifo entry: {}", err))?;
+                        .context("coult not send fifo entry")?;
                 }
                 EntryKind::Directory => {
                     log::debug!("adding '{}' to tar", path.display());
@@ -671,7 +667,7 @@ where
                         tarencoder
                             .add_entry(&mut header, path, tokio::io::empty())
                             .await
-                            .map_err(|err| format_err!("could not send dir entry: {}", err))?;
+                            .context("could not send dir entry")?;
                     }
                 }
                 EntryKind::Device(device) => {
@@ -690,7 +686,7 @@ where
                     tarencoder
                         .add_entry(&mut header, path, tokio::io::empty())
                         .await
-                        .map_err(|err| format_err!("could not send device entry: {}", err))?;
+                        .context("could not send device entry")?;
                 }
                 _ => {} // ignore all else
             }
@@ -714,7 +710,7 @@ where
     let file = root
         .lookup(&path)
         .await?
-        .ok_or_else(|| format_err!("error opening '{:?}'", path.as_ref()))?;
+        .with_context(|| format!("error opening {:?}", path.as_ref()))?;
 
     let prefix = {
         let mut components = file.entry().path().components();
@@ -756,13 +752,13 @@ where
                     );
                     zip.add_entry(entry, decoder.contents())
                         .await
-                        .map_err(|err| format_err!("could not send file entry: {}", err))?;
+                        .context("could not send file entry")?;
                 }
                 EntryKind::Hardlink(_) => {
                     let entry = root
                         .lookup(&path)
                         .await?
-                        .ok_or_else(|| format_err!("error looking up '{:?}'", path))?;
+                        .with_context(|| format!("error looking up {:?}", path))?;
                     let realfile = accessor.follow_hardlink(&entry).await?;
                     let metadata = realfile.entry().metadata();
                     log::debug!("adding '{}' to zip", path.display());
@@ -774,7 +770,7 @@ where
                     );
                     zip.add_entry(entry, decoder.contents())
                         .await
-                        .map_err(|err| format_err!("could not send file entry: {}", err))?;
+                        .context("could not send file entry")?;
                 }
                 EntryKind::Directory => {
                     log::debug!("adding '{}' to zip", path.display());
@@ -806,26 +802,14 @@ where
         None,
         Some(CreateOptions::new().perm(Mode::from_bits_truncate(0o700))),
     )
-    .map_err(|err| {
-        format_err!(
-            "error creating directory {:?}: {}",
-            destination.as_ref(),
-            err
-        )
-    })?;
+    .with_context(|| format!("error creating directory {:?}", destination.as_ref()))?;
 
     let dir = Dir::open(
         destination.as_ref(),
         OFlag::O_DIRECTORY | OFlag::O_CLOEXEC,
         Mode::empty(),
     )
-    .map_err(|err| {
-        format_err!(
-            "unable to open target directory {:?}: {}",
-            destination.as_ref(),
-            err,
-        )
-    })?;
+    .with_context(|| format!("unable to open target directory {:?}", destination.as_ref()))?;
 
     Ok(Extractor::new(dir, metadata, false, false, Flags::DEFAULT))
 }
@@ -850,7 +834,7 @@ where
     let file = root
         .lookup(&path)
         .await?
-        .ok_or_else(|| format_err!("error opening '{:?}'", path.as_ref()))?;
+        .with_context(|| format!("error opening {:?}", path.as_ref()))?;
 
     recurse_files_extractor(&mut extractor, file).await
 }
@@ -866,7 +850,7 @@ where
     decoder.enable_goodbye_entries(true);
     let root = match decoder.next().await {
         Some(Ok(root)) => root,
-        Some(Err(err)) => bail!("error getting root entry from pxar: {}", err),
+        Some(Err(err)) => return Err(err).context("error getting root entry from pxar"),
         None => bail!("cannot extract empty archive"),
     };
 
@@ -920,8 +904,8 @@ fn get_filename(entry: &Entry) -> Result<(OsString, CString), Error> {
         bail!("archive file entry contains slashes, which is invalid and a security concern");
     }
 
-    let file_name = CString::new(file_name_os.as_bytes())
-        .map_err(|_| format_err!("encountered file name with null-bytes"))?;
+    let file_name =
+        CString::new(file_name_os.as_bytes()).context("encountered file name with null-bytes")?;
 
     Ok((file_name_os, file_name))
 }
@@ -943,7 +927,7 @@ where
         EntryKind::Directory => {
             extractor
                 .enter_directory(file_name_os.to_owned(), metadata.clone(), true)
-                .map_err(|err| format_err!("error at entry {:?}: {}", file_name_os, err))?;
+                .with_context(|| format!("error at entry {file_name_os:?}"))?;
 
             let dir = file.enter_directory().await?;
             let mut seq_decoder = dir.decode_full().await?;
@@ -957,9 +941,10 @@ where
                     &file_name,
                     metadata,
                     *size,
-                    &mut file.contents().await.map_err(|_| {
-                        format_err!("found regular file entry without contents in archive")
-                    })?,
+                    &mut file
+                        .contents()
+                        .await
+                        .context("found regular file entry without contents in archive")?,
                     extractor.overwrite,
                 )
                 .await?
@@ -997,7 +982,7 @@ where
                     dir_level += 1;
                     extractor
                         .enter_directory(file_name_os.to_owned(), metadata.clone(), true)
-                        .map_err(|err| format_err!("error at entry {:?}: {}", file_name_os, err))?;
+                        .with_context(|| format!("error at entry {file_name_os:?}"))?;
                 }
                 EntryKind::File { size, .. } => {
                     extractor
@@ -1005,9 +990,9 @@ where
                             &file_name,
                             metadata,
                             *size,
-                            &mut decoder.contents().ok_or_else(|| {
-                                format_err!("found regular file entry without contents in archive")
-                            })?,
+                            &mut decoder
+                                .contents()
+                                .context("found regular file entry without contents in archive")?,
                             extractor.overwrite,
                         )
                         .await?
diff --git a/pbs-client/src/pxar/metadata.rs b/pbs-client/src/pxar/metadata.rs
index cff3cb34..745785bf 100644
--- a/pbs-client/src/pxar/metadata.rs
+++ b/pbs-client/src/pxar/metadata.rs
@@ -2,7 +2,7 @@ use std::ffi::{CStr, CString};
 use std::os::unix::io::{AsRawFd, RawFd};
 use std::path::Path;
 
-use anyhow::{bail, format_err, Error};
+use anyhow::{anyhow, bail, Context, Error};
 use nix::errno::Errno;
 use nix::fcntl::OFlag;
 use nix::sys::stat::Mode;
@@ -106,7 +106,7 @@ pub fn apply(
         .or_else(&mut *on_error)?;
     add_fcaps(flags, c_proc_path.as_ptr(), metadata, &mut skip_xattrs).or_else(&mut *on_error)?;
     apply_acls(flags, &c_proc_path, metadata, path_info)
-        .map_err(|err| format_err!("failed to apply acls: {}", err))
+        .context("failed to apply acls")
         .or_else(&mut *on_error)?;
     apply_quota_project_id(flags, fd, metadata).or_else(&mut *on_error)?;
 
@@ -118,7 +118,7 @@ pub fn apply(
         })
         .map(drop)
         .or_else(allow_notsupp)
-        .map_err(|err| format_err!("failed to change file mode: {}", err))
+        .context("failed to change file mode")
         .or_else(&mut *on_error)?;
     }
 
@@ -134,11 +134,9 @@ pub fn apply(
         Ok(_) => (),
         Err(ref err) if err.is_errno(Errno::EOPNOTSUPP) => (),
         Err(err) => {
-            on_error(format_err!(
-                "failed to restore mtime attribute on {:?}: {}",
-                path_info,
-                err
-            ))?;
+            on_error(anyhow!(err).context(format!(
+                "failed to restore mtime attribute on {path_info:?}"
+            )))?;
         }
     }
 
@@ -167,7 +165,7 @@ pub fn apply_ownership(
         ))
         .map(drop)
         .or_else(allow_notsupp)
-        .map_err(|err| format_err!("failed to set ownership: {}", err))
+        .context("failed to set ownership")
         .or_else(&mut *on_error)?;
     }
     Ok(())
@@ -198,9 +196,7 @@ fn add_fcaps(
     })
     .map(drop)
     .or_else(|err| allow_notsupp_remember(err, skip_xattrs))
-    .map_err(|err| format_err!("failed to apply file capabilities: {}", err))?;
-
-    Ok(())
+    .context("failed to apply file capabilities")
 }
 
 fn apply_xattrs(
@@ -234,7 +230,7 @@ fn apply_xattrs(
         })
         .map(drop)
         .or_else(|err| allow_notsupp_remember(err, &mut *skip_xattrs))
-        .map_err(|err| format_err!("failed to apply extended attributes: {}", err))?;
+        .context("failed to apply extended attributes")?;
     }
 
     Ok(())
@@ -348,21 +344,13 @@ fn apply_quota_project_id(flags: Flags, fd: RawFd, metadata: &Metadata) -> Resul
 
     let mut fsxattr = fs::FSXAttr::default();
     unsafe {
-        fs::fs_ioc_fsgetxattr(fd, &mut fsxattr).map_err(|err| {
-            format_err!(
-                "error while getting fsxattr to restore quota project id - {}",
-                err
-            )
-        })?;
+        fs::fs_ioc_fsgetxattr(fd, &mut fsxattr)
+            .context("error while getting fsxattr to restore quota project id")?;
 
         fsxattr.fsx_projid = projid.projid as u32;
 
-        fs::fs_ioc_fssetxattr(fd, &fsxattr).map_err(|err| {
-            format_err!(
-                "error while setting fsxattr to restore quota project id - {}",
-                err
-            )
-        })?;
+        fs::fs_ioc_fssetxattr(fd, &fsxattr)
+            .context("error while setting fsxattr to restore quota project id")?;
     }
 
     Ok(())
@@ -386,7 +374,7 @@ fn apply_chattr(fd: RawFd, chattr: libc::c_long, mask: libc::c_long) -> Result<(
         Err(errno) if errno_is_unsupported(errno) => {
             return Ok(());
         }
-        Err(err) => bail!("failed to read file attributes: {}", err),
+        Err(err) => return Err(err).context("failed to read file attributes"),
     }
 
     let attr = (chattr & mask) | (fattr & !mask);
@@ -398,7 +386,7 @@ fn apply_chattr(fd: RawFd, chattr: libc::c_long, mask: libc::c_long) -> Result<(
     match unsafe { fs::write_attr_fd(fd, &attr) } {
         Ok(_) => Ok(()),
         Err(errno) if errno_is_unsupported(errno) => Ok(()),
-        Err(err) => bail!("failed to set file attributes: {}", err),
+        Err(err) => Err(err).context("failed to set file attributes"),
     }
 }
 
@@ -412,7 +400,7 @@ fn apply_flags(flags: Flags, fd: RawFd, entry_flags: u64) -> Result<(), Error> {
         match unsafe { fs::write_fat_attr_fd(fd, &fatattr) } {
             Ok(_) => (),
             Err(errno) if errno_is_unsupported(errno) => (),
-            Err(err) => bail!("failed to set file FAT attributes: {}", err),
+            Err(err) => return Err(err).context("failed to set file FAT attributes"),
         }
     }
 
diff --git a/pbs-client/src/pxar/tools.rs b/pbs-client/src/pxar/tools.rs
index 844a0f73..0cfbaf5b 100644
--- a/pbs-client/src/pxar/tools.rs
+++ b/pbs-client/src/pxar/tools.rs
@@ -4,7 +4,7 @@ use std::ffi::OsStr;
 use std::os::unix::ffi::OsStrExt;
 use std::path::Path;
 
-use anyhow::{bail, format_err, Error};
+use anyhow::{bail, Context, Error};
 use nix::sys::stat::Mode;
 
 use pxar::{format::StatxTimestamp, mode, Entry, EntryKind, Metadata};
@@ -12,10 +12,13 @@ use pxar::{format::StatxTimestamp, mode, Entry, EntryKind, Metadata};
 /// Get the file permissions as `nix::Mode`
 pub fn perms_from_metadata(meta: &Metadata) -> Result<Mode, Error> {
     let mode = meta.stat.get_permission_bits();
+
     u32::try_from(mode)
-        .map_err(drop)
-        .and_then(|mode| Mode::from_bits(mode).ok_or(()))
-        .map_err(|_| format_err!("mode contains illegal bits: 0x{:x} (0o{:o})", mode, mode))
+        .context("couldn't narrow permission bits")
+        .and_then(|mode| {
+            Mode::from_bits(mode)
+                .with_context(|| format!("mode contains illegal bits: 0x{:x} (0o{:o})", mode, mode))
+        })
 }
 
 /// Make sure path is relative and not '.' or '..'.
-- 
2.39.2





  reply	other threads:[~2023-07-17  8:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-17  8:04 [pbs-devel] [PATCH v3 proxmox-backup/pve-container 0/5] improve error handling of pxar extractor Max Carrara
2023-07-17  8:04 ` Max Carrara [this message]
2023-07-17  8:04 ` [pbs-devel] [PATCH v3 proxmox-backup 2/5] pbs-client: pxar: refactor body of `extract_archive` to `ExtractorIter` Max Carrara
2023-07-17  8:04 ` [pbs-devel] [PATCH v3 proxmox-backup 3/5] pbs-client: pxar: add PxarExtractContext Max Carrara
2023-07-17  8:04 ` [pbs-devel] [PATCH v3 proxmox-backup 4/5] proxmox-backup-client: restore: add 'ignore-extract-device-errors' flag Max Carrara
2023-07-17  8:04 ` [pbs-devel] [PATCH v3 pve-container 5/5] fix #3460: restore: honor '--ignore-unpack-errors' flag for pbs Max Carrara
2023-08-23  8:06   ` [pbs-devel] applied: " Wolfgang Bumiller
2023-07-17 12:07 ` [pbs-devel] partially-applied: [PATCH v3 proxmox-backup/pve-container 0/5] improve error handling of pxar extractor Wolfgang Bumiller

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=20230717080410.429362-2-m.carrara@proxmox.com \
    --to=m.carrara@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal