[thin_dump] Prompt for repair only on input errors

Separate output errors from input errors for callers to determine the
error type. A broken pipe error (EPIPE) is also treated as an output
error since the Rust std library ignores SIGPIPE by default [1][2].
thin_dump then displays the hint if there's any error in metadata, or
exit gracefully if it is an output error.

[1] rust-lang/rust#13158
[2] rust-lang/rust#62569

Author: mingnus

src/commands/thin_dump.rs

@@ -8,6 +8,7 @@ use std::sync::Arc;
 
 use crate::commands::utils::*;
 use crate::commands::Command;
+use crate::error::IoError;
 use crate::report::*;
 use crate::thin::dump::{dump, ThinDumpOptions};
 use crate::thin::metadata_repair::SuperblockOverrides;
@@ -128,13 +129,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 let IoError::Input(reason) = e {
+                report.fatal(&format!("{:?}", reason));
+                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(())
     }
 }

src/error.rs

@@ -0,0 +1,22 @@
+// An error type for applications need to distinguish input & output errors
+pub enum IoError {
+    Input(anyhow::Error),
+    Output(anyhow::Error),
+}
+
+pub fn to_input_err(e: anyhow::Error) -> IoError {
+    IoError::Input(e)
+}
+
+pub fn to_output_err(e: anyhow::Error) -> IoError {
+    IoError::Output(e)
+}
+
+impl From<IoError> for anyhow::Error {
+    fn from(e: IoError) -> anyhow::Error {
+        match e {
+            IoError::Input(inner) => inner,
+            IoError::Output(inner) => inner,
+        }
+    }
+}

src/lib.rs

@@ -20,6 +20,7 @@ pub mod checksum;
 pub mod commands;
 pub mod copier;
 pub mod era;
+pub mod error;
 pub mod file_utils;
 pub mod grid_layout;
 pub mod io_engine;

src/shrink/toplevel.rs

