diff --git a/src/backup/catalog_shell.rs b/src/backup/catalog_shell.rs index 99552cd9..795d8bb3 100644 --- a/src/backup/catalog_shell.rs +++ b/src/backup/catalog_shell.rs @@ -3,7 +3,7 @@ use std::ffi::{CStr, CString, OsStr, OsString}; use std::future::Future; use std::io::Write; use std::mem; -use std::os::unix::ffi::OsStrExt; +use std::os::unix::ffi::{OsStrExt, OsStringExt}; use std::path::{Path, PathBuf}; use std::pin::Pin; @@ -1073,6 +1073,7 @@ impl<'a> ExtractorState<'a> { } self.path.extend(&entry.name); + self.extractor.set_path(OsString::from_vec(self.path.clone())); self.handle_entry(entry).await?; } diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs index 0a9228fe..16c6ec66 100644 --- a/src/bin/proxmox-backup-client.rs +++ b/src/bin/proxmox-backup-client.rs @@ -1383,6 +1383,7 @@ async fn restore(param: Value) -> Result { println!("{:?}", path); } }, + None, ) .map_err(|err| format_err!("error extracting archive - {}", err))?; } else { diff --git a/src/bin/pxar.rs b/src/bin/pxar.rs index a3951bb9..4d2308bf 100644 --- a/src/bin/pxar.rs +++ b/src/bin/pxar.rs @@ -3,8 +3,10 @@ use std::ffi::OsStr; use std::fs::OpenOptions; use std::os::unix::fs::OpenOptionsExt; use std::path::{Path, PathBuf}; +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; -use anyhow::{format_err, Error}; +use anyhow::{bail, format_err, Error}; use futures::future::FutureExt; use futures::select; use tokio::signal::unix::{signal, SignalKind}; @@ -25,6 +27,7 @@ fn extract_archive_from_reader( verbose: bool, match_list: &[MatchEntry], extract_match_default: bool, + on_error: Option Result<(), Error> + Send>>, ) -> Result<(), Error> { proxmox_backup::pxar::extract_archive( pxar::decoder::Decoder::from_std(reader)?, @@ -38,6 +41,7 @@ fn extract_archive_from_reader( println!("{:?}", path); } }, + on_error, ) } @@ -104,6 +108,11 @@ fn extract_archive_from_reader( optional: true, default: false, }, + strict: { + description: "Stop on errors. Otherwise most errors will simply warn.", + optional: true, + default: false, + }, }, }, )] @@ -121,6 +130,7 @@ fn extract_archive( no_device_nodes: bool, no_fifos: bool, no_sockets: bool, + strict: bool, ) -> Result<(), Error> { let mut feature_flags = Flags::DEFAULT; if no_xattrs { @@ -166,6 +176,20 @@ fn extract_archive( let extract_match_default = match_list.is_empty(); + let was_ok = Arc::new(AtomicBool::new(true)); + let on_error = if strict { + // by default errors are propagated up + None + } else { + let was_ok = Arc::clone(&was_ok); + // otherwise we want to log them but not act on them + Some(Box::new(move |err| { + was_ok.store(false, Ordering::Release); + eprintln!("error: {}", err); + Ok(()) + }) as Box Result<(), Error> + Send>) + }; + if archive == "-" { let stdin = std::io::stdin(); let mut reader = stdin.lock(); @@ -177,6 +201,7 @@ fn extract_archive( verbose, &match_list, extract_match_default, + on_error, )?; } else { if verbose { @@ -192,9 +217,14 @@ fn extract_archive( verbose, &match_list, extract_match_default, + on_error, )?; } + if !was_ok.load(Ordering::Acquire) { + bail!("there were errors"); + } + Ok(()) } diff --git a/src/pxar/extract.rs b/src/pxar/extract.rs index 4a2993a0..4a111922 100644 --- a/src/pxar/extract.rs +++ b/src/pxar/extract.rs @@ -6,6 +6,7 @@ use std::io; use std::os::unix::ffi::OsStrExt; use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; use std::path::Path; +use std::sync::{Arc, Mutex}; use anyhow::{bail, format_err, Error}; use nix::dir::Dir; @@ -20,8 +21,8 @@ use proxmox::c_result; use proxmox::tools::fs::{create_path, CreateOptions}; use crate::pxar::dir_stack::PxarDirStack; -use crate::pxar::Flags; use crate::pxar::metadata; +use crate::pxar::Flags; pub fn extract_archive( mut decoder: pxar::decoder::Decoder, @@ -31,6 +32,7 @@ pub fn extract_archive( feature_flags: Flags, allow_existing_dirs: bool, mut callback: F, + on_error: Option Result<(), Error> + Send>>, ) -> Result<(), Error> where T: pxar::decoder::SeqRead, @@ -69,7 +71,12 @@ where feature_flags, ); + if let Some(on_error) = on_error { + extractor.on_error(on_error); + } + let mut match_stack = Vec::new(); + let mut err_path_stack = Vec::new(); let mut current_match = extract_match_default; while let Some(entry) = decoder.next() { use pxar::EntryKind; @@ -88,6 +95,8 @@ where let metadata = entry.metadata(); + extractor.set_path(entry.path().as_os_str().to_owned()); + let match_result = match_list.matches( entry.path().as_os_str().as_bytes(), Some(metadata.file_type() as u32), @@ -103,17 +112,32 @@ where callback(entry.path()); let create = current_match && match_result != Some(MatchType::Exclude); - extractor.enter_directory(file_name_os.to_owned(), metadata.clone(), create)?; + extractor + .enter_directory(file_name_os.to_owned(), metadata.clone(), create) + .map_err(|err| format_err!("error at entry {:?}: {}", file_name_os, err))?; // We're starting a new directory, push our old matching state and replace it with // our new one: match_stack.push(current_match); current_match = did_match; + // When we hit the goodbye table we'll try to apply metadata to the directory, but + // the Goodbye entry will not contain the path, so push it to our path stack for + // error messages: + err_path_stack.push(extractor.clone_path()); + Ok(()) } (_, 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", + file_name_os + ) + })?); + extractor .leave_directory() .map_err(|err| format_err!("error at entry {:?}: {}", file_name_os, err))?; @@ -182,6 +206,13 @@ pub(crate) struct Extractor { feature_flags: Flags, allow_existing_dirs: bool, dir_stack: PxarDirStack, + + /// For better error output we need to track the current path in the Extractor state. + current_path: Arc>, + + /// Error callback. Includes `current_path` in the reformatted error, should return `Ok` to + /// continue extracting or the passed error as `Err` to bail out. + on_error: Box Result<(), Error> + Send>, } impl Extractor { @@ -196,9 +227,30 @@ impl Extractor { dir_stack: PxarDirStack::new(root_dir, metadata), allow_existing_dirs, feature_flags, + current_path: Arc::new(Mutex::new(OsString::new())), + on_error: Box::new(|err| Err(err)), } } + /// We call this on errors. The error will be reformatted to include `current_path`. The + /// callback should decide whether this error was fatal (simply return it) to bail out early, + /// or log/remember/accumulate errors somewhere and return `Ok(())` in its place to continue + /// extracting. + 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)) + }); + } + + pub fn set_path(&mut self, path: OsString) { + *self.current_path.lock().unwrap() = path; + } + + pub fn clone_path(&self) -> OsString { + self.current_path.lock().unwrap().clone() + } + /// When encountering a directory during extraction, this is used to keep track of it. If /// `create` is true it is immediately created and its metadata will be updated once we leave /// it. If `create` is false it will only be created if it is going to have any actual content. @@ -217,7 +269,7 @@ impl Extractor { Ok(()) } - /// When done with a directory we need to make sure we're + /// When done with a directory we can apply its metadata if it has been created. pub fn leave_directory(&mut self) -> Result<(), Error> { let dir = self .dir_stack @@ -231,6 +283,7 @@ impl Extractor { dir.metadata(), fd, &CString::new(dir.file_name().as_bytes())?, + &mut self.on_error, ) .map_err(|err| format_err!("failed to apply directory metadata: {}", err))?; } @@ -256,14 +309,16 @@ impl Extractor { ) -> Result<(), Error> { let parent = self.parent_fd()?; nix::unistd::symlinkat(link, Some(parent), file_name)?; - metadata::apply_at(self.feature_flags, metadata, parent, file_name) + metadata::apply_at( + self.feature_flags, + metadata, + parent, + file_name, + &mut self.on_error, + ) } - pub fn extract_hardlink( - &mut self, - file_name: &CStr, - link: &OsStr, - ) -> Result<(), Error> { + pub fn extract_hardlink(&mut self, file_name: &CStr, link: &OsStr) -> Result<(), Error> { crate::pxar::tools::assert_relative_path(link)?; let parent = self.parent_fd()?; @@ -307,7 +362,13 @@ impl Extractor { unsafe { c_result!(libc::mknodat(parent, file_name.as_ptr(), mode, device)) } .map_err(|err| format_err!("failed to create device node: {}", err))?; - metadata::apply_at(self.feature_flags, metadata, parent, file_name) + metadata::apply_at( + self.feature_flags, + metadata, + parent, + file_name, + &mut self.on_error, + ) } pub fn extract_file( @@ -319,16 +380,23 @@ impl Extractor { ) -> Result<(), Error> { let parent = self.parent_fd()?; let mut file = unsafe { - std::fs::File::from_raw_fd(nix::fcntl::openat( - parent, - file_name, - OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_WRONLY | OFlag::O_CLOEXEC, - Mode::from_bits(0o600).unwrap(), + std::fs::File::from_raw_fd( + nix::fcntl::openat( + parent, + file_name, + OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_WRONLY | OFlag::O_CLOEXEC, + Mode::from_bits(0o600).unwrap(), + ) + .map_err(|err| format_err!("failed to create file {:?}: {}", file_name, err))?, ) - .map_err(|err| format_err!("failed to create file {:?}: {}", file_name, err))?) }; - metadata::apply_initial_flags(self.feature_flags, metadata, file.as_raw_fd())?; + metadata::apply_initial_flags( + self.feature_flags, + metadata, + file.as_raw_fd(), + &mut self.on_error, + )?; let extracted = io::copy(&mut *contents, &mut file) .map_err(|err| format_err!("failed to copy file contents: {}", err))?; @@ -336,7 +404,13 @@ impl Extractor { bail!("extracted {} bytes of a file of {} bytes", extracted, size); } - metadata::apply(self.feature_flags, metadata, file.as_raw_fd(), file_name) + metadata::apply( + self.feature_flags, + metadata, + file.as_raw_fd(), + file_name, + &mut self.on_error, + ) } pub async fn async_extract_file( @@ -348,16 +422,23 @@ impl Extractor { ) -> Result<(), Error> { let parent = self.parent_fd()?; let mut file = tokio::fs::File::from_std(unsafe { - std::fs::File::from_raw_fd(nix::fcntl::openat( - parent, - file_name, - OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_WRONLY | OFlag::O_CLOEXEC, - Mode::from_bits(0o600).unwrap(), + std::fs::File::from_raw_fd( + nix::fcntl::openat( + parent, + file_name, + OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_WRONLY | OFlag::O_CLOEXEC, + Mode::from_bits(0o600).unwrap(), + ) + .map_err(|err| format_err!("failed to create file {:?}: {}", file_name, err))?, ) - .map_err(|err| format_err!("failed to create file {:?}: {}", file_name, err))?) }); - metadata::apply_initial_flags(self.feature_flags, metadata, file.as_raw_fd())?; + metadata::apply_initial_flags( + self.feature_flags, + metadata, + file.as_raw_fd(), + &mut self.on_error, + )?; let extracted = tokio::io::copy(&mut *contents, &mut file) .await @@ -366,6 +447,12 @@ impl Extractor { bail!("extracted {} bytes of a file of {} bytes", extracted, size); } - metadata::apply(self.feature_flags, metadata, file.as_raw_fd(), file_name) + metadata::apply( + self.feature_flags, + metadata, + file.as_raw_fd(), + file_name, + &mut self.on_error, + ) } } diff --git a/src/pxar/metadata.rs b/src/pxar/metadata.rs index 6df00230..ae72890c 100644 --- a/src/pxar/metadata.rs +++ b/src/pxar/metadata.rs @@ -62,6 +62,7 @@ pub fn apply_at( metadata: &Metadata, parent: RawFd, file_name: &CStr, + on_error: &mut (dyn FnMut(Error) -> Result<(), Error> + Send), ) -> Result<(), Error> { let fd = proxmox::tools::fd::Fd::openat( &unsafe { RawFdNum::from_raw_fd(parent) }, @@ -70,20 +71,32 @@ pub fn apply_at( Mode::empty(), )?; - apply(flags, metadata, fd.as_raw_fd(), file_name) + apply(flags, metadata, fd.as_raw_fd(), file_name, on_error) } pub fn apply_initial_flags( flags: Flags, metadata: &Metadata, fd: RawFd, + on_error: &mut (dyn FnMut(Error) -> Result<(), Error> + Send), ) -> Result<(), Error> { let entry_flags = Flags::from_bits_truncate(metadata.stat.flags); - apply_chattr(fd, entry_flags.to_initial_chattr(), flags.to_initial_chattr())?; + apply_chattr( + fd, + entry_flags.to_initial_chattr(), + flags.to_initial_chattr(), + ) + .or_else(on_error)?; Ok(()) } -pub fn apply(flags: Flags, metadata: &Metadata, fd: RawFd, file_name: &CStr) -> Result<(), Error> { +pub fn apply( + flags: Flags, + metadata: &Metadata, + fd: RawFd, + file_name: &CStr, + on_error: &mut (dyn FnMut(Error) -> Result<(), Error> + Send), +) -> Result<(), Error> { let c_proc_path = CString::new(format!("/proc/self/fd/{}", fd)).unwrap(); unsafe { @@ -95,15 +108,18 @@ pub fn apply(flags: Flags, metadata: &Metadata, fd: RawFd, file_name: &CStr) -> )) .map(drop) .or_else(allow_notsupp) - .map_err(|err| format_err!("failed to set ownership: {}", err))?; + .map_err(|err| format_err!("failed to set ownership: {}", err)) + .or_else(&mut *on_error)?; } let mut skip_xattrs = false; - apply_xattrs(flags, c_proc_path.as_ptr(), metadata, &mut skip_xattrs)?; - add_fcaps(flags, c_proc_path.as_ptr(), metadata, &mut skip_xattrs)?; + apply_xattrs(flags, c_proc_path.as_ptr(), metadata, &mut skip_xattrs) + .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) - .map_err(|err| format_err!("failed to apply acls: {}", err))?; - apply_quota_project_id(flags, fd, metadata)?; + .map_err(|err| format_err!("failed to apply acls: {}", err)) + .or_else(&mut *on_error)?; + apply_quota_project_id(flags, fd, metadata).or_else(&mut *on_error)?; // Finally mode and time. We may lose access with mode, but the changing the mode also // affects times. @@ -113,11 +129,12 @@ pub fn apply(flags: Flags, metadata: &Metadata, fd: RawFd, file_name: &CStr) -> }) .map(drop) .or_else(allow_notsupp) - .map_err(|err| format_err!("failed to change file mode: {}", err))?; + .map_err(|err| format_err!("failed to change file mode: {}", err)) + .or_else(&mut *on_error)?; } if metadata.stat.flags != 0 { - apply_flags(flags, fd, metadata.stat.flags)?; + apply_flags(flags, fd, metadata.stat.flags).or_else(&mut *on_error)?; } let res = c_result!(unsafe { @@ -131,13 +148,13 @@ pub fn apply(flags: Flags, metadata: &Metadata, fd: RawFd, file_name: &CStr) -> match res { Ok(_) => (), Err(ref err) if err.is_errno(Errno::EOPNOTSUPP) => (), - Err(ref err) if err.is_errno(Errno::EPERM) => { - println!( + Err(err) => { + on_error(format_err!( "failed to restore mtime attribute on {:?}: {}", - file_name, err - ); + file_name, + err + ))?; } - Err(err) => return Err(err.into()), } Ok(()) @@ -189,7 +206,7 @@ fn apply_xattrs( } if !xattr::is_valid_xattr_name(xattr.name()) { - println!("skipping invalid xattr named {:?}", xattr.name()); + eprintln!("skipping invalid xattr named {:?}", xattr.name()); continue; }