StaticHeader::setup Improve error reporting

Author: GuillaumeGomez

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

@@ -178,7 +178,11 @@ 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)) => {
+                setup_warn(&header, err);
+                header
+            }
             None => return Ok(None),
         };
 
@@ -272,10 +276,49 @@ impl BDA {
     where
         F: Read + Seek + SyncAll,
     {
-        StaticHeader::setup(f).map(|sh| sh.map(|sh| (sh.pool_uuid, sh.dev_uuid)))
+        match StaticHeader::setup(f) {
+            Ok(Some(SetupResult::Ok(sh))) => Ok(Some((sh.pool_uuid, sh.dev_uuid))),
+            Ok(Some(SetupResult::OkWithError(sh, err))) => {
+                setup_warn(&sh, err);
+                Ok(Some((sh.pool_uuid, sh.dev_uuid)))
+            }
+            Ok(None) => Ok(None),
+            Err(err) => Err(err),
+        }
     }
 }
 
+fn setup_warn(header: &StaticHeader, err: StratisError) {
+    warn!(
+        "Experienced an I/O error while attempting to repair an ill-formed, \
+         unreadable, or stale signature block: {:?}.\n\
+         Returning device \"{:?}\" from pool \"{:?}\".",
+        err, header.dev_uuid, header.pool_uuid
+    );
+}
+
+fn bda_write_check<F>(
+    f: &mut F,
+    bda_buf: &[u8],
+    which: MetadataLocation,
+    header: StaticHeader,
+) -> StratisResult<Option<SetupResult>>
+where
+    F: Read + Seek + SyncAll,
+{
+    Ok(match BDA::write(f, &bda_buf, which) {
+        Ok(_) => Some(SetupResult::Ok(header)),
+        Err(err) => Some(SetupResult::OkWithError(header, StratisError::Io(err))),
+    })
+}
+
+#[derive(Debug)]
+#[must_use]
+pub enum SetupResult {
+    Ok(StaticHeader),
+    OkWithError(StaticHeader, StratisError),
+}
+
 #[derive(Eq, PartialEq)]
 pub struct StaticHeader {
     blkdev_size: Sectors,
@@ -312,17 +355,23 @@ impl StaticHeader {
     }
 
     /// Try to find a valid StaticHeader on a device.
+    ///
     /// Return the latest copy that validates as a Stratis BDA, however verify both
     /// copies and if one validates but one does not, re-write the one that is incorrect.  If both
     /// copies are valid, but one is newer than the other, rewrite the older one to match.
+    ///
     /// Return None if it's not a Stratis device.
+    ///
     /// Return an error if the metadata seems to indicate that the device is
     /// a Stratis device, but no well-formed signature block could be read.
+    ///
     /// Return an error if neither sigblock location can be read.
+    ///
     /// 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>>
+    ///
+    /// Return the latest copy alongside the associated error if a write intended to repair
+    /// an ill-formed, unreadable, or stale signature failed.
+    fn setup<F>(f: &mut F) -> StratisResult<Option<SetupResult>>
     where
         F: Read + Seek + SyncAll,
     {
@@ -332,43 +381,40 @@ impl StaticHeader {
                 StaticHeader::sigblock_from_buf(&buf_loc_1),
                 StaticHeader::sigblock_from_buf(&buf_loc_2),
             ) {
-                (Ok(loc_1), Ok(loc_2)) => match (loc_1, loc_2) {
-                    (Some(loc_1), Some(loc_2)) => {
-                        if loc_1 == loc_2 {
-                            Ok(Some(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()))
-                        } 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))
-                        } 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(loc_1), Ok(loc_2)) => {
+                    match (loc_1, loc_2) {
+                        (Some(loc_1), Some(loc_2)) => {
+                            if loc_1 == loc_2 {
+                                Ok(Some(SetupResult::Ok(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()))
+                            } 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_check(f, &buf_loc_1, MetadataLocation::Second, loc_1)
+                            } else {
+                                // The second header block must be newer, so overwrite first
+                                // with contents of second.
+                                bda_write_check(f, &buf_loc_2, MetadataLocation::First, 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_check(f, &buf_loc_1, MetadataLocation::Second, loc_1)
+                        }
+                        (None, Some(loc_2)) => {
+                            // Copy 2 has valid Stratis BDA, copy 1 has no magic, re-write copy 1
+                            bda_write_check(f, &buf_loc_2, MetadataLocation::First, 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))
-                    }
-                    (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(loc_1), Err(loc_2)) => {
-                    if loc_1.is_some() {
-                        BDA::write(f, &buf_loc_1, MetadataLocation::Second)?;
-                        Ok(loc_1)
+                    // Re-write copy 2
+                    if let Some(loc_1) = loc_1 {
+                        bda_write_check(f, &buf_loc_1, MetadataLocation::Second, 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
@@ -377,9 +423,9 @@ impl StaticHeader {
                     }
                 }
                 (Err(loc_1), Ok(loc_2)) => {
-                    if loc_2.is_some() {
-                        BDA::write(f, &buf_loc_2, MetadataLocation::First)?;
-                        Ok(loc_2)
+                    // Re-write copy 1
+                    if let Some(loc_2) = loc_2 {
+                        bda_write_check(f, &buf_loc_2, MetadataLocation::First, 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
@@ -395,10 +441,11 @@ impl StaticHeader {
             // Copy 1 read OK, 2 resulted in an IO error
             (Ok(buf_loc_1), Err(_)) => match StaticHeader::sigblock_from_buf(&buf_loc_1) {
                 Ok(loc_1) => {
-                    if loc_1.is_some() {
-                        BDA::write(f, &buf_loc_1, MetadataLocation::Second)?;
+                    if let Some(loc_1) = loc_1 {
+                        bda_write_check(f, &buf_loc_1, MetadataLocation::Second, loc_1)
+                    } else {
+                        Ok(None)
                     }
-                    Ok(loc_1)
                 }
                 Err(e) => {
                     // Unable to determine if location 2 has a signature, but location 1 did,
@@ -410,10 +457,11 @@ impl StaticHeader {
             // Copy 2 read OK, 1 resulted in IO Error
             (Err(_), Ok(buf_loc_2)) => match StaticHeader::sigblock_from_buf(&buf_loc_2) {
                 Ok(loc_2) => {
-                    if loc_2.is_some() {
-                        BDA::write(f, &buf_loc_2, MetadataLocation::First)?;
+                    if let Some(loc_2) = loc_2 {
+                        bda_write_check(f, &buf_loc_2, MetadataLocation::First, loc_2)
+                    } else {
+                        Ok(None)
                     }
-                    Ok(loc_2)
                 }
                 Err(e) => {
                     // Unable to determine if location 1 has a signature, but location 2 did,