public inbox for pbs-devel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal