From a18fd60f3f27fbfbd868a716356edcf8d8af23ad Mon Sep 17 00:00:00 2001
From: Ming-Hung Tsai <mtsai@redhat.com>
Date: Thu, 9 Sep 2021 16:46:39 +0800
Subject: [PATCH] [thin_check (rust)] Fix auto-repair related errors

- Returns error on metadata leaks
- Clear needs_check flag on success
- Check auto-repair write errors
- Fix file open flags, and correct spelling
---
 src/bin/thin_check.rs          |  7 +++++--
 src/io_engine.rs               | 12 ++++++------
 src/pdata/space_map_checker.rs |  7 ++++++-
 src/thin/check.rs              | 34 ++++++++++++++++++++++++++++++++++
 4 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/src/bin/thin_check.rs b/src/bin/thin_check.rs
index cf74f4a..371dd21 100644
--- a/src/bin/thin_check.rs
+++ b/src/bin/thin_check.rs
@@ -109,16 +109,18 @@ fn main() {
     }
 
     let engine: Arc<dyn IoEngine + Send + Sync>;
+    let writable = matches.is_present("AUTO_REPAIR") || matches.is_present("CLEAR_NEEDS_CHECK");
 
     if matches.is_present("ASYNC_IO") {
         engine = Arc::new(
-            AsyncIoEngine::new(&input_file, MAX_CONCURRENT_IO, false)
+            AsyncIoEngine::new(&input_file, MAX_CONCURRENT_IO, writable)
                 .expect("unable to open input file"),
         );
     } else {
         let nr_threads = std::cmp::max(8, num_cpus::get() * 2);
         engine = Arc::new(
-            SyncIoEngine::new(&input_file, nr_threads, false).expect("unable to open input file"),
+            SyncIoEngine::new(&input_file, nr_threads, writable)
+                .expect("unable to open input file"),
         );
     }
 
@@ -128,6 +130,7 @@ fn main() {
         skip_mappings: matches.is_present("SKIP_MAPPINGS"),
         ignore_non_fatal: matches.is_present("IGNORE_NON_FATAL"),
         auto_repair: matches.is_present("AUTO_REPAIR"),
+        clear_needs_check: matches.is_present("CLEAR_NEEDS_CHECK"),
         report,
     };
 
diff --git a/src/io_engine.rs b/src/io_engine.rs
index 2bd7687..37f0d49 100644
--- a/src/io_engine.rs
+++ b/src/io_engine.rs
@@ -131,16 +131,16 @@ impl<'a> Drop for FileGuard<'a> {
 }
 
 impl SyncIoEngine {
-    fn open_file(path: &Path, writeable: bool) -> Result<File> {
-        let file = OpenOptions::new().read(true).write(writeable).open(path)?;
+    fn open_file(path: &Path, writable: bool) -> Result<File> {
+        let file = OpenOptions::new().read(true).write(writable).open(path)?;
 
         Ok(file)
     }
 
-    pub fn new(path: &Path, nr_files: usize, writeable: bool) -> Result<SyncIoEngine> {
+    pub fn new(path: &Path, nr_files: usize, writable: bool) -> Result<SyncIoEngine> {
         let mut files = Vec::with_capacity(nr_files);
         for _n in 0..nr_files {
-            files.push(SyncIoEngine::open_file(path, writeable)?);
+            files.push(SyncIoEngine::open_file(path, writable)?);
         }
 
         Ok(SyncIoEngine {
@@ -231,10 +231,10 @@ pub struct AsyncIoEngine {
 }
 
 impl AsyncIoEngine {
-    pub fn new(path: &Path, queue_len: u32, writeable: bool) -> Result<AsyncIoEngine> {
+    pub fn new(path: &Path, queue_len: u32, writable: bool) -> Result<AsyncIoEngine> {
         let input = OpenOptions::new()
             .read(true)
-            .write(writeable)
+            .write(writable)
             .custom_flags(libc::O_DIRECT)
             .open(path)?;
 
diff --git a/src/pdata/space_map_checker.rs b/src/pdata/space_map_checker.rs
index 3e01561..1ae7f6b 100644
--- a/src/pdata/space_map_checker.rs
+++ b/src/pdata/space_map_checker.rs
@@ -309,7 +309,12 @@ pub fn repair_space_map(
         }
     }
 
-    engine.write_many(&write_blocks[0..])?;
+    let results = engine.write_many(&write_blocks[0..])?;
+    for ret in results {
+        if ret.is_err() {
+            return Err(anyhow!("Unable to repair space map: {:?}", ret));
+        }
+    }
     Ok(())
 }
 
diff --git a/src/thin/check.rs b/src/thin/check.rs
index 1c5836e..7fdf9d4 100644
--- a/src/thin/check.rs
+++ b/src/thin/check.rs
@@ -87,6 +87,7 @@ pub struct ThinCheckOptions {
     pub skip_mappings: bool,
     pub ignore_non_fatal: bool,
     pub auto_repair: bool,
+    pub clear_needs_check: bool,
     pub report: Arc<Report>,
 }
 
@@ -276,6 +277,10 @@ pub fn check(opts: ThinCheckOptions) -> Result<()> {
     )?;
 
     if opts.skip_mappings {
+        let cleared = clear_needs_check_flag(ctx.engine.clone())?;
+        if cleared {
+            ctx.report.info("Cleared needs_check flag");
+        }
         return Ok(());
     }
 
@@ -331,6 +336,26 @@ pub fn check(opts: ThinCheckOptions) -> Result<()> {
             ctx.report.info("Repairing metadata leaks.");
             repair_space_map(ctx.engine.clone(), metadata_leaks, metadata_sm.clone())?;
         }
+
+        let cleared = clear_needs_check_flag(ctx.engine.clone())?;
+        if cleared {
+            ctx.report.info("Cleared needs_check flag");
+        }
+    } else if !opts.ignore_non_fatal {
+        if !data_leaks.is_empty() {
+            return Err(anyhow!("data space map contains leaks"));
+        }
+
+        if !metadata_leaks.is_empty() {
+            return Err(anyhow!("metadata space map contains leaks"));
+        }
+
+        if opts.clear_needs_check {
+            let cleared = clear_needs_check_flag(ctx.engine.clone())?;
+            if cleared {
+                ctx.report.info("Cleared needs_check flag");
+            }
+        }
     }
 
     stop_progress.store(true, Ordering::Relaxed);
@@ -339,6 +364,15 @@ pub fn check(opts: ThinCheckOptions) -> Result<()> {
     Ok(())
 }
 
+pub fn clear_needs_check_flag(engine: Arc<dyn IoEngine + Send + Sync>) -> Result<bool> {
+    let mut sb = read_superblock(engine.as_ref(), SUPERBLOCK_LOCATION)?;
+    if !sb.flags.needs_check {
+        return Ok(false);
+    }
+    sb.flags.needs_check = false;
+    write_superblock(engine.as_ref(), SUPERBLOCK_LOCATION, &sb).map(|_| true)
+}
+
 //------------------------------------------
 
 // Some callers wish to know which blocks are allocated.