From d436f35ed3d0c707f0c401c84013f05ac4b9b0a4 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Fri, 10 Sep 2021 10:33:53 +0800 Subject: [PATCH] [thin_dump/thin_repair/thin_restore (rust)] Fix errors in tests - Move error messages to stderr - Fix the transaction_id and data_block_size in test fixture, and check the data_block_size in thin_restore. - Disable the missing_something tests for Rust. The transaction_id and nr_data_blocks now could be recovered automatically. - Limit the use of override options in test cases. Only broken superblocks could be overridden, and the provided values should be compatible to the original metadata. - Remove unused option --- src/bin/thin_dump.rs | 2 +- src/bin/thin_restore.rs | 7 ------- src/thin/restore.rs | 4 ++++ tests/common/thin_xml_generator.rs | 4 ++-- tests/thin_dump.rs | 31 ++++++++++++++---------------- tests/thin_repair.rs | 28 +++++++++++++-------------- tests/thin_restore.rs | 5 ++++- 7 files changed, 38 insertions(+), 43 deletions(-) diff --git a/src/bin/thin_dump.rs b/src/bin/thin_dump.rs index 45ab904..59ac543 100644 --- a/src/bin/thin_dump.rs +++ b/src/bin/thin_dump.rs @@ -138,7 +138,7 @@ fn main() { }; if let Err(reason) = dump(opts) { - println!("{}", reason); + eprintln!("{}", reason); process::exit(1); } } diff --git a/src/bin/thin_restore.rs b/src/bin/thin_restore.rs index f064594..125e148 100644 --- a/src/bin/thin_restore.rs +++ b/src/bin/thin_restore.rs @@ -43,13 +43,6 @@ fn main() { .long("output") .value_name("FILE") .required(true), - ) - .arg( - Arg::with_name("OVERRIDE_MAPPING_ROOT") - .help("Specify a mapping root to use") - .long("override-mapping-root") - .value_name("OVERRIDE_MAPPING_ROOT") - .takes_value(true), ); let matches = parser.get_matches(); diff --git a/src/thin/restore.rs b/src/thin/restore.rs index 38380fa..ecaa7a0 100644 --- a/src/thin/restore.rs +++ b/src/thin/restore.rs @@ -207,6 +207,10 @@ impl<'a> MetadataVisitor for Restorer<'a> { return Err(anyhow!("duplicated superblock")); } + if !(128..=2097152).contains(&sb.data_block_size) || (sb.data_block_size & 0x7F != 0) { + return Err(anyhow!("invalid data block size")); + } + self.sb = Some(sb.clone()); self.data_sm = Some(core_sm(sb.nr_data_blocks, u32::MAX)); let b = self.w.alloc()?; diff --git a/tests/common/thin_xml_generator.rs b/tests/common/thin_xml_generator.rs index 9d56856..a0cc66c 100644 --- a/tests/common/thin_xml_generator.rs +++ b/tests/common/thin_xml_generator.rs @@ -29,10 +29,10 @@ fn common_sb(nr_blocks: u64) -> ir::Superblock { ir::Superblock { uuid: "".to_string(), time: 0, - transaction: 0, + transaction: 1, flags: None, version: None, - data_block_size: 32, + data_block_size: 128, nr_data_blocks: nr_blocks, metadata_snap: None, } diff --git a/tests/thin_dump.rs b/tests/thin_dump.rs index c1bd3c4..206acfb 100644 --- a/tests/thin_dump.rs +++ b/tests/thin_dump.rs @@ -1,7 +1,6 @@ use anyhow::Result; use std::fs::OpenOptions; use std::io::Write; -use std::str::from_utf8; mod common; @@ -129,6 +128,7 @@ fn no_stderr() -> Result<()> { // test superblock overriding & repair // TODO: share with thin_repair +#[cfg(not(feature = "rust_tests"))] fn override_something(flag: &str, value: &str, pattern: &str) -> Result<()> { let mut td = TestDir::new()?; let md = mk_valid_md(&mut td)?; @@ -137,21 +137,24 @@ fn override_something(flag: &str, value: &str, pattern: &str) -> Result<()> { if !cfg!(feature = "rust_tests") { assert_eq!(output.stderr.len(), 0); } - assert!(from_utf8(&output.stdout[0..])?.contains(pattern)); + assert!(std::str::from_utf8(&output.stdout[0..])?.contains(pattern)); Ok(()) } #[test] +#[cfg(not(feature = "rust_tests"))] fn override_transaction_id() -> Result<()> { override_something("--transaction-id", "2345", "transaction=\"2345\"") } #[test] +#[cfg(not(feature = "rust_tests"))] fn override_data_block_size() -> Result<()> { override_something("--data-block-size", "8192", "data_block_size=\"8192\"") } #[test] +#[cfg(not(feature = "rust_tests"))] fn override_nr_data_blocks() -> Result<()> { override_something("--nr-data-blocks", "234500", "nr_data_blocks=\"234500\"") } @@ -161,24 +164,16 @@ fn override_nr_data_blocks() -> Result<()> { fn repair_superblock() -> Result<()> { let mut td = TestDir::new()?; let md = mk_valid_md(&mut td)?; - let before = run_ok_raw( - THIN_DUMP, - args![ - "--transaction-id=5", - "--data-block-size=128", - "--nr-data-blocks=4096000", - &md - ], - )?; + let before = run_ok_raw(THIN_DUMP, args![&md])?; damage_superblock(&md)?; let after = run_ok_raw( THIN_DUMP, args![ "--repair", - "--transaction-id=5", + "--transaction-id=1", "--data-block-size=128", - "--nr-data-blocks=4096000", + "--nr-data-blocks=20480", &md ], )?; @@ -195,6 +190,7 @@ fn repair_superblock() -> Result<()> { // TODO: share with thin_repair #[test] +#[cfg(not(feature = "rust_tests"))] fn missing_transaction_id() -> Result<()> { let mut td = TestDir::new()?; let md = mk_valid_md(&mut td)?; @@ -204,7 +200,7 @@ fn missing_transaction_id() -> Result<()> { args![ "--repair", "--data-block-size=128", - "--nr-data-blocks=4096000", + "--nr-data-blocks=20480", &md ], )?; @@ -221,8 +217,8 @@ fn missing_data_block_size() -> Result<()> { THIN_DUMP, args![ "--repair", - "--transaction-id=5", - "--nr-data-blocks=4096000", + "--transaction-id=1", + "--nr-data-blocks=20480", &md ], )?; @@ -231,6 +227,7 @@ fn missing_data_block_size() -> Result<()> { } #[test] +#[cfg(not(feature = "rust_tests"))] fn missing_nr_data_blocks() -> Result<()> { let mut td = TestDir::new()?; let md = mk_valid_md(&mut td)?; @@ -239,7 +236,7 @@ fn missing_nr_data_blocks() -> Result<()> { THIN_DUMP, args![ "--repair", - "--transaction-id=5", + "--transaction-id=1", "--data-block-size=128", &md ], diff --git a/tests/thin_repair.rs b/tests/thin_repair.rs index da54a7a..a5acf31 100644 --- a/tests/thin_repair.rs +++ b/tests/thin_repair.rs @@ -116,6 +116,7 @@ fn dont_repair_xml() -> Result<()> { // TODO: share with thin_dump +#[cfg(not(feature = "rust_tests"))] fn override_thing(flag: &str, val: &str, pattern: &str) -> Result<()> { let mut td = TestDir::new()?; let md1 = mk_valid_md(&mut td)?; @@ -128,16 +129,19 @@ fn override_thing(flag: &str, val: &str, pattern: &str) -> Result<()> { } #[test] +#[cfg(not(feature = "rust_tests"))] fn override_transaction_id() -> Result<()> { override_thing("--transaction-id", "2345", "transaction=\"2345\"") } #[test] +#[cfg(not(feature = "rust_tests"))] fn override_data_block_size() -> Result<()> { override_thing("--data-block-size", "8192", "data_block_size=\"8192\"") } #[test] +#[cfg(not(feature = "rust_tests"))] fn override_nr_data_blocks() -> Result<()> { override_thing("--nr-data-blocks", "234500", "nr_data_blocks=\"234500\"") } @@ -147,15 +151,7 @@ fn override_nr_data_blocks() -> Result<()> { fn superblock_succeeds() -> Result<()> { let mut td = TestDir::new()?; let md1 = mk_valid_md(&mut td)?; - let original = run_ok_raw( - THIN_DUMP, - args![ - "--transaction-id=5", - "--data-block-size=128", - "--nr-data-blocks=4096000", - &md1 - ], - )?; + let original = run_ok_raw(THIN_DUMP, args![&md1])?; if !cfg!(feature = "rust_tests") { assert_eq!(original.stderr.len(), 0); } @@ -164,9 +160,9 @@ fn superblock_succeeds() -> Result<()> { run_ok( THIN_REPAIR, args![ - "--transaction-id=5", + "--transaction-id=1", "--data-block-size=128", - "--nr-data-blocks=4096000", + "--nr-data-blocks=20480", "-i", &md1, "-o", @@ -196,10 +192,11 @@ fn missing_thing(flag1: &str, flag2: &str, pattern: &str) -> Result<()> { } #[test] +#[cfg(not(feature = "rust_tests"))] fn missing_transaction_id() -> Result<()> { missing_thing( "--data-block-size=128", - "--nr-data-blocks=4096000", + "--nr-data-blocks=20480", "transaction id", ) } @@ -207,16 +204,17 @@ fn missing_transaction_id() -> Result<()> { #[test] fn missing_data_block_size() -> Result<()> { missing_thing( - "--transaction-id=5", - "--nr-data-blocks=4096000", + "--transaction-id=1", + "--nr-data-blocks=20480", "data block size", ) } #[test] +#[cfg(not(feature = "rust_tests"))] fn missing_nr_data_blocks() -> Result<()> { missing_thing( - "--transaction-id=5", + "--transaction-id=1", "--data-block-size=128", "nr data blocks", ) diff --git a/tests/thin_restore.rs b/tests/thin_restore.rs index e1ec504..da2d90a 100644 --- a/tests/thin_restore.rs +++ b/tests/thin_restore.rs @@ -124,7 +124,7 @@ fn accepts_quiet() -> Result<()> { //----------------------------------------- // TODO: share with thin_dump - +#[cfg(not(feature = "rust_tests"))] fn override_something(flag: &str, value: &str, pattern: &str) -> Result<()> { let mut td = TestDir::new()?; let xml = mk_valid_xml(&mut td)?; @@ -138,16 +138,19 @@ fn override_something(flag: &str, value: &str, pattern: &str) -> Result<()> { } #[test] +#[cfg(not(feature = "rust_tests"))] fn override_transaction_id() -> Result<()> { override_something("--transaction-id", "2345", "transaction=\"2345\"") } #[test] +#[cfg(not(feature = "rust_tests"))] fn override_data_block_size() -> Result<()> { override_something("--data-block-size", "8192", "data_block_size=\"8192\"") } #[test] +#[cfg(not(feature = "rust_tests"))] fn override_nr_data_blocks() -> Result<()> { override_something("--nr-data-blocks", "234500", "nr_data_blocks=\"234500\"") }