StaticHeader::setup Improve error reporting

Author: GuillaumeGomez

src/engine/strat_engine/backstore/metadata/bda.rs

@@ -152,7 +152,8 @@ impl BDA {
         F: Read + Seek + SyncAll,
     {
         let header = match StaticHeader::setup(f)? {
-            Some(header) => header,
+            Some(SetupResult::Ok(header)) => header,
+            Some(SetupResult::OkWithError(header, _err)) => header,
             None => return Ok(None),
         };
 
@@ -241,6 +242,22 @@ impl BDA {
     }
 }
 
+#[derive(Debug)]
+pub enum SetupResult {
+    Ok(StaticHeader),
+    OkWithError(StaticHeader, StratisError),
+}
+
+impl SetupResult {
+    fn new(header: StaticHeader) -> SetupResult {
+        SetupResult::Ok(header)
+    }
+
+    fn new_with_error(header: StaticHeader, error: StratisError) -> SetupResult {
+        SetupResult::OkWithError(header, error)
+    }
+}
+
 #[derive(Eq, PartialEq)]
 pub struct StaticHeader {
     blkdev_size: Sectors,
@@ -283,7 +300,7 @@ impl StaticHeader {
     /// Return an error if the sigblocks differ in some unaccountable way.
     /// Returns an error if a write intended to repair an ill-formed,
     /// unreadable, or stale signature block failed.
-    fn setup<F>(f: &mut F) -> StratisResult<Option<StaticHeader>>
+    fn setup<F>(f: &mut F) -> StratisResult<Option<SetupResult>>
     where
         F: Read + Seek + SyncAll,
     {
@@ -298,41 +315,44 @@ impl StaticHeader {
                         match (loc_1, loc_2) {
                             (Some(loc_1), Some(loc_2)) => {
                                 if loc_1 == loc_2 {
-                                    Ok(Some(loc_1))
+                                    Ok(Some(SetupResult::new(loc_1)))
                                 } else if loc_1.initialization_time == loc_2.initialization_time {
                                     // Inexplicable disagreement among static headers
                                     let err_str = "Appeared to be a Stratis device, but signature blocks disagree.";
-                                    Err(StratisError::Engine(ErrorEnum::Invalid, err_str.into()))
+                                    Ok(Some(
+                                        SetupResult::new_with_error(
+                                            loc_1,
+                                            StratisError::Engine(ErrorEnum::Invalid, err_str.into()))))
                                 } else if loc_1.initialization_time > loc_2.initialization_time {
                                     // If the first header block is newer, overwrite second with
                                     // contents of first.
                                     BDA::write(f, &buf_loc_1, MetadataLocation::Second)?;
-                                    Ok(Some(loc_1))
+                                    Ok(Some(SetupResult::new(loc_1)))
                                 } else {
                                     // The second header block must be newer, so overwrite first
                                     // with contents of second.
                                     BDA::write(f, &buf_loc_2, MetadataLocation::First)?;
-                                    Ok(Some(loc_2))
+                                    Ok(Some(SetupResult::new(loc_2)))
                                 }
                             }
                             (None, None) => Ok(None),
                             (Some(loc_1), None) => {
                                 // Copy 1 has valid Stratis BDA, copy 2 has no magic, re-write copy 2
                                 BDA::write(f, &buf_loc_1, MetadataLocation::Second)?;
-                                Ok(Some(loc_1))
+                                Ok(Some(SetupResult::new(loc_1)))
                             }
                             (None, Some(loc_2)) => {
                                 // Copy 2 has valid Stratis BDA, copy 1 has no magic, re-write copy 1
                                 BDA::write(f, &buf_loc_2, MetadataLocation::First)?;
-                                Ok(Some(loc_2))
+                                Ok(Some(SetupResult::new(loc_2)))
                             }
                         }
                     }
                     (Ok(loc_1), Err(loc_2)) => {
                         // Re-write copy 2
-                        if loc_1.is_some() {
+                        if let Some(loc_1) = loc_1 {
                             BDA::write(f, &buf_loc_1, MetadataLocation::Second)?;
-                            Ok(loc_1)
+                            Ok(Some(SetupResult::new(loc_1)))
                         } else {
                             // Location 1 doesn't have a signature, but location 2 did, but it got an error,
                             // lets return the error instead as this appears to be a stratis device that
@@ -342,9 +362,9 @@ impl StaticHeader {
                     }
                     (Err(loc_1), Ok(loc_2)) => {
                         // Re-write copy 1
-                        if loc_2.is_some() {
+                        if let Some(loc_2) = loc_2 {
                             BDA::write(f, &buf_loc_2, MetadataLocation::First)?;
-                            Ok(loc_2)
+                            Ok(Some(SetupResult::new(loc_2)))
                         } else {
                             // Location 2 doesn't have a signature, but location 1 did, but it got an error,
                             // lets return the error instead as this appears to be a stratis device that
@@ -363,10 +383,12 @@ impl StaticHeader {
                 // Copy 1 read OK, 2 resulted in an IO error
                 match StaticHeader::sigblock_from_buf(&buf_loc_1) {
                     Ok(loc_1) => {
-                        if loc_1.is_some() {
+                        if let Some(loc_1) = loc_1 {
                             BDA::write(f, &buf_loc_1, MetadataLocation::Second)?;
+                            Ok(Some(SetupResult::new(loc_1)))
+                        } else {
+                            Ok(None)
                         }
-                        Ok(loc_1)
                     }
                     Err(e) => {
                         // Unable to determine if location 2 has a signature, but location 1 did,
@@ -380,10 +402,12 @@ impl StaticHeader {
                 // Copy 2 read OK, 1 resulted in IO Error
                 match StaticHeader::sigblock_from_buf(&buf_loc_2) {
                     Ok(loc_2) => {
-                        if loc_2.is_some() {
+                        if let Some(loc_2) = loc_2 {
                             BDA::write(f, &buf_loc_2, MetadataLocation::First)?;
+                            Ok(Some(SetupResult::new(loc_2)))
+                        } else {
+                            Ok(None)
                         }
-                        Ok(loc_2)
                     }
                     Err(e) => {
                         // Unable to determine if location 1 has a signature, but location 2 did,
@@ -411,7 +435,8 @@ impl StaticHeader {
         // it must also have correct CRC, no weird stuff in fields,
         // etc!
         match StaticHeader::setup(f) {
-            Ok(Some(sh)) => Ok(Some((sh.pool_uuid, sh.dev_uuid))),
+            Ok(Some(SetupResult::Ok(sh))) => Ok(Some((sh.pool_uuid, sh.dev_uuid))),
+            Ok(Some(SetupResult::OkWithError(sh, _err))) => Ok(Some((sh.pool_uuid, sh.dev_uuid))),
             Ok(None) => Ok(None),
             Err(err) => Err(err),
         }