From 3641a9009e7f0b638e08fbf23fc77af78c9bf759 Mon Sep 17 00:00:00 2001 From: mulhern Date: Thu, 8 Aug 2019 08:44:51 -0400 Subject: [PATCH 1/4] Tidy up a test about static header rewriting Remove the dependence on BDA and improve the name and comments. Improve assertions so that they will be more helpful on failure. Signed-off-by: mulhern --- .../strat_engine/backstore/metadata/bda.rs | 41 ++++++++----------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/src/engine/strat_engine/backstore/metadata/bda.rs b/src/engine/strat_engine/backstore/metadata/bda.rs index e3f2053851..8287c06c1a 100644 --- a/src/engine/strat_engine/backstore/metadata/bda.rs +++ b/src/engine/strat_engine/backstore/metadata/bda.rs @@ -730,29 +730,22 @@ mod tests { /// the two static headers is corrupted. Verify expected behavior /// if both are corrupted, which varies depending on whether the /// Stratis magic number or some other part of the header is corrupted. - fn bda_test_recovery(primary in option::of(0..bytes!(static_header_size::SIGBLOCK_SECTORS)), + fn test_corrupted_sigblock_recovery(primary in option::of(0..bytes!(static_header_size::SIGBLOCK_SECTORS)), secondary in option::of(0..bytes!(static_header_size::SIGBLOCK_SECTORS))) { let sh = random_static_header(10000, 4); - let buf_size = *sh.mda_size.sectors().bytes() as usize + bytes!(static_header_size::STATIC_HEADER_SECTORS); - let mut buf = Cursor::new(vec![0; buf_size]); - BDA::initialize( - &mut buf, - sh.pool_uuid, - sh.dev_uuid, - sh.mda_size.region_size().data_size(), - sh.blkdev_size, - Utc::now().timestamp() as u64, - ).unwrap(); + let buf_size = bytes!(static_header_size::STATIC_HEADER_SECTORS); - let reference_buf = buf.clone(); + let mut reference_buf = Cursor::new(vec![0; buf_size]); + sh.write(&mut reference_buf, MetadataLocation::Both).unwrap(); + + let mut buf = Cursor::new(vec![0; buf_size]); + sh.write(&mut buf, MetadataLocation::Both).unwrap(); if let Some(index) = primary { - // Corrupt primary copy corrupt_byte(&mut buf, (bytes!(static_header_size::FIRST_SIGBLOCK_START_SECTORS) + index) as u64).unwrap(); } if let Some(index) = secondary { - // Corrupt secondary copy corrupt_byte(&mut buf, (bytes!(static_header_size::SECOND_SIGBLOCK_START_SECTORS) + index) as u64).unwrap(); } @@ -760,27 +753,27 @@ mod tests { match (primary, secondary) { (Some(p_index), Some(s_index)) => { - // Setup should fail to find a usable Stratis BDA match (p_index, s_index) { + // Both magic locations are corrupted, conclusion is + // that this is not a Stratis static header. (4..=19, 4..=19) => { - // When we corrupt both magics then we believe that - // the signature is not ours and will return Ok(None) - prop_assert!(setup_result.is_ok() && setup_result.unwrap().is_none()); + prop_assert!(setup_result.is_ok()); + prop_assert_eq!(setup_result.unwrap(), None); } + // Both sigblocks were corrupted, but at least one + // was recognized as a Stratis sigblock. _ => { prop_assert!(setup_result.is_err()); } } - - // Check buffer, should be different + // No healing was attempted. prop_assert_ne!(reference_buf.get_ref(), buf.get_ref()); } + // Only one header was corrupted, so the other was healed. _ => { - // Setup should work and buffer should be corrected - prop_assert!(setup_result.is_ok() && setup_result.unwrap().is_some()); - - // Check buffer, should be corrected. + prop_assert!(setup_result.is_ok()); + prop_assert_eq!(setup_result.unwrap(), Some(sh)); prop_assert_eq!(reference_buf.get_ref(), buf.get_ref()); } } From 08ec3315fef81eced1d8795862986c806304c525 Mon Sep 17 00:00:00 2001 From: mulhern Date: Thu, 8 Aug 2019 08:47:34 -0400 Subject: [PATCH 2/4] Move corrupt_byte to the one place where it is used Signed-off-by: mulhern --- .../strat_engine/backstore/metadata/bda.rs | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/engine/strat_engine/backstore/metadata/bda.rs b/src/engine/strat_engine/backstore/metadata/bda.rs index 8287c06c1a..3d0b956f03 100644 --- a/src/engine/strat_engine/backstore/metadata/bda.rs +++ b/src/engine/strat_engine/backstore/metadata/bda.rs @@ -530,21 +530,6 @@ mod tests { use super::*; - /// Corrupt a byte at the specified position. - fn corrupt_byte(f: &mut F, position: u64) -> io::Result<()> - where - F: Read + Seek + SyncAll, - { - let mut byte_to_corrupt = [0; 1]; - f.seek(SeekFrom::Start(position))?; - f.read_exact(&mut byte_to_corrupt)?; - byte_to_corrupt[0] = !byte_to_corrupt[0]; - f.seek(SeekFrom::Start(position))?; - f.write_all(&byte_to_corrupt)?; - f.sync_all()?; - Ok(()) - } - /// Return a static header with random block device and MDA size. /// The block device is less than the minimum, for efficiency in testing. fn random_static_header(blkdev_size: u64, mda_size_factor: u32) -> StaticHeader { @@ -732,6 +717,21 @@ mod tests { /// Stratis magic number or some other part of the header is corrupted. fn test_corrupted_sigblock_recovery(primary in option::of(0..bytes!(static_header_size::SIGBLOCK_SECTORS)), secondary in option::of(0..bytes!(static_header_size::SIGBLOCK_SECTORS))) { + // Corrupt a byte at the specified position. + fn corrupt_byte(f: &mut F, position: u64) -> io::Result<()> + where + F: Read + Seek + SyncAll, + { + let mut byte_to_corrupt = [0; 1]; + f.seek(SeekFrom::Start(position))?; + f.read_exact(&mut byte_to_corrupt)?; + byte_to_corrupt[0] = !byte_to_corrupt[0]; + f.seek(SeekFrom::Start(position))?; + f.write_all(&byte_to_corrupt)?; + f.sync_all()?; + Ok(()) + } + let sh = random_static_header(10000, 4); let buf_size = bytes!(static_header_size::STATIC_HEADER_SECTORS); From aab869d62aa1627793c770e0ce9752f8d2e7090f Mon Sep 17 00:00:00 2001 From: mulhern Date: Thu, 8 Aug 2019 08:56:31 -0400 Subject: [PATCH 3/4] Tidy up corrupt_byte just a bit Signed-off-by: mulhern --- src/engine/strat_engine/backstore/metadata/bda.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/engine/strat_engine/backstore/metadata/bda.rs b/src/engine/strat_engine/backstore/metadata/bda.rs index 3d0b956f03..346a300685 100644 --- a/src/engine/strat_engine/backstore/metadata/bda.rs +++ b/src/engine/strat_engine/backstore/metadata/bda.rs @@ -717,19 +717,21 @@ mod tests { /// Stratis magic number or some other part of the header is corrupted. fn test_corrupted_sigblock_recovery(primary in option::of(0..bytes!(static_header_size::SIGBLOCK_SECTORS)), secondary in option::of(0..bytes!(static_header_size::SIGBLOCK_SECTORS))) { + // Corrupt a byte at the specified position. fn corrupt_byte(f: &mut F, position: u64) -> io::Result<()> where F: Read + Seek + SyncAll, { let mut byte_to_corrupt = [0; 1]; - f.seek(SeekFrom::Start(position))?; - f.read_exact(&mut byte_to_corrupt)?; + f.seek(SeekFrom::Start(position)) + .and_then(|_| f.read_exact(&mut byte_to_corrupt))?; + byte_to_corrupt[0] = !byte_to_corrupt[0]; - f.seek(SeekFrom::Start(position))?; - f.write_all(&byte_to_corrupt)?; - f.sync_all()?; - Ok(()) + + f.seek(SeekFrom::Start(position)) + .and_then(|_| f.write_all(&byte_to_corrupt)) + .and_then(|_| f.sync_all()) } let sh = random_static_header(10000, 4); From 77748a52ac572ddb1c9d5132db1501bd8ab26284 Mon Sep 17 00:00:00 2001 From: mulhern Date: Thu, 8 Aug 2019 09:07:11 -0400 Subject: [PATCH 4/4] Improve some indentation rustfmt is still working on formatting within macros, and through various edits these two lines became way too long and were not automatically reformatted. Signed-off-by: mulhern --- src/engine/strat_engine/backstore/metadata/bda.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/engine/strat_engine/backstore/metadata/bda.rs b/src/engine/strat_engine/backstore/metadata/bda.rs index 346a300685..074c68b2f0 100644 --- a/src/engine/strat_engine/backstore/metadata/bda.rs +++ b/src/engine/strat_engine/backstore/metadata/bda.rs @@ -744,11 +744,19 @@ mod tests { sh.write(&mut buf, MetadataLocation::Both).unwrap(); if let Some(index) = primary { - corrupt_byte(&mut buf, (bytes!(static_header_size::FIRST_SIGBLOCK_START_SECTORS) + index) as u64).unwrap(); + corrupt_byte( + &mut buf, + (bytes!(static_header_size::FIRST_SIGBLOCK_START_SECTORS) + index) as u64, + ) + .unwrap(); } if let Some(index) = secondary { - corrupt_byte(&mut buf, (bytes!(static_header_size::SECOND_SIGBLOCK_START_SECTORS) + index) as u64).unwrap(); + corrupt_byte( + &mut buf, + (bytes!(static_header_size::SECOND_SIGBLOCK_START_SECTORS) + index) as u64, + ) + .unwrap(); } let setup_result = StaticHeader::setup(&mut buf);