const fn dataset quotas (#6643)

While adding another dataset I ran across this at-runtime array
initialization and wanted to make it all `const`.

First, we make the `ByteCount::from_{kibi,mebi,gibi}byte_u32` methods
`const fn`. Trait methods cannot be used in `const` contexts yet so we
directly construct the object and verify in tests that the maximum
inputs to these methods don't violate the type invariant.

The rest makes the sled-storage arrays `const` and cleans related code
up.

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

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

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<ByteCount> = 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<T, Fut>(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(),

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

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
     }