[thin_check (rust)] Pull out space map checking routines

src/pdata/btree_walker.rs

@@ -567,3 +567,51 @@ pub fn btree_to_map_with_path<V: Unpack + Copy>(
 }
 
 //------------------------------------------
+
+struct NoopVisitor<V> {
+    dummy: std::marker::PhantomData<V>,
+}
+
+impl<V> NoopVisitor<V> {
+    pub fn new() -> NoopVisitor<V> {
+        NoopVisitor {
+            dummy: std::marker::PhantomData,
+        }
+    }
+}
+
+impl<V: Unpack> NodeVisitor<V> for NoopVisitor<V> {
+    fn visit(
+        &self,
+        _path: &[u64],
+        _kr: &KeyRange,
+        _header: &NodeHeader,
+        _keys: &[u64],
+        _values: &[V],
+    ) -> Result<()> {
+        Ok(())
+    }
+
+    //fn visit_again(&self, _path: &[u64], _b: u64) -> Result<()> {
+    fn visit_again(&self, _path: &[u64], _b: u64) -> Result<()> {
+        Ok(())
+    }
+
+    fn end_walk(&self) -> Result<()> {
+        Ok(())
+    }
+}
+
+pub fn count_btree_blocks<V: Unpack>(
+    engine: Arc<dyn IoEngine + Send + Sync>,
+    path: &mut Vec<u64>,
+    root: u64,
+    metadata_sm: ASpaceMap,
+    ignore_non_fatal: bool,
+) -> Result<()> {
+    let w = BTreeWalker::new_with_sm(engine, metadata_sm, ignore_non_fatal)?;
+    let v = NoopVisitor::<V>::new();
+    w.walk(path, &v, root)
+}
+
+//------------------------------------------

src/pdata/mod.rs

@@ -7,5 +7,6 @@ pub mod btree_merge;
 pub mod btree_leaf_walker;
 pub mod btree_walker;
 pub mod space_map;
+pub mod space_map_checker;
 pub mod unpack;
 

src/pdata/space_map_checker.rs

