From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id D7729D877 for ; Mon, 17 Jul 2023 10:04:50 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B9008AA7E for ; Mon, 17 Jul 2023 10:04:20 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 17 Jul 2023 10:04:18 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 2F62C425E3 for ; Mon, 17 Jul 2023 10:04:18 +0200 (CEST) From: Max Carrara To: pbs-devel@lists.proxmox.com Date: Mon, 17 Jul 2023 10:04:06 +0200 Message-Id: <20230717080410.429362-2-m.carrara@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230717080410.429362-1-m.carrara@proxmox.com> References: <20230717080410.429362-1-m.carrara@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.011 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 v3 proxmox-backup 1/5] pbs-client: pxar: preserve error context X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Jul 2023 08:04:50 -0000 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 --- 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 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 { 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