From ed4a2485b435d327b3144acc8b8ed229b6dd436b Mon Sep 17 00:00:00 2001 From: eabdullin Date: Tue, 30 Apr 2024 15:41:47 +0000 Subject: [PATCH] import UBI device-mapper-persistent-data-1.0.9-3.el9_4 --- .device-mapper-persistent-data.metadata | 4 +- .gitignore | 4 +- ...o.toml-to-work-with-vendor-directory.patch | 22 +- ...he-ioctl-request-code-for-the-powerp.patch | 217 --- ...correct-index_entry.nr_free-while-ex.patch | 181 +++ ...s-Verify-ioctl-request-code-in-tests.patch | 120 -- ...child-keys-checking-on-the-node-with.patch | 69 + ...non-zero-values-in-unused-index-bloc.patch | 269 ++++ ...boundary-check-on-the-bitset-for-cac.patch | 111 ++ ...-Print-suggestive-hints-for-improvin.patch | 62 + ...-print-error-messages-on-BrokenPipe-.patch | 225 +++ ...ck-Allow-long-format-for-input-and-o.patch | 34 + ...sion-string-compatibility-issue-with.patch | 1292 +++++++++++++++++ ...-print-error-messages-on-BrokenPipe-.patch | 109 ++ SPECS/device-mapper-persistent-data.spec | 40 +- 15 files changed, 2393 insertions(+), 366 deletions(-) delete mode 100644 SOURCES/0002-file_utils-Fix-the-ioctl-request-code-for-the-powerp.patch create mode 100644 SOURCES/0002-space-map-Fix-incorrect-index_entry.nr_free-while-ex.patch delete mode 100644 SOURCES/0003-file_utils-Verify-ioctl-request-code-in-tests.patch create mode 100644 SOURCES/0003-thin_repair-Fix-child-keys-checking-on-the-node-with.patch create mode 100644 SOURCES/0004-space_map-Allow-non-zero-values-in-unused-index-bloc.patch create mode 100644 SOURCES/0005-cache_check-Fix-boundary-check-on-the-bitset-for-cac.patch create mode 100644 SOURCES/0006-thin-cache_check-Print-suggestive-hints-for-improvin.patch create mode 100644 SOURCES/0007-thin_dump-Do-not-print-error-messages-on-BrokenPipe-.patch create mode 100644 SOURCES/0008-thin_metadata_pack-Allow-long-format-for-input-and-o.patch create mode 100644 SOURCES/0009-commands-Fix-version-string-compatibility-issue-with.patch create mode 100644 SOURCES/0010-thin_dump-Do-not-print-error-messages-on-BrokenPipe-.patch diff --git a/.device-mapper-persistent-data.metadata b/.device-mapper-persistent-data.metadata index 5717445..c6c4b43 100644 --- a/.device-mapper-persistent-data.metadata +++ b/.device-mapper-persistent-data.metadata @@ -1,2 +1,2 @@ -c526655cc4ed18b4f6fdcc4687a7a653d310c018 SOURCES/dmpd106-vendor.tar.gz -0667b7ee41ea165dc2ea0322b4af67f2b0c374a7 SOURCES/v1.0.6.tar.gz +d336474a4772430fa2b8a5f76a6c2f2a78caa698 SOURCES/dmpd109-vendor.tar.gz +8862f41bf5fad011879b6d819ab5e5b11afcda7e SOURCES/v1.0.9.tar.gz diff --git a/.gitignore b/.gitignore index 09595e6..f94aaaa 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,2 @@ -SOURCES/dmpd106-vendor.tar.gz -SOURCES/v1.0.6.tar.gz +SOURCES/dmpd109-vendor.tar.gz +SOURCES/v1.0.9.tar.gz diff --git a/SOURCES/0001-Tweak-cargo.toml-to-work-with-vendor-directory.patch b/SOURCES/0001-Tweak-cargo.toml-to-work-with-vendor-directory.patch index 2e51e47..1bdbce3 100644 --- a/SOURCES/0001-Tweak-cargo.toml-to-work-with-vendor-directory.patch +++ b/SOURCES/0001-Tweak-cargo.toml-to-work-with-vendor-directory.patch @@ -1,4 +1,4 @@ -From 732ff5861a1525944a927439d1c075ac269788ce Mon Sep 17 00:00:00 2001 +From 0d5347bd771e960294cd0c2f083d96448613ab9c Mon Sep 17 00:00:00 2001 From: Marian Csontos Date: Thu, 27 Jul 2023 11:37:01 +0200 Subject: [PATCH] Tweak cargo.toml to work with vendor directory @@ -7,24 +7,16 @@ Mock works offline, cargo would try to download the files from github. So cargo vendor has to be run first, and then change the Cargo.toml to make mock happy. --- - Cargo.toml | 4 +++- - 1 file changed, 3 insertions(+), 1 deletion(-) + Cargo.toml | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml -index c2b496ac..562c40d3 100644 +index 500345a4..d4aa38a6 100644 --- a/Cargo.toml +++ b/Cargo.toml -@@ -11,6 +11,7 @@ anyhow = "1.0" - base64 = "0.21" - byteorder = "1.4" - clap = { version = "4.3", default-features = false, features = ["std", "help", "usage", "error-context", "suggestions"] } -+#crc32c = { git = "https://github.com/zowens/crc32c", branch = "master" } - crc32c = "0.6" - data-encoding = "2.4" - exitcode = "1.1.2" -@@ -27,7 +28,8 @@ quick-xml = "0.29" +@@ -27,7 +27,8 @@ quick-xml = "0.29" rand = "0.8" - rangemap = "1.3" + rangemap = "1.4" roaring = "0.10" -rio = { git = "https://github.com/jthornber/rio", branch = "master", optional = true } +#rio = { git = "https://github.com/jthornber/rio", branch = "master", optional = true } @@ -33,5 +25,5 @@ index c2b496ac..562c40d3 100644 threadpool = "1.8" thiserror = "1.0" -- -2.41.0 +2.43.0 diff --git a/SOURCES/0002-file_utils-Fix-the-ioctl-request-code-for-the-powerp.patch b/SOURCES/0002-file_utils-Fix-the-ioctl-request-code-for-the-powerp.patch deleted file mode 100644 index e2a6f0e..0000000 --- a/SOURCES/0002-file_utils-Fix-the-ioctl-request-code-for-the-powerp.patch +++ /dev/null @@ -1,217 +0,0 @@ -From 6de336c0e974989010c32693c9540180d4f28f0b Mon Sep 17 00:00:00 2001 -From: Ming-Hung Tsai -Date: Sat, 26 Aug 2023 18:19:15 +0800 -Subject: [PATCH 2/3] [file_utils] Fix the ioctl request code for the powerpc - architecture - -The PowerPC architecture employees a different bitwise layout in the -request code than other platforms, resulting in different request code -values. - -(cherry picked from commit afcbcd7d85902fc7f1e51fc5302230851303ab85) ---- - src/file_utils.rs | 14 ++---- - src/ioctl.rs | 106 ++++++++++++++++++++++++++++++++++++++++++++++ - src/lib.rs | 1 + - src/thin/trim.rs | 11 ++--- - 4 files changed, 115 insertions(+), 17 deletions(-) - create mode 100644 src/ioctl.rs - -diff --git a/src/file_utils.rs b/src/file_utils.rs -index 81d4a8a7..97400272 100644 ---- a/src/file_utils.rs -+++ b/src/file_utils.rs -@@ -5,6 +5,8 @@ use std::os::unix::ffi::OsStrExt; - use std::os::unix::io::AsRawFd; - use std::path::Path; - -+use crate::ioctl::{self, *}; -+ - //--------------------------------------- - - fn test_bit(mode: u32, flag: u32) -> bool { -@@ -41,10 +43,7 @@ pub fn is_file(path: &Path) -> io::Result { - - //--------------------------------------- - --#[cfg(target_pointer_width = "32")] --const BLKGETSIZE64: u32 = 0x80041272; --#[cfg(target_pointer_width = "64")] --const BLKGETSIZE64: u32 = 0x80081272; -+const BLKGETSIZE64: ioctl::RequestType = crate::request_code_read!(0x12, 114, usize); - - pub fn fail(msg: &str) -> io::Result { - let e = io::Error::new(io::ErrorKind::Other, msg); -@@ -56,13 +55,8 @@ fn get_device_size>(path: P) -> io::Result { - let fd = file.as_raw_fd(); - let mut cap = 0u64; - -- #[cfg(target_env = "musl")] -- type RequestType = libc::c_int; -- #[cfg(not(target_env = "musl"))] -- type RequestType = libc::c_ulong; -- - unsafe { -- if libc::ioctl(fd, BLKGETSIZE64 as RequestType, &mut cap) == 0 { -+ if libc::ioctl(fd, BLKGETSIZE64, &mut cap) == 0 { - Ok(cap) - } else { - Err(io::Error::last_os_error()) -diff --git a/src/ioctl.rs b/src/ioctl.rs -new file mode 100644 -index 00000000..84933648 ---- /dev/null -+++ b/src/ioctl.rs -@@ -0,0 +1,106 @@ -+/* Rust port of kernel include/uapi/asm-generic/ioctl.h */ -+ -+//------------------------------------------ -+ -+#[cfg(target_env = "musl")] -+pub type RequestType = libc::c_int; -+#[cfg(not(target_env = "musl"))] -+pub type RequestType = libc::c_ulong; -+ -+#[cfg(any( -+ target_arch = "mips", -+ target_arch = "mips64", -+ target_arch = "powerpc", -+ target_arch = "powerpc64", -+ target_arch = "powerpc64le", -+ target_arch = "sparc", -+ target_arch = "sparc64" -+))] -+mod defs { -+ use super::RequestType; -+ pub const IOC_NONE: RequestType = 1; -+ pub const IOC_READ: RequestType = 2; -+ pub const IOC_WRITE: RequestType = 4; -+ pub const IOC_DIRBITS: RequestType = 3; -+ pub const IOC_SIZEBITS: RequestType = 13; -+} -+ -+#[cfg(not(any( -+ target_arch = "mips", -+ target_arch = "mips64", -+ target_arch = "powerpc", -+ target_arch = "powerpc64", -+ target_arch = "powerpc64le", -+ target_arch = "sparc", -+ target_arch = "sparc64" -+)))] -+mod defs { -+ use super::RequestType; -+ pub const IOC_NONE: RequestType = 0; -+ pub const IOC_WRITE: RequestType = 1; -+ pub const IOC_READ: RequestType = 2; -+ pub const IOC_DIRBITS: RequestType = 2; -+ pub const IOC_SIZEBITS: RequestType = 14; -+} -+ -+pub use defs::*; -+ -+pub const IOC_NRBITS: RequestType = 8; -+pub const IOC_TYPEBITS: RequestType = 8; -+ -+pub const IOC_NRMASK: RequestType = (1 << IOC_NRBITS) - 1; -+pub const IOC_TYPEMASK: RequestType = (1 << IOC_TYPEBITS) - 1; -+pub const IOC_SIZEMASK: RequestType = (1 << IOC_SIZEBITS) - 1; -+pub const IOC_DIRMASK: RequestType = (1 << IOC_DIRBITS) - 1; -+ -+pub const IOC_NRSHIFT: RequestType = 0; -+pub const IOC_TYPESHIFT: RequestType = IOC_NRSHIFT + IOC_NRBITS; -+pub const IOC_SIZESHIFT: RequestType = IOC_TYPESHIFT + IOC_TYPEBITS; -+pub const IOC_DIRSHIFT: RequestType = IOC_SIZESHIFT + IOC_SIZEBITS; -+ -+#[macro_export] -+macro_rules! ioc { -+ ($dir: expr, $typ: expr, $nr: expr, $size: expr) => { -+ (($dir as RequestType & IOC_DIRMASK) << IOC_DIRSHIFT) -+ | (($typ as RequestType & IOC_TYPEMASK) << IOC_TYPESHIFT) -+ | (($nr as RequestType & IOC_NRMASK) << IOC_NRSHIFT) -+ | (($size as RequestType & IOC_SIZEMASK) << IOC_SIZESHIFT) -+ }; -+} -+ -+//------------------------------------------ -+ -+#[macro_export] -+macro_rules! request_code_none { -+ ($typ: expr, $nr: expr) => { -+ $crate::ioc!(IOC_NONE, $typ, $nr, 0) -+ }; -+} -+ -+#[macro_export] -+macro_rules! request_code_read { -+ ($typ: expr, $nr: expr, $size_type: ty) => { -+ $crate::ioc!(IOC_READ, $typ, $nr, ::std::mem::size_of::<$size_type>()) -+ }; -+} -+ -+#[macro_export] -+macro_rules! request_code_write { -+ ($typ: expr, $nr: expr, $size_type: ty) => { -+ $crate::ioc!(IOC_WRITE, $typ, $nr, ::std::mem::size_of::<$size_type>()) -+ }; -+} -+ -+#[macro_export] -+macro_rules! request_code_readwrite { -+ ($typ: expr, $nr: expr, $size_type: ty) => { -+ $crate::ioc!( -+ IOC_READ | IOC_WRITE, -+ $typ, -+ $nr, -+ ::std::mem::size_of::<$size_type>() -+ ) -+ }; -+} -+ -+//------------------------------------------ -diff --git a/src/lib.rs b/src/lib.rs -index b12146ef..1371baac 100644 ---- a/src/lib.rs -+++ b/src/lib.rs -@@ -14,6 +14,7 @@ pub mod era; - pub mod file_utils; - pub mod grid_layout; - pub mod io_engine; -+pub mod ioctl; - pub mod math; - pub mod pack; - pub mod pdata; -diff --git a/src/thin/trim.rs b/src/thin/trim.rs -index 0d1fb590..b92a4bbd 100644 ---- a/src/thin/trim.rs -+++ b/src/thin/trim.rs -@@ -8,6 +8,7 @@ use std::sync::Arc; - use crate::commands::engine::*; - use crate::file_utils::file_size; - use crate::io_engine::*; -+use crate::ioctl::{self, *}; - use crate::pdata::btree_walker::*; - use crate::pdata::space_map::common::*; - use crate::pdata::unpack::unpack; -@@ -132,15 +133,11 @@ impl<'a> Iterator for RangeIterator<'a> { - - //------------------------------------------ - --const BLKDISCARD: u32 = 0x1277; --fn ioctl_blkdiscard(fd: i32, range: &[u64; 2]) -> std::io::Result<()> { -- #[cfg(target_env = "musl")] -- type RequestType = libc::c_int; -- #[cfg(not(target_env = "musl"))] -- type RequestType = libc::c_ulong; -+const BLKDISCARD: ioctl::RequestType = crate::request_code_none!(0x12, 119); - -+fn ioctl_blkdiscard(fd: i32, range: &[u64; 2]) -> std::io::Result<()> { - unsafe { -- if libc::ioctl(fd, BLKDISCARD as RequestType, range) == 0 { -+ if libc::ioctl(fd, BLKDISCARD, range) == 0 { - Ok(()) - } else { - Err(std::io::Error::last_os_error()) --- -2.41.0 - diff --git a/SOURCES/0002-space-map-Fix-incorrect-index_entry.nr_free-while-ex.patch b/SOURCES/0002-space-map-Fix-incorrect-index_entry.nr_free-while-ex.patch new file mode 100644 index 0000000..082ba6b --- /dev/null +++ b/SOURCES/0002-space-map-Fix-incorrect-index_entry.nr_free-while-ex.patch @@ -0,0 +1,181 @@ +From f088ce90f5f7934489a8d1062ebfd3d23e4aec38 Mon Sep 17 00:00:00 2001 +From: Ming-Hung Tsai +Date: Tue, 12 Dec 2023 14:25:27 +0800 +Subject: [PATCH 2/6] [space map] Fix incorrect index_entry.nr_free while + expansion + +Do not truncate the nr_free value of the last index_entry to the space +map boundary, to address the issue in kernel that sm_ll_extend() doesn't +extend the nr_free value of the last index_entry while expanding the +space map. Without this fix, we'll have incorrect free space estimation +in later block allocations, leading to under-utilized space maps. + +(cherry picked from commit 9a405f57c591020321fc16f00efdb5197e1df2c0) +--- + src/pdata/space_map/common.rs | 20 +++++--- + src/pdata/space_map/tests.rs | 96 +++++++++++++++++++++++++++++++++++ + 2 files changed, 108 insertions(+), 8 deletions(-) + +diff --git a/src/pdata/space_map/common.rs b/src/pdata/space_map/common.rs +index d92b8115..4be9c303 100644 +--- a/src/pdata/space_map/common.rs ++++ b/src/pdata/space_map/common.rs +@@ -211,22 +211,28 @@ pub fn write_common( + let len = std::cmp::min(nr_blocks - begin, ENTRIES_PER_BITMAP as u64); + let mut entries = Vec::with_capacity(ENTRIES_PER_BITMAP); + let mut first_free: Option = None; +- let mut nr_free: u32 = 0; ++ let mut nr_free = ENTRIES_PER_BITMAP as u32; // do not truncate to the sm size boundary + + for i in 0..len { + let b = begin + i; + let rc = sm.get(b)?; + let e = match rc { + 0 => { +- nr_free += 1; + if first_free.is_none() { + first_free = Some(i as u32); + } + Small(0) + } +- 1 => Small(1), +- 2 => Small(2), ++ 1 => { ++ nr_free -= 1; ++ Small(1) ++ } ++ 2 => { ++ nr_free -= 1; ++ Small(2) ++ } + _ => { ++ nr_free -= 1; + overflow_builder.push_value(w, b, rc)?; + Overflow + } +@@ -334,16 +340,14 @@ pub fn write_metadata_common(w: &mut WriteBatcher) -> anyhow::Result<(Vec Result<()> { ++ let engine = Arc::new(CoreIoEngine::new(nr_blocks)); ++ let meta_sm = core_metadata_sm(engine.get_nr_blocks(), u32::MAX); ++ ++ let mut w = WriteBatcher::new(engine.clone(), meta_sm.clone(), engine.get_batch_size()); ++ w.alloc()?; // reserved for the superblock ++ let root = write_metadata_sm(&mut w)?; ++ ++ let b = engine.read(root.bitmap_root)?; ++ let entries = check_and_unpack_metadata_index(&b)?.indexes; ++ ensure!(entries.len() as u64 == div_up(nr_blocks, ENTRIES_PER_BITMAP as u64)); ++ ++ // the number of blocks observed by index_entries must be multiple of ENTRIES_PER_BITMAP ++ let nr_allocated = meta_sm.lock().unwrap().get_nr_allocated()?; ++ let nr_free: u64 = entries.iter().map(|ie| ie.nr_free as u64).sum(); ++ ensure!(nr_allocated + nr_free == (entries.len() * ENTRIES_PER_BITMAP) as u64); ++ ++ Ok(()) ++ } ++ ++ #[test] ++ fn check_single_index_entry() -> Result<()> { ++ check_index_entries(1000) ++ } ++ ++ #[test] ++ fn check_multiple_index_entries() -> Result<()> { ++ check_index_entries(ENTRIES_PER_BITMAP as u64 * 16 + 1000) ++ } ++} ++ ++//------------------------------------------ ++ ++mod disk_sm { ++ use anyhow::{ensure, Result}; ++ use std::ops::Deref; ++ use std::sync::Arc; ++ ++ use crate::io_engine::core::CoreIoEngine; ++ use crate::io_engine::*; ++ use crate::math::div_up; ++ use crate::pdata::btree_walker::btree_to_value_vec; ++ use crate::pdata::space_map::common::{IndexEntry, ENTRIES_PER_BITMAP}; ++ use crate::pdata::space_map::disk::*; ++ use crate::pdata::space_map::metadata::*; ++ use crate::pdata::space_map::*; ++ use crate::write_batcher::WriteBatcher; ++ ++ fn check_index_entries(nr_blocks: u64) -> Result<()> { ++ let engine = Arc::new(CoreIoEngine::new(1024)); ++ let meta_sm = core_metadata_sm(engine.get_nr_blocks(), u32::MAX); ++ ++ let mut w = WriteBatcher::new(engine.clone(), meta_sm.clone(), engine.get_batch_size()); ++ w.alloc()?; // reserved for the superblock ++ ++ let data_sm = core_sm(nr_blocks, u32::MAX); ++ data_sm.lock().unwrap().inc(0, 100)?; ++ ++ let root = write_disk_sm(&mut w, data_sm.lock().unwrap().deref())?; ++ ++ let entries = ++ btree_to_value_vec::(&mut Vec::new(), engine, false, root.bitmap_root)?; ++ ensure!(entries.len() as u64 == div_up(nr_blocks, ENTRIES_PER_BITMAP as u64)); ++ ++ // the number of blocks observed by index_entries must be a multiple of ENTRIES_PER_BITMAP ++ let nr_allocated = data_sm.lock().unwrap().get_nr_allocated()?; ++ let nr_free: u64 = entries.iter().map(|ie| ie.nr_free as u64).sum(); ++ ensure!(nr_allocated + nr_free == (entries.len() * ENTRIES_PER_BITMAP) as u64); ++ ++ Ok(()) ++ } ++ ++ #[test] ++ fn check_single_index_entry() -> Result<()> { ++ check_index_entries(1000) ++ } ++ ++ #[test] ++ fn check_multiple_index_entries() -> Result<()> { ++ check_index_entries(ENTRIES_PER_BITMAP as u64 * 16 + 1000) ++ } ++} ++ ++//------------------------------------------ +-- +2.43.0 + diff --git a/SOURCES/0003-file_utils-Verify-ioctl-request-code-in-tests.patch b/SOURCES/0003-file_utils-Verify-ioctl-request-code-in-tests.patch deleted file mode 100644 index 7e37307..0000000 --- a/SOURCES/0003-file_utils-Verify-ioctl-request-code-in-tests.patch +++ /dev/null @@ -1,120 +0,0 @@ -From 38e497f4200a0d2fcb043941d4a5792c76e47bbb Mon Sep 17 00:00:00 2001 -From: Ming-Hung Tsai -Date: Wed, 30 Aug 2023 18:11:45 +0800 -Subject: [PATCH 3/3] [file_utils] Verify ioctl request code in tests - -(cherry picked from commit f049fda90bbf74ab26bfd38e26e7c92de8f50e30) ---- - src/ioctl.rs | 3 ++ - src/ioctl/tests.rs | 86 ++++++++++++++++++++++++++++++++++++++++++++++ - 2 files changed, 89 insertions(+) - create mode 100644 src/ioctl/tests.rs - -diff --git a/src/ioctl.rs b/src/ioctl.rs -index 84933648..221bd4e9 100644 ---- a/src/ioctl.rs -+++ b/src/ioctl.rs -@@ -1,5 +1,8 @@ - /* Rust port of kernel include/uapi/asm-generic/ioctl.h */ - -+#[cfg(test)] -+mod tests; -+ - //------------------------------------------ - - #[cfg(target_env = "musl")] -diff --git a/src/ioctl/tests.rs b/src/ioctl/tests.rs -new file mode 100644 -index 00000000..17a395df ---- /dev/null -+++ b/src/ioctl/tests.rs -@@ -0,0 +1,86 @@ -+use crate::ioctl::*; -+ -+//------------------------------------------ -+ -+#[cfg(any( -+ target_arch = "mips", -+ target_arch = "mips64", -+ target_arch = "powerpc", -+ target_arch = "powerpc64", -+ target_arch = "powerpc64le", -+ target_arch = "sparc", -+ target_arch = "sparc64" -+))] -+mod expected { -+ use super::RequestType; -+ pub const BLKDISCARD: RequestType = 0x20001277; -+ -+ #[cfg(target_pointer_width = "32")] -+ mod sized { -+ use super::RequestType; -+ pub const BLKBSZSET: RequestType = 0x80041271; -+ pub const BLKGETSIZE64: RequestType = 0x40041272; -+ } -+ -+ #[cfg(target_pointer_width = "64")] -+ mod sized { -+ use super::RequestType; -+ pub const BLKBSZSET: RequestType = 0x80081271; -+ pub const BLKGETSIZE64: RequestType = 0x40081272; -+ } -+ -+ pub use sized::*; -+} -+ -+#[cfg(not(any( -+ target_arch = "mips", -+ target_arch = "mips64", -+ target_arch = "powerpc", -+ target_arch = "powerpc64", -+ target_arch = "powerpc64le", -+ target_arch = "sparc", -+ target_arch = "sparc64" -+)))] -+mod expected { -+ use super::RequestType; -+ pub const BLKDISCARD: RequestType = 0x1277; -+ -+ #[cfg(target_pointer_width = "32")] -+ mod sized { -+ use super::RequestType; -+ pub const BLKBSZSET: RequestType = 0x40041271; -+ pub const BLKGETSIZE64: RequestType = 0x80041272; -+ } -+ -+ #[cfg(target_pointer_width = "64")] -+ mod sized { -+ use super::RequestType; -+ pub const BLKBSZSET: RequestType = 0x40081271; -+ pub const BLKGETSIZE64: RequestType = 0x80081272; -+ } -+ -+ pub use sized::*; -+} -+ -+#[test] -+fn test_ioc_none() { -+ assert_eq!(crate::request_code_none!(0x12, 119), expected::BLKDISCARD); -+} -+ -+#[test] -+fn test_ioc_read_usize() { -+ assert_eq!( -+ crate::request_code_read!(0x12, 114, usize), -+ expected::BLKGETSIZE64 -+ ); -+} -+ -+#[test] -+fn test_ioc_write_usize() { -+ assert_eq!( -+ crate::request_code_write!(0x12, 113, usize), -+ expected::BLKBSZSET -+ ); -+} -+ -+//------------------------------------------ --- -2.41.0 - diff --git a/SOURCES/0003-thin_repair-Fix-child-keys-checking-on-the-node-with.patch b/SOURCES/0003-thin_repair-Fix-child-keys-checking-on-the-node-with.patch new file mode 100644 index 0000000..511af7b --- /dev/null +++ b/SOURCES/0003-thin_repair-Fix-child-keys-checking-on-the-node-with.patch @@ -0,0 +1,69 @@ +From add81da22a3998503a6f340350d7e59ed3b52e28 Mon Sep 17 00:00:00 2001 +From: Ming-Hung Tsai +Date: Wed, 10 Jan 2024 15:56:39 +0800 +Subject: [PATCH 3/6] [thin_repair] Fix child keys checking on the node with a + zero key + +Fix the issue that keys overlapping between the second and the first +child nodes indexed by zero not being checked. + +(cherry picked from commit 386123bd0f74f7603e993bf3c26aac002162d5db) +--- + src/thin/metadata_repair.rs | 27 +++++++++++++++------------ + 1 file changed, 15 insertions(+), 12 deletions(-) + +diff --git a/src/thin/metadata_repair.rs b/src/thin/metadata_repair.rs +index 9716b1e3..8fece4b9 100644 +--- a/src/thin/metadata_repair.rs ++++ b/src/thin/metadata_repair.rs +@@ -128,10 +128,11 @@ impl DevInfo { + } + + fn push_child(&mut self, child: &DevInfo) -> Result<()> { +- if self.key_high > 0 && child.key_low <= self.key_high { +- return Err(anyhow!("incompatible child")); +- } +- if !self.pushed { ++ if self.pushed { ++ if child.key_low <= self.key_high { ++ return Err(anyhow!("incompatible child")); ++ } ++ } else { + self.key_low = child.key_low; + self.pushed = true; + } +@@ -175,10 +176,11 @@ impl MappingsInfo { + } + + fn push_child(&mut self, child: &MappingsInfo) -> Result<()> { +- if self.key_high > 0 && child.key_low <= self.key_high { +- return Err(anyhow!("incompatible child")); +- } +- if !self.pushed { ++ if self.pushed { ++ if child.key_low <= self.key_high { ++ return Err(anyhow!("incompatible child")); ++ } ++ } else { + self.key_low = child.key_low; + self.pushed = true; + } +@@ -221,10 +223,11 @@ impl DetailsInfo { + } + + fn push_child(&mut self, child: &DetailsInfo) -> Result<()> { +- if self.key_high > 0 && child.key_low <= self.key_high { +- return Err(anyhow!("incompatible child")); +- } +- if !self.pushed { ++ if self.pushed { ++ if child.key_low <= self.key_high { ++ return Err(anyhow!("incompatible child")); ++ } ++ } else { + self.key_low = child.key_low; + self.pushed = true; + } +-- +2.43.0 + diff --git a/SOURCES/0004-space_map-Allow-non-zero-values-in-unused-index-bloc.patch b/SOURCES/0004-space_map-Allow-non-zero-values-in-unused-index-bloc.patch new file mode 100644 index 0000000..f49b859 --- /dev/null +++ b/SOURCES/0004-space_map-Allow-non-zero-values-in-unused-index-bloc.patch @@ -0,0 +1,269 @@ +From 9b948a9639b00fe9226065ca4dd6fd2257cc126c Mon Sep 17 00:00:00 2001 +From: Ming-Hung Tsai +Date: Thu, 11 Jan 2024 22:57:36 +0800 +Subject: [PATCH 4/6] [space_map] Allow non-zero values in unused index block + entries + +Previously, we assumed unused entries in the index block were all +zero-initialized, leading to issues while loading the block with +unexpected bytes and a valid checksum [1]. The updated approach loads +index entries based on actual size information from superblock and +therefore improves compatibility. + +[1] stratis-storage/stratisd#3520 + +(cherry picked from commit d5fe6a1e1c539a0f260a96eb4d7ed9b83c3f84c9) +--- + src/commands/engine.rs | 6 ++++- + src/pdata/space_map/allocated_blocks.rs | 15 ++++++----- + src/pdata/space_map/checker.rs | 12 ++++++--- + src/pdata/space_map/metadata.rs | 22 +++++++++------- + src/pdata/space_map/tests.rs | 35 +++++++++++++++++++++++-- + src/thin/damage_generator.rs | 4 +-- + src/thin/stat.rs | 6 +++-- + 7 files changed, 75 insertions(+), 25 deletions(-) + +diff --git a/src/commands/engine.rs b/src/commands/engine.rs +index 1e752d6e..e8b75301 100644 +--- a/src/commands/engine.rs ++++ b/src/commands/engine.rs +@@ -137,7 +137,11 @@ fn thin_valid_blocks>(path: P, opts: &EngineOptions) -> RoaringBi + return all_blocks(e.get_nr_blocks() as u32); + } + let metadata_root = metadata_root.unwrap(); +- let valid_blocks = allocated_blocks(e.clone(), metadata_root.bitmap_root); ++ let valid_blocks = allocated_blocks( ++ e.clone(), ++ metadata_root.bitmap_root, ++ metadata_root.nr_blocks, ++ ); + valid_blocks.unwrap_or_else(|_| all_blocks(e.get_nr_blocks() as u32)) + } + +diff --git a/src/pdata/space_map/allocated_blocks.rs b/src/pdata/space_map/allocated_blocks.rs +index aa75cb7a..71497a76 100644 +--- a/src/pdata/space_map/allocated_blocks.rs ++++ b/src/pdata/space_map/allocated_blocks.rs +@@ -17,18 +17,21 @@ struct IndexInfo { + pub fn allocated_blocks( + engine: Arc, + sm_root: u64, ++ nr_blocks: u64, + ) -> Result { + // Walk index tree to find where the bitmaps are. + let b = engine.read(sm_root)?; +- let (_, indexes) = MetadataIndex::unpack(b.get_data())?; ++ let indexes = load_metadata_index(&b, nr_blocks)?; + +- let mut infos = Vec::new(); +- for (key, entry) in indexes.indexes.iter().enumerate() { +- infos.push(IndexInfo { ++ let mut infos: Vec<_> = indexes ++ .indexes ++ .iter() ++ .enumerate() ++ .map(|(key, entry)| IndexInfo { + key: key as u64, + loc: entry.blocknr, +- }); +- } ++ }) ++ .collect(); + + // Read bitmaps in sequence + infos.sort_by(|lhs, rhs| lhs.loc.partial_cmp(&rhs.loc).unwrap()); +diff --git a/src/pdata/space_map/checker.rs b/src/pdata/space_map/checker.rs +index cfafa79b..7cc8286b 100644 +--- a/src/pdata/space_map/checker.rs ++++ b/src/pdata/space_map/checker.rs +@@ -194,10 +194,11 @@ fn gather_disk_index_entries( + fn gather_metadata_index_entries( + engine: Arc, + bitmap_root: u64, ++ nr_blocks: u64, + metadata_sm: ASpaceMap, + ) -> Result> { + let b = engine.read(bitmap_root)?; +- let entries = check_and_unpack_metadata_index(&b)?.indexes; ++ let entries = load_metadata_index(&b, nr_blocks)?.indexes; + metadata_sm.lock().unwrap().inc(bitmap_root, 1)?; + inc_entries(&metadata_sm, &entries[0..])?; + +@@ -254,8 +255,13 @@ pub fn check_metadata_space_map( + metadata_sm.clone(), + false, + )?; +- let entries = +- gather_metadata_index_entries(engine.clone(), root.bitmap_root, metadata_sm.clone())?; ++ ++ let entries = gather_metadata_index_entries( ++ engine.clone(), ++ root.bitmap_root, ++ root.nr_blocks, ++ metadata_sm.clone(), ++ )?; + + // check overflow ref-counts + { +diff --git a/src/pdata/space_map/metadata.rs b/src/pdata/space_map/metadata.rs +index b466c135..be232389 100644 +--- a/src/pdata/space_map/metadata.rs ++++ b/src/pdata/space_map/metadata.rs +@@ -6,6 +6,7 @@ use std::sync::{Arc, Mutex}; + + use crate::checksum; + use crate::io_engine::*; ++use crate::math::div_up; + use crate::pdata::space_map::common::*; + use crate::pdata::space_map::*; + use crate::pdata::unpack::*; +@@ -32,14 +33,11 @@ impl Unpack for MetadataIndex { + let (i, _csum) = le_u32(i)?; + let (i, _padding) = le_u32(i)?; + let (i, blocknr) = le_u64(i)?; +- let (i, indexes) = nom::multi::count(IndexEntry::unpack, MAX_METADATA_BITMAPS)(i)?; ++ let (i, mut indexes) = nom::multi::count(IndexEntry::unpack, MAX_METADATA_BITMAPS)(i)?; + +- // Filter out unused entries +- let indexes: Vec = indexes +- .iter() +- .take_while(|e| e.blocknr != 0) +- .cloned() +- .collect(); ++ // Drop unused entries that point to block 0 ++ let nr_bitmaps = indexes.iter().take_while(|e| e.blocknr != 0).count(); ++ indexes.truncate(nr_bitmaps); + + Ok((i, MetadataIndex { blocknr, indexes })) + } +@@ -69,9 +67,15 @@ fn verify_checksum(b: &Block) -> Result<()> { + } + } + +-pub fn check_and_unpack_metadata_index(b: &Block) -> Result { ++pub fn load_metadata_index(b: &Block, nr_blocks: u64) -> Result { + verify_checksum(b)?; +- unpack::(b.get_data()).map_err(|e| e.into()) ++ let mut entries = unpack::(b.get_data())?; ++ if entries.blocknr != b.loc { ++ return Err(anyhow!("blocknr mismatch")); ++ } ++ let nr_bitmaps = div_up(nr_blocks, ENTRIES_PER_BITMAP as u64) as usize; ++ entries.indexes.truncate(nr_bitmaps); ++ Ok(entries) + } + + //------------------------------------------ +diff --git a/src/pdata/space_map/tests.rs b/src/pdata/space_map/tests.rs +index fb08a9dc..fa118189 100644 +--- a/src/pdata/space_map/tests.rs ++++ b/src/pdata/space_map/tests.rs +@@ -171,10 +171,11 @@ mod metadata_sm { + let mut w = WriteBatcher::new(engine.clone(), meta_sm.clone(), engine.get_batch_size()); + w.alloc()?; // reserved for the superblock + let root = write_metadata_sm(&mut w)?; ++ drop(w); + + let b = engine.read(root.bitmap_root)?; +- let entries = check_and_unpack_metadata_index(&b)?.indexes; +- ensure!(entries.len() as u64 == div_up(nr_blocks, ENTRIES_PER_BITMAP as u64)); ++ let entries = load_metadata_index(&b, root.nr_blocks)?.indexes; ++ ensure!(entries.len() == div_up(nr_blocks, ENTRIES_PER_BITMAP as u64) as usize); + + // the number of blocks observed by index_entries must be multiple of ENTRIES_PER_BITMAP + let nr_allocated = meta_sm.lock().unwrap().get_nr_allocated()?; +@@ -193,6 +194,35 @@ mod metadata_sm { + fn check_multiple_index_entries() -> Result<()> { + check_index_entries(ENTRIES_PER_BITMAP as u64 * 16 + 1000) + } ++ ++ #[test] ++ fn ignore_junk_bytes_in_index_block() -> Result<()> { ++ use crate::checksum; ++ use crate::pdata::space_map::common::IndexEntry; ++ use crate::pdata::unpack::Unpack; ++ ++ let nr_blocks = ENTRIES_PER_BITMAP as u64 * 4 + 1000; ++ let nr_bitmaps = div_up(nr_blocks, ENTRIES_PER_BITMAP as u64) as usize; ++ let engine = Arc::new(CoreIoEngine::new(nr_blocks)); ++ let meta_sm = core_metadata_sm(engine.get_nr_blocks(), u32::MAX); ++ ++ let mut w = WriteBatcher::new(engine.clone(), meta_sm.clone(), engine.get_batch_size()); ++ w.alloc()?; // reserved for the superblock ++ let root = write_metadata_sm(&mut w)?; ++ ++ // append junk bytes to the unused entry ++ let index_block = w.read(root.bitmap_root)?; ++ index_block.get_data()[nr_bitmaps * IndexEntry::disk_size() as usize + 16] = 1; ++ w.write(index_block, checksum::BT::INDEX)?; ++ w.flush()?; ++ drop(w); ++ ++ let b = engine.read(root.bitmap_root)?; ++ let entries = load_metadata_index(&b, root.nr_blocks)?.indexes; ++ ensure!(entries.len() == nr_bitmaps); ++ ++ Ok(()) ++ } + } + + //------------------------------------------ +@@ -223,6 +253,7 @@ mod disk_sm { + data_sm.lock().unwrap().inc(0, 100)?; + + let root = write_disk_sm(&mut w, data_sm.lock().unwrap().deref())?; ++ drop(w); + + let entries = + btree_to_value_vec::(&mut Vec::new(), engine, false, root.bitmap_root)?; +diff --git a/src/thin/damage_generator.rs b/src/thin/damage_generator.rs +index 56685d4d..df1be4cd 100644 +--- a/src/thin/damage_generator.rs ++++ b/src/thin/damage_generator.rs +@@ -23,7 +23,7 @@ fn find_blocks_of_rc( + let mut found = Vec::::new(); + if ref_count < 3 { + let b = engine.read(sm_root.bitmap_root)?; +- let entries = check_and_unpack_metadata_index(&b)?.indexes; ++ let entries = load_metadata_index(&b, sm_root.nr_blocks)?.indexes; + let bitmaps: Vec = entries.iter().map(|ie| ie.blocknr).collect(); + let nr_bitmaps = bitmaps.len(); + +@@ -75,7 +75,7 @@ fn adjust_bitmap_entries( + }; + + let index_block = engine.read(sm_root.bitmap_root)?; +- let entries = check_and_unpack_metadata_index(&index_block)?.indexes; ++ let entries = load_metadata_index(&index_block, sm_root.nr_blocks)?.indexes; + + let bi = blocks_to_bitmaps(blocks); + let bitmaps: Vec = bi.iter().map(|i| entries[*i].blocknr).collect(); +diff --git a/src/thin/stat.rs b/src/thin/stat.rs +index 03ae6845..c6f2bf44 100644 +--- a/src/thin/stat.rs ++++ b/src/thin/stat.rs +@@ -72,9 +72,10 @@ fn gather_btree_index_entries( + fn gather_metadata_index_entries( + engine: Arc, + bitmap_root: u64, ++ nr_blocks: u64, + ) -> Result> { + let b = engine.read(bitmap_root)?; +- let entries = check_and_unpack_metadata_index(&b)?.indexes; ++ let entries = load_metadata_index(&b, nr_blocks)?.indexes; + Ok(entries) + } + +@@ -152,7 +153,8 @@ fn stat_metadata_block_ref_counts( + ) -> Result> { + let mut histogram = BTreeMap::::new(); + +- let index_entries = gather_metadata_index_entries(engine.clone(), root.bitmap_root)?; ++ let index_entries = ++ gather_metadata_index_entries(engine.clone(), root.bitmap_root, root.nr_blocks)?; + stat_low_ref_counts(engine.clone(), &index_entries, &mut histogram)?; + + let histogram = stat_overflow_ref_counts(engine, root.ref_count_root, histogram)?; +-- +2.43.0 + diff --git a/SOURCES/0005-cache_check-Fix-boundary-check-on-the-bitset-for-cac.patch b/SOURCES/0005-cache_check-Fix-boundary-check-on-the-bitset-for-cac.patch new file mode 100644 index 0000000..1a69672 --- /dev/null +++ b/SOURCES/0005-cache_check-Fix-boundary-check-on-the-bitset-for-cac.patch @@ -0,0 +1,111 @@ +From 96fc598f76beac1deb0c9564dc67416f70ae4ac4 Mon Sep 17 00:00:00 2001 +From: Ming-Hung Tsai +Date: Tue, 30 Jan 2024 15:20:31 +0800 +Subject: [PATCH 5/6] [cache_check] Fix boundary check on the bitset for cached + blocks + +The bitset for cached block addresses grows dynamically if the metadata +is not shutdown properly, in which the size hint of the slow (backing) +device is not available. Fix a bug in determining whether resizing is +needed (bz2258485). + +(cherry picked from commit d2390a50f38d88f0f32b13e59444bbbca7e660b3) +--- + src/cache/check.rs | 6 +++--- + tests/cache_check.rs | 45 ++++++++++++++++++++++++++++++++++++++++++++ + 2 files changed, 48 insertions(+), 3 deletions(-) + +diff --git a/src/cache/check.rs b/src/cache/check.rs +index 18bd51b3..c71d3059 100644 +--- a/src/cache/check.rs ++++ b/src/cache/check.rs +@@ -19,7 +19,7 @@ use crate::report::*; + + //------------------------------------------ + +-// 16m entries is capable for a 1TB cache with 64KB block size ++// 16m entries is capable to address a 1TB device with 64KB block size + const DEFAULT_OBLOCKS: usize = 16777216; + + fn inc_superblock(sm: &ASpaceMap) -> anyhow::Result<()> { +@@ -82,7 +82,7 @@ mod format1 { + } + let mut seen_oblocks = self.seen_oblocks.lock().unwrap(); + +- if m.oblock as usize > seen_oblocks.len() { ++ if m.oblock as usize >= seen_oblocks.len() { + seen_oblocks.grow(m.oblock as usize + 1); + } else if seen_oblocks.contains(m.oblock as usize) { + return Err(array::value_err("origin block already mapped".to_string())); +@@ -179,7 +179,7 @@ mod format2 { + )); + } + +- if m.oblock as usize > seen_oblocks.len() { ++ if m.oblock as usize >= seen_oblocks.len() { + seen_oblocks.grow(m.oblock as usize + 1); + } else if seen_oblocks.contains(m.oblock as usize) { + return Err(array::value_err("origin block already mapped".to_string())); +diff --git a/tests/cache_check.rs b/tests/cache_check.rs +index 81f4c578..8988694a 100644 +--- a/tests/cache_check.rs ++++ b/tests/cache_check.rs +@@ -11,6 +11,8 @@ use common::program::*; + use common::target::*; + use common::test_dir::*; + ++use std::io::Write; ++ + //------------------------------------------ + + const USAGE: &str = "Validates cache metadata on a device or file. +@@ -294,3 +296,46 @@ fn no_clear_needs_check_if_error() -> Result<()> { + } + + //------------------------------------------ ++ ++fn metadata_without_slow_dev_size_info(use_v1: bool) -> Result<()> { ++ let mut td = TestDir::new()?; ++ ++ // The input metadata has a cached oblock with address equals to the default bitset size ++ // boundary (DEFAULT_OBLOCKS = 16777216), triggering bitset resize. ++ let xml = td.mk_path("meta.xml"); ++ let mut file = std::fs::File::create(&xml)?; ++ file.write_all(b" ++ ++ ++ ++ ++ ++ ++")?; ++ ++ let md = td.mk_path("meta.bin"); ++ thinp::file_utils::create_sized_file(&md, 4096 * 4096)?; ++ ++ let cache_restore_args = if use_v1 { ++ args!["-i", &xml, "-o", &md, "--metadata-version=1"] ++ } else { ++ args!["-i", &xml, "-o", &md, "--metadata-version=2"] ++ }; ++ ++ run_ok(cache_restore_cmd(cache_restore_args))?; ++ run_ok(cache_check_cmd(args![&md]))?; ++ ++ Ok(()) ++} ++ ++#[test] ++fn metadata_v1_without_slow_dev_size_info() -> Result<()> { ++ metadata_without_slow_dev_size_info(true) ++} ++ ++#[test] ++fn metadata_v2_without_slow_dev_size_info() -> Result<()> { ++ metadata_without_slow_dev_size_info(false) ++} ++ ++//------------------------------------------ +-- +2.43.0 + diff --git a/SOURCES/0006-thin-cache_check-Print-suggestive-hints-for-improvin.patch b/SOURCES/0006-thin-cache_check-Print-suggestive-hints-for-improvin.patch new file mode 100644 index 0000000..df75f43 --- /dev/null +++ b/SOURCES/0006-thin-cache_check-Print-suggestive-hints-for-improvin.patch @@ -0,0 +1,62 @@ +From e295610fad85ec1a64e569cba425ca557a56c6e6 Mon Sep 17 00:00:00 2001 +From: Ming-Hung Tsai +Date: Wed, 31 Jan 2024 11:36:17 +0800 +Subject: [PATCH 6/6] [thin/cache_check] Print suggestive hints for improving + error resolution + +Enhance error messages to instruct users on addressing recoverable +errors, eliminating the guesswork (bz2233177). + +(cherry picked from commit aaf3b396574709902ffba47e03a5c7ded6a103c5) +--- + src/cache/check.rs | 5 ++++- + src/thin/check.rs | 10 ++++++++-- + 2 files changed, 12 insertions(+), 3 deletions(-) + +diff --git a/src/cache/check.rs b/src/cache/check.rs +index c71d3059..17d1af77 100644 +--- a/src/cache/check.rs ++++ b/src/cache/check.rs +@@ -387,7 +387,10 @@ pub fn check(opts: CacheCheckOptions) -> anyhow::Result<()> { + ctx.report.warning("Repairing metadata leaks."); + repair_space_map(ctx.engine.clone(), metadata_leaks, metadata_sm.clone())?; + } else if !opts.ignore_non_fatal { +- return Err(anyhow!("metadata space map contains leaks")); ++ return Err(anyhow!(concat!( ++ "metadata space map contains leaks\n", ++ "perhaps you wanted to run with --auto-repair" ++ ))); + } + } + +diff --git a/src/thin/check.rs b/src/thin/check.rs +index 8b829899..f6fde359 100644 +--- a/src/thin/check.rs ++++ b/src/thin/check.rs +@@ -1246,7 +1246,10 @@ pub fn check(opts: ThinCheckOptions) -> Result<()> { + report.warning("Repairing data leaks."); + repair_space_map(engine.clone(), data_leaks, data_sm.clone())?; + } else if !opts.ignore_non_fatal { +- return Err(anyhow!("data space map contains leaks")); ++ return Err(anyhow!(concat!( ++ "data space map contains leaks\n", ++ "perhaps you wanted to run with --auto-repair" ++ ))); + } + } + +@@ -1255,7 +1258,10 @@ pub fn check(opts: ThinCheckOptions) -> Result<()> { + report.warning("Repairing metadata leaks."); + repair_space_map(engine.clone(), metadata_leaks, metadata_sm.clone())?; + } else if !opts.ignore_non_fatal { +- return Err(anyhow!("metadata space map contains leaks")); ++ return Err(anyhow!(concat!( ++ "metadata space map contains leaks\n", ++ "perhaps you wanted to run with --auto-repair" ++ ))); + } + } + +-- +2.43.0 + diff --git a/SOURCES/0007-thin_dump-Do-not-print-error-messages-on-BrokenPipe-.patch b/SOURCES/0007-thin_dump-Do-not-print-error-messages-on-BrokenPipe-.patch new file mode 100644 index 0000000..cec4d5c --- /dev/null +++ b/SOURCES/0007-thin_dump-Do-not-print-error-messages-on-BrokenPipe-.patch @@ -0,0 +1,225 @@ +From 6f7f70da8341cfe8447c6c70ebb4085629983b0d Mon Sep 17 00:00:00 2001 +From: Ming-Hung Tsai +Date: Tue, 20 Feb 2024 04:16:46 +0800 +Subject: [PATCH 07/10] [thin_dump] Do not print error messages on BrokenPipe + (EPIPE) + +Considering BrokenPipe is an expected result in most use cases, +thin_dump should not print error messages in this situation but should +return a non-zero exit code instead. For instance, lvconvert fetches +only the first line of thin_dump then closes the pipe immediately, +causing BrokenPipe to terminate thin_dump intentionally. + +Changes made include removing the unused NodeVisitor abstraction for +MappingVisitor to keep the error type from low-level io, and printing +messages based on the error type returned upward (bz2233177). + +(cherry picked from commit d7b91f9f7e9e4118f1f47df19d81fc9bc6965ec5) +--- + src/commands/utils.rs | 16 ++++++--- + src/thin/dump.rs | 31 +++--------------- + tests/common/process.rs | 2 +- + tests/thin_dump.rs | 72 ++++++++++++++++++++++++++++++++++++++++- + 4 files changed, 89 insertions(+), 32 deletions(-) + +diff --git a/src/commands/utils.rs b/src/commands/utils.rs +index 0be4fa7f..38751603 100644 +--- a/src/commands/utils.rs ++++ b/src/commands/utils.rs +@@ -162,10 +162,18 @@ pub fn check_overwrite_metadata(report: &Report, path: &Path) -> Result<()> { + + pub fn to_exit_code(report: &Report, result: anyhow::Result) -> exitcode::ExitCode { + if let Err(e) = result { +- if e.chain().len() > 1 { +- report.fatal(&format!("{}: {}", e, e.root_cause())); +- } else { +- report.fatal(&format!("{}", e)); ++ let root_cause = e.root_cause(); ++ let is_broken_pipe = root_cause ++ .downcast_ref::>() // quick_xml::Error::Io wraps io::Error in Arc ++ .map(|err| err.kind() == std::io::ErrorKind::BrokenPipe) ++ .unwrap_or(false); ++ ++ if !is_broken_pipe { ++ if e.chain().len() > 1 { ++ report.fatal(&format!("{}: {}", e, root_cause)); ++ } else { ++ report.fatal(&format!("{}", e)); ++ } + } + + // FIXME: we need a way of getting more meaningful error codes +diff --git a/src/thin/dump.rs b/src/thin/dump.rs +index f6046487..561ea566 100644 +--- a/src/thin/dump.rs ++++ b/src/thin/dump.rs +@@ -10,8 +10,7 @@ use crate::checksum; + use crate::commands::engine::*; + use crate::dump_utils::*; + use crate::io_engine::*; +-use crate::pdata::btree::{self, *}; +-use crate::pdata::btree_walker::*; ++use crate::pdata::btree::*; + use crate::pdata::space_map::common::*; + use crate::pdata::unpack::*; + use crate::report::*; +@@ -104,9 +103,7 @@ impl<'a> MappingVisitor<'a> { + }), + } + } +-} + +-impl<'a> NodeVisitor for MappingVisitor<'a> { + fn visit( + &self, + _path: &[u64], +@@ -114,39 +111,21 @@ impl<'a> NodeVisitor for MappingVisitor<'a> { + _h: &NodeHeader, + keys: &[u64], + values: &[BlockTime], +- ) -> btree::Result<()> { ++ ) -> Result<()> { + let mut inner = self.inner.lock().unwrap(); + for (k, v) in keys.iter().zip(values.iter()) { + if let Some(run) = inner.builder.next(*k, v.block, v.time) { +- // FIXME: BTreeError should carry more information than a string +- // so the caller could identify the actual root cause, +- // e.g., a broken pipe error or something. +- inner +- .md_out +- .map(&run) +- .map_err(|e| btree::value_err(format!("{}", e)))?; ++ inner.md_out.map(&run)?; + } + } + + Ok(()) + } + +- fn visit_again(&self, _path: &[u64], b: u64) -> btree::Result<()> { +- let mut inner = self.inner.lock().unwrap(); +- inner +- .md_out +- .ref_shared(&format!("{}", b)) +- .map_err(|e| btree::value_err(format!("{}", e)))?; +- Ok(()) +- } +- +- fn end_walk(&self) -> btree::Result<()> { ++ fn end_walk(&self) -> Result<()> { + let mut inner = self.inner.lock().unwrap(); + if let Some(run) = inner.builder.complete() { +- inner +- .md_out +- .map(&run) +- .map_err(|e| btree::value_err(format!("{}", e)))?; ++ inner.md_out.map(&run)?; + } + Ok(()) + } +diff --git a/tests/common/process.rs b/tests/common/process.rs +index 99f10dc7..ad26c38b 100644 +--- a/tests/common/process.rs ++++ b/tests/common/process.rs +@@ -39,7 +39,7 @@ impl Command { + Command { program, args } + } + +- fn to_expr(&self) -> duct::Expression { ++ pub fn to_expr(&self) -> duct::Expression { + duct::cmd(&self.program, &self.args) + } + } +diff --git a/tests/thin_dump.rs b/tests/thin_dump.rs +index 28cb237d..6e2cd3db 100644 +--- a/tests/thin_dump.rs ++++ b/tests/thin_dump.rs +@@ -129,7 +129,7 @@ fn dump_restore_cycle() -> Result<()> { + // test no stderr with a normal dump + + #[test] +-fn no_stderr() -> Result<()> { ++fn no_stderr_on_success() -> Result<()> { + let mut td = TestDir::new()?; + + let md = mk_valid_md(&mut td)?; +@@ -139,6 +139,76 @@ fn no_stderr() -> Result<()> { + Ok(()) + } + ++//------------------------------------------ ++// test no stderr on broken pipe errors ++ ++#[test] ++fn no_stderr_on_broken_pipe() -> Result<()> { ++ use anyhow::ensure; ++ ++ let mut td = TestDir::new()?; ++ ++ // use the metadata producing dump more than 64KB (the default pipe buffer size), ++ // such that thin_dump will be blocked on writing to the pipe, then hits EPIPE. ++ let md = prep_metadata(&mut td)?; ++ ++ let mut pipefd = [0i32; 2]; ++ unsafe { ++ ensure!(libc::pipe2(pipefd.as_mut_slice().as_mut_ptr(), libc::O_CLOEXEC) == 0); ++ } ++ ++ let cmd = thin_dump_cmd(args![&md]) ++ .to_expr() ++ .stdout_file(pipefd[1]) ++ .stderr_capture(); ++ let handle = cmd.unchecked().start()?; ++ ++ // wait for thin_dump to fill the pipe buffer ++ std::thread::sleep(std::time::Duration::from_millis(1000)); ++ ++ unsafe { ++ libc::close(pipefd[1]); // close the unused write-end ++ libc::close(pipefd[0]); // causing broken pipe ++ } ++ ++ let output = handle.wait()?; ++ ensure!(!output.status.success()); ++ ensure!(output.stderr.is_empty()); ++ ++ Ok(()) ++} ++ ++#[test] ++fn no_stderr_on_broken_fifo() -> Result<()> { ++ use anyhow::ensure; ++ ++ let mut td = TestDir::new()?; ++ ++ // use the metadata producing dump more than 64KB (the default pipe buffer size), ++ // such that thin_dump will be blocked on writing to the pipe, then hits EPIPE. ++ let md = prep_metadata(&mut td)?; ++ ++ let out_fifo = td.mk_path("out_fifo"); ++ unsafe { ++ let c_str = std::ffi::CString::new(out_fifo.as_os_str().as_encoded_bytes()).unwrap(); ++ ensure!(libc::mkfifo(c_str.as_ptr(), 0o666) == 0); ++ }; ++ ++ let cmd = thin_dump_cmd(args![&md, "-o", &out_fifo]) ++ .to_expr() ++ .stderr_capture(); ++ let handle = cmd.unchecked().start()?; ++ ++ let fifo = std::fs::File::open(out_fifo)?; ++ drop(fifo); // causing broken pipe ++ ++ let output = handle.wait()?; ++ ensure!(!output.status.success()); ++ ensure!(output.stderr.is_empty()); ++ ++ Ok(()) ++} ++ + //------------------------------------------ + // test dump metadata snapshot from a live metadata + // here we use a corrupted metadata to ensure that "thin_dump -m" reads the +-- +2.43.0 + diff --git a/SOURCES/0008-thin_metadata_pack-Allow-long-format-for-input-and-o.patch b/SOURCES/0008-thin_metadata_pack-Allow-long-format-for-input-and-o.patch new file mode 100644 index 0000000..6603391 --- /dev/null +++ b/SOURCES/0008-thin_metadata_pack-Allow-long-format-for-input-and-o.patch @@ -0,0 +1,34 @@ +From e6659ff342516f1861cdb1f365dfb4f79bb45c54 Mon Sep 17 00:00:00 2001 +From: mulhern +Date: Sun, 7 Jan 2024 20:17:13 -0500 +Subject: [PATCH 08/10] thin_metadata_pack: Allow long format for input and + output + +The options match the man pages that way. + +(cherry picked from commit b5e7028effc51ad1c303424a9f1d161d2f7b0c7c) +--- + src/commands/thin_metadata_pack.rs | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/src/commands/thin_metadata_pack.rs b/src/commands/thin_metadata_pack.rs +index 5aad973c..265439c9 100644 +--- a/src/commands/thin_metadata_pack.rs ++++ b/src/commands/thin_metadata_pack.rs +@@ -26,11 +26,13 @@ impl ThinMetadataPackCommand { + .help("Specify thinp metadata binary device/file") + .required(true) + .short('i') ++ .long("input") + .value_name("DEV")) + .arg(Arg::new("OUTPUT") + .help("Specify packed output file") + .required(true) + .short('o') ++ .long("output") + .value_name("FILE")) + } + } +-- +2.43.0 + diff --git a/SOURCES/0009-commands-Fix-version-string-compatibility-issue-with.patch b/SOURCES/0009-commands-Fix-version-string-compatibility-issue-with.patch new file mode 100644 index 0000000..9d3e117 --- /dev/null +++ b/SOURCES/0009-commands-Fix-version-string-compatibility-issue-with.patch @@ -0,0 +1,1292 @@ +From 255429f9323f68006dc0233a0668a39200342b2c Mon Sep 17 00:00:00 2001 +From: Ming-Hung Tsai +Date: Wed, 21 Feb 2024 15:25:53 +0800 +Subject: [PATCH 09/10] [commands] Fix version string compatibility issue with + LVM + +Make the displayed version string backward-compatible to LVM's +_check_tool_version() parser, which is used to check cache_check's +version while activating a cache device (issue #294). + +(cherry picked from commit 3cb749b91e5ed0dbd284ddd15b08998898f6d802) +--- + src/commands/cache_check.rs | 5 ++- + src/commands/cache_dump.rs | 5 ++- + src/commands/cache_generate_metadata.rs | 5 ++- + src/commands/cache_metadata_size.rs | 9 ++++- + src/commands/cache_repair.rs | 5 ++- + src/commands/cache_restore.rs | 5 ++- + src/commands/cache_writeback.rs | 5 ++- + src/commands/era_check.rs | 5 ++- + src/commands/era_dump.rs | 5 ++- + src/commands/era_generate_metadata.rs | 5 ++- + src/commands/era_invalidate.rs | 5 ++- + src/commands/era_repair.rs | 5 ++- + src/commands/era_restore.rs | 5 ++- + src/commands/thin_check.rs | 5 ++- + src/commands/thin_delta.rs | 49 +++++++++++++++++-------- + src/commands/thin_dump.rs | 5 ++- + src/commands/thin_explore.rs | 3 ++ + src/commands/thin_generate_damage.rs | 5 ++- + src/commands/thin_generate_metadata.rs | 5 ++- + src/commands/thin_ls.rs | 5 ++- + src/commands/thin_metadata_pack.rs | 9 ++++- + src/commands/thin_metadata_size.rs | 9 ++++- + src/commands/thin_metadata_unpack.rs | 5 ++- + src/commands/thin_repair.rs | 5 ++- + src/commands/thin_restore.rs | 5 ++- + src/commands/thin_rmap.rs | 5 ++- + src/commands/thin_shrink.rs | 9 ++++- + src/commands/thin_stat.rs | 5 ++- + src/commands/thin_trim.rs | 5 ++- + src/version.rs | 32 ++++++++++++++++ + tests/common/common_args.rs | 4 +- + tests/thin_delta.rs | 12 ++---- + 32 files changed, 194 insertions(+), 57 deletions(-) + +diff --git a/src/commands/cache_check.rs b/src/commands/cache_check.rs +index dd86d492..3af29b8c 100644 +--- a/src/commands/cache_check.rs ++++ b/src/commands/cache_check.rs +@@ -8,6 +8,7 @@ use crate::commands::engine::*; + use crate::commands::utils::*; + use crate::commands::Command; + use crate::report::*; ++use crate::version::*; + + //------------------------------------------ + +@@ -18,6 +19,7 @@ impl CacheCheckCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Validates cache metadata on a device or file.") + // flags + .arg( +@@ -76,7 +78,7 @@ impl CacheCheckCommand { + .required(true) + .index(1), + ); +- verbose_args(engine_args(cmd)) ++ verbose_args(engine_args(version_args(cmd))) + } + } + +@@ -87,6 +89,7 @@ impl<'a> Command<'a> for CacheCheckCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + +diff --git a/src/commands/cache_dump.rs b/src/commands/cache_dump.rs +index f087536d..43126495 100644 +--- a/src/commands/cache_dump.rs ++++ b/src/commands/cache_dump.rs +@@ -7,6 +7,7 @@ use crate::cache::dump::{dump, CacheDumpOptions}; + use crate::commands::engine::*; + use crate::commands::utils::*; + use crate::commands::Command; ++use crate::version::*; + + //------------------------------------------ + +@@ -17,6 +18,7 @@ impl CacheDumpCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Dump the cache metadata to stdout in XML format") + .arg( + Arg::new("REPAIR") +@@ -40,7 +42,7 @@ impl CacheDumpCommand { + .required(true) + .index(1), + ); +- engine_args(cmd) ++ engine_args(version_args(cmd)) + } + } + +@@ -51,6 +53,7 @@ impl<'a> Command<'a> for CacheDumpCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + let output_file = matches.get_one::("OUTPUT").map(Path::new); +diff --git a/src/commands/cache_generate_metadata.rs b/src/commands/cache_generate_metadata.rs +index 449dfad4..0818e46b 100644 +--- a/src/commands/cache_generate_metadata.rs ++++ b/src/commands/cache_generate_metadata.rs +@@ -8,6 +8,7 @@ use crate::cache::metadata_generator::*; + use crate::commands::engine::*; + use crate::commands::utils::*; + use crate::commands::Command; ++use crate::version::*; + + //------------------------------------------ + +@@ -18,6 +19,7 @@ impl CacheGenerateMetadataCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("A tool for creating synthetic cache metadata.") + // flags + .arg( +@@ -128,7 +130,7 @@ impl CacheGenerateMetadataCommand { + ]) + .required(true), + ); +- engine_args(cmd) ++ engine_args(version_args(cmd)) + } + } + +@@ -139,6 +141,7 @@ impl<'a> Command<'a> for CacheGenerateMetadataCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let output_file = Path::new(matches.get_one::("OUTPUT").unwrap()); + +diff --git a/src/commands/cache_metadata_size.rs b/src/commands/cache_metadata_size.rs +index 8189e8cf..0083f44b 100644 +--- a/src/commands/cache_metadata_size.rs ++++ b/src/commands/cache_metadata_size.rs +@@ -11,6 +11,7 @@ use crate::commands::Command; + use crate::math::div_up; + use crate::report::mk_simple_report; + use crate::units::*; ++use crate::version::*; + + //------------------------------------------ + +@@ -40,9 +41,10 @@ pub struct CacheMetadataSizeCommand; + + impl CacheMetadataSizeCommand { + fn cli(&self) -> clap::Command { +- clap::Command::new(self.name()) ++ let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Estimate the size of the metadata device needed for a given configuration.") + .override_usage("cache_metadata_size [OPTIONS] <--device-size --block-size | --nr-blocks >") + // flags +@@ -98,7 +100,9 @@ impl CacheMetadataSizeCommand { + .value_name("UNIT") + .value_parser(value_parser!(Units)) + .default_value("sector"), +- ) ++ ); ++ ++ version_args(cmd) + } + + fn parse_args(&self, args: I) -> Result<(CacheMetadataSizeOptions, Units, OutputFormat)> +@@ -107,6 +111,7 @@ impl CacheMetadataSizeCommand { + T: Into + Clone, + { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let nr_blocks = matches.get_one::("NR_BLOCKS"); + let device_size = matches.get_one::("DEVICE_SIZE"); +diff --git a/src/commands/cache_repair.rs b/src/commands/cache_repair.rs +index 2098548e..658e096c 100644 +--- a/src/commands/cache_repair.rs ++++ b/src/commands/cache_repair.rs +@@ -8,6 +8,7 @@ use crate::commands::engine::*; + use crate::commands::utils::*; + use crate::commands::Command; + use crate::report::*; ++use crate::version::*; + + pub struct CacheRepairCommand; + +@@ -16,6 +17,7 @@ impl CacheRepairCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Repair binary cache metadata, and write it to a different device or file") + .arg( + Arg::new("QUIET") +@@ -44,7 +46,7 @@ impl CacheRepairCommand { + // a dummy argument for compatibility with lvconvert + .arg(Arg::new("DUMMY").required(false).hide(true).index(1)); + +- verbose_args(engine_args(cmd)) ++ verbose_args(engine_args(version_args(cmd))) + } + } + +@@ -55,6 +57,7 @@ impl<'a> Command<'a> for CacheRepairCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let report = mk_report(matches.get_flag("QUIET")); + let log_level = match parse_log_level(&matches) { +diff --git a/src/commands/cache_restore.rs b/src/commands/cache_restore.rs +index 609d2a10..b88d9c92 100644 +--- a/src/commands/cache_restore.rs ++++ b/src/commands/cache_restore.rs +@@ -9,6 +9,7 @@ use crate::commands::engine::*; + use crate::commands::utils::*; + use crate::commands::Command; + use crate::report::{parse_log_level, verbose_args}; ++use crate::version::*; + + pub struct CacheRestoreCommand; + +@@ -17,6 +18,7 @@ impl CacheRestoreCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Convert XML format metadata to binary.") + .arg( + Arg::new("QUIET") +@@ -58,7 +60,7 @@ impl CacheRestoreCommand { + .value_name("FILE") + .required(true), + ); +- verbose_args(engine_args(cmd)) ++ verbose_args(engine_args(version_args(cmd))) + } + } + +@@ -69,6 +71,7 @@ impl<'a> Command<'a> for CacheRestoreCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + let output_file = Path::new(matches.get_one::("OUTPUT").unwrap()); +diff --git a/src/commands/cache_writeback.rs b/src/commands/cache_writeback.rs +index 3e7be9a2..875188a3 100644 +--- a/src/commands/cache_writeback.rs ++++ b/src/commands/cache_writeback.rs +@@ -7,6 +7,7 @@ use crate::commands::engine::*; + use crate::commands::utils::*; + use crate::commands::Command; + use crate::report::{parse_log_level, verbose_args}; ++use crate::version::*; + + pub struct CacheWritebackCommand; + +@@ -15,6 +16,7 @@ impl CacheWritebackCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Repair binary cache metadata, and write it to a different device or file") + .arg( + Arg::new("QUIET") +@@ -88,7 +90,7 @@ impl CacheWritebackCommand { + .value_parser(value_parser!(u32)) + .default_value("0"), + ); +- verbose_args(engine_args(cmd)) ++ verbose_args(engine_args(version_args(cmd))) + } + } + +@@ -99,6 +101,7 @@ impl<'a> Command<'a> for CacheWritebackCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let metadata_dev = Path::new(matches.get_one::("METADATA_DEV").unwrap()); + let origin_dev = Path::new(matches.get_one::("ORIGIN_DEV").unwrap()); +diff --git a/src/commands/era_check.rs b/src/commands/era_check.rs +index 7ee03ded..fefd772b 100644 +--- a/src/commands/era_check.rs ++++ b/src/commands/era_check.rs +@@ -8,6 +8,7 @@ use crate::commands::utils::*; + use crate::commands::Command; + use crate::era::check::{check, EraCheckOptions}; + use crate::report::*; ++use crate::version::*; + + //------------------------------------------ + +@@ -18,6 +19,7 @@ impl EraCheckCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Validate era metadata on device or file.") + // flags + .arg( +@@ -46,7 +48,7 @@ impl EraCheckCommand { + .required(true) + .index(1), + ); +- verbose_args(engine_args(cmd)) ++ verbose_args(engine_args(version_args(cmd))) + } + } + +@@ -57,6 +59,7 @@ impl<'a> Command<'a> for EraCheckCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + +diff --git a/src/commands/era_dump.rs b/src/commands/era_dump.rs +index 35100b05..e0f9919e 100644 +--- a/src/commands/era_dump.rs ++++ b/src/commands/era_dump.rs +@@ -7,6 +7,7 @@ use crate::commands::engine::*; + use crate::commands::utils::*; + use crate::commands::Command; + use crate::era::dump::{dump, EraDumpOptions}; ++use crate::version::*; + + //------------------------------------------ + +@@ -17,6 +18,7 @@ impl EraDumpCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Dump the era metadata to stdout in XML format") + .arg( + Arg::new("LOGICAL") +@@ -46,7 +48,7 @@ impl EraDumpCommand { + .required(true) + .index(1), + ); +- engine_args(cmd) ++ engine_args(version_args(cmd)) + } + } + +@@ -57,6 +59,7 @@ impl<'a> Command<'a> for EraDumpCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + let output_file = matches.get_one::("OUTPUT").map(Path::new); +diff --git a/src/commands/era_generate_metadata.rs b/src/commands/era_generate_metadata.rs +index 58c46f25..9028c5da 100644 +--- a/src/commands/era_generate_metadata.rs ++++ b/src/commands/era_generate_metadata.rs +@@ -7,6 +7,7 @@ use crate::commands::engine::*; + use crate::commands::utils::*; + use crate::commands::Command; + use crate::era::metadata_generator::*; ++use crate::version::*; + + //------------------------------------------ + +@@ -17,6 +18,7 @@ impl EraGenerateMetadataCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("A tool for creating synthetic era metadata.") + // flags + .arg( +@@ -67,7 +69,7 @@ impl EraGenerateMetadataCommand { + .required(true), + ) + .group(ArgGroup::new("commands").args(["FORMAT"]).required(true)); +- engine_args(cmd) ++ engine_args(version_args(cmd)) + } + } + +@@ -78,6 +80,7 @@ impl<'a> Command<'a> for EraGenerateMetadataCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let output_file = Path::new(matches.get_one::("OUTPUT").unwrap()); + +diff --git a/src/commands/era_invalidate.rs b/src/commands/era_invalidate.rs +index cf5c14f9..58032112 100644 +--- a/src/commands/era_invalidate.rs ++++ b/src/commands/era_invalidate.rs +@@ -5,6 +5,7 @@ use crate::commands::engine::*; + use crate::commands::utils::*; + use crate::commands::Command; + use crate::era::invalidate::{invalidate, EraInvalidateOptions}; ++use crate::version::*; + + //------------------------------------------ + +@@ -15,6 +16,7 @@ impl EraInvalidateCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("List blocks that may have changed since a given era") + .arg( + Arg::new("METADATA_SNAPSHOT") +@@ -44,7 +46,7 @@ impl EraInvalidateCommand { + .required(true) + .index(1), + ); +- engine_args(cmd) ++ engine_args(version_args(cmd)) + } + } + +@@ -55,6 +57,7 @@ impl<'a> Command<'a> for EraInvalidateCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + let output_file = matches.get_one::("OUTPUT").map(Path::new); +diff --git a/src/commands/era_repair.rs b/src/commands/era_repair.rs +index e4f095c8..825c37d8 100644 +--- a/src/commands/era_repair.rs ++++ b/src/commands/era_repair.rs +@@ -8,6 +8,7 @@ use crate::commands::utils::*; + use crate::commands::Command; + use crate::era::repair::{repair, EraRepairOptions}; + use crate::report::*; ++use crate::version::*; + + pub struct EraRepairCommand; + +@@ -16,6 +17,7 @@ impl EraRepairCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Repair binary era metadata, and write it to a different device or file") + .arg( + Arg::new("QUIET") +@@ -41,7 +43,7 @@ impl EraRepairCommand { + .value_name("FILE") + .required(true), + ); +- verbose_args(engine_args(cmd)) ++ verbose_args(engine_args(version_args(cmd))) + } + } + +@@ -52,6 +54,7 @@ impl<'a> Command<'a> for EraRepairCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + let output_file = Path::new(matches.get_one::("OUTPUT").unwrap()); +diff --git a/src/commands/era_restore.rs b/src/commands/era_restore.rs +index 9c1184be..b779ac98 100644 +--- a/src/commands/era_restore.rs ++++ b/src/commands/era_restore.rs +@@ -8,6 +8,7 @@ use crate::commands::utils::*; + use crate::commands::Command; + use crate::era::restore::{restore, EraRestoreOptions}; + use crate::report::{parse_log_level, verbose_args}; ++use crate::version::*; + + pub struct EraRestoreCommand; + +@@ -16,6 +17,7 @@ impl EraRestoreCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Convert XML format metadata to binary.") + // flags + .arg( +@@ -42,7 +44,7 @@ impl EraRestoreCommand { + .value_name("FILE") + .required(true), + ); +- verbose_args(engine_args(cmd)) ++ verbose_args(engine_args(version_args(cmd))) + } + } + +@@ -53,6 +55,7 @@ impl<'a> Command<'a> for EraRestoreCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + let output_file = Path::new(matches.get_one::("OUTPUT").unwrap()); +diff --git a/src/commands/thin_check.rs b/src/commands/thin_check.rs +index c505c821..f8eeafc4 100644 +--- a/src/commands/thin_check.rs ++++ b/src/commands/thin_check.rs +@@ -6,6 +6,7 @@ use crate::commands::utils::*; + use crate::commands::Command; + use crate::report::{parse_log_level, verbose_args}; + use crate::thin::check::{check, ThinCheckOptions}; ++use crate::version::*; + + pub struct ThinCheckCommand; + +@@ -14,6 +15,7 @@ impl ThinCheckCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Validates thin provisioning metadata on a device or file.") + // flags + .arg( +@@ -85,7 +87,7 @@ impl ThinCheckCommand { + .required(true) + .index(1), + ); +- verbose_args(engine_args(cmd)) ++ verbose_args(engine_args(version_args(cmd))) + } + } + +@@ -96,6 +98,7 @@ impl<'a> Command<'a> for ThinCheckCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + +diff --git a/src/commands/thin_delta.rs b/src/commands/thin_delta.rs +index 0d4371ac..9d50d641 100644 +--- a/src/commands/thin_delta.rs ++++ b/src/commands/thin_delta.rs +@@ -9,6 +9,7 @@ use crate::commands::utils::*; + use crate::commands::Command; + use crate::thin::delta::*; + use crate::thin::delta_visitor::Snap; ++use crate::version::*; + + //------------------------------------------ + +@@ -19,6 +20,7 @@ impl ThinDeltaCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Print the differences in the mappings between two thin devices") + .arg( + Arg::new("METADATA_SNAPSHOT") +@@ -72,17 +74,15 @@ impl ThinDeltaCommand { + .index(1), + ) + // groups +- .group( +- ArgGroup::new("SNAP1") +- .args(["ROOT1", "THIN1"]) +- .required(true), +- ) +- .group( +- ArgGroup::new("SNAP2") +- .args(["ROOT2", "THIN2"]) +- .required(true), +- ); +- engine_args(cmd) ++ // ++ // Due to a bug in clap 4.4 that doesn't handle exclusive Args ++ // and ArgGroup::required(true) properly, we avoid using the ++ // 'required(true)' flag on ArgGroup, and perform manual checks ++ // instead. (see github clap-rs/clap#5041) ++ .group(ArgGroup::new("SNAP1").args(["ROOT1", "THIN1"])) ++ .group(ArgGroup::new("SNAP2").args(["ROOT2", "THIN2"])); ++ ++ engine_args(version_args(cmd)) + } + } + +@@ -93,6 +93,7 @@ impl<'a> Command<'a> for ThinDeltaCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + +@@ -102,16 +103,34 @@ impl<'a> Command<'a> for ThinDeltaCommand { + return to_exit_code::<()>(&report, Err(e)); + } + +- let snap1 = match matches.get_one::("SNAP1").unwrap().as_str() { ++ let snap1 = match matches ++ .get_one::("SNAP1") ++ .unwrap_or(&clap::Id::default()) ++ .as_str() ++ { + "THIN1" => Snap::DeviceId(*matches.get_one::("THIN1").unwrap()), + "ROOT1" => Snap::RootBlock(*matches.get_one::("ROOT1").unwrap()), +- _ => return to_exit_code::<()>(&report, Err(anyhow!("unknown option"))), ++ _ => { ++ return to_exit_code::<()>( ++ &report, ++ Err(anyhow!("--thin1 or --root1 not specified")), ++ ) ++ } + }; + +- let snap2 = match matches.get_one::("SNAP2").unwrap().as_str() { ++ let snap2 = match matches ++ .get_one::("SNAP2") ++ .unwrap_or(&clap::Id::default()) ++ .as_str() ++ { + "THIN2" => Snap::DeviceId(*matches.get_one::("THIN2").unwrap()), + "ROOT2" => Snap::RootBlock(*matches.get_one::("ROOT2").unwrap()), +- _ => return to_exit_code::<()>(&report, Err(anyhow!("unknown option"))), ++ _ => { ++ return to_exit_code::<()>( ++ &report, ++ Err(anyhow!("--thin2 or --root2 not specified")), ++ ) ++ } + }; + + let engine_opts = parse_engine_opts(ToolType::Thin, &matches); +diff --git a/src/commands/thin_dump.rs b/src/commands/thin_dump.rs +index 96cd8e87..d876066f 100644 +--- a/src/commands/thin_dump.rs ++++ b/src/commands/thin_dump.rs +@@ -10,6 +10,7 @@ use crate::commands::Command; + use crate::report::*; + use crate::thin::dump::{dump, OutputFormat, ThinDumpOptions}; + use crate::thin::metadata_repair::SuperblockOverrides; ++use crate::version::*; + + pub struct ThinDumpCommand; + +@@ -18,6 +19,7 @@ impl ThinDumpCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Dump thin-provisioning metadata to stdout in XML format") + .arg( + Arg::new("QUIET") +@@ -108,7 +110,7 @@ impl ThinDumpCommand { + .required(true) + .index(1), + ); +- verbose_args(engine_args(cmd)) ++ verbose_args(engine_args(version_args(cmd))) + } + } + +@@ -119,6 +121,7 @@ impl<'a> Command<'a> for ThinDumpCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + let output_file = matches.get_one::("OUTPUT").map(Path::new); +diff --git a/src/commands/thin_explore.rs b/src/commands/thin_explore.rs +index 79ca6ff2..5b6c7fac 100644 +--- a/src/commands/thin_explore.rs ++++ b/src/commands/thin_explore.rs +@@ -39,6 +39,7 @@ use crate::pdata::unpack::*; + use crate::thin::block_time::*; + use crate::thin::device_detail::*; + use crate::thin::superblock::*; ++use crate::version::*; + + //------------------------------------ + +@@ -879,6 +880,7 @@ impl ThinExploreCommand { + clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("A text user interface for examining thin metadata.") + .arg( + Arg::new("NODE_PATH") +@@ -903,6 +905,7 @@ impl<'a> Command<'a> for ThinExploreCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let node_path = matches + .get_one::("NODE_PATH") +diff --git a/src/commands/thin_generate_damage.rs b/src/commands/thin_generate_damage.rs +index f6d9f47c..080a0325 100644 +--- a/src/commands/thin_generate_damage.rs ++++ b/src/commands/thin_generate_damage.rs +@@ -5,6 +5,7 @@ use std::process; + use crate::commands::engine::*; + use crate::commands::utils::*; + use crate::thin::damage_generator::*; ++use crate::version::*; + + //------------------------------------------ + use crate::commands::Command; +@@ -16,6 +17,7 @@ impl ThinGenerateDamageCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("A tool for creating synthetic thin metadata.") + .arg( + Arg::new("CREATE_METADATA_LEAKS") +@@ -92,7 +94,7 @@ impl ThinGenerateDamageCommand { + .args(["MAPPING_ROOT", "DETAILS_ROOT", "METADATA_SNAPSHOT"]) + .multiple(true), + ); +- engine_args(cmd) ++ engine_args(version_args(cmd)) + } + } + +@@ -103,6 +105,7 @@ impl<'a> Command<'a> for ThinGenerateDamageCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let report = mk_report(false); + +diff --git a/src/commands/thin_generate_metadata.rs b/src/commands/thin_generate_metadata.rs +index 14a59a01..7e445d28 100644 +--- a/src/commands/thin_generate_metadata.rs ++++ b/src/commands/thin_generate_metadata.rs +@@ -5,6 +5,7 @@ use std::process; + use crate::commands::engine::*; + use crate::commands::utils::*; + use crate::thin::metadata_generator::*; ++use crate::version::*; + + //------------------------------------------ + use crate::commands::Command; +@@ -16,6 +17,7 @@ impl ThinGenerateMetadataCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("A tool for creating synthetic thin metadata.") + // flags + .arg( +@@ -64,7 +66,7 @@ impl ThinGenerateMetadataCommand { + .args(["FORMAT", "SET_NEEDS_CHECK"]) + .required(true), + ); +- engine_args(cmd) ++ engine_args(version_args(cmd)) + } + } + +@@ -75,6 +77,7 @@ impl<'a> Command<'a> for ThinGenerateMetadataCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let report = mk_report(false); + +diff --git a/src/commands/thin_ls.rs b/src/commands/thin_ls.rs +index 7d778d31..20dcd582 100644 +--- a/src/commands/thin_ls.rs ++++ b/src/commands/thin_ls.rs +@@ -8,6 +8,7 @@ use crate::commands::utils::*; + use crate::commands::Command; + use crate::report::{parse_log_level, verbose_args}; + use crate::thin::ls::*; ++use crate::version::*; + + pub struct ThinLsCommand; + +@@ -16,6 +17,7 @@ impl ThinLsCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("List thin volumes within a pool") + .arg( + Arg::new("NO_HEADERS") +@@ -47,7 +49,7 @@ impl ThinLsCommand { + .required(true) + .index(1), + ); +- verbose_args(engine_args(cmd)) ++ verbose_args(engine_args(version_args(cmd))) + } + } + +@@ -60,6 +62,7 @@ impl<'a> Command<'a> for ThinLsCommand { + use OutputField::*; + + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + +diff --git a/src/commands/thin_metadata_pack.rs b/src/commands/thin_metadata_pack.rs +index 265439c9..be0c7585 100644 +--- a/src/commands/thin_metadata_pack.rs ++++ b/src/commands/thin_metadata_pack.rs +@@ -6,14 +6,16 @@ use std::path::Path; + use crate::commands::utils::*; + use crate::commands::Command; + use crate::report::*; ++use crate::version::*; + + pub struct ThinMetadataPackCommand; + + impl ThinMetadataPackCommand { + fn cli(&self) -> clap::Command { +- clap::Command::new(self.name()) ++ let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Produces a compressed file of thin metadata. Only packs metadata blocks that are actually used.") + // flags + .arg(Arg::new("FORCE") +@@ -33,7 +35,9 @@ impl ThinMetadataPackCommand { + .required(true) + .short('o') + .long("output") +- .value_name("FILE")) ++ .value_name("FILE")); ++ ++ version_args(cmd) + } + } + +@@ -44,6 +48,7 @@ impl<'a> Command<'a> for ThinMetadataPackCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + let output_file = Path::new(matches.get_one::("OUTPUT").unwrap()); +diff --git a/src/commands/thin_metadata_size.rs b/src/commands/thin_metadata_size.rs +index 86f04b3a..b1c18678 100644 +--- a/src/commands/thin_metadata_size.rs ++++ b/src/commands/thin_metadata_size.rs +@@ -10,6 +10,7 @@ use crate::commands::Command; + use crate::report::mk_simple_report; + use crate::thin::metadata_size::*; + use crate::units::*; ++use crate::version::*; + + //------------------------------------------ + +@@ -39,9 +40,10 @@ pub struct ThinMetadataSizeCommand; + + impl ThinMetadataSizeCommand { + fn cli(&self) -> clap::Command { +- clap::Command::new(self.name()) ++ let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Estimate the size of the metadata device needed for a given configuration.") + // flags + .arg( +@@ -94,7 +96,9 @@ impl ThinMetadataSizeCommand { + .value_name("UNIT") + .value_parser(value_parser!(Units)) + .default_value("sector"), +- ) ++ ); ++ ++ version_args(cmd) + } + + fn parse_args(&self, args: I) -> Result<(ThinMetadataSizeOptions, Units, OutputFormat)> +@@ -103,6 +107,7 @@ impl ThinMetadataSizeCommand { + T: Into + Clone, + { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let pool_size = matches + .get_one::("POOL_SIZE") +diff --git a/src/commands/thin_metadata_unpack.rs b/src/commands/thin_metadata_unpack.rs +index 7e383ce3..6fbb8437 100644 +--- a/src/commands/thin_metadata_unpack.rs ++++ b/src/commands/thin_metadata_unpack.rs +@@ -8,6 +8,7 @@ use crate::commands::utils::*; + use crate::commands::Command; + use crate::pack::toplevel::unpack; + use crate::report::mk_simple_report; ++use crate::version::*; + + pub struct ThinMetadataUnpackCommand; + +@@ -16,6 +17,7 @@ impl ThinMetadataUnpackCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Unpack a compressed file of thin metadata.") + // flags + .arg( +@@ -40,7 +42,7 @@ impl ThinMetadataUnpackCommand { + .short('o') + .value_name("DEV"), + ); +- engine_args(cmd) ++ engine_args(version_args(cmd)) + } + } + +@@ -51,6 +53,7 @@ impl<'a> Command<'a> for ThinMetadataUnpackCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + let output_file = Path::new(matches.get_one::("OUTPUT").unwrap()); +diff --git a/src/commands/thin_repair.rs b/src/commands/thin_repair.rs +index 6f4384cc..5abc5e36 100644 +--- a/src/commands/thin_repair.rs ++++ b/src/commands/thin_repair.rs +@@ -9,6 +9,7 @@ use crate::commands::Command; + use crate::report::{parse_log_level, verbose_args}; + use crate::thin::metadata_repair::SuperblockOverrides; + use crate::thin::repair::{repair, ThinRepairOptions}; ++use crate::version::*; + + pub struct ThinRepairCommand; + +@@ -17,6 +18,7 @@ impl ThinRepairCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Repair thin-provisioning metadata, and write it to different device or file") + .arg( + Arg::new("QUIET") +@@ -66,7 +68,7 @@ impl ThinRepairCommand { + // a dummy argument for compatibility with lvconvert + .arg(Arg::new("DUMMY").required(false).hide(true).index(1)); + +- verbose_args(engine_args(cmd)) ++ verbose_args(engine_args(version_args(cmd))) + } + } + +@@ -77,6 +79,7 @@ impl<'a> Command<'a> for ThinRepairCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + let output_file = Path::new(matches.get_one::("OUTPUT").unwrap()); +diff --git a/src/commands/thin_restore.rs b/src/commands/thin_restore.rs +index feab577a..43becfcb 100644 +--- a/src/commands/thin_restore.rs ++++ b/src/commands/thin_restore.rs +@@ -9,6 +9,7 @@ use crate::commands::Command; + use crate::report::{parse_log_level, verbose_args}; + use crate::thin::metadata_repair::SuperblockOverrides; + use crate::thin::restore::{restore, ThinRestoreOptions}; ++use crate::version::*; + + pub struct ThinRestoreCommand; + +@@ -17,6 +18,7 @@ impl ThinRestoreCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Convert XML format metadata to binary.") + .arg( + Arg::new("QUIET") +@@ -63,7 +65,7 @@ impl ThinRestoreCommand { + .value_name("NUM") + .value_parser(value_parser!(u64)), + ); +- verbose_args(engine_args(cmd)) ++ verbose_args(engine_args(version_args(cmd))) + } + } + +@@ -74,6 +76,7 @@ impl<'a> Command<'a> for ThinRestoreCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + let output_file = Path::new(matches.get_one::("OUTPUT").unwrap()); +diff --git a/src/commands/thin_rmap.rs b/src/commands/thin_rmap.rs +index fef2e36a..109a8a0f 100644 +--- a/src/commands/thin_rmap.rs ++++ b/src/commands/thin_rmap.rs +@@ -8,6 +8,7 @@ use crate::commands::engine::*; + use crate::commands::utils::*; + use crate::commands::Command; + use crate::thin::rmap::*; ++use crate::version::*; + + //------------------------------------------ + +@@ -18,6 +19,7 @@ impl ThinRmapCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Output reverse map of a thin provisioned region of blocks") + // options + .arg( +@@ -39,7 +41,7 @@ impl ThinRmapCommand { + .required(true) + .index(1), + ); +- engine_args(cmd) ++ engine_args(version_args(cmd)) + } + } + +@@ -50,6 +52,7 @@ impl<'a> Command<'a> for ThinRmapCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + let input_file = Path::new(matches.get_one::("INPUT").unwrap()); + + let report = mk_report(false); +diff --git a/src/commands/thin_shrink.rs b/src/commands/thin_shrink.rs +index 38783d41..772024cd 100644 +--- a/src/commands/thin_shrink.rs ++++ b/src/commands/thin_shrink.rs +@@ -13,14 +13,16 @@ use crate::commands::utils::*; + use crate::commands::Command; + use crate::report::*; + use crate::thin::shrink::{shrink, ThinShrinkOptions}; ++use crate::version::*; + + pub struct ThinShrinkCommand; + + impl ThinShrinkCommand { + fn cli(&self) -> clap::Command { +- clap::Command::new(self.name()) ++ let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Rewrite xml metadata and move data in an inactive pool.") + .arg( + Arg::new("INPUT") +@@ -64,7 +66,9 @@ impl ThinShrinkCommand { + .help("Perform binary metadata rebuild rather than XML rewrite") + .long("binary") + .action(ArgAction::SetTrue), +- ) ++ ); ++ ++ version_args(cmd) + } + + fn parse_args(&self, args: I) -> io::Result +@@ -73,6 +77,7 @@ impl ThinShrinkCommand { + T: Into + Clone, + { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let input = Path::new(matches.get_one::("INPUT").unwrap()); + let output = Path::new(matches.get_one::("OUTPUT").unwrap()); +diff --git a/src/commands/thin_stat.rs b/src/commands/thin_stat.rs +index e62e7e8f..62ecb9c6 100644 +--- a/src/commands/thin_stat.rs ++++ b/src/commands/thin_stat.rs +@@ -5,6 +5,7 @@ use crate::commands::engine::*; + use crate::commands::utils::*; + use crate::report::mk_simple_report; + use crate::thin::stat::*; ++use crate::version::*; + + //------------------------------------------ + use crate::commands::Command; +@@ -16,6 +17,7 @@ impl ThinStatCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Tool to show metadata statistics") + .arg( + Arg::new("OP") +@@ -30,7 +32,7 @@ impl ThinStatCommand { + .index(1), + ); + +- engine_args(cmd) ++ engine_args(version_args(cmd)) + } + } + +@@ -41,6 +43,7 @@ impl<'a> Command<'a> for ThinStatCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + let report = std::sync::Arc::new(mk_simple_report()); + + let engine_opts = parse_engine_opts(ToolType::Thin, &matches); +diff --git a/src/commands/thin_trim.rs b/src/commands/thin_trim.rs +index 9e742e15..79df88dc 100644 +--- a/src/commands/thin_trim.rs ++++ b/src/commands/thin_trim.rs +@@ -8,6 +8,7 @@ use crate::commands::utils::*; + use crate::report::{parse_log_level, verbose_args}; + use crate::thin::check::{check, ThinCheckOptions}; + use crate::thin::trim::{trim, ThinTrimOptions}; ++use crate::version::*; + + //------------------------------------------ + use crate::commands::Command; +@@ -19,6 +20,7 @@ impl ThinTrimCommand { + let cmd = clap::Command::new(self.name()) + .next_display_order(None) + .version(crate::tools_version!()) ++ .disable_version_flag(true) + .about("Issue discard requests for free pool space (offline tool).") + .arg( + Arg::new("QUIET") +@@ -42,7 +44,7 @@ impl ThinTrimCommand { + .value_name("FILE") + .required(true), + ); +- verbose_args(engine_args(cmd)) ++ verbose_args(engine_args(version_args(cmd))) + } + } + +@@ -53,6 +55,7 @@ impl<'a> Command<'a> for ThinTrimCommand { + + fn run(&self, args: &mut dyn Iterator) -> exitcode::ExitCode { + let matches = self.cli().get_matches_from(args); ++ display_version(&matches); + + let metadata_dev = Path::new(matches.get_one::("METADATA_DEV").unwrap()); + let data_dev = Path::new(matches.get_one::("DATA_DEV").unwrap()); +diff --git a/src/version.rs b/src/version.rs +index af094ef4..bf4e8dda 100644 +--- a/src/version.rs ++++ b/src/version.rs +@@ -1,6 +1,38 @@ ++use clap::ArgMatches; ++use std::io::Write; ++ ++//------------------------------------------ ++ + #[macro_export] + macro_rules! tools_version { + () => { + env!("CARGO_PKG_VERSION") + }; + } ++ ++pub fn version_args(cmd: clap::Command) -> clap::Command { ++ use clap::Arg; ++ ++ cmd.arg( ++ Arg::new("VERSION") ++ .help("Print version") ++ .short('V') ++ .long("version") ++ .exclusive(true) ++ .action(clap::ArgAction::SetTrue), ++ ) ++} ++ ++pub fn display_version(matches: &ArgMatches) { ++ if matches.get_flag("VERSION") { ++ let mut stdout = std::io::stdout(); ++ // ignore broken pipe errors ++ let _ = stdout.write_all(tools_version!().as_bytes()); ++ let _ = stdout.write_all(b"\n"); ++ let _ = stdout.flush(); ++ ++ std::process::exit(0); ++ } ++} ++ ++//------------------------------------------ +diff --git a/tests/common/common_args.rs b/tests/common/common_args.rs +index b2da8ead..c5286749 100644 +--- a/tests/common/common_args.rs ++++ b/tests/common/common_args.rs +@@ -50,7 +50,7 @@ where + P: Program<'a>, + { + let stdout = run_ok(P::cmd(args!["-V"]))?; +- assert!(stdout.contains(tools_version!())); ++ assert!(stdout.starts_with(tools_version!())); + Ok(()) + } + +@@ -59,7 +59,7 @@ where + P: Program<'a>, + { + let stdout = run_ok(P::cmd(args!["--version"]))?; +- assert!(stdout.contains(tools_version!())); ++ assert!(stdout.starts_with(tools_version!())); + Ok(()) + } + +diff --git a/tests/thin_delta.rs b/tests/thin_delta.rs +index 0732038a..13556bf4 100644 +--- a/tests/thin_delta.rs ++++ b/tests/thin_delta.rs +@@ -13,7 +13,7 @@ use common::thin::*; + + const USAGE: &str = "Print the differences in the mappings between two thin devices + +-Usage: thin_delta [OPTIONS] <--root1 |--thin1 > <--root2 |--thin2 > ++Usage: thin_delta [OPTIONS] + + Arguments: + Specify the input device +@@ -71,10 +71,7 @@ fn snap1_unspecified() -> Result<()> { + let mut td = TestDir::new()?; + let md = mk_valid_md(&mut td)?; + let stderr = run_fail(thin_delta_cmd(args!["--snap2", "45", &md]))?; +- assert!(stderr.contains( +- "the following required arguments were not provided: +- <--root1 |--thin1 >" +- )); ++ assert!(stderr.contains("--thin1 or --root1 not specified")); + Ok(()) + } + +@@ -83,10 +80,7 @@ fn snap2_unspecified() -> Result<()> { + let mut td = TestDir::new()?; + let md = mk_valid_md(&mut td)?; + let stderr = run_fail(thin_delta_cmd(args!["--snap1", "45", &md]))?; +- assert!(stderr.contains( +- "the following required arguments were not provided: +- <--root2 |--thin2 >" +- )); ++ assert!(stderr.contains("--thin2 or --root2 not specified")); + Ok(()) + } + +-- +2.43.0 + diff --git a/SOURCES/0010-thin_dump-Do-not-print-error-messages-on-BrokenPipe-.patch b/SOURCES/0010-thin_dump-Do-not-print-error-messages-on-BrokenPipe-.patch new file mode 100644 index 0000000..0990e80 --- /dev/null +++ b/SOURCES/0010-thin_dump-Do-not-print-error-messages-on-BrokenPipe-.patch @@ -0,0 +1,109 @@ +From a557bc55ef3c136b1fca3f27cd55fdb0014dc6e7 Mon Sep 17 00:00:00 2001 +From: Ming-Hung Tsai +Date: Fri, 23 Feb 2024 20:20:01 +0800 +Subject: [PATCH 10/10] [thin_dump] Do not print error messages on BrokenPipe + (EPIPE) + +Handle the case that doesn't write through quick_xml, +e.g., thin_dump --format human_readable + +(cherry picked from commit b3e05f2eb9b704af897f14215536dadde5c13b2d) +--- + src/commands/utils.rs | 7 +++++-- + tests/thin_dump.rs | 34 +++++++++++++++++++++++++++------- + 2 files changed, 32 insertions(+), 9 deletions(-) + +diff --git a/src/commands/utils.rs b/src/commands/utils.rs +index 38751603..82b2529e 100644 +--- a/src/commands/utils.rs ++++ b/src/commands/utils.rs +@@ -165,8 +165,11 @@ pub fn to_exit_code(report: &Report, result: anyhow::Result) -> exitcode:: + let root_cause = e.root_cause(); + let is_broken_pipe = root_cause + .downcast_ref::>() // quick_xml::Error::Io wraps io::Error in Arc +- .map(|err| err.kind() == std::io::ErrorKind::BrokenPipe) +- .unwrap_or(false); ++ .map_or_else( ++ || root_cause.downcast_ref::(), ++ |err| Some(err.as_ref()), ++ ) ++ .map_or(false, |err| err.kind() == std::io::ErrorKind::BrokenPipe); + + if !is_broken_pipe { + if e.chain().len() > 1 { +diff --git a/tests/thin_dump.rs b/tests/thin_dump.rs +index 6e2cd3db..81982188 100644 +--- a/tests/thin_dump.rs ++++ b/tests/thin_dump.rs +@@ -142,8 +142,7 @@ fn no_stderr_on_success() -> Result<()> { + //------------------------------------------ + // test no stderr on broken pipe errors + +-#[test] +-fn no_stderr_on_broken_pipe() -> Result<()> { ++fn test_no_stderr_on_broken_pipe(extra_args: &[&std::ffi::OsStr]) -> Result<()> { + use anyhow::ensure; + + let mut td = TestDir::new()?; +@@ -157,7 +156,9 @@ fn no_stderr_on_broken_pipe() -> Result<()> { + ensure!(libc::pipe2(pipefd.as_mut_slice().as_mut_ptr(), libc::O_CLOEXEC) == 0); + } + +- let cmd = thin_dump_cmd(args![&md]) ++ let mut args = args![&md].to_vec(); ++ args.extend_from_slice(extra_args); ++ let cmd = thin_dump_cmd(args) + .to_expr() + .stdout_file(pipefd[1]) + .stderr_capture(); +@@ -179,7 +180,16 @@ fn no_stderr_on_broken_pipe() -> Result<()> { + } + + #[test] +-fn no_stderr_on_broken_fifo() -> Result<()> { ++fn no_stderr_on_broken_pipe_xml() -> Result<()> { ++ test_no_stderr_on_broken_pipe(&[]) ++} ++ ++#[test] ++fn no_stderr_on_broken_pipe_humanreadable() -> Result<()> { ++ test_no_stderr_on_broken_pipe(&args!["--format", "human_readable"]) ++} ++ ++fn test_no_stderr_on_broken_fifo(extra_args: &[&std::ffi::OsStr]) -> Result<()> { + use anyhow::ensure; + + let mut td = TestDir::new()?; +@@ -194,9 +204,9 @@ fn no_stderr_on_broken_fifo() -> Result<()> { + ensure!(libc::mkfifo(c_str.as_ptr(), 0o666) == 0); + }; + +- let cmd = thin_dump_cmd(args![&md, "-o", &out_fifo]) +- .to_expr() +- .stderr_capture(); ++ let mut args = args![&md, "-o", &out_fifo].to_vec(); ++ args.extend_from_slice(extra_args); ++ let cmd = thin_dump_cmd(args).to_expr().stderr_capture(); + let handle = cmd.unchecked().start()?; + + let fifo = std::fs::File::open(out_fifo)?; +@@ -209,6 +219,16 @@ fn no_stderr_on_broken_fifo() -> Result<()> { + Ok(()) + } + ++#[test] ++fn no_stderr_on_broken_fifo_xml() -> Result<()> { ++ test_no_stderr_on_broken_fifo(&[]) ++} ++ ++#[test] ++fn no_stderr_on_broken_fifo_humanreadable() -> Result<()> { ++ test_no_stderr_on_broken_fifo(&args!["--format", "human_readable"]) ++} ++ + //------------------------------------------ + // test dump metadata snapshot from a live metadata + // here we use a corrupted metadata to ensure that "thin_dump -m" reads the +-- +2.43.0 + diff --git a/SPECS/device-mapper-persistent-data.spec b/SPECS/device-mapper-persistent-data.spec index 5901d04..a84fdd3 100644 --- a/SPECS/device-mapper-persistent-data.spec +++ b/SPECS/device-mapper-persistent-data.spec @@ -9,17 +9,26 @@ Summary: Device-mapper Persistent Data Tools Name: device-mapper-persistent-data -Version: 1.0.6 +Version: 1.0.9 Release: 3%{?dist}%{?release_suffix} License: GPLv3+ URL: https://github.com/jthornber/thin-provisioning-tools #Source0: https://github.com/jthornber/thin-provisioning-tools/archive/thin-provisioning-tools-%%{version}.tar.gz Source0: https://github.com/jthornber/thin-provisioning-tools/archive/v%{version}%{?version_suffix}.tar.gz -Source1: dmpd106-vendor.tar.gz +Source1: dmpd109-vendor.tar.gz Patch1: 0001-Tweak-cargo.toml-to-work-with-vendor-directory.patch -# BZ 2233533: -Patch2: 0002-file_utils-Fix-the-ioctl-request-code-for-the-powerp.patch -Patch3: 0003-file_utils-Verify-ioctl-request-code-in-tests.patch +Patch2: 0002-space-map-Fix-incorrect-index_entry.nr_free-while-ex.patch +Patch3: 0003-thin_repair-Fix-child-keys-checking-on-the-node-with.patch +Patch4: 0004-space_map-Allow-non-zero-values-in-unused-index-bloc.patch +Patch5: 0005-cache_check-Fix-boundary-check-on-the-bitset-for-cac.patch +Patch6: 0006-thin-cache_check-Print-suggestive-hints-for-improvin.patch +# RHEL-26521: +Patch7: 0007-thin_dump-Do-not-print-error-messages-on-BrokenPipe-.patch +# RHEL-26520: +Patch8: 0008-thin_metadata_pack-Allow-long-format-for-input-and-o.patch +Patch9: 0009-commands-Fix-version-string-compatibility-issue-with.patch +# RHEL-26521: +Patch10: 0010-thin_dump-Do-not-print-error-messages-on-BrokenPipe-.patch BuildRequires: rust-packaging BuildRequires: rust >= 1.35 @@ -57,7 +66,10 @@ echo %{version}-%{release} > VERSION %if %{with check} %check -RUST_BACKTRACE=1 %cargo_test || true +# aarch64 is failing, but only in brew environment, tests are passing when +# running locally +#%%cargo_test +RUST_BACKTRACE=1 %%cargo_test -- --nocapture --test-threads=1 || true %endif %install @@ -111,11 +123,19 @@ make DESTDIR=%{buildroot} MANDIR=%{_mandir} install #% {_sbindir}/thin_show_duplicates %changelog -* Mon Sep 11 2023 Marian Csontos - 1.0.6-3 -- Fix build target. +* Mon Mar 04 2024 Marian Csontos - 1.0.9-3 +- Fix --version string compatibility with LVM tools. +- Fix confusing Broken pipe warning when used in lvconvert --repair. -* Thu Aug 31 2023 Marian Csontos - 1.0.6-2 -- Fix broken installation on ppc64le caused by incorrect ioctl call. +* Thu Feb 08 2024 Marian Csontos - 1.0.9-2 +- Allow non-zero values in unused index block entries. +- Fix boundary check on the bitset for cached blocks. +- Fix incorrect index_entry.nr_free on expansion affecting free space estimate. +- Fix thin_repair checking keys on a node with a zero key. +- Enhance error message in cache_check suggesting fix when applicable. + +* Wed Dec 13 2023 Marian Csontos - 1.0.9-1 +- Update to latest upstream release 1.0.9. * Wed Aug 09 2023 Marian Csontos - 1.0.6-1 - Update to latest upstream release 1.0.6.