@@ -0,0 +1,308 @@
+use anyhow::{anyhow, Result};
+use std::io::Cursor;
+use std::sync::Arc;
+
+use crate::checksum;
+use crate::io_engine::IoEngine;
+use crate::pdata::btree::{self, *};
+use crate::pdata::btree_walker::*;
+use crate::pdata::space_map::*;
+use crate::pdata::unpack::*;
+use crate::report::Report;
+
+//------------------------------------------
+
+pub struct BitmapLeak {
+    blocknr: u64, // blocknr for the first entry in the bitmap
+    loc: u64,     // location of the bitmap
+}
+
+//------------------------------------------
+
+struct OverflowChecker<'a> {
+    kind: &'a str,
+    sm: &'a dyn SpaceMap,
+}
+
+impl<'a> OverflowChecker<'a> {
+    fn new(kind: &'a str, sm: &'a dyn SpaceMap) -> OverflowChecker<'a> {
+        OverflowChecker { kind, sm }
+    }
+}
+
+impl<'a> NodeVisitor<u32> for OverflowChecker<'a> {
+    fn visit(
+        &self,
+        _path: &[u64],
+        _kr: &KeyRange,
+        _h: &NodeHeader,
+        keys: &[u64],
+        values: &[u32],
+    ) -> btree::Result<()> {
+        for n in 0..keys.len() {
+            let k = keys[n];
+            let v = values[n];
+            let expected = self.sm.get(k).unwrap();
+            if expected != v {
+                return Err(value_err(format!("Bad reference count for {} block {}.  Expected {}, but space map contains {}.",
+                    self.kind, k, expected, v)));
+            }
+        }
+
+        Ok(())
+    }
+
+    fn visit_again(&self, _path: &[u64], _b: u64) -> btree::Result<()> {
+        Ok(())
+    }
+
+    fn end_walk(&self) -> btree::Result<()> {
+        Ok(())
+    }
+}
+
+//------------------------------------------
+
+fn inc_entries(sm: &ASpaceMap, entries: &[IndexEntry]) -> Result<()> {
+    let mut sm = sm.lock().unwrap();
+    for ie in entries {
+        // FIXME: checksumming bitmaps?
+        sm.inc(ie.blocknr, 1)?;
+    }
+    Ok(())
+}
+
+// Compare the refernece counts in bitmaps against the expected values
+//
+// `sm` - The in-core space map of expected reference counts
+fn check_low_ref_counts<'a>(
+    engine: Arc<dyn IoEngine + Send + Sync>,
+    report: Arc<Report>,
+    kind: &'a str,
+    entries: Vec<IndexEntry>,
+    sm: ASpaceMap,
+) -> Result<Vec<BitmapLeak>> {
+    // gathering bitmap blocknr
+    let mut blocks = Vec::with_capacity(entries.len());
+    for i in &entries {
+        blocks.push(i.blocknr);
+    }
+
+    // read bitmap blocks
+    // FIXME: we should do this in batches
+    let blocks = engine.read_many(&blocks)?;
+
+    // compare ref-counts in bitmap blocks
+    let mut leaks = 0;
+    let mut blocknr = 0;
+    let mut bitmap_leaks = Vec::new();
+    let sm = sm.lock().unwrap();
+    let nr_blocks = sm.get_nr_blocks()?;
+    for b in blocks.iter().take(entries.len()) {
+        match b {
+            Err(_e) => {
+                return Err(anyhow!("Unable to read bitmap block"));
+            }
+            Ok(b) => {
+                if checksum::metadata_block_type(&b.get_data()) != checksum::BT::BITMAP {
+                    report.fatal(&format!(
+                        "Index entry points to block ({}) that isn't a bitmap",
+                        b.loc
+                    ));
+
+                    // FIXME: revert the ref-count at b.loc?
+                }
+
+                let bitmap = unpack::<Bitmap>(b.get_data())?;
+                let first_blocknr = blocknr;
+                let mut contains_leak = false;
+                for e in bitmap.entries.iter() {
+                    if blocknr >= nr_blocks {
+                        break;
+                    }
+
+                    match e {
+                        BitmapEntry::Small(actual) => {
+                            let expected = sm.get(blocknr)?;
+                            if *actual == 1 && expected == 0 {
+                                leaks += 1;
+                                contains_leak = true;
+                            } else if *actual != expected as u8 {
+                                report.fatal(&format!("Bad reference count for {} block {}.  Expected {}, but space map contains {}.",
+                                          kind, blocknr, expected, actual));
+                            }
+                        }
+                        BitmapEntry::Overflow => {
+                            let expected = sm.get(blocknr)?;
+                            if expected < 3 {
+                                report.fatal(&format!("Bad reference count for {} block {}.  Expected {}, but space map says it's >= 3.",
+                                                  kind, blocknr, expected));
+                            }
+                        }
+                    }
+                    blocknr += 1;
+                }
+                if contains_leak {
+                    bitmap_leaks.push(BitmapLeak {
+                        blocknr: first_blocknr,
+                        loc: b.loc,
+                    });
+                }
+            }
+        }
+    }
+
+    if leaks > 0 {
+        report.non_fatal(&format!("{} {} blocks have leaked.", leaks, kind));
+    }
+
+    Ok(bitmap_leaks)
+}
+
+fn gather_disk_index_entries(
+    engine: Arc<dyn IoEngine + Send + Sync>,
+    bitmap_root: u64,
+    metadata_sm: ASpaceMap,
+    ignore_non_fatal: bool,
+) -> Result<Vec<IndexEntry>> {
+    let entries_map = btree_to_map_with_sm::<IndexEntry>(
+        &mut vec![0],
+        engine,
+        metadata_sm.clone(),
+        ignore_non_fatal,
+        bitmap_root,
+    )?;
+
+    let entries: Vec<IndexEntry> = entries_map.values().cloned().collect();
+    inc_entries(&metadata_sm, &entries[0..])?;
+
+    Ok(entries)
+}
+
+fn gather_metadata_index_entries(
+    engine: Arc<dyn IoEngine + Send + Sync>,
+    bitmap_root: u64,
+    metadata_sm: ASpaceMap,
+) -> Result<Vec<IndexEntry>> {
+    let b = engine.read(bitmap_root)?;
+    let entries = unpack::<MetadataIndex>(b.get_data())?.indexes;
+
+    // Filter out unused entries with block 0
+    let entries: Vec<IndexEntry> = entries
+        .iter()
+        .take_while(|e| e.blocknr != 0)
+        .cloned()
+        .collect();
+
+    metadata_sm.lock().unwrap().inc(bitmap_root, 1)?;
+    inc_entries(&metadata_sm, &entries[0..])?;
+
+    Ok(entries)
+}
+
+//------------------------------------------
+
+// This checks the space map and returns any leak blocks for auto-repair to process.
+//
+// `disk_sm` - The in-core space map of expected data block ref-counts
+// `metadata_sm` - The in-core space for storing ref-counts of verified blocks
+pub fn check_disk_space_map(
+    engine: Arc<dyn IoEngine + Send + Sync>,
+    report: Arc<Report>,
+    root: SMRoot,
+    disk_sm: ASpaceMap,
+    metadata_sm: ASpaceMap,
+    ignore_non_fatal: bool,
+) -> Result<Vec<BitmapLeak>> {
+    let entries = gather_disk_index_entries(engine.clone(), root.bitmap_root, metadata_sm.clone(), ignore_non_fatal)?;
+
+    // check overflow ref-counts
+    {
+        let sm = disk_sm.lock().unwrap();
+        let v = OverflowChecker::new("data", &*sm);
+        let w = BTreeWalker::new_with_sm(engine.clone(), metadata_sm.clone(), false)?;
+        w.walk(&mut vec![0], &v, root.ref_count_root)?;
+    }
+
+    // check low ref-counts in bitmaps
+    check_low_ref_counts(engine, report, "data", entries, disk_sm)
+}
+
+// This checks the space map and returns any leak blocks for auto-repair to process.
+//
+// `metadata_sm`: The in-core space map of expected metadata block ref-counts
+pub fn check_metadata_space_map(
+    engine: Arc<dyn IoEngine + Send + Sync>,
+    report: Arc<Report>,
+    root: SMRoot,
+    metadata_sm: ASpaceMap,
+    ignore_non_fatal: bool,
+) -> Result<Vec<BitmapLeak>> {
+    count_btree_blocks::<u32>(engine.clone(), &mut vec![0], root.ref_count_root, metadata_sm.clone(), false)?;
+    let entries = gather_metadata_index_entries(engine.clone(), root.bitmap_root, metadata_sm.clone())?;
+
+    // check overflow ref-counts
+    {
+        let sm = metadata_sm.lock().unwrap();
+        let v = OverflowChecker::new("metadata", &*sm);
+        let w = BTreeWalker::new(engine.clone(), ignore_non_fatal);
+        w.walk(&mut vec![0], &v, root.ref_count_root)?;
+    }
+
+    // check low ref-counts in bitmaps
+    check_low_ref_counts(engine, report, "metadata", entries, metadata_sm)
+}
+
+// This assumes the only errors in the space map are leaks.  Entries should just be
+// those that contain leaks.
+pub fn repair_space_map(
+    engine: Arc<dyn IoEngine + Send + Sync>,
+    entries: Vec<BitmapLeak>,
+    sm: ASpaceMap
+) -> Result<()> {
+    let sm = sm.lock().unwrap();
+
+    let mut blocks = Vec::with_capacity(entries.len());
+    for i in &entries {
+        blocks.push(i.loc);
+    }
+
+    // FIXME: we should do this in batches
+    let rblocks = engine.read_many(&blocks[0..])?;
+    let mut write_blocks = Vec::new();
+
+    for (i, rb) in rblocks.into_iter().enumerate() {
+        if let Ok(b) = rb {
+            let be = &entries[i];
+            let mut blocknr = be.blocknr;
+            let mut bitmap = unpack::<Bitmap>(b.get_data())?;
+            for e in bitmap.entries.iter_mut() {
+                if blocknr >= sm.get_nr_blocks()? {
+                    break;
+                }
+
+                if let BitmapEntry::Small(actual) = e {
+                    let expected = sm.get(blocknr)?;
+                    if *actual == 1 && expected == 0 {
+                        *e = BitmapEntry::Small(0);
+                    }
+                }
+
+                blocknr += 1;
+            }
+
+            let mut out = Cursor::new(b.get_data());
+            bitmap.pack(&mut out)?;
+            checksum::write_checksum(b.get_data(), checksum::BT::BITMAP)?;
+
+            write_blocks.push(b);
+        } else {
+            return Err(anyhow!("Unable to reread bitmap blocks for repair"));
+        }
+    }
+
+    engine.write_many(&write_blocks[0..])?;
+    Ok(())
+}
+
+//------------------------------------------

