[pbs-devel] [PATCH proxmox-backup 2/2] api2/tape/restore: restore_snapshot_archive: only ignore tape errors
Dominik Csapak
d.csapak at proxmox.com
Tue Apr 13 12:58:59 CEST 2021
we sometimes want to ignore tape errors, e.g., if a file was incomplete,
so we must check those conditions only when the error really comes
from the tape side and not from the storage side.
Introduce a 'RestoreError', which maps anyhow Errors as type 'Tape' and
io Errors as type 'Disk'
Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
---
src/api2/tape/restore.rs | 97 ++++++++++++++++++++++------------------
1 file changed, 54 insertions(+), 43 deletions(-)
diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
index 9f79f06f..8058765d 100644
--- a/src/api2/tape/restore.rs
+++ b/src/api2/tape/restore.rs
@@ -18,6 +18,7 @@ use proxmox::{
schema::parse_property_string,
section_config::SectionConfigData,
},
+ io_format_err,
tools::{
Uuid,
io::ReadExt,
@@ -646,6 +647,16 @@ fn restore_chunk_archive<'a>(
Ok(Some(chunks))
}
+// we want to differentiate between errors that come from disk and tape side,
+// so map anyhow::Error to tape and io::Error to disk
+#[derive(thiserror::Error, Debug)]
+pub enum RestoreError {
+ #[error("tape error: {0}")]
+ Tape(#[from] Error),
+ #[error("{0}")]
+ Disk(#[from] std::io::Error),
+}
+
fn restore_snapshot_archive<'a>(
worker: &WorkerTask,
reader: Box<dyn 'a + TapeRead>,
@@ -655,7 +666,7 @@ fn restore_snapshot_archive<'a>(
let mut decoder = pxar::decoder::sync::Decoder::from_std(reader)?;
match try_restore_snapshot_archive(worker, &mut decoder, snapshot_path) {
Ok(()) => Ok(true),
- Err(err) => {
+ Err(RestoreError::Tape(err)) => {
let reader = decoder.input();
// check if this stream is marked incomplete
@@ -671,26 +682,25 @@ fn restore_snapshot_archive<'a>(
// else the archive is corrupt
Err(err)
}
+ Err(RestoreError::Disk(err)) => Err(err.into()),
}
}
+// we have to map the errors here according to RestoreError
fn try_restore_snapshot_archive<R: pxar::decoder::SeqRead>(
worker: &WorkerTask,
decoder: &mut pxar::decoder::sync::Decoder<R>,
snapshot_path: &Path,
-) -> Result<(), Error> {
+) -> Result<(), RestoreError> {
- let _root = match decoder.next() {
- None => bail!("missing root entry"),
- Some(root) => {
- let root = root?;
- match root.kind() {
- pxar::EntryKind::Directory => { /* Ok */ }
- _ => bail!("wrong root entry type"),
- }
- root
- }
- };
+ let root = decoder
+ .next()
+ .ok_or_else(|| format_err!("missing root entry"))?
+ .map_err(Error::from)?;
+
+ if !matches!(root.kind(), pxar::EntryKind::Directory) {
+ return Err(format_err!("wrong root entry type").into());
+ }
let root_path = Path::new("/");
let manifest_file_name = OsStr::new(MANIFEST_BLOB_NAME);
@@ -702,29 +712,27 @@ fn try_restore_snapshot_archive<R: pxar::decoder::SeqRead>(
let entry = match decoder.next() {
None => break,
- Some(entry) => entry?,
+ Some(entry) => entry.map_err(Error::from)?,
};
+
let entry_path = entry.path();
- match entry.kind() {
- pxar::EntryKind::File { .. } => { /* Ok */ }
- _ => bail!("wrong entry type for {:?}", entry_path),
+ if !matches!(entry.kind(), pxar::EntryKind::File { .. }) {
+ return Err(format_err!("wrong entry type for {:?}", entry_path).into());
}
- match entry_path.parent() {
- None => bail!("wrong parent for {:?}", entry_path),
- Some(p) => {
- if p != root_path {
- bail!("wrong parent for {:?}", entry_path);
- }
- }
+
+ let path = entry_path
+ .parent()
+ .ok_or_else(|| format_err!("wrong parent for {:?}", entry_path))?;
+
+ if path != root_path {
+ return Err(format_err!("wrong parent for {:?}", entry_path).into());
}
let filename = entry.file_name();
- let mut contents = match decoder.contents() {
- None => bail!("missing file content"),
- Some(contents) => contents,
- };
-
+ let mut contents = decoder
+ .contents()
+ .ok_or_else(|| format_err!("missing file content"))?;
let mut archive_path = snapshot_path.to_owned();
archive_path.push(&filename);
@@ -733,31 +741,30 @@ fn try_restore_snapshot_archive<R: pxar::decoder::SeqRead>(
if filename == manifest_file_name {
- let blob = DataBlob::load_from_reader(&mut contents)?;
+ let blob = DataBlob::load_from_reader(&mut contents).map_err(Error::from)?;
let options = CreateOptions::new();
replace_file(&tmp_path, blob.raw_data(), options)?;
- manifest = Some(BackupManifest::try_from(blob)?);
+ manifest = Some(BackupManifest::try_from(blob).map_err(Error::from)?);
} else {
let mut tmpfile = std::fs::OpenOptions::new()
.write(true)
.create(true)
.read(true)
.open(&tmp_path)
- .map_err(|err| format_err!("restore {:?} failed - {}", tmp_path, err))?;
+ .map_err(|err| io_format_err!("restore {:?} failed - {}", tmp_path, err))?;
- std::io::copy(&mut contents, &mut tmpfile)?;
+ // error type is a bit ambiguous, but err o safe side and assume
+ // a tape error
+ std::io::copy(&mut contents, &mut tmpfile).map_err(Error::from)?;
- if let Err(err) = std::fs::rename(&tmp_path, &archive_path) {
- bail!("Atomic rename file {:?} failed - {}", archive_path, err);
- }
+ std::fs::rename(&tmp_path, &archive_path).map_err(|err| {
+ io_format_err!("Atomic rename file {:?} failed - {}", archive_path, err)
+ })?;
}
}
- let manifest = match manifest {
- None => bail!("missing manifest"),
- Some(manifest) => manifest,
- };
+ let manifest = manifest.ok_or_else(|| format_err!("missing manifest"))?;
for item in manifest.files() {
let mut archive_path = snapshot_path.to_owned();
@@ -788,9 +795,13 @@ fn try_restore_snapshot_archive<R: pxar::decoder::SeqRead>(
let mut tmp_manifest_path = manifest_path.clone();
tmp_manifest_path.set_extension("tmp");
- if let Err(err) = std::fs::rename(&tmp_manifest_path, &manifest_path) {
- bail!("Atomic rename manifest {:?} failed - {}", manifest_path, err);
- }
+ std::fs::rename(&tmp_manifest_path, &manifest_path).map_err(|err| {
+ io_format_err!(
+ "Atomic rename manifest {:?} failed - {}",
+ manifest_path,
+ err
+ )
+ })?;
Ok(())
}
--
2.20.1
More information about the pbs-devel
mailing list