diff --git a/Cargo.lock b/Cargo.lock index 6da6054f6a..636a78eac4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10378,7 +10378,6 @@ dependencies = [ "omicron-test-utils", "omicron-uuid-kinds", "omicron-workspace-hack", - "once_cell", "rand", "schemars", "serde", diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 5ca1c87f01..05129c689b 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -627,31 +627,31 @@ const GiB: u64 = MiB * 1024; const TiB: u64 = GiB * 1024; impl ByteCount { - pub fn from_kibibytes_u32(kibibytes: u32) -> ByteCount { - ByteCount::try_from(KiB * u64::from(kibibytes)).unwrap() + // None of these three constructors can create a value larger than + // `i64::MAX`. (Note that a `from_tebibytes_u32` could overflow u64.) + pub const fn from_kibibytes_u32(kibibytes: u32) -> ByteCount { + ByteCount(KiB * kibibytes as u64) } - - pub fn from_mebibytes_u32(mebibytes: u32) -> ByteCount { - ByteCount::try_from(MiB * u64::from(mebibytes)).unwrap() + pub const fn from_mebibytes_u32(mebibytes: u32) -> ByteCount { + ByteCount(MiB * mebibytes as u64) } - - pub fn from_gibibytes_u32(gibibytes: u32) -> ByteCount { - ByteCount::try_from(GiB * u64::from(gibibytes)).unwrap() + pub const fn from_gibibytes_u32(gibibytes: u32) -> ByteCount { + ByteCount(GiB * gibibytes as u64) } - pub fn to_bytes(&self) -> u64 { + pub const fn to_bytes(&self) -> u64 { self.0 } - pub fn to_whole_kibibytes(&self) -> u64 { + pub const fn to_whole_kibibytes(&self) -> u64 { self.to_bytes() / KiB } - pub fn to_whole_mebibytes(&self) -> u64 { + pub const fn to_whole_mebibytes(&self) -> u64 { self.to_bytes() / MiB } - pub fn to_whole_gibibytes(&self) -> u64 { + pub const fn to_whole_gibibytes(&self) -> u64 { self.to_bytes() / GiB } - pub fn to_whole_tebibytes(&self) -> u64 { + pub const fn to_whole_tebibytes(&self) -> u64 { self.to_bytes() / TiB } } @@ -3254,6 +3254,21 @@ mod test { let bogus = ByteCount::try_from(i64::MIN).unwrap_err(); assert_eq!(bogus.to_string(), "value is too small for a byte count"); + // The largest input value to the `from_*_u32` methods do not create + // a value larger than i64::MAX. + assert!( + ByteCount::from_kibibytes_u32(u32::MAX).to_bytes() + <= u64::try_from(i64::MAX).unwrap() + ); + assert!( + ByteCount::from_mebibytes_u32(u32::MAX).to_bytes() + <= u64::try_from(i64::MAX).unwrap() + ); + assert!( + ByteCount::from_gibibytes_u32(u32::MAX).to_bytes() + <= u64::try_from(i64::MAX).unwrap() + ); + // We've now exhaustively tested both sides of all boundary conditions // for all three constructors (to the extent that that's possible). // Check non-trivial cases for the various accessor functions. This diff --git a/sled-agent/src/backing_fs.rs b/sled-agent/src/backing_fs.rs index 1b1e105a28..265c0152e2 100644 --- a/sled-agent/src/backing_fs.rs +++ b/sled-agent/src/backing_fs.rs @@ -27,7 +27,6 @@ use illumos_utils::zfs::{ }; use omicron_common::api::external::ByteCount; use omicron_common::disk::CompressionAlgorithm; -use once_cell::sync::Lazy; use std::io; #[derive(Debug, thiserror::Error)] @@ -102,19 +101,18 @@ const BACKING_FMD_DATASET: &'static str = "fmd"; const BACKING_FMD_MOUNTPOINT: &'static str = "/var/fm/fmd"; const BACKING_FMD_SUBDIRS: [&'static str; 3] = ["rsrc", "ckpt", "xprt"]; const BACKING_FMD_SERVICE: &'static str = "svc:/system/fmd:default"; -const BACKING_FMD_QUOTA: u64 = 500 * (1 << 20); // 500 MiB +const BACKING_FMD_QUOTA: ByteCount = ByteCount::from_mebibytes_u32(500); const BACKING_COMPRESSION: CompressionAlgorithm = CompressionAlgorithm::On; const BACKINGFS_COUNT: usize = 1; -static BACKINGFS: Lazy<[BackingFs; BACKINGFS_COUNT]> = Lazy::new(|| { +const BACKINGFS: [BackingFs; BACKINGFS_COUNT] = [BackingFs::new(BACKING_FMD_DATASET) .mountpoint(BACKING_FMD_MOUNTPOINT) .subdirs(&BACKING_FMD_SUBDIRS) - .quota(ByteCount::try_from(BACKING_FMD_QUOTA).unwrap()) + .quota(BACKING_FMD_QUOTA) .compression(BACKING_COMPRESSION) - .service(BACKING_FMD_SERVICE)] -}); + .service(BACKING_FMD_SERVICE)]; /// Ensure that the backing filesystems are mounted. /// If the underlying dataset for a backing fs does not exist on the specified diff --git a/sled-agent/src/zone_bundle.rs b/sled-agent/src/zone_bundle.rs index 88cb853652..7a66ca088f 100644 --- a/sled-agent/src/zone_bundle.rs +++ b/sled-agent/src/zone_bundle.rs @@ -1733,8 +1733,6 @@ mod illumos_tests { use chrono::TimeZone; use chrono::Timelike; use chrono::Utc; - use omicron_common::api::external::ByteCount; - use once_cell::sync::Lazy; use rand::RngCore; use sled_storage::manager_test_harness::StorageManagerTestHarness; use slog::Drain; @@ -1921,9 +1919,7 @@ mod illumos_tests { // i.e., the "ashift" value. An empty dataset is unlikely to contain more // than one megabyte of overhead, so use that as a conservative test size to // avoid issues. - static TEST_QUOTA: Lazy = Lazy::new(|| { - sled_storage::dataset::DEBUG_DATASET_QUOTA.try_into().unwrap() - }); + use sled_storage::dataset::DEBUG_DATASET_QUOTA as TEST_QUOTA; async fn run_test_with_zfs_dataset(test: T) where @@ -1971,7 +1967,7 @@ mod illumos_tests { // If this needs to change, go modify the "add_vdevs" call in // "setup_storage". assert!( - *TEST_QUOTA + TEST_QUOTA < StorageManagerTestHarness::DEFAULT_VDEV_SIZE .try_into() .unwrap(), diff --git a/sled-storage/Cargo.toml b/sled-storage/Cargo.toml index 27555ce96d..2439c52aa7 100644 --- a/sled-storage/Cargo.toml +++ b/sled-storage/Cargo.toml @@ -20,7 +20,6 @@ illumos-utils.workspace = true key-manager.workspace = true omicron-common.workspace = true omicron-uuid-kinds.workspace = true -once_cell.workspace = true rand.workspace = true schemars = { workspace = true, features = [ "chrono", "uuid1" ] } serde.workspace = true diff --git a/sled-storage/src/dataset.rs b/sled-storage/src/dataset.rs index 3be7b0afa4..a715a33a69 100644 --- a/sled-storage/src/dataset.rs +++ b/sled-storage/src/dataset.rs @@ -7,7 +7,6 @@ use crate::config::MountConfig; use crate::keyfile::KeyFile; use camino::Utf8PathBuf; -use cfg_if::cfg_if; use illumos_utils::zfs::{ self, DestroyDatasetErrorVariant, EncryptionDetails, Keypath, Mountpoint, SizeDetails, Zfs, @@ -19,7 +18,6 @@ use omicron_common::api::internal::shared::DatasetKind; use omicron_common::disk::{ CompressionAlgorithm, DatasetName, DiskIdentity, DiskVariant, GzipLevel, }; -use once_cell::sync::Lazy; use rand::distributions::{Alphanumeric, DistString}; use slog::{debug, info, Logger}; use std::process::Stdio; @@ -33,19 +31,18 @@ pub const CONFIG_DATASET: &'static str = "config"; pub const M2_DEBUG_DATASET: &'static str = "debug"; pub const M2_BACKING_DATASET: &'static str = "backing"; -cfg_if! { - if #[cfg(any(test, feature = "testing"))] { +pub const DEBUG_DATASET_QUOTA: ByteCount = + if cfg!(any(test, feature = "testing")) { // Tuned for zone_bundle tests - pub const DEBUG_DATASET_QUOTA: u64 = 1 << 20; + ByteCount::from_mebibytes_u32(1) } else { - // TODO-correctness: This value of 100GiB is a pretty wild guess, and should be - // tuned as needed. - pub const DEBUG_DATASET_QUOTA: u64 = 100 * (1 << 30); - } -} + // TODO-correctness: This value of 100GiB is a pretty wild guess, and + // should be tuned as needed. + ByteCount::from_gibibytes_u32(100) + }; // TODO-correctness: This value of 100GiB is a pretty wild guess, and should be // tuned as needed. -pub const DUMP_DATASET_QUOTA: u64 = 100 * (1 << 30); +pub const DUMP_DATASET_QUOTA: ByteCount = ByteCount::from_gibibytes_u32(100); // passed to zfs create -o compression= pub const DUMP_DATASET_COMPRESSION: CompressionAlgorithm = CompressionAlgorithm::GzipN { level: GzipLevel::new::<9>() }; @@ -59,50 +56,41 @@ pub const U2_DEBUG_DATASET: &'static str = "crypt/debug"; pub const CRYPT_DATASET: &'static str = "crypt"; const U2_EXPECTED_DATASET_COUNT: usize = 2; -static U2_EXPECTED_DATASETS: Lazy< - [ExpectedDataset; U2_EXPECTED_DATASET_COUNT], -> = Lazy::new(|| { - [ - // Stores filesystems for zones - ExpectedDataset::new(ZONE_DATASET).wipe(), - // For storing full kernel RAM dumps - ExpectedDataset::new(DUMP_DATASET) - .quota(ByteCount::try_from(DUMP_DATASET_QUOTA).unwrap()) - .compression(DUMP_DATASET_COMPRESSION), - ] -}); +const U2_EXPECTED_DATASETS: [ExpectedDataset; U2_EXPECTED_DATASET_COUNT] = [ + // Stores filesystems for zones + ExpectedDataset::new(ZONE_DATASET).wipe(), + // For storing full kernel RAM dumps + ExpectedDataset::new(DUMP_DATASET) + .quota(DUMP_DATASET_QUOTA) + .compression(DUMP_DATASET_COMPRESSION), +]; const M2_EXPECTED_DATASET_COUNT: usize = 6; -static M2_EXPECTED_DATASETS: Lazy< - [ExpectedDataset; M2_EXPECTED_DATASET_COUNT], -> = Lazy::new(|| { - [ - // Stores software images. - // - // Should be duplicated to both M.2s. - ExpectedDataset::new(INSTALL_DATASET), - // Stores crash dumps. - ExpectedDataset::new(CRASH_DATASET), - // Backing store for OS data that should be persisted across reboots. - // Its children are selectively overlay mounted onto parts of the ramdisk - // root. - ExpectedDataset::new(M2_BACKING_DATASET), - // Stores cluter configuration information. - // - // Should be duplicated to both M.2s. - ExpectedDataset::new(CLUSTER_DATASET), - // Stores configuration data, including: - // - What services should be launched on this sled - // - Information about how to initialize the Sled Agent - // - (For scrimlets) RSS setup information - // - // Should be duplicated to both M.2s. - ExpectedDataset::new(CONFIG_DATASET), - // Store debugging data, such as service bundles. - ExpectedDataset::new(M2_DEBUG_DATASET) - .quota(ByteCount::try_from(DEBUG_DATASET_QUOTA).unwrap()), - ] -}); +const M2_EXPECTED_DATASETS: [ExpectedDataset; M2_EXPECTED_DATASET_COUNT] = [ + // Stores software images. + // + // Should be duplicated to both M.2s. + ExpectedDataset::new(INSTALL_DATASET), + // Stores crash dumps. + ExpectedDataset::new(CRASH_DATASET), + // Backing store for OS data that should be persisted across reboots. + // Its children are selectively overlay mounted onto parts of the ramdisk + // root. + ExpectedDataset::new(M2_BACKING_DATASET), + // Stores cluter configuration information. + // + // Should be duplicated to both M.2s. + ExpectedDataset::new(CLUSTER_DATASET), + // Stores configuration data, including: + // - What services should be launched on this sled + // - Information about how to initialize the Sled Agent + // - (For scrimlets) RSS setup information + // + // Should be duplicated to both M.2s. + ExpectedDataset::new(CONFIG_DATASET), + // Store debugging data, such as service bundles. + ExpectedDataset::new(M2_DEBUG_DATASET).quota(DEBUG_DATASET_QUOTA), +]; // Helper type for describing expected datasets and their optional quota. #[derive(Clone, Copy, Debug)] @@ -127,7 +115,7 @@ impl ExpectedDataset { } } - fn quota(mut self, quota: ByteCount) -> Self { + const fn quota(mut self, quota: ByteCount) -> Self { self.quota = Some(quota); self }