src/thin/check.rs

@@ -1,17 +1,16 @@
 use anyhow::{anyhow, Result};
 use std::collections::BTreeMap;
-use std::io::Cursor;
 use std::path::Path;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, Mutex};
 use std::thread::{self, JoinHandle};
 use threadpool::ThreadPool;
 
-use crate::checksum;
 use crate::io_engine::{AsyncIoEngine, IoEngine, SyncIoEngine};
 use crate::pdata::btree::{self, *};
 use crate::pdata::btree_walker::*;
 use crate::pdata::space_map::*;
+use crate::pdata::space_map_checker::*;
 use crate::pdata::unpack::*;
 use crate::report::*;
 use crate::thin::block_time::*;
@@ -72,211 +71,6 @@ impl NodeVisitor<BlockTime> for BottomLevelVisitor {
 
 //------------------------------------------
 
-struct OverflowChecker<'a> {
-    data_sm: &'a dyn SpaceMap,
-}
-
-impl<'a> OverflowChecker<'a> {
-    fn new(data_sm: &'a dyn SpaceMap) -> OverflowChecker<'a> {
-        OverflowChecker { data_sm }
-    }
-}
-
-impl<'a> NodeVisitor<u32> for OverflowChecker<'a> {
-    fn visit(
-        &self,
-        _path: &[u64],
-        _kr: &KeyRange,
-        _h: &NodeHeader,
-        keys: &[u64],
-        values: &[u32],
-    ) -> btree::Result<()> {
-        for n in 0..keys.len() {
-            let k = keys[n];
-            let v = values[n];
-            let expected = self.data_sm.get(k).unwrap();
-            if expected != v {
-                return Err(value_err(format!("Bad reference count for data block {}.  Expected {}, but space map contains {}.",
-                                  k, expected, v)));
-            }
-        }
-
-        Ok(())
-    }
-
-    fn visit_again(&self, _path: &[u64], _b: u64) -> btree::Result<()> {
-        Ok(())
-    }
-
-    fn end_walk(&self) -> btree::Result<()> {
-        Ok(())
-    }
-}
-
-//------------------------------------------
-
-struct BitmapLeak {
-    blocknr: u64, // blocknr for the first entry in the bitmap
-    loc: u64,     // location of the bitmap
-}
-
-// This checks the space map and returns any leak blocks for auto-repair to process.
-fn check_space_map(
-    path: &mut Vec<u64>,
-    ctx: &Context,
-    kind: &str,
-    entries: Vec<IndexEntry>,
-    metadata_sm: Option<ASpaceMap>,
-    sm: ASpaceMap,
-    root: SMRoot,
-) -> Result<Vec<BitmapLeak>> {
-    let report = ctx.report.clone();
-    let engine = ctx.engine.clone();
-
-    let sm = sm.lock().unwrap();
-
-    // overflow btree
-    {
-        let v = OverflowChecker::new(&*sm);
-        let w;
-        if metadata_sm.is_none() {
-            w = BTreeWalker::new(engine.clone(), false);
-        } else {
-            w = BTreeWalker::new_with_sm(engine.clone(), metadata_sm.unwrap().clone(), false)?;
-        }
-        w.walk(path, &v, root.ref_count_root)?;
-    }
-
-    let mut blocks = Vec::with_capacity(entries.len());
-    for i in &entries {
-        blocks.push(i.blocknr);
-    }
-
-    // FIXME: we should do this in batches
-    let blocks = engine.read_many(&blocks)?;
-
-    let mut leaks = 0;
-    let mut blocknr = 0;
-    let mut bitmap_leaks = Vec::new();
-    for b in blocks.iter().take(entries.len()) {
-        match b {
-            Err(_e) => {
-                return Err(anyhow!("Unable to read bitmap block"));
-            }
-            Ok(b) => {
-                if checksum::metadata_block_type(&b.get_data()) != checksum::BT::BITMAP {
-                    report.fatal(&format!(
-                        "Index entry points to block ({}) that isn't a bitmap",
-                        b.loc
-                    ));
-                }
-
-                let bitmap = unpack::<Bitmap>(b.get_data())?;
-                let first_blocknr = blocknr;
-                let mut contains_leak = false;
-                for e in bitmap.entries.iter() {
-                    if blocknr >= root.nr_blocks {
-                        break;
-                    }
-
-                    match e {
-                        BitmapEntry::Small(actual) => {
-                            let expected = sm.get(blocknr)?;
-                            if *actual == 1 && expected == 0 {
-                                leaks += 1;
-                                contains_leak = true;
-                            } else if *actual != expected as u8 {
-                                report.fatal(&format!("Bad reference count for {} block {}.  Expected {}, but space map contains {}.",
-                                          kind, blocknr, expected, actual));
-                            }
-                        }
-                        BitmapEntry::Overflow => {
-                            let expected = sm.get(blocknr)?;
-                            if expected < 3 {
-                                report.fatal(&format!("Bad reference count for {} block {}.  Expected {}, but space map says it's >= 3.",
-                                                  kind, blocknr, expected));
-                            }
-                        }
-                    }
-                    blocknr += 1;
-                }
-                if contains_leak {
-                    bitmap_leaks.push(BitmapLeak {
-                        blocknr: first_blocknr,
-                        loc: b.loc,
-                    });
-                }
-            }
-        }
-    }
-
-    if leaks > 0 {
-        report.non_fatal(&format!("{} {} blocks have leaked.", leaks, kind));
-    }
-
-    Ok(bitmap_leaks)
-}
-
-// This assumes the only errors in the space map are leaks.  Entries should just be
-// those that contain leaks.
-fn repair_space_map(ctx: &Context, entries: Vec<BitmapLeak>, sm: ASpaceMap) -> Result<()> {
-    let engine = ctx.engine.clone();
-
-    let sm = sm.lock().unwrap();
-
-    let mut blocks = Vec::with_capacity(entries.len());
-    for i in &entries {
-        blocks.push(i.loc);
-    }
-
-    // FIXME: we should do this in batches
-    let rblocks = engine.read_many(&blocks[0..])?;
-    let mut write_blocks = Vec::new();
-
-    for (i, rb) in rblocks.into_iter().enumerate() {
-        if let Ok(b) = rb {
-            let be = &entries[i];
-            let mut blocknr = be.blocknr;
-            let mut bitmap = unpack::<Bitmap>(b.get_data())?;
-            for e in bitmap.entries.iter_mut() {
-                if blocknr >= sm.get_nr_blocks()? {
-                    break;
-                }
-
-                if let BitmapEntry::Small(actual) = e {
-                    let expected = sm.get(blocknr)?;
-                    if *actual == 1 && expected == 0 {
-                        *e = BitmapEntry::Small(0);
-                    }
-                }
-
-                blocknr += 1;
-            }
-
-            let mut out = Cursor::new(b.get_data());
-            bitmap.pack(&mut out)?;
-            checksum::write_checksum(b.get_data(), checksum::BT::BITMAP)?;
-
-            write_blocks.push(b);
-        } else {
-            return Err(anyhow!("Unable to reread bitmap blocks for repair"));
-        }
-    }
-
-    engine.write_many(&write_blocks[0..])?;
-    Ok(())
-}
-
-//------------------------------------------
-
-fn inc_entries(sm: &ASpaceMap, entries: &[IndexEntry]) -> Result<()> {
-    let mut sm = sm.lock().unwrap();
-    for ie in entries {
-        sm.inc(ie.blocknr, 1)?;
-    }
-    Ok(())
-}
-
 fn inc_superblock(sm: &ASpaceMap) -> Result<()> {
     let mut sm = sm.lock().unwrap();
     sm.inc(SUPERBLOCK_LOCATION, 1)?;
@@ -506,30 +300,22 @@ pub fn check(opts: ThinCheckOptions) -> Result<()> {
     check_mapping_bottom_level(&ctx, &metadata_sm, &data_sm, &roots)?;
     bail_out(&ctx, "mapping tree")?;
 
+    //-----------------------------------------
+
     report.set_sub_title("data space map");
     let root = unpack::<SMRoot>(&sb.data_sm_root[0..])?;
-
+    let data_leaks = check_disk_space_map(
-    let entries = btree_to_map_with_sm::<IndexEntry>(
-        &mut path,
         engine.clone(),
+        report.clone(),
+        root,
+        data_sm.clone(),
         metadata_sm.clone(),
         opts.ignore_non_fatal,
-        root.bitmap_root,
-    )?;
-    let entries: Vec<IndexEntry> = entries.values().cloned().collect();
-    inc_entries(&metadata_sm, &entries[0..])?;
-
-    let data_leaks = check_space_map(
-        &mut path,
-        &ctx,
-        "data",
-        entries,
-        Some(metadata_sm.clone()),
-        data_sm.clone(),
-        root,
     )?;
     bail_out(&ctx, "data space map")?;
 
+    //-----------------------------------------
+
     report.set_sub_title("metadata space map");
     let root = unpack::<SMRoot>(&sb.metadata_sm_root[0..])?;
     report.info(&format!(
@@ -537,49 +323,28 @@ pub fn check(opts: ThinCheckOptions) -> Result<()> {
         root.nr_blocks - root.nr_allocated
     ));
 
-    let b = engine.read(root.bitmap_root)?;
+    // Now the counts should be correct and we can check it.
-    metadata_sm.lock().unwrap().inc(root.bitmap_root, 1)?;
+    let metadata_leaks = check_metadata_space_map(
-    let entries = unpack::<MetadataIndex>(b.get_data())?.indexes;
-
-    // Unused entries will point to block 0
-    let entries: Vec<IndexEntry> = entries
-        .iter()
-        .take_while(|e| e.blocknr != 0)
-        .cloned()
-        .collect();
-    inc_entries(&metadata_sm, &entries[0..])?;
-
-    // We call this for the side effect of incrementing the ref counts
-    // for the metadata that holds the tree.
-    let _counts = btree_to_map_with_sm::<u32>(
-        &mut path,
         engine.clone(),
+        report.clone(),
+        root,
         metadata_sm.clone(),
         opts.ignore_non_fatal,
-        root.ref_count_root,
     )?;
 
-    // Now the counts should be correct and we can check it.
-    let metadata_leaks = check_space_map(
-        &mut path,
-        &ctx,
-        "metadata",
-        entries,
-        None,
-        metadata_sm.clone(),
-        root,
-    )?;
     bail_out(&ctx, "metadata space map")?;
 
+    //-----------------------------------------
+
     if opts.auto_repair {
         if !data_leaks.is_empty() {
             ctx.report.info("Repairing data leaks.");
-            repair_space_map(&ctx, data_leaks, data_sm.clone())?;
+            repair_space_map(ctx.engine.clone(), data_leaks, data_sm.clone())?;
         }
 
         if !metadata_leaks.is_empty() {
             ctx.report.info("Repairing metadata leaks.");
-            repair_space_map(&ctx, metadata_leaks, metadata_sm.clone())?;
+            repair_space_map(ctx.engine.clone(), metadata_leaks, metadata_sm.clone())?;
         }
     }