diff --git a/.github/workflows/cargo.yml b/.github/workflows/cargo.yml index 3d99812940..0b25005186 100644 --- a/.github/workflows/cargo.yml +++ b/.github/workflows/cargo.yml @@ -51,7 +51,7 @@ jobs: - uses: dtolnay/rust-toolchain@master with: components: cargo - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN - name: Check out ci repo run: git clone https://github.com/stratis-storage/ci.git - name: Run comparisons of version specs with available Fedora packages diff --git a/.github/workflows/fedora.yml b/.github/workflows/fedora.yml index 850f0f957a..ca4a51acde 100644 --- a/.github/workflows/fedora.yml +++ b/.github/workflows/fedora.yml @@ -40,39 +40,45 @@ jobs: matrix: include: - task: make -f Makefile clippy - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: clippy - task: PROFILEDIR=debug make -f Makefile build - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo - task: PROFILEDIR=debug make -f Makefile build-min + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + components: cargo + - task: PROFILEDIR=debug make -f Makefile build-min-no-systemd toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo - - task: PROFILEDIR=debug make -f Makefile build-no-ipc + - task: PROFILEDIR=debug make -f Makefile build-utils toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo + - task: PROFILEDIR=debug make -f Makefile build-no-ipc + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + components: cargo - task: PROFILEDIR=debug make -f Makefile stratisd-tools - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo - task: make -f Makefile docs-ci - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo - task: make -f Makefile test - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo - task: >- TANG_URL=localhost make -f Makefile test-clevis-loop-should-fail - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo - task: make -f Makefile build - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo - task: make -f Makefile build-min - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo - task: make -f Makefile build-no-ipc - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo runs-on: ubuntu-22.04 container: @@ -107,7 +113,7 @@ jobs: matrix: include: - task: RUST_LOG=stratisd=debug make -f Makefile test-loop - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo runs-on: ubuntu-22.04 container: diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 4ebf9f2aba..273d0c47b4 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -40,10 +40,10 @@ jobs: matrix: include: - task: make -f Makefile fmt-ci - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: rustfmt - task: make -f Makefile check-typos - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo runs-on: ubuntu-22.04 container: @@ -83,7 +83,7 @@ jobs: TANG_URL=tang RUST_LOG=stratisd=debug make -f Makefile test-clevis-loop - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo image: fedora:39 # CURRENT DEVELOPMENT ENVIRONMENT runs-on: ubuntu-22.04 @@ -217,7 +217,7 @@ jobs: - uses: dtolnay/rust-toolchain@master with: components: cargo - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN - name: Run stratisd-min cli tests run: make test-stratisd-min - name: Run stratis-min cli tests @@ -267,7 +267,7 @@ jobs: - uses: dtolnay/rust-toolchain@master with: components: cargo - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN - name: Build stratisd run: PROFILEDIR=debug make -f Makefile build-all - name: Install stratisd diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml index 392c3c8e6a..dad447688d 100644 --- a/.github/workflows/ubuntu.yml +++ b/.github/workflows/ubuntu.yml @@ -40,34 +40,40 @@ jobs: matrix: include: - task: make -f Makefile clippy - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: clippy - task: PROFILEDIR=debug make -f Makefile build - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo - task: PROFILEDIR=debug make -f Makefile build-min + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + components: cargo + - task: PROFILEDIR=debug make -f Makefile build-min-no-systemd toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo - - task: PROFILEDIR=debug make -f Makefile build-no-ipc + - task: PROFILEDIR=debug make -f Makefile build-utils toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo + - task: PROFILEDIR=debug make -f Makefile build-no-ipc + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + components: cargo - task: PROFILEDIR=debug make -f Makefile stratisd-tools - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo - task: make -f Makefile docs-ci - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo - task: make -f Makefile test - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo - task: make -f Makefile build - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo - task: make -f Makefile build-min - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo - task: make -f Makefile build-no-ipc - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo runs-on: ubuntu-22.04 container: @@ -105,7 +111,7 @@ jobs: matrix: include: - task: RUST_LOG=stratisd=debug make -f Makefile test-loop - toolchain: 1.75.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN + toolchain: 1.76.0 # CURRENT DEVELOPMENT RUST TOOLCHAIN components: cargo runs-on: ubuntu-22.04 container: diff --git a/CHANGES.txt b/CHANGES.txt index 1076121f45..7d16584851 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,26 @@ +stratisd 3.6.5 +============== +Recommended Rust toolchain version: 1.76.0 +Recommended development platform for Python development: Fedora 39 + +* Cherry-picked commits: + * Improve readability of locking code + * Change comment + * Relax fairness restriction on AllOrSomeLock + * Fix log message in SetCreateAction + * Remove redundant remove_stratis_dm_devices() + * Remove remove_stratis_dm_devices entirely + * Set timeout of 30 sec after interrupt signal sent + * Kill stratisd process if a terminate has not worked + * Add some Python logging in test infrastructure + * Make dump_metadata module private + * create_pool(): reformat for black 24.1.1 + * github actions: update recommended Rust to 1.76.0 + * Refactor stratis-utils for better modularity and more convenient dispatch + * Build min targets without systemd enabled + * Build the -utils binary separately + + stratisd 3.6.4 ============== Recommended Rust toolchain version: 1.75.0 diff --git a/Cargo.lock b/Cargo.lock index ddca983aa1..8caddeca09 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1280,7 +1280,7 @@ checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" [[package]] name = "stratisd" -version = "3.6.4" +version = "3.6.5" dependencies = [ "assert_cmd", "assert_matches", diff --git a/Cargo.toml b/Cargo.toml index b39a90d2d3..5d1d623497 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "stratisd" -version = "3.6.4" +version = "3.6.5" authors = ["Stratis Developers "] edition = "2021" rust-version = "1.71.1" # LOWEST SUPPORTED RUST TOOLCHAIN diff --git a/Makefile b/Makefile index 0269ede4c1..438378fe7c 100644 --- a/Makefile +++ b/Makefile @@ -47,6 +47,7 @@ NO_IPC_FEATURES = --no-default-features --features engine SYSTEMD_FEATURES = --no-default-features --features engine,min,systemd_compat EXTRAS_FEATURES = --no-default-features --features engine,extras,min UDEV_FEATURES = --no-default-features --features udev_scripts +UTILS_FEATURES = --no-default-features --features engine,systemd_compat DENY = -D warnings -D future-incompatible -D unused -D rust_2018_idioms -D nonstandard_style @@ -209,14 +210,37 @@ build-tests: RUSTFLAGS="${DENY}" \ cargo test --no-run ${RELEASE_FLAG} ${TARGET_ARGS} +## Build stratis-utils only +build-utils: + PKG_CONFIG_ALLOW_CROSS=1 \ + RUSTFLAGS="${DENY}" \ + cargo ${BUILD} ${RELEASE_FLAG} \ + --bin=stratis-utils \ + ${UTILS_FEATURES} ${TARGET_ARGS} + +build-utils-no-systemd: + PKG_CONFIG_ALLOW_CROSS=1 \ + RUSTFLAGS="${DENY}" \ + cargo ${BUILD} ${RELEASE_FLAG} \ + --bin=stratis-utils \ + ${NO_IPC_FEATURES} ${TARGET_ARGS} + ## Build stratisd-min and stratis-min for early userspace build-min: PKG_CONFIG_ALLOW_CROSS=1 \ RUSTFLAGS="${DENY}" \ cargo ${BUILD} ${RELEASE_FLAG} \ - --bin=stratis-min --bin=stratisd-min --bin=stratis-utils \ + --bin=stratis-min --bin=stratisd-min \ ${SYSTEMD_FEATURES} ${TARGET_ARGS} +## Build min targets without systemd support enabled +build-min-no-systemd: + PKG_CONFIG_ALLOW_CROSS=1 \ + RUSTFLAGS="${DENY}" \ + cargo ${BUILD} ${RELEASE_FLAG} \ + --bin=stratis-min --bin=stratisd-min \ + ${MIN_FEATURES} ${TARGET_ARGS} + ## Build stratisd-min and stratis-min for early userspace build-no-ipc: PKG_CONFIG_ALLOW_CROSS=1 \ @@ -344,7 +368,7 @@ install-daemons: install: install-udev-cfg install-man-cfg install-dbus-cfg install-dracut-cfg install-systemd-cfg install-binaries install-udev-binaries install-fstab-script install-daemons ## Build all Rust artifacts -build-all-rust: build build-min build-udev-utils stratisd-tools +build-all-rust: build build-min build-utils build-udev-utils stratisd-tools ## Build all man pages build-all-man: docs/stratisd.8 docs/stratis-dumpmetadata.8 @@ -468,12 +492,16 @@ clippy-min: clippy-udev-utils: RUSTFLAGS="${DENY}" cargo clippy ${CLIPPY_OPTS} ${UDEV_FEATURES} -- ${CLIPPY_DENY} ${CLIPPY_PEDANTIC} ${CLIPPY_PEDANTIC_USELESS} +## Run clippy on the utils binary +clippy-utils: + RUSTFLAGS="${DENY}" cargo clippy ${CLIPPY_OPTS} ${UTILS_FEATURES} -- ${CLIPPY_DENY} ${CLIPPY_PEDANTIC} ${CLIPPY_PEDANTIC_USELESS} + ## Run clippy on no-ipc-build clippy-no-ipc: RUSTFLAGS="${DENY}" cargo clippy ${CLIPPY_OPTS} ${NO_IPC_FEATURES} -- ${CLIPPY_DENY} ${CLIPPY_PEDANTIC} ${CLIPPY_PEDANTIC_USELESS} ## Run clippy on the current source tree -clippy: clippy-macros clippy-min clippy-udev-utils clippy-no-ipc +clippy: clippy-macros clippy-min clippy-udev-utils clippy-no-ipc clippy-utils RUSTFLAGS="${DENY}" cargo clippy ${CLIPPY_OPTS} -- ${CLIPPY_DENY} ${CLIPPY_PEDANTIC} ${CLIPPY_PEDANTIC_USELESS} .PHONY: diff --git a/src/bin/stratis-utils.rs b/src/bin/stratis-utils.rs index fba94ba430..2127c4dc71 100644 --- a/src/bin/stratis-utils.rs +++ b/src/bin/stratis-utils.rs @@ -2,440 +2,63 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at http://mozilla.org/MPL/2.0/. -#[cfg(feature = "systemd_compat")] -mod generators; +mod utils; -use std::{ - env, - error::Error, - fmt::{self, Display}, -}; +use std::{env, error::Error, path::Path}; -use clap::{Arg, ArgAction, Command}; +use clap::{Arg, Command}; -#[cfg(feature = "systemd_compat")] -use clap::builder::Str; +use crate::utils::{cmds, ExecutableError}; -use serde_json::{json, Value}; - -use devicemapper::{Bytes, Sectors}; - -use stratisd::engine::{crypt_metadata_size, ThinPoolSizeParams, BDA}; - -#[cfg(feature = "systemd_compat")] -use crate::generators::{stratis_clevis_setup_generator, stratis_setup_generator}; - -// 2^FS_SIZE_START_POWER is the logical size of the smallest Stratis -// filesystem for which usage data exists in FSSizeLookup::internal, i.e., -// 512 MiB. -const FS_SIZE_START_POWER: usize = 29; - -const FS_SIZE_LOOKUP_TABLE_LEN: usize = 27; -const FS_LOGICAL_SIZE_MAX: u128 = 36_028_797_018_963_968; // 32 PiB -const FS_LOGICAL_SIZE_MIN: u128 = 536_870_912; // 512 MiB - -struct FSSizeLookup { - internal: Vec, -} - -impl FSSizeLookup { - /// Calculate a predicted usage for the given logical filesystem size. - /// Find the index of the entry in the table such that the logical - /// filesystem size for which the data was recorded is at least as much as - /// the logical_size argument, but no greater than twice as much, and - /// return the value. - /// The formula is: - /// predicted_size = table[(log_2(logical_size) + 1) - FS_SIZE_START_POWER] - fn lookup(&self, logical_size: Bytes) -> Sectors { - let raw_size = *logical_size; - assert!(raw_size >= FS_LOGICAL_SIZE_MIN); - assert!(raw_size < FS_LOGICAL_SIZE_MAX); - // At FS_LOGICAL_SIZE_MAX there is an 8 integer range of floating - // point value that the u128 value may occupy. Given the large size - // of the number and that it is the log that is being taken this is - // of no concern for the correctness of the value. - #[allow(clippy::cast_precision_loss)] - let lg = f64::log2(raw_size as f64); - // The value of lg is in the double decimal digits; truncation is - // impossible. - #[allow(clippy::cast_possible_truncation)] - let result = f64::floor(lg) as usize + 1; - let index = result - FS_SIZE_START_POWER; - - // The values are in Sectors, they are real values from a test - // so they must be realistic in that sense. - Bytes(self.internal[index]).sectors() - } - - // Returns a table of recorded usage for filesystems of increasing - // logical size. The first entry corresponds to the usage recorded for a - // filesystem of logical size 2^FS_SIZE_START_POWER, which is 512 MiB. - // Each subsequent value represents the usage recorded for a filesystem - // double the size of the previous one in the list. - fn new() -> Self { - let internal = vec![ - 20_971_520, - 20_971_520, - 22_020_096, - 23_068_672, - 23_068_672, - 31_457_280, - 34_603_008, - 51_380_224, - 84_934_656, - 152_043_520, - 286_261_248, - 571_473_920, - 1_108_344_832, - 2_171_600_896, - 2_171_600_896, - 2_171_600_896, - 2_171_600_896, - 2_205_155_328, - 2_273_312_768, - 2_407_530_496, - 2_675_965_952, - 3_212_836_864, - 4_286_578_688, - 6_434_062_336, - 10_729_029_632, - 19_318_964_224, - 36_498_833_408, - ]; - assert!(internal.len() == FS_SIZE_LOOKUP_TABLE_LEN); - FSSizeLookup { internal } - } -} - -#[derive(Debug)] -struct ExecutableError(String); - -impl Display for ExecutableError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.0) - } -} - -impl Error for ExecutableError {} - -// Get a prediction of filesystem size given a list of filesystem sizes and -// whether or not the pool allows overprovisioning. -fn get_filesystem_prediction( - overprovisioned: bool, - filesystem_sizes: Vec, -) -> Result> { - filesystem_sizes - .iter() - .map(|&val| { - if !(FS_LOGICAL_SIZE_MIN..FS_LOGICAL_SIZE_MAX).contains(&val) { - Err(Box::new(ExecutableError(format!( - "Specified filesystem size {val} is not within allowed limits." - )))) - } else if val.sectors().bytes() != val { - Err(Box::new(ExecutableError(format!( - "Specified filesystem size {val} is not a multiple of sector size, 512." - )))) - } else { - Ok(val) - } - }) - .collect::, _>>()?; - - Ok(if overprovisioned { - let lookup = FSSizeLookup::new(); - - filesystem_sizes.iter().map(|&sz| lookup.lookup(sz)).sum() - } else { - filesystem_sizes.iter().map(|sz| sz.sectors()).sum() - }) +fn basename(path: &str) -> Option<&Path> { + Path::new(path).file_name().map(Path::new) } -// Print predicted filesystem usage -fn predict_filesystem_usage( - overprovisioned: bool, - filesystem_sizes: Vec, -) -> Result<(), Box> { - let fs_used = get_filesystem_prediction(overprovisioned, filesystem_sizes)?; - - let used_size_str = Value::String((*(fs_used.bytes())).to_string()); - - let json = json! { - {"used": used_size_str} - }; +fn main() -> Result<(), Box> { + let executable_name = "stratis-utils"; - println!("{json}"); + let args = env::args().collect::>(); + let argv1 = &args[0]; - Ok(()) -} + let stripped_args = if basename(argv1.as_str()) + .map(|n| n == Path::new(executable_name)) + .unwrap_or(false) + { + let command = Command::new(executable_name) + .arg( + Arg::new("executable") + .required(true) + .value_name("EXECUTABLE") + .value_parser(cmds().iter().map(|x| x.name()).collect::>()), + ) + .arg_required_else_help(true); -// Predict usage for a newly created pool given information about whether -// or not the pool is encrypted, a list of device sizes, and an optional list -// of filesystem sizes. -fn predict_pool_usage( - encrypted: bool, - overprovisioned: bool, - device_sizes: Vec, - filesystem_sizes: Option>, -) -> Result<(), Box> { - let fs_used = filesystem_sizes - .map(|sizes| get_filesystem_prediction(overprovisioned, sizes)) - .transpose()?; + let truncated_args = if args.len() > 1 { + vec![argv1, &args[1]] + } else { + vec![argv1] + }; - let crypt_metadata_size = if encrypted { - crypt_metadata_size() + command.get_matches_from(truncated_args); + args[1..].to_vec() } else { - Bytes(0) + args }; - let crypt_metadata_size_sectors = crypt_metadata_size.sectors(); - - // verify that crypt metadata size is divisible by sector size - assert_eq!(crypt_metadata_size_sectors.bytes(), crypt_metadata_size); - - let device_sizes = device_sizes.iter().map(|s| s.sectors()).collect::>(); - - let stratis_device_sizes = device_sizes - .iter() - .map(|&s| { - (*s).checked_sub(*crypt_metadata_size_sectors) - .map(Sectors) - .ok_or_else(|| { - Box::new(ExecutableError( - "Some device sizes too small for encryption metadata.".into(), - )) - }) - }) - .collect::, _>>()?; - - let stratis_metadata_size = BDA::default().extended_size().sectors(); - let stratis_avail_sizes = stratis_device_sizes - .iter() - .map(|&s| { - (*s).checked_sub(*stratis_metadata_size) - .map(Sectors) - .ok_or_else(|| { - Box::new(ExecutableError( - "Some device sizes too small for Stratis metadata.".into(), - )) - }) - }) - .collect::, _>>()?; - - let total_size: Sectors = device_sizes.iter().cloned().sum(); - let non_metadata_size: Sectors = stratis_avail_sizes.iter().cloned().sum(); - - let size_params = ThinPoolSizeParams::new(non_metadata_size)?; - let total_non_data = 2usize * size_params.meta_size() + size_params.mdv_size(); - - let avail_size = (non_metadata_size) - .checked_sub(*total_non_data) - .map(Sectors) - .ok_or_else(|| { - Box::new(ExecutableError( - "Sum of all device sizes too small for a Stratis pool.".into(), - )) - })?; - - let avail_size = (*avail_size) - .checked_sub(*fs_used.unwrap_or(Sectors(0))) - .map(Sectors) - .ok_or_else(|| { - Box::new(ExecutableError( - "Filesystems will take up too much space on specified pool.".into(), - )) - })?; - - let used_size = total_size - avail_size; - - let total_size_str = Value::String((*(total_size.bytes())).to_string()); - let used_size_str = Value::String((*(used_size.bytes())).to_string()); - let avail_size_str = Value::String((*(avail_size.bytes())).to_string()); - let stratis_admin_str = Value::String((*(total_non_data.bytes())).to_string()); - let stratis_metadata_str = - Value::String((*((total_size - non_metadata_size).bytes())).to_string()); - - let json = json! { - {"total": total_size_str, "used": used_size_str, "free": avail_size_str, "stratis-admin-space": stratis_admin_str, "stratis-metadata-space": stratis_metadata_str} - }; - - println!("{json}"); - - Ok(()) -} - -// Parse arguments for stratis_predict_usage command. -fn predict_usage_parse_args() -> Command { - Command::new("stratis-predict-usage") - .about("Predicts space usage for Stratis.") - .subcommand_required(true) - .subcommands(vec![ - Command::new("pool") - .about("Predicts the space usage when creating a Stratis pool.") - .arg(Arg::new("encrypted") - .long("encrypted") - .action(ArgAction::SetTrue) - .help("Whether the pool will be encrypted."), - ) - .arg( - Arg::new("no-overprovision") - .long("no-overprovision") - .action(ArgAction::SetTrue) - .help("Indicates that the pool does not allow overprovisioning"), - ) - .arg( - Arg::new("device-size") - .long("device-size") - .num_args(1) - .action(ArgAction::Append) - .required(true) - .help("Size of device to be included in the pool. May be specified multiple times. Units are bytes.") - .next_line_help(true) - ) - .arg( - Arg::new("filesystem-size") - .long("filesystem-size") - .num_args(1) - .action(ArgAction::Append) - .help("Size of filesystem to be made for this pool. May be specified multiple times, one for each filesystem. Units are bytes. Must be at least 512 MiB and less than 4 PiB.") - .next_line_help(true) - ), - Command::new("filesystem") - .about("Predicts the space usage when creating a Stratis filesystem.") - .arg( - Arg::new("filesystem-size") - .long("filesystem-size") - .num_args(1) - .action(ArgAction::Append) - .required(true) - .help("Size of filesystem to be made for this pool. May be specified multiple times, one for each filesystem. Units are bytes. Must be at least 512 MiB and less than 4 PiB.") - .next_line_help(true) - ) - .arg( - Arg::new("no-overprovision") - .long("no-overprovision") - .action(ArgAction::SetTrue) - .help("Indicates that the pool does not allow overprovisioning"), - )] - ) -} - -// Parse arguments for stratis generator commands. -#[cfg(feature = "systemd_compat")] -fn generator_parse_args(generator: impl Into) -> Command { - Command::new(generator) - .arg( - Arg::new("normal_priority_dir") - .required(true) - .help("Directory in which to write a unit file for normal priority"), - ) - .arg( - Arg::new("early_priority_dir") - .required(true) - .help("Directory in which to write a unit file for early priority"), - ) - .arg( - Arg::new("late_priority_dir") - .required(true) - .help("Directory in which to write a unit file for late priority"), - ) -} - -/// Parse the arguments based on which hard link was accessed. -fn parse_args() -> Result<(), Box> { - let args = env::args().collect::>(); - let argv1 = args[0].as_str(); - - if argv1.ends_with("stratis-predict-usage") { - let parser = predict_usage_parse_args(); - let matches = parser.get_matches_from(&args); - match matches.subcommand() { - Some(("pool", sub_m)) => predict_pool_usage( - sub_m.get_flag("encrypted"), - !sub_m.get_flag("no-overprovision"), - sub_m - .get_many::("device-size") - .map(|szs| { - szs.map(|sz| sz.parse::().map(Bytes)) - .collect::, _>>() - }) - .expect("required argument")?, - sub_m - .get_many::("filesystem-size") - .map(|szs| { - szs.map(|sz| sz.parse::().map(Bytes)) - .collect::, _>>() - }) - .transpose()?, - )?, - Some(("filesystem", sub_m)) => predict_filesystem_usage( - !sub_m.get_flag("no-overprovision"), - sub_m - .get_many::("filesystem-size") - .map(|szs| { - szs.map(|sz| sz.parse::().map(Bytes)) - .collect::, _>>() - }) - .expect("required argument")?, - )?, - - _ => panic!("Impossible subcommand name"), - } - } else if argv1.ends_with("stratis-clevis-setup-generator") - || argv1.ends_with("stratis-setup-generator") - { - #[cfg(feature = "systemd_compat")] - if argv1.ends_with("stratis-clevis-setup-generator") { - let parser = generator_parse_args("stratis-clevis-setup-generator"); - let matches = parser.get_matches_from(&args); - stratis_clevis_setup_generator::generator( - matches - .get_one::("early_priority_dir") - .expect("required") - .to_owned(), - )?; - } else if argv1.ends_with("stratis-setup-generator") { - let parser = generator_parse_args("stratis-setup-generator"); - let matches = parser.get_matches_from(&args); - stratis_setup_generator::generator( - matches - .get_one::("early_priority_dir") - .expect("required") - .to_owned(), - )?; + let command_name = match basename(&stripped_args[0]).and_then(|n| n.to_str()) { + Some(name) => name, + None => { + return Err(Box::new(ExecutableError( + "command name does not convert to string".to_string(), + ))); } + }; - #[cfg(not(feature = "systemd_compat"))] - return Err(Box::new(ExecutableError( - "systemd compatibility disabled for this build".into(), - ))); + if let Some(c) = cmds().iter().find(|x| command_name == x.name()) { + c.run(stripped_args) } else { - return Err(Box::new(ExecutableError(format!( - "{argv1} is not a recognized executable name" - )))); - } - - Ok(()) -} - -/// This is the main method that dispatches the desired method based on the first -/// argument (the executable name). This will vary based on the hard link that was -/// invoked. -fn main() -> Result<(), Box> { - parse_args() -} - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn test_predict_usage_parse_args() { - predict_usage_parse_args().debug_assert(); - } - - #[test] - #[cfg(feature = "systemd_compat")] - fn test_generator_parse_args() { - generator_parse_args("stratis-generator").debug_assert(); + Err(Box::new(ExecutableError(format!( + "{command_name} is not a recognized executable name" + )))) } } diff --git a/src/bin/tools/mod.rs b/src/bin/tools/mod.rs index b7d2ac52d2..06f90e0b4e 100644 --- a/src/bin/tools/mod.rs +++ b/src/bin/tools/mod.rs @@ -3,6 +3,6 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. mod cmds; -pub mod dump_metadata; +mod dump_metadata; pub use cmds::cmds; diff --git a/src/bin/utils/cmds.rs b/src/bin/utils/cmds.rs new file mode 100644 index 0000000000..3aed22a87d --- /dev/null +++ b/src/bin/utils/cmds.rs @@ -0,0 +1,240 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +use std::{ + error::Error, + fmt::{self, Display}, +}; + +use clap::{Arg, ArgAction, Command}; + +#[cfg(feature = "systemd_compat")] +use clap::builder::Str; + +use devicemapper::Bytes; + +use crate::utils::predict_usage; + +#[cfg(feature = "systemd_compat")] +use crate::utils::generators::{stratis_clevis_setup_generator, stratis_setup_generator}; + +#[derive(Debug)] +pub struct ExecutableError(pub String); + +impl Display for ExecutableError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} + +impl Error for ExecutableError {} + +pub trait UtilCommand<'a> { + fn name(&self) -> &'a str; + fn run(&self, command_line_args: Vec) -> Result<(), Box>; +} + +struct StratisPredictUsage; + +impl StratisPredictUsage { + fn cmd() -> Command { + Command::new("stratis-predict-usage") + .about("Predicts space usage for Stratis.") + .subcommand_required(true) + .subcommands(vec![ + Command::new("pool") + .about("Predicts the space usage when creating a Stratis pool.") + .arg(Arg::new("encrypted") + .long("encrypted") + .action(ArgAction::SetTrue) + .help("Whether the pool will be encrypted."), + ) + .arg( + Arg::new("no-overprovision") + .long("no-overprovision") + .action(ArgAction::SetTrue) + .help("Indicates that the pool does not allow overprovisioning"), + ) + .arg( + Arg::new("device-size") + .long("device-size") + .num_args(1) + .action(ArgAction::Append) + .required(true) + .help("Size of device to be included in the pool. May be specified multiple times. Units are bytes.") + .next_line_help(true) + ) + .arg( + Arg::new("filesystem-size") + .long("filesystem-size") + .num_args(1) + .action(ArgAction::Append) + .help("Size of filesystem to be made for this pool. May be specified multiple times, one for each filesystem. Units are bytes. Must be at least 512 MiB and less than 4 PiB.") + .next_line_help(true) + ), + Command::new("filesystem") + .about("Predicts the space usage when creating a Stratis filesystem.") + .arg( + Arg::new("filesystem-size") + .long("filesystem-size") + .num_args(1) + .action(ArgAction::Append) + .required(true) + .help("Size of filesystem to be made for this pool. May be specified multiple times, one for each filesystem. Units are bytes. Must be at least 512 MiB and less than 4 PiB.") + .next_line_help(true) + ) + .arg( + Arg::new("no-overprovision") + .long("no-overprovision") + .action(ArgAction::SetTrue) + .help("Indicates that the pool does not allow overprovisioning"), + )] + ) + } +} + +impl<'a> UtilCommand<'a> for StratisPredictUsage { + fn name(&self) -> &'a str { + "stratis-predict-usage" + } + + fn run(&self, command_line_args: Vec) -> Result<(), Box> { + let matches = StratisPredictUsage::cmd().get_matches_from(command_line_args); + match matches.subcommand() { + Some(("pool", sub_m)) => predict_usage::predict_pool_usage( + sub_m.get_flag("encrypted"), + !sub_m.get_flag("no-overprovision"), + sub_m + .get_many::("device-size") + .map(|szs| { + szs.map(|sz| sz.parse::().map(Bytes)) + .collect::, _>>() + }) + .expect("required argument")?, + sub_m + .get_many::("filesystem-size") + .map(|szs| { + szs.map(|sz| sz.parse::().map(Bytes)) + .collect::, _>>() + }) + .transpose()?, + ), + Some(("filesystem", sub_m)) => predict_usage::predict_filesystem_usage( + !sub_m.get_flag("no-overprovision"), + sub_m + .get_many::("filesystem-size") + .map(|szs| { + szs.map(|sz| sz.parse::().map(Bytes)) + .collect::, _>>() + }) + .expect("required argument")?, + ), + _ => unreachable!("Impossible subcommand name"), + } + } +} + +#[cfg(feature = "systemd_compat")] +fn stratis_setup_generator_cmd(generator: impl Into) -> Command { + Command::new(generator) + .arg( + Arg::new("normal_priority_dir") + .required(true) + .help("Directory in which to write a unit file for normal priority"), + ) + .arg( + Arg::new("early_priority_dir") + .required(true) + .help("Directory in which to write a unit file for early priority"), + ) + .arg( + Arg::new("late_priority_dir") + .required(true) + .help("Directory in which to write a unit file for late priority"), + ) +} + +struct StratisSetupGenerator; + +impl<'a> UtilCommand<'a> for StratisSetupGenerator { + fn name(&self) -> &'a str { + "stratis-setup-generator" + } + + #[cfg(feature = "systemd_compat")] + fn run(&self, command_line_args: Vec) -> Result<(), Box> { + let matches = stratis_setup_generator_cmd("stratis-setup-generator") + .get_matches_from(command_line_args); + + stratis_setup_generator::generator( + matches + .get_one::("early_priority_dir") + .expect("required") + .to_owned(), + ) + } + + #[cfg(not(feature = "systemd_compat"))] + fn run(&self, _command_line_args: Vec) -> Result<(), Box> { + Err(Box::new(ExecutableError( + "systemd compatibility disabled for this build".into(), + ))) + } +} + +struct StratisClevisSetupGenerator; + +impl<'a> UtilCommand<'a> for StratisClevisSetupGenerator { + fn name(&self) -> &'a str { + "stratis-clevis-setup-generator" + } + + #[cfg(feature = "systemd_compat")] + fn run(&self, command_line_args: Vec) -> Result<(), Box> { + let matches = stratis_setup_generator_cmd("stratis-clevis-setup-generator") + .get_matches_from(command_line_args); + + stratis_clevis_setup_generator::generator( + matches + .get_one::("early_priority_dir") + .expect("required") + .to_owned(), + ) + } + + #[cfg(not(feature = "systemd_compat"))] + fn run(&self, _command_line_args: Vec) -> Result<(), Box> { + Err(Box::new(ExecutableError( + "systemd compatibility disabled for this build".into(), + ))) + } +} + +pub fn cmds<'a>() -> Vec>> { + vec![ + Box::new(StratisPredictUsage), + Box::new(StratisSetupGenerator), + Box::new(StratisClevisSetupGenerator), + ] +} + +#[cfg(test)] +mod tests { + + use super::StratisPredictUsage; + + #[cfg(feature = "systemd_compat")] + use super::stratis_setup_generator_cmd; + + #[test] + fn test_predictusage_parse_args() { + StratisPredictUsage::cmd().debug_assert(); + } + + #[test] + #[cfg(feature = "systemd_compat")] + fn test_generator_parse_args() { + stratis_setup_generator_cmd("stratis-generator").debug_assert(); + } +} diff --git a/src/bin/generators/lib.rs b/src/bin/utils/generators/lib.rs similarity index 100% rename from src/bin/generators/lib.rs rename to src/bin/utils/generators/lib.rs diff --git a/src/bin/generators/mod.rs b/src/bin/utils/generators/mod.rs similarity index 100% rename from src/bin/generators/mod.rs rename to src/bin/utils/generators/mod.rs diff --git a/src/bin/generators/stratis_clevis_setup_generator.rs b/src/bin/utils/generators/stratis_clevis_setup_generator.rs similarity index 100% rename from src/bin/generators/stratis_clevis_setup_generator.rs rename to src/bin/utils/generators/stratis_clevis_setup_generator.rs diff --git a/src/bin/generators/stratis_setup_generator.rs b/src/bin/utils/generators/stratis_setup_generator.rs similarity index 100% rename from src/bin/generators/stratis_setup_generator.rs rename to src/bin/utils/generators/stratis_setup_generator.rs diff --git a/src/bin/utils/mod.rs b/src/bin/utils/mod.rs new file mode 100644 index 0000000000..43c9be897e --- /dev/null +++ b/src/bin/utils/mod.rs @@ -0,0 +1,10 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +mod cmds; +#[cfg(feature = "systemd_compat")] +mod generators; +mod predict_usage; + +pub use cmds::{cmds, ExecutableError}; diff --git a/src/bin/utils/predict_usage.rs b/src/bin/utils/predict_usage.rs new file mode 100644 index 0000000000..8ac63628b9 --- /dev/null +++ b/src/bin/utils/predict_usage.rs @@ -0,0 +1,252 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +use std::{ + error::Error, + fmt::{self, Display}, +}; + +use serde_json::{json, Value}; + +use devicemapper::{Bytes, Sectors}; + +use stratisd::engine::{crypt_metadata_size, ThinPoolSizeParams, BDA}; + +// 2^FS_SIZE_START_POWER is the logical size of the smallest Stratis +// filesystem for which usage data exists in FSSizeLookup::internal, i.e., +// 512 MiB. +const FS_SIZE_START_POWER: usize = 29; + +const FS_SIZE_LOOKUP_TABLE_LEN: usize = 27; +const FS_LOGICAL_SIZE_MAX: u128 = 36_028_797_018_963_968; // 32 PiB +const FS_LOGICAL_SIZE_MIN: u128 = 536_870_912; // 512 MiB + +struct FSSizeLookup { + internal: Vec, +} + +impl FSSizeLookup { + /// Calculate a predicted usage for the given logical filesystem size. + /// Find the index of the entry in the table such that the logical + /// filesystem size for which the data was recorded is at least as much as + /// the logical_size argument, but no greater than twice as much, and + /// return the value. + /// The formula is: + /// predicted_size = table[(log_2(logical_size) + 1) - FS_SIZE_START_POWER] + fn lookup(&self, logical_size: Bytes) -> Sectors { + let raw_size = *logical_size; + assert!(raw_size >= FS_LOGICAL_SIZE_MIN); + assert!(raw_size < FS_LOGICAL_SIZE_MAX); + // At FS_LOGICAL_SIZE_MAX there is an 8 integer range of floating + // point value that the u128 value may occupy. Given the large size + // of the number and that it is the log that is being taken this is + // of no concern for the correctness of the value. + #[allow(clippy::cast_precision_loss)] + let lg = f64::log2(raw_size as f64); + // The value of lg is in the double decimal digits; truncation is + // impossible. + #[allow(clippy::cast_possible_truncation)] + let result = f64::floor(lg) as usize + 1; + let index = result - FS_SIZE_START_POWER; + + // The values are in Sectors, they are real values from a test + // so they must be realistic in that sense. + Bytes(self.internal[index]).sectors() + } + + // Returns a table of recorded usage for filesystems of increasing + // logical size. The first entry corresponds to the usage recorded for a + // filesystem of logical size 2^FS_SIZE_START_POWER, which is 512 MiB. + // Each subsequent value represents the usage recorded for a filesystem + // double the size of the previous one in the list. + fn new() -> Self { + let internal = vec![ + 20_971_520, + 20_971_520, + 22_020_096, + 23_068_672, + 23_068_672, + 31_457_280, + 34_603_008, + 51_380_224, + 84_934_656, + 152_043_520, + 286_261_248, + 571_473_920, + 1_108_344_832, + 2_171_600_896, + 2_171_600_896, + 2_171_600_896, + 2_171_600_896, + 2_205_155_328, + 2_273_312_768, + 2_407_530_496, + 2_675_965_952, + 3_212_836_864, + 4_286_578_688, + 6_434_062_336, + 10_729_029_632, + 19_318_964_224, + 36_498_833_408, + ]; + assert!(internal.len() == FS_SIZE_LOOKUP_TABLE_LEN); + FSSizeLookup { internal } + } +} + +#[derive(Debug)] +struct PredictError(String); + +impl Display for PredictError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} + +impl Error for PredictError {} + +// Get a prediction of filesystem size given a list of filesystem sizes and +// whether or not the pool allows overprovisioning. +fn get_filesystem_prediction( + overprovisioned: bool, + filesystem_sizes: Vec, +) -> Result> { + filesystem_sizes + .iter() + .map(|&val| { + if !(FS_LOGICAL_SIZE_MIN..FS_LOGICAL_SIZE_MAX).contains(&val) { + Err(Box::new(PredictError(format!( + "Specified filesystem size {val} is not within allowed limits." + )))) + } else if val.sectors().bytes() != val { + Err(Box::new(PredictError(format!( + "Specified filesystem size {val} is not a multiple of sector size, 512." + )))) + } else { + Ok(val) + } + }) + .collect::, _>>()?; + + Ok(if overprovisioned { + let lookup = FSSizeLookup::new(); + + filesystem_sizes.iter().map(|&sz| lookup.lookup(sz)).sum() + } else { + filesystem_sizes.iter().map(|sz| sz.sectors()).sum() + }) +} + +// Print predicted filesystem usage +pub fn predict_filesystem_usage( + overprovisioned: bool, + filesystem_sizes: Vec, +) -> Result<(), Box> { + let fs_used = get_filesystem_prediction(overprovisioned, filesystem_sizes)?; + + let used_size_str = Value::String((*(fs_used.bytes())).to_string()); + + let json = json! { + {"used": used_size_str} + }; + + println!("{json}"); + + Ok(()) +} + +// Predict usage for a newly created pool given information about whether +// or not the pool is encrypted, a list of device sizes, and an optional list +// of filesystem sizes. +pub fn predict_pool_usage( + encrypted: bool, + overprovisioned: bool, + device_sizes: Vec, + filesystem_sizes: Option>, +) -> Result<(), Box> { + let fs_used = filesystem_sizes + .map(|sizes| get_filesystem_prediction(overprovisioned, sizes)) + .transpose()?; + + let crypt_metadata_size = if encrypted { + crypt_metadata_size() + } else { + Bytes(0) + }; + + let crypt_metadata_size_sectors = crypt_metadata_size.sectors(); + + // verify that crypt metadata size is divisible by sector size + assert_eq!(crypt_metadata_size_sectors.bytes(), crypt_metadata_size); + + let device_sizes = device_sizes.iter().map(|s| s.sectors()).collect::>(); + + let stratis_device_sizes = device_sizes + .iter() + .map(|&s| { + (*s).checked_sub(*crypt_metadata_size_sectors) + .map(Sectors) + .ok_or_else(|| { + Box::new(PredictError( + "Some device sizes too small for encryption metadata.".into(), + )) + }) + }) + .collect::, _>>()?; + + let stratis_metadata_size = BDA::default().extended_size().sectors(); + let stratis_avail_sizes = stratis_device_sizes + .iter() + .map(|&s| { + (*s).checked_sub(*stratis_metadata_size) + .map(Sectors) + .ok_or_else(|| { + Box::new(PredictError( + "Some device sizes too small for Stratis metadata.".into(), + )) + }) + }) + .collect::, _>>()?; + + let total_size: Sectors = device_sizes.iter().cloned().sum(); + let non_metadata_size: Sectors = stratis_avail_sizes.iter().cloned().sum(); + + let size_params = ThinPoolSizeParams::new(non_metadata_size)?; + let total_non_data = 2usize * size_params.meta_size() + size_params.mdv_size(); + + let avail_size = (non_metadata_size) + .checked_sub(*total_non_data) + .map(Sectors) + .ok_or_else(|| { + Box::new(PredictError( + "Sum of all device sizes too small for a Stratis pool.".into(), + )) + })?; + + let avail_size = (*avail_size) + .checked_sub(*fs_used.unwrap_or(Sectors(0))) + .map(Sectors) + .ok_or_else(|| { + Box::new(PredictError( + "Filesystems will take up too much space on specified pool.".into(), + )) + })?; + + let used_size = total_size - avail_size; + + let total_size_str = Value::String((*(total_size.bytes())).to_string()); + let used_size_str = Value::String((*(used_size.bytes())).to_string()); + let avail_size_str = Value::String((*(avail_size.bytes())).to_string()); + let stratis_admin_str = Value::String((*(total_non_data.bytes())).to_string()); + let stratis_metadata_str = + Value::String((*((total_size - non_metadata_size).bytes())).to_string()); + + let json = json! { + {"total": total_size_str, "used": used_size_str, "free": avail_size_str, "stratis-admin-space": stratis_admin_str, "stratis-metadata-space": stratis_metadata_str} + }; + + println!("{json}"); + + Ok(()) +} diff --git a/src/engine/structures/lock.rs b/src/engine/structures/lock.rs index 7a63bf473e..a849a28b9b 100644 --- a/src/engine/structures/lock.rs +++ b/src/engine/structures/lock.rs @@ -146,6 +146,9 @@ where { /// * Asserts that tasks performing an actions either are performing an action immediately /// after being spawned or are in the list of woken tasks. + /// + /// NOTE: This method has the side effect of clearing a woken waiter if it is the waiter that + /// is currently acquiring the lock. fn woken_or_new(&mut self, wait_type: Option<&WaitType>, idx: u64) { if self.woken.contains_key(&idx) { let woken = self.woken.remove(&idx); @@ -159,7 +162,10 @@ where /// after being spawned or are in the list of woken tasks. /// * Asserts that the current task never conflicts with tasks that have been woken but /// not processed yet. - fn assert(&mut self, wait_type: &WaitType, idx: u64) { + /// + /// NOTE: This method has the side effect of clearing a woken waiter if it is the waiter that + /// is currently acquiring the lock. + fn pre_acquire_assertion(&mut self, wait_type: &WaitType, idx: u64) { self.woken_or_new(Some(wait_type), idx); assert!(!self.conflicts_with_woken(wait_type)); } @@ -176,7 +182,7 @@ where } if let Some(i) = idx { - self.assert(&WaitType::SomeRead(uuid), i); + self.pre_acquire_assertion(&WaitType::SomeRead(uuid), i); } trace!("Lock record after acquisition: {}", self); @@ -201,7 +207,7 @@ where self.write_locked.insert(uuid); if let Some(i) = idx { - self.assert(&WaitType::SomeWrite(uuid), i); + self.pre_acquire_assertion(&WaitType::SomeWrite(uuid), i); } trace!("Lock record after acquisition: {}", self); @@ -218,7 +224,7 @@ where fn add_read_all_lock(&mut self, idx: u64) { self.all_read_locked += 1; - self.assert(&WaitType::AllRead, idx); + self.pre_acquire_assertion(&WaitType::AllRead, idx); trace!("Lock record after acquisition: {}", self); } @@ -237,7 +243,7 @@ where fn add_write_all_lock(&mut self, idx: u64) { self.all_write_locked = true; - self.assert(&WaitType::AllWrite, idx); + self.pre_acquire_assertion(&WaitType::AllWrite, idx); trace!("Lock record after acquisition: {}", self); } @@ -286,8 +292,7 @@ where /// Returns true if the current request should be put in the wait queue. /// * Always returns false if the index for the given request is in the record of woken /// tasks. - /// * Otherwise, returns true if any of the following conditions are met: - /// * There are already tasks waiting in the queue. + /// * Otherwise, returns true if either of the following conditions are met: /// * The lock already has a conflicting acquisition. /// * The request conflicts with any tasks that have already been woken up. fn should_wait(&self, ty: &WaitType, idx: u64) -> bool { @@ -299,9 +304,7 @@ where ); false } else { - let should_wait = !self.waiting.is_empty() - || self.already_acquired(ty) - || self.conflicts_with_woken(ty); + let should_wait = self.already_acquired(ty) || self.conflicts_with_woken(ty); if should_wait { trace!( "Putting task with index {}, wait type {:?} to sleep", @@ -366,25 +369,17 @@ where } } - /// Determines whether a task should be woken up from the queue. - /// Returns true if: - /// * The waiting task does not conflict with any already woken tasks. - /// * The waiting task does not conflict with any locks currently held. - fn should_wake(&self) -> bool { - if let Some(w) = self.waiting.front() { - !self.conflicts_with_woken(&w.ty) && !self.already_acquired(&w.ty) - } else { - false - } - } - - /// Wake all non-conflicting tasks in the queue and stop at the first conflicting task. + /// Wake all non-conflicting tasks in the queue. /// Adds all woken tasks to the record of woken tasks. fn wake(&mut self) { - while self.should_wake() { - if let Some(w) = self.waiting.pop_front() { - self.woken.insert(w.idx, w.ty); - w.waker.wake(); + let mut waiting = VecDeque::new(); + std::mem::swap(&mut waiting, &mut self.waiting); + for waiter in waiting.drain(..) { + if !self.conflicts_with_woken(&waiter.ty) && !self.already_acquired(&waiter.ty) { + self.woken.insert(waiter.idx, waiter.ty); + waiter.waker.wake(); + } else { + self.waiting.push_back(waiter); } } } diff --git a/src/engine/types/actions.rs b/src/engine/types/actions.rs index 1e029748d2..02e3dc9683 100644 --- a/src/engine/types/actions.rs +++ b/src/engine/types/actions.rs @@ -359,7 +359,7 @@ impl Display for SetCreateAction { if self.changed.is_empty() { write!( f, - "The requested filesystems already exist; no action taken" + "The specified devices already belong to the pool; no action taken" ) } else { write!( @@ -574,7 +574,7 @@ impl Display for DeleteAction { } } -/// An action which may create multiple things. +/// An action which may delete multiple things. #[derive(Debug, PartialEq, Eq)] pub struct SetDeleteAction { changed: Vec, diff --git a/tests/client-dbus/tests/udev/_dm.py b/tests/client-dbus/tests/udev/_dm.py index f99d83da77..a71168348c 100644 --- a/tests/client-dbus/tests/udev/_dm.py +++ b/tests/client-dbus/tests/udev/_dm.py @@ -63,6 +63,9 @@ def remove_stratis_setup(): if _remove_device(dev): devices.remove(dev) + if _get_stratis_devices() != []: + raise RuntimeError("Some devices were not removed") + if __name__ == "__main__": remove_stratis_setup() diff --git a/tests/client-dbus/tests/udev/_utils.py b/tests/client-dbus/tests/udev/_utils.py index 88a5067f64..cc8548f9f8 100644 --- a/tests/client-dbus/tests/udev/_utils.py +++ b/tests/client-dbus/tests/udev/_utils.py @@ -16,6 +16,7 @@ """ # isort: STDLIB +import logging import os import random import signal @@ -43,7 +44,7 @@ ) from stratisd_client_dbus._constants import TOP_OBJECT -from ._dm import _get_stratis_devices, remove_stratis_setup +from ._dm import remove_stratis_setup from ._loopback import LoopBackDevices _STRATISD = os.environ["STRATISD"] @@ -81,12 +82,12 @@ def create_pool( { "name": name, "devices": devices, - "key_desc": (False, "") - if key_description is None - else (True, key_description), - "clevis_info": (False, ("", "")) - if clevis_info is None - else (True, clevis_info), + "key_desc": ( + (False, "") if key_description is None else (True, key_description) + ), + "clevis_info": ( + (False, ("", "")) if clevis_info is None else (True, clevis_info) + ), }, ) @@ -232,17 +233,6 @@ def processes(name): pass -def remove_stratis_dm_devices(): - """ - Remove Stratis device mapper devices, fail with a runtime error if - some have been missed. - :raises RuntimeError: if some devices are remaining - """ - remove_stratis_setup() - if _get_stratis_devices() != []: - raise RuntimeError("Some devices were not removed") - - class _Service: """ Start and stop stratisd. @@ -297,7 +287,7 @@ def stop_service(self): :return: None """ self._service.send_signal(signal.SIGINT) - self._service.wait() + self._service.wait(timeout=30) if next(processes("stratisd"), None) is not None: raise RuntimeError("Failed to stop stratisd service") @@ -426,10 +416,17 @@ def _clean_up(self): """ stratisds = list(processes("stratisd")) for process in stratisds: + logging.warning("stratisd process %s still running, terminating", process) process.terminate() - psutil.wait_procs(stratisds) + (_, alive) = psutil.wait_procs(stratisds, timeout=10) + for process in alive: + logging.warning( + "stratisd process %s did not respond to terminate signal, killing", + process, + ) + process.kill() - remove_stratis_dm_devices() + remove_stratis_setup() self._lb_mgr.destroy_all() def wait_for_pools(self, expected_num, *, name=None): diff --git a/tests/client-dbus/tests/udev/test_udev.py b/tests/client-dbus/tests/udev/test_udev.py index 0bb3685041..caf8564508 100644 --- a/tests/client-dbus/tests/udev/test_udev.py +++ b/tests/client-dbus/tests/udev/test_udev.py @@ -16,6 +16,7 @@ """ # isort: STDLIB +import logging import random # isort: LOCAL @@ -23,6 +24,7 @@ from stratisd_client_dbus._constants import TOP_OBJECT from stratisd_client_dbus._stratisd_constants import EncryptionMethod +from ._dm import remove_stratis_setup from ._loopback import UDEV_ADD_EVENT, UDEV_REMOVE_EVENT from ._utils import ( CRYPTO_LUKS_FS_TYPE, @@ -33,12 +35,13 @@ create_pool, get_devnodes, random_string, - remove_stratis_dm_devices, settle, wait_for_udev, wait_for_udev_count, ) +logging.basicConfig(level=logging.INFO) + class UdevTest1(UdevTest): """ @@ -79,7 +82,7 @@ def _test_driver(self, number_of_pools, dev_count_pool): create_pool(pool_name, self._lb_mgr.device_files(device_tokens)) pool_data[pool_name] = device_tokens - remove_stratis_dm_devices() + remove_stratis_setup() all_tokens = [ dev for device_tokens in pool_data.values() for dev in device_tokens @@ -91,7 +94,7 @@ def _test_driver(self, number_of_pools, dev_count_pool): with ServiceContextManager(): self.wait_for_pools(number_of_pools) - remove_stratis_dm_devices() + remove_stratis_setup() self._lb_mgr.unplug(all_tokens) @@ -124,8 +127,6 @@ def _test_driver(self, number_of_pools, dev_count_pool): for name in pool_data: self.wait_for_pools(1, name=name) - remove_stratis_dm_devices() - def test_generic(self): """ See _test_driver for description. @@ -170,12 +171,12 @@ def _single_pool(self, num_devices, *, num_hotplugs=0): self.assertEqual(len(device_object_paths), len(devnodes)) wait_for_udev(STRATIS_FS_TYPE, get_devnodes(device_object_paths)) - remove_stratis_dm_devices() + remove_stratis_setup() with ServiceContextManager(): self.wait_for_pools(1) - remove_stratis_dm_devices() + remove_stratis_setup() self._lb_mgr.unplug(device_tokens) @@ -199,8 +200,6 @@ def _single_pool(self, num_devices, *, num_hotplugs=0): self.wait_for_pools(1) - remove_stratis_dm_devices() - def test_simultaneous(self): """ See documentation for _single_pool. @@ -259,7 +258,7 @@ def _simple_initial_discovery_test( pool_uuid = Pool.Properties.Uuid.Get(get_object(pool_object_path)) if take_down_dm: - remove_stratis_dm_devices() + remove_stratis_setup() with OptionalKeyServiceContextManager(key_spec=key_spec): ((changed, _), exit_code, _) = Manager.Methods.StartPool( @@ -281,8 +280,6 @@ def _simple_initial_discovery_test( self.wait_for_pools(1) - remove_stratis_dm_devices() - def test_encryption_simple_initial_discovery(self): """ See documentation for _simple_initial_discovery_test. @@ -346,7 +343,7 @@ def _simple_event_test(self, *, key_spec=None): # pylint: disable=too-many-loca self.wait_for_pools(1) pool_uuid = Pool.Properties.Uuid.Get(get_object(pool_object_path)) - remove_stratis_dm_devices() + remove_stratis_setup() self._lb_mgr.unplug(device_tokens) wait_for_udev(udev_wait_type, []) @@ -404,8 +401,6 @@ def _simple_event_test(self, *, key_spec=None): # pylint: disable=too-many-loca self.wait_for_pools(1) - remove_stratis_dm_devices() - def test_simple_event(self): """ See documentation for _simple_event_test. @@ -474,7 +469,7 @@ def test_duplicate_pool_name( pool_tokens.append(this_pool) - remove_stratis_dm_devices() + remove_stratis_setup() self._lb_mgr.unplug(this_pool) @@ -557,8 +552,6 @@ def test_duplicate_pool_name( self.wait_for_pools(num_pools) - remove_stratis_dm_devices() - class UdevTest6(UdevTest): """ @@ -602,7 +595,7 @@ def _simple_stop_test(self): self.wait_for_pools(2) - remove_stratis_dm_devices() + remove_stratis_setup() with OptionalKeyServiceContextManager(): self.wait_for_pools(2) @@ -626,7 +619,7 @@ def _simple_stop_test(self): 1, ) - remove_stratis_dm_devices() + remove_stratis_setup() with OptionalKeyServiceContextManager(): self.wait_for_pools(1)