Skip to content

Commit

Permalink
[thin_dump] Prompt for repair only on input errors
Browse files Browse the repository at this point in the history
thin_dump should display the reparing hint only if there's any error
in the input metadata rather than the output process. A context
therefore is added to the returned error for callers to determine the
error type. Note that a broken pipe error (EPIPE) is treated as an
output error since the Rust std library ignores SIGPIPE by default
[1][2].

[1] rust-lang/rust#13158
[2] rust-lang/rust#62569
  • Loading branch information
mingnus committed May 26, 2022
1 parent fa3b3c6 commit 94a42b2
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 40 deletions.
22 changes: 13 additions & 9 deletions src/commands/thin_dump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::sync::Arc;
use crate::commands::utils::*;
use crate::commands::Command;
use crate::report::*;
use crate::thin::dump::{dump, ThinDumpOptions};
use crate::thin::dump::{dump, OutputError, ThinDumpOptions};
use crate::thin::metadata_repair::SuperblockOverrides;

pub struct ThinDumpCommand;
Expand Down Expand Up @@ -128,13 +128,17 @@ impl<'a> Command<'a> for ThinDumpCommand {
},
};

dump(opts).map_err(|reason| {
report.fatal(&format!("{}", reason));
report.fatal(
"metadata contains errors (run thin_check for details).\n\
perhaps you wanted to run with --repair ?",
);
std::io::Error::from_raw_os_error(libc::EPERM)
})
if let Err(e) = dump(opts) {
if !e.is::<OutputError>() {
report.fatal(&format!("{:?}", e));
report.fatal(
"metadata contains errors (run thin_check for details).\n\
perhaps you wanted to run with --repair ?",
);
}
return Err(std::io::Error::from_raw_os_error(libc::EPERM));
}

Ok(())
}
}
68 changes: 37 additions & 31 deletions src/thin/dump.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use anyhow::{anyhow, Result};
use anyhow::{anyhow, Context, Result};
use std::fs::File;
use std::io::BufWriter;
use std::io::Write;
Expand Down Expand Up @@ -153,56 +153,59 @@ pub struct ThinDumpOptions<'a> {
pub overrides: SuperblockOverrides,
}

struct Context {
struct ThinDumpContext {
report: Arc<Report>,
engine: Arc<dyn IoEngine + Send + Sync>,
}

fn mk_context(opts: &ThinDumpOptions) -> Result<Context> {
fn mk_context(opts: &ThinDumpOptions) -> Result<ThinDumpContext> {
let engine: Arc<dyn IoEngine + Send + Sync> = if opts.async_io {
Arc::new(AsyncIoEngine::new(opts.input, MAX_CONCURRENT_IO, false)?)
} else {
let nr_threads = std::cmp::max(8, num_cpus::get() * 2);
Arc::new(SyncIoEngine::new(opts.input, nr_threads, false)?)
};

Ok(Context {
Ok(ThinDumpContext {
report: opts.report.clone(),
engine,
})
}

//------------------------------------------

fn emit_leaf(v: &mut MappingVisitor, b: &Block) -> Result<()> {
// a wrapper for callers to indicate the error type
#[derive(Debug)]
pub struct OutputError;

impl std::fmt::Display for OutputError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "output error")
}
}

fn emit_leaf(v: &MappingVisitor, b: &Block) -> Result<()> {
use Node::*;
let path = Vec::new();
let kr = KeyRange::new();

let bt = checksum::metadata_block_type(b.get_data());
if bt != checksum::BT::NODE {
return Err(anyhow!(format!(
"checksum failed for node {}, {:?}",
b.loc, bt
)));
return Err(anyhow!("checksum failed for node {}, {:?}", b.loc, bt));
}

let node = unpack_node::<BlockTime>(&path, b.get_data(), true, true)?;

match node {
Internal { .. } => {
return Err(anyhow!("not a leaf"));
}
Internal { .. } => Err(anyhow!("block {} is not a leaf", b.loc)),
Leaf {
header,
keys,
values,
} => {
v.visit(&path, &kr, &header, &keys, &values)?;
}
} => v
.visit(&path, &kr, &header, &keys, &values)
.context(OutputError),
}

Ok(())
}

fn read_for<T>(engine: Arc<dyn IoEngine>, blocks: &[u64], mut t: T) -> Result<()>
Expand All @@ -214,7 +217,8 @@ where
.read_many(cs)
.map_err(|_e| anyhow!("read_many failed"))?
{
t(b.map_err(|_e| anyhow!("read of individual block failed"))?)?;
let blk = b.map_err(|_e| anyhow!("read of individual block failed"))?;
t(blk)?;
}
}

Expand All @@ -226,15 +230,14 @@ fn emit_leaves(
out: &mut dyn MetadataVisitor,
leaves: &[u64],
) -> Result<()> {
let mut v = MappingVisitor::new(out);
let v = MappingVisitor::new(out);
let proc = |b| {
emit_leaf(&mut v, &b)?;
emit_leaf(&v, &b)?;
Ok(())
};

read_for(engine, leaves, proc)?;
v.end_walk()
.map_err(|e| anyhow!("failed to emit leaves: {}", e))
v.end_walk().context(OutputError)
}

fn emit_entries(
Expand All @@ -255,7 +258,7 @@ fn emit_entries(
leaves.clear();
}
let str = format!("{}", id);
out.ref_shared(&str)?;
out.ref_shared(&str).context(OutputError)?;
}
}
}
Expand Down Expand Up @@ -284,12 +287,13 @@ pub fn dump_metadata(
nr_data_blocks: data_root.nr_blocks,
metadata_snap: None,
};
out.superblock_b(&out_sb)?;
out.superblock_b(&out_sb).context(OutputError)?;

for d in &md.defs {
out.def_shared_b(&format!("{}", d.def_id))?;
out.def_shared_b(&format!("{}", d.def_id))
.context(OutputError)?;
emit_entries(engine.clone(), out, &d.map.entries)?;
out.def_shared_e()?;
out.def_shared_e().context(OutputError)?;
}

for dev in &md.devs {
Expand All @@ -300,12 +304,12 @@ pub fn dump_metadata(
creation_time: dev.detail.creation_time,
snap_time: dev.detail.snapshotted_time,
};
out.device_b(&device)?;
out.device_b(&device).context(OutputError)?;
emit_entries(engine.clone(), out, &dev.map.entries)?;
out.device_e()?;
out.device_e().context(OutputError)?;
}
out.superblock_e()?;
out.eof()?;
out.superblock_e().context(OutputError)?;
out.eof().context(OutputError)?;

Ok(())
}
Expand All @@ -327,11 +331,13 @@ pub fn dump(opts: ThinDumpOptions) -> Result<()> {
read_superblock(ctx.engine.as_ref(), SUPERBLOCK_LOCATION)
.and_then(|sb| sb.overrides(&opts.overrides))?
};

let md = build_metadata(ctx.engine.clone(), &sb)?;
let md = optimise_metadata(md)?;

let writer: Box<dyn Write> = if opts.output.is_some() {
Box::new(BufWriter::new(File::create(opts.output.unwrap())?))
let f = File::create(opts.output.unwrap()).context(OutputError)?;
Box::new(BufWriter::new(f))
} else {
Box::new(BufWriter::new(std::io::stdout()))
};
Expand Down

0 comments on commit 94a42b2

Please sign in to comment.