From 34f927d9896b320fb0040fd461ecce05b9370334 Mon Sep 17 00:00:00 2001
From: Ming-Hung Tsai <mtsai@redhat.com>
Date: Thu, 9 Sep 2021 17:42:36 +0800
Subject: [PATCH] [thin_check (rust)] Make better use of Rust's Result type

Replace the bail_out checking with the returned Result, which helps
decoupling the internal state of Report from application logic.
---
 src/pdata/space_map_checker.rs | 10 +++++++++-
 src/thin/check.rs              | 19 -------------------
 2 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/src/pdata/space_map_checker.rs b/src/pdata/space_map_checker.rs
index 1ae7f6b..ad5998b 100644
--- a/src/pdata/space_map_checker.rs
+++ b/src/pdata/space_map_checker.rs
@@ -98,6 +98,7 @@ fn check_low_ref_counts(
 
     // compare ref-counts in bitmap blocks
     let mut leaks = 0;
+    let mut failed = false;
     let mut blocknr = 0;
     let mut bitmap_leaks = Vec::new();
     let sm = sm.lock().unwrap();
@@ -113,6 +114,7 @@ fn check_low_ref_counts(
                         "Index entry points to block ({}) that isn't a bitmap",
                         b.loc
                     ));
+                    failed = true;
 
                     // FIXME: revert the ref-count at b.loc?
                 }
@@ -134,6 +136,7 @@ fn check_low_ref_counts(
                             } else if *actual != expected as u8 {
                                 report.fatal(&format!("Bad reference count for {} block {}.  Expected {}, but space map contains {}.",
                                           kind, blocknr, expected, actual));
+                                failed = true;
                             }
                         }
                         BitmapEntry::Overflow => {
@@ -141,6 +144,7 @@ fn check_low_ref_counts(
                             if expected < 3 {
                                 report.fatal(&format!("Bad reference count for {} block {}.  Expected {}, but space map says it's >= 3.",
                                                   kind, blocknr, expected));
+                                failed = true;
                             }
                         }
                     }
@@ -160,7 +164,11 @@ fn check_low_ref_counts(
         report.non_fatal(&format!("{} {} blocks have leaked.", leaks, kind));
     }
 
-    Ok(bitmap_leaks)
+    if failed {
+        Err(anyhow!("Fatal errors in {} space map", kind))
+    } else {
+        Ok(bitmap_leaks)
+    }
 }
 
 fn gather_disk_index_entries(
diff --git a/src/thin/check.rs b/src/thin/check.rs
index cf61296..e3e8a93 100644
--- a/src/thin/check.rs
+++ b/src/thin/check.rs
@@ -206,18 +206,6 @@ fn mk_context(engine: Arc<dyn IoEngine + Send + Sync>, report: Arc<Report>) -> R
     })
 }
 
-fn bail_out(ctx: &Context, task: &str) -> Result<()> {
-    use ReportOutcome::*;
-
-    match ctx.report.get_outcome() {
-        Fatal => Err(anyhow!(format!(
-            "Check of {} failed, ending check early.",
-            task
-        ))),
-        _ => Ok(()),
-    }
-}
-
 pub fn check(opts: ThinCheckOptions) -> Result<()> {
     let ctx = mk_context(opts.engine.clone(), opts.report.clone())?;
 
@@ -289,7 +277,6 @@ pub fn check(opts: ThinCheckOptions) -> Result<()> {
     let root = unpack::<SMRoot>(&sb.data_sm_root[0..])?;
     let data_sm = core_sm(root.nr_blocks, nr_devs as u32);
     check_mapping_bottom_level(&ctx, &metadata_sm, &data_sm, &roots, opts.ignore_non_fatal)?;
-    bail_out(&ctx, "mapping tree")?;
 
     //-----------------------------------------
 
@@ -303,7 +290,6 @@ pub fn check(opts: ThinCheckOptions) -> Result<()> {
         metadata_sm.clone(),
         opts.ignore_non_fatal,
     )?;
-    bail_out(&ctx, "data space map")?;
 
     //-----------------------------------------
 
@@ -323,8 +309,6 @@ pub fn check(opts: ThinCheckOptions) -> Result<()> {
         opts.ignore_non_fatal,
     )?;
 
-    bail_out(&ctx, "metadata space map")?;
-
     //-----------------------------------------
 
     if opts.auto_repair {
@@ -434,7 +418,6 @@ pub fn check_with_maps(
     let root = unpack::<SMRoot>(&sb.data_sm_root[0..])?;
     let data_sm = core_sm(root.nr_blocks, nr_devs as u32);
     check_mapping_bottom_level(&ctx, &metadata_sm, &data_sm, &roots, false)?;
-    bail_out(&ctx, "mapping tree")?;
 
     //-----------------------------------------
 
@@ -448,7 +431,6 @@ pub fn check_with_maps(
         metadata_sm.clone(),
         false,
     )?;
-    bail_out(&ctx, "data space map")?;
 
     //-----------------------------------------
 
@@ -462,7 +444,6 @@ pub fn check_with_maps(
     // Now the counts should be correct and we can check it.
     let _metadata_leaks =
         check_metadata_space_map(engine.clone(), report, root, metadata_sm.clone(), false)?;
-    bail_out(&ctx, "metadata space map")?;
 
     //-----------------------------------------