Switch to predicting usage for v2 pools

Leave --encrypted option, for backwards compatibility but add help
indicating that it doesn't matter if you set it or not.
Add better error messages for test failures.
Optionally log some deductions for metadata.

Signed-off-by: mulhern <amulhern@redhat.com>

.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):
         """