@@ -543,7 +543,9 @@ fn rebuild_metadata(opts: ThinShrinkOptions) -> Result<()> {
     let mut w = WriteBatcher::new(output.clone(), sm, output.get_batch_size());
     let mut restorer = Restorer::new(&mut w, opts.report);
     let mut remapper = DataRemapper::new(&mut restorer, opts.nr_blocks, remaps);
-    dump_metadata(input, &mut remapper, &sb, &md)
+    dump_metadata(input, &mut remapper, &sb, &md)?;
+
+    Ok(())
 }
 
 pub fn shrink(opts: ThinShrinkOptions) -> Result<()> {

src/thin/dump.rs

@@ -6,6 +6,7 @@ use std::path::Path;
 use std::sync::{Arc, Mutex};
 
 use crate::checksum;
+use crate::error::*;
 use crate::io_engine::{AsyncIoEngine, Block, IoEngine, SyncIoEngine};
 use crate::pdata::btree::{self, *};
 use crate::pdata::btree_walker::*;
@@ -174,47 +175,51 @@ fn mk_context(opts: &ThinDumpOptions) -> Result<Context> {
 
 //------------------------------------------
 
-fn emit_leaf(v: &mut MappingVisitor, b: &Block) -> Result<()> {
+fn emit_leaf(v: &mut MappingVisitor, b: &Block) -> Result<(), IoError> {
     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!(
+        return Err(IoError::Input(anyhow!(
             "checksum failed for node {}, {:?}",
-            b.loc, bt
+            b.loc,
+            bt
         )));
     }
 
-    let node = unpack_node::<BlockTime>(&path, b.get_data(), true, true)?;
+    let node = unpack_node::<BlockTime>(&path, b.get_data(), true, true)
+        .map_err(|e| IoError::Input(e.into()))?;
 
     match node {
         Internal { .. } => {
-            return Err(anyhow!("not a leaf"));
+            return Err(IoError::Input(anyhow!("node {} is not a leaf", b.loc)));
         }
         Leaf {
             header,
             keys,
             values,
         } => {
-            v.visit(&path, &kr, &header, &keys, &values)?;
+            v.visit(&path, &kr, &header, &keys, &values)
+                .map_err(|e| IoError::Output(e.into()))?;
         }
     }
 
     Ok(())
 }
 
-fn read_for<T>(engine: Arc<dyn IoEngine>, blocks: &[u64], mut t: T) -> Result<()>
+fn read_for<T>(engine: Arc<dyn IoEngine>, blocks: &[u64], mut t: T) -> Result<(), IoError>
 where
-    T: FnMut(Block) -> Result<()>,
+    T: FnMut(Block) -> Result<(), IoError>,
 {
     for cs in blocks.chunks(engine.get_batch_size()) {
         for b in engine
             .read_many(cs)
-            .map_err(|_e| anyhow!("read_many failed"))?
+            .map_err(|_e| IoError::Input(anyhow!("read_many failed")))?
         {
-            t(b.map_err(|_e| anyhow!("read of individual block failed"))?)?;
+            let blk = b.map_err(|_e| IoError::Input(anyhow!("read of individual block failed")))?;
+            t(blk)?;
         }
     }
 
@@ -225,7 +230,7 @@ fn emit_leaves(
     engine: Arc<dyn IoEngine>,
     out: &mut dyn MetadataVisitor,
     leaves: &[u64],
-) -> Result<()> {
+) -> Result<(), IoError> {
     let mut v = MappingVisitor::new(out);
     let proc = |b| {
         emit_leaf(&mut v, &b)?;
@@ -234,14 +239,14 @@ fn emit_leaves(
 
     read_for(engine, leaves, proc)?;
     v.end_walk()
-        .map_err(|e| anyhow!("failed to emit leaves: {}", e))
+        .map_err(|e| IoError::Output(anyhow!("failed to emit leaves: {}", e)))
 }
 
 fn emit_entries(
     engine: Arc<dyn IoEngine>,
     out: &mut dyn MetadataVisitor,
     entries: &[Entry],
-) -> Result<()> {
+) -> Result<(), IoError> {
     let mut leaves = Vec::new();
 
     for e in entries {
@@ -255,7 +260,7 @@ fn emit_entries(
                     leaves.clear();
                 }
                 let str = format!("{}", id);
-                out.ref_shared(&str)?;
+                out.ref_shared(&str).map_err(to_output_err)?;
             }
         }
     }
@@ -272,8 +277,9 @@ pub fn dump_metadata(
     out: &mut dyn MetadataVisitor,
     sb: &Superblock,
     md: &Metadata,
-) -> Result<()> {
-    let data_root = unpack::<SMRoot>(&sb.data_sm_root[0..])?;
+) -> Result<(), IoError> {
+    let data_root =
+        unpack::<SMRoot>(&sb.data_sm_root[0..]).map_err(|e| IoError::Input(e.into()))?;
     let out_sb = ir::Superblock {
         uuid: "".to_string(),
         time: sb.time,
@@ -284,12 +290,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).map_err(to_output_err)?;
 
     for d in &md.defs {
-        out.def_shared_b(&format!("{}", d.def_id))?;
+        out.def_shared_b(&format!("{}", d.def_id))
+            .map_err(to_output_err)?;
         emit_entries(engine.clone(), out, &d.map.entries)?;
-        out.def_shared_e()?;
+        out.def_shared_e().map_err(to_output_err)?;
     }
 
     for dev in &md.devs {
@@ -300,38 +307,42 @@ pub fn dump_metadata(
             creation_time: dev.detail.creation_time,
             snap_time: dev.detail.snapshotted_time,
         };
-        out.device_b(&device)?;
+        out.device_b(&device).map_err(to_output_err)?;
         emit_entries(engine.clone(), out, &dev.map.entries)?;
-        out.device_e()?;
+        out.device_e().map_err(to_output_err)?;
     }
-    out.superblock_e()?;
-    out.eof()?;
+    out.superblock_e().map_err(to_output_err)?;
+    out.eof().map_err(to_output_err)?;
 
     Ok(())
 }
 
 //------------------------------------------
 
-pub fn dump(opts: ThinDumpOptions) -> Result<()> {
-    let ctx = mk_context(&opts)?;
+pub fn dump(opts: ThinDumpOptions) -> Result<(), IoError> {
+    let ctx = mk_context(&opts).map_err(to_input_err)?;
     let sb = if opts.repair {
         read_or_rebuild_superblock(
             ctx.engine.clone(),
             ctx.report.clone(),
             SUPERBLOCK_LOCATION,
             &opts.overrides,
-        )?
+        )
     } else if opts.use_metadata_snap {
-        read_superblock_snap(ctx.engine.as_ref())?
+        read_superblock_snap(ctx.engine.as_ref())
     } else {
         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)?;
+            .and_then(|sb| sb.overrides(&opts.overrides))
+    }
+    .map_err(to_input_err)?;
+
+    let md = build_metadata(ctx.engine.clone(), &sb).map_err(to_input_err)?;
+    let md = optimise_metadata(md).map_err(to_input_err)?;
 
     let writer: Box<dyn Write> = if opts.output.is_some() {
-        Box::new(BufWriter::new(File::create(opts.output.unwrap())?))
+        Box::new(BufWriter::new(
+            File::create(opts.output.unwrap()).map_err(|e| IoError::Output(e.into()))?,
+        ))
     } else {
         Box::new(BufWriter::new(std::io::stdout()))
     };

src/thin/repair.rs

@@ -72,7 +72,9 @@ pub fn repair(opts: ThinRepairOptions) -> Result<()> {
     );
     let mut restorer = Restorer::new(&mut w, ctx.report);
 
-    dump_metadata(ctx.engine_in, &mut restorer, &sb, &md)
+    dump_metadata(ctx.engine_in, &mut restorer, &sb, &md)?;
+
+    Ok(())
 }
 
 //------------------------------------------