Merge pull request stratis-storage#3679 from mulkieran/issue_stratisd…

…_3663_a

Update prediction script for new metadata layout

.github/workflows/nightly.yml

@@ -195,7 +195,6 @@ jobs:
       matrix:
         include:
           - toolchain: 1.80.1  # CURRENT DEVELOPMENT RUST TOOLCHAIN
-          - toolchain: 1.71.1  # LOWEST SUPPORTED RUST TOOLCHAIN
     runs-on: ubuntu-22.04
     steps:
       - uses: actions/checkout@v4

src/bin/utils/cmds.rs

@@ -57,7 +57,10 @@ impl StratisPredictUsage {
                     .arg(Arg::new("encrypted")
                         .long("encrypted")
                         .action(ArgAction::SetTrue)
-                        .help("Whether the pool will be encrypted."),
+                        .help("Whether the pool will be encrypted.")
+                        .long_help(
+"Since space for crypt metadata is allocated regardless of whether or not the
+pool is encrypted, setting this option has no effect on the prediction."),
                     )
                     .arg(
                         Arg::new("no-overprovision")
@@ -112,7 +115,6 @@ impl<'a> UtilCommand<'a> for StratisPredictUsage {
         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::<String>("device-size")

src/bin/utils/predict_usage.rs

@@ -8,12 +8,14 @@ use std::{
 };
 
 use env_logger::Builder;
-use log::LevelFilter;
+use log::{info, LevelFilter};
 use serde_json::{json, Value};
 
 use devicemapper::{Bytes, Sectors};
 
-use stratisd::engine::{crypt_metadata_size, ThinPoolSizeParams, BDA};
+use stratisd::engine::{
+    crypt_metadata_size, integrity_meta_space, raid_meta_space, 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.,
@@ -161,11 +163,50 @@ pub fn predict_filesystem_usage(
     Ok(())
 }
 
+fn predict_pool_metadata_usage(device_sizes: Vec<Sectors>) -> Result<Sectors, Box<dyn Error>> {
+    let raid_metadata_alloc = raid_meta_space();
+    let stratis_metadata_alloc = BDA::default().extended_size().sectors();
+    let stratis_avail_sizes = device_sizes
+        .iter()
+        .map(|&s| {
+            info!("Total size of device: {:}", s);
+
+            let integrity_deduction = integrity_meta_space(s);
+            info!(
+                "Deduction for stratis metadata: {:}",
+                stratis_metadata_alloc
+            );
+
+            info!("Deduction for integrity space: {:}", integrity_deduction);
+            info!("Deduction for raid space: {:}", raid_metadata_alloc);
+            (*s).checked_sub(*stratis_metadata_alloc)
+                .and_then(|r| r.checked_sub(*raid_metadata_alloc))
+                .and_then(|r| r.checked_sub(*integrity_deduction))
+                .map(Sectors)
+                .map(|x| {
+                    info!("Size after deductions: {:}", x);
+                    x
+                })
+                .ok_or_else(|| {
+                    Box::new(PredictError(
+                        "Some device sizes too small for DM metadata.".into(),
+                    ))
+                })
+        })
+        .collect::<Result<Vec<_>, _>>()?;
+
+    let crypt_metadata_size = crypt_metadata_size();
+    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);
+
+    Ok(stratis_avail_sizes.iter().cloned().sum::<Sectors>() - crypt_metadata_size_sectors)
+}
+
 // 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<Bytes>,
     filesystem_sizes: Option<Vec<Bytes>>,
@@ -177,48 +218,10 @@ pub fn predict_pool_usage(
         .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::<Vec<_>>();
-
-    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::<Result<Vec<_>, _>>()?;
-
-    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::<Result<Vec<_>, _>>()?;
-
     let total_size: Sectors = device_sizes.iter().cloned().sum();
-    let non_metadata_size: Sectors = stratis_avail_sizes.iter().cloned().sum();
+
+    let non_metadata_size = predict_pool_metadata_usage(device_sizes)?;
 
     let size_params = ThinPoolSizeParams::new(non_metadata_size)?;
     let total_non_data = 2usize * size_params.meta_size() + size_params.mdv_size();

src/engine/mod.rs

@@ -4,14 +4,16 @@
 
 #[cfg(feature = "test_extras")]
 pub use self::strat_engine::{ProcessedPathInfos, StratPool};
+
 pub use self::{
     engine::{BlockDev, Engine, Filesystem, KeyActions, Pool, Report},
     shared::{total_allocated, total_used},
     sim_engine::SimEngine,
     strat_engine::{
-        crypt_metadata_size, get_dm, get_dm_init, register_clevis_token, set_up_crypt_logging,
-        unshare_mount_namespace, StaticHeader, StaticHeaderResult, StratEngine, StratKeyActions,
-        ThinPoolSizeParams, BDA, CLEVIS_TANG_TRUST_URL,
+        crypt_metadata_size, get_dm, get_dm_init, integrity_meta_space, raid_meta_space,
+        register_clevis_token, set_up_crypt_logging, unshare_mount_namespace, StaticHeader,
+        StaticHeaderResult, StratEngine, StratKeyActions, ThinPoolSizeParams, BDA,
+        CLEVIS_TANG_TRUST_URL,
     },
     structures::{AllLockReadGuard, ExclusiveGuard, SharedGuard, Table},
     types::{

src/engine/strat_engine/backstore/mod.rs

@@ -12,8 +12,10 @@ mod devices;
 mod range_alloc;
 mod shared;
 
-pub use self::devices::{
-    find_stratis_devs_by_uuid, get_devno_from_path, ProcessedPathInfos, UnownedDevices,
+pub use self::{
+    blockdev::v2::{integrity_meta_space, raid_meta_space},
+    devices::{find_stratis_devs_by_uuid, get_devno_from_path, ProcessedPathInfos, UnownedDevices},
 };
+
 #[cfg(test)]
 pub use self::devices::{initialize_devices, initialize_devices_legacy};

src/engine/strat_engine/mod.rs

@@ -24,7 +24,9 @@ mod writing;
 
 #[cfg(feature = "test_extras")]
 pub use self::{backstore::ProcessedPathInfos, pool::v1::StratPool};
+
 pub use self::{
+    backstore::{integrity_meta_space, raid_meta_space},
     crypt::{
         crypt_metadata_size, register_clevis_token, set_up_crypt_logging, CLEVIS_TANG_TRUST_URL,
     },

tests/client-dbus/tests/misc/test_predict.py

@@ -43,7 +43,7 @@ def _call_predict_usage_pool(
     :param bool overprovision: whether it is allowed to overprovision the pool
     """
     with subprocess.Popen(
-        [_STRATIS_PREDICT_USAGE, "pool"]
+        [_STRATIS_PREDICT_USAGE, "--log-level=trace", "pool"]
         + [f"--device-size={size.magnitude}" for size in device_sizes]
         + (
             []
@@ -75,7 +75,7 @@ def _call_predict_usage_filesystem(fs_sizes, overprovision):
     """
 
     with subprocess.Popen(
-        [_STRATIS_PREDICT_USAGE, "filesystem"]
+        [_STRATIS_PREDICT_USAGE, "--log-level=trace", "filesystem"]
         + [f"--filesystem-size={size.magnitude}" for size in fs_sizes]
         + ([] if overprovision else ["--no-overprovision"]),
         stdout=subprocess.PIPE,

tests/client-dbus/tests/udev/test_predict.py

@@ -61,7 +61,7 @@ def _call_predict_usage(encrypted, device_sizes, *, fs_specs=None, overprovision
     :param bool overprovision: whether it is allowed to overprovision the pool
     """
     with subprocess.Popen(
-        [_STRATIS_PREDICT_USAGE, "pool"]
+        [_STRATIS_PREDICT_USAGE, "--log-level=trace", "pool"]
         + [f"--device-size={size}" for size in device_sizes]
         + (
             []
@@ -210,8 +210,6 @@ def _check_prediction(self, prediction, mopool):
         :param str prediction: result of calling script, JSON format
         :param MOPool mopool: object with pool properties
         """
-        encrypted = mopool.Encrypted()
-
         (success, total_physical_used) = mopool.TotalPhysicalUsed()
         if not success:
             raise RuntimeError("Pool's TotalPhysicalUsed property was invalid.")
@@ -221,17 +219,22 @@ def _check_prediction(self, prediction, mopool):
             prediction["total"],
         )
 
-        if encrypted:
-            self.assertLess(mopool.TotalPhysicalSize(), total_prediction)
-            self.assertLess(total_physical_used, used_prediction)
-
-            diff1 = Range(total_prediction) - Range(mopool.TotalPhysicalSize())
-            diff2 = Range(used_prediction) - Range(total_physical_used)
-
-            self.assertEqual(diff1, diff2)
-        else:
-            self.assertEqual(mopool.TotalPhysicalSize(), total_prediction)
-            self.assertEqual(total_physical_used, used_prediction)
+        self.assertEqual(
+            mopool.TotalPhysicalSize(),
+            total_prediction,
+            msg=(
+                "Total physical size predictions do not match. "
+                f"Predicted sizes: {prediction}"
+            ),
+        )
+        self.assertEqual(
+            total_physical_used,
+            used_prediction,
+            msg=(
+                "Total physical used predictions do not match. "
+                f"Predicted sizes: {prediction}"
+            ),
+        )
 
     def _test_prediction(self, pool_name, *, fs_specs=None, overprovision=True):
         """