Skip to content

Commit

Permalink
Make D-Bus signature for integrity tag spec a string
Browse files Browse the repository at this point in the history
Define an enum to represent the tag size.

Store the IntegrityTagSpec value in the pool-level metadata.
Nothing is gained by turning the enum into bits before storing it in the
pool-level metadata.

Signed-off-by: mulhern <[email protected]>
  • Loading branch information
mulkieran committed Dec 16, 2024
1 parent 0a68e34 commit 18b1495
Show file tree
Hide file tree
Showing 17 changed files with 135 additions and 75 deletions.
21 changes: 14 additions & 7 deletions src/bin/utils/cmds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@ use std::{
str::FromStr,
};

use clap::{Arg, ArgAction, Command};
use clap::{builder::PossibleValuesParser, Arg, ArgAction, Command};

#[cfg(feature = "systemd_compat")]
use clap::builder::Str;
use log::LevelFilter;
use strum::VariantNames;

use devicemapper::Bytes;

use stratisd::engine::{IntegrityTagSpec, DEFAULT_INTEGRITY_TAG_SPEC};

use crate::utils::predict_usage;

#[cfg(feature = "systemd_compat")]
Expand Down Expand Up @@ -88,10 +91,12 @@ pool is encrypted, setting this option has no effect on the prediction."),
.next_line_help(true)
)
.arg(
Arg::new("integrity_tag_size")
.long("integrity-tag-size")
Arg::new("integrity_tag_spec")
.long("integrity-tag-spec")
.num_args(1)
.help("Size of the integrity checksums to be stored in the integrity metadata. The checksum size depends on the algorithm used for checksums. Units are bytes.")
.value_parser(PossibleValuesParser::new(IntegrityTagSpec::VARIANTS))
.default_value(DEFAULT_INTEGRITY_TAG_SPEC.as_ref())
.help("Integrity tag specification defining the size of the tag to store a checksum or other value for each block on a device.")
.next_line_help(true)
)
.arg(
Expand Down Expand Up @@ -153,9 +158,11 @@ impl<'a> UtilCommand<'a> for StratisPredictUsage {
})
.transpose()?,
sub_m
.get_one::<String>("integrity_tag_size")
.map(|s| s.parse::<u8>().map(Bytes::from))
.transpose()?,
.get_one::<String>("integrity_tag_spec")
.map(|sz| {
IntegrityTagSpec::try_from(sz.as_str()).expect("parser ensures valid value")
})
.expect("default specified by parser"),
LevelFilter::from_str(
matches
.get_one::<String>("log-level")
Expand Down
10 changes: 5 additions & 5 deletions src/bin/utils/predict_usage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ use serde_json::{json, Value};
use devicemapper::{Bytes, Sectors};

use stratisd::engine::{
crypt_metadata_size, integrity_meta_space, ThinPoolSizeParams, BDA,
DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE, DEFAULT_INTEGRITY_TAG_SIZE,
crypt_metadata_size, integrity_meta_space, IntegrityTagSpec, ThinPoolSizeParams, BDA,
DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE,
};

// 2^FS_SIZE_START_POWER is the logical size of the smallest Stratis
Expand Down Expand Up @@ -167,7 +167,7 @@ pub fn predict_filesystem_usage(
fn predict_pool_metadata_usage(
device_sizes: Vec<Sectors>,
journal_size: Option<Sectors>,
tag_size: Option<Bytes>,
tag_size: IntegrityTagSpec,
) -> Result<Sectors, Box<dyn Error>> {
let stratis_metadata_alloc = BDA::default().extended_size().sectors();
let stratis_avail_sizes = device_sizes
Expand All @@ -179,7 +179,7 @@ fn predict_pool_metadata_usage(
s,
journal_size.unwrap_or(DEFAULT_INTEGRITY_JOURNAL_SIZE.sectors()),
DEFAULT_INTEGRITY_BLOCK_SIZE,
tag_size.unwrap_or(DEFAULT_INTEGRITY_TAG_SIZE),
tag_size,
);
info!(
"Deduction for stratis metadata: {:}",
Expand Down Expand Up @@ -218,7 +218,7 @@ pub fn predict_pool_usage(
device_sizes: Vec<Bytes>,
filesystem_sizes: Option<Vec<Bytes>>,
journal_size: Option<Sectors>,
tag_size: Option<Bytes>,
tag_size: IntegrityTagSpec,
log_level: LevelFilter,
) -> Result<(), Box<dyn Error>> {
Builder::new().filter(None, log_level).init();
Expand Down
9 changes: 5 additions & 4 deletions src/dbus_api/api/manager_3_8/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,14 @@ pub fn create_pool_method(f: &Factory<MTSync<TData>, TData>) -> Method<MTSync<TD
//
// Rust representation: (bool, u64)
.in_arg(("journal_size", "(bt)"))
// Optional tag size for integrity metadata reservation.
// Optional tag size or specification for integrity metadata
// reservation.
// b: true if the size should be specified.
// false if the default should be used.
// i: Integer representing tag size in bytes.
// q: Tag size specification.
//
// Rust representation: (bool, u8)
.in_arg(("tag_size", "(by)"))
// Rust representation: (bool, String)
.in_arg(("tag_size_spec", "(bs)"))
// In order from left to right:
// b: true if a pool was created and object paths were returned
// o: Object path for Pool
Expand Down
19 changes: 15 additions & 4 deletions src/dbus_api/api/manager_3_8/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ use crate::{
util::{engine_to_dbus_err_tuple, get_next_arg, tuple_to_option},
},
engine::{
CreateAction, EncryptionInfo, KeyDescription, Name, PoolIdentifier, PoolUuid, StartAction,
UnlockMethod,
CreateAction, EncryptionInfo, IntegrityTagSpec, KeyDescription, Name, PoolIdentifier,
PoolUuid, StartAction, UnlockMethod,
},
stratis::StratisError,
};
Expand Down Expand Up @@ -158,7 +158,7 @@ pub fn create_pool(m: &MethodInfo<'_, MTSync<TData>, TData>) -> MethodResult {
Some(get_next_arg(&mut iter, 3)?),
);
let journal_size_tuple: (bool, u64) = get_next_arg(&mut iter, 4)?;
let tag_size_tuple: (bool, u8) = get_next_arg(&mut iter, 5)?;
let tag_size_tuple: (bool, String) = get_next_arg(&mut iter, 5)?;

let return_message = message.method_return();

Expand Down Expand Up @@ -188,7 +188,18 @@ pub fn create_pool(m: &MethodInfo<'_, MTSync<TData>, TData>) -> MethodResult {
};

let journal_size = tuple_to_option(journal_size_tuple).map(Bytes::from);
let tag_size = tuple_to_option(tag_size_tuple).map(Bytes::from);
let tag_size = match tuple_to_option(tag_size_tuple)
.map(|s| IntegrityTagSpec::try_from(s.as_str()))
.transpose()
{
Ok(s) => s,
Err(e) => {
let (rc, rs) = engine_to_dbus_err_tuple(&StratisError::Msg(format!(
"Failed to parse integrity tag specification: {e}"
)));
return Ok(vec![return_message.append3(default_return, rc, rs)]);
}
};

let dbus_context = m.tree.get_data();
let create_result = handle_action!(block_on(dbus_context.engine.create_pool(
Expand Down
13 changes: 7 additions & 6 deletions src/engine/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ use crate::{
structures::{AllLockReadGuard, AllLockWriteGuard, SomeLockReadGuard, SomeLockWriteGuard},
types::{
ActionAvailability, BlockDevTier, Clevis, CreateAction, DeleteAction, DevUuid,
EncryptionInfo, FilesystemUuid, GrowAction, Key, KeyDescription, LockedPoolsInfo,
MappingCreateAction, MappingDeleteAction, Name, PoolDiff, PoolEncryptionInfo,
PoolIdentifier, PoolUuid, RegenAction, RenameAction, ReportType, SetCreateAction,
SetDeleteAction, SetUnlockAction, StartAction, StopAction, StoppedPoolsInfo,
StratFilesystemDiff, StratSigblockVersion, UdevEngineEvent, UnlockMethod,
EncryptionInfo, FilesystemUuid, GrowAction, IntegrityTagSpec, Key, KeyDescription,
LockedPoolsInfo, MappingCreateAction, MappingDeleteAction, Name, PoolDiff,
PoolEncryptionInfo, PoolIdentifier, PoolUuid, RegenAction, RenameAction, ReportType,
SetCreateAction, SetDeleteAction, SetUnlockAction, StartAction, StopAction,
StoppedPoolsInfo, StratFilesystemDiff, StratSigblockVersion, UdevEngineEvent,
UnlockMethod,
},
},
stratis::StratisResult,
Expand Down Expand Up @@ -382,7 +383,7 @@ pub trait Engine: Debug + Report + Send + Sync {
blockdev_paths: &[&Path],
encryption_info: Option<&EncryptionInfo>,
journal_size: Option<Bytes>,
tag_size: Option<Bytes>,
tag_size: Option<IntegrityTagSpec>,
) -> StratisResult<CreateAction<PoolUuid>>;

/// Handle a libudev event.
Expand Down
6 changes: 3 additions & 3 deletions src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ pub use self::{
crypt_metadata_size, get_dm, get_dm_init, integrity_meta_space, register_clevis_token,
set_up_crypt_logging, unshare_mount_namespace, StaticHeader, StaticHeaderResult,
StratEngine, StratKeyActions, ThinPoolSizeParams, BDA, CLEVIS_TANG_TRUST_URL,
DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE, DEFAULT_INTEGRITY_TAG_SIZE,
DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE, DEFAULT_INTEGRITY_TAG_SPEC,
},
structures::{AllLockReadGuard, ExclusiveGuard, SharedGuard, Table},
types::{
ActionAvailability, BlockDevTier, ClevisInfo, CreateAction, DeleteAction, DevUuid, Diff,
EncryptionInfo, EngineAction, FilesystemUuid, GrowAction, KeyDescription, Lockable,
LockedPoolInfo, LockedPoolsInfo, MappingCreateAction, MappingDeleteAction,
EncryptionInfo, EngineAction, FilesystemUuid, GrowAction, IntegrityTagSpec, KeyDescription,
Lockable, LockedPoolInfo, LockedPoolsInfo, MappingCreateAction, MappingDeleteAction,
MaybeInconsistent, Name, PoolDiff, PoolEncryptionInfo, PoolIdentifier, PoolUuid,
PropChangeAction, RenameAction, ReportType, SetCreateAction, SetDeleteAction,
SetUnlockAction, StartAction, StopAction, StoppedPoolInfo, StoppedPoolsInfo,
Expand Down
8 changes: 4 additions & 4 deletions src/engine/sim_engine/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ use crate::{
},
types::{
CreateAction, DeleteAction, DevUuid, EncryptionInfo, Features, FilesystemUuid,
LockedPoolsInfo, Name, PoolDevice, PoolDiff, PoolIdentifier, PoolUuid, RenameAction,
ReportType, SetUnlockAction, StartAction, StopAction, StoppedPoolInfo,
StoppedPoolsInfo, StratFilesystemDiff, UdevEngineEvent, UnlockMethod,
IntegrityTagSpec, LockedPoolsInfo, Name, PoolDevice, PoolDiff, PoolIdentifier,
PoolUuid, RenameAction, ReportType, SetUnlockAction, StartAction, StopAction,
StoppedPoolInfo, StoppedPoolsInfo, StratFilesystemDiff, UdevEngineEvent, UnlockMethod,
},
StratSigblockVersion,
},
Expand Down Expand Up @@ -132,7 +132,7 @@ impl Engine for SimEngine {
blockdev_paths: &[&Path],
encryption_info: Option<&EncryptionInfo>,
_: Option<Bytes>,
_: Option<Bytes>,
_: Option<IntegrityTagSpec>,
) -> StratisResult<CreateAction<PoolUuid>> {
validate_name(name)?;
let name = Name::new(name.to_owned());
Expand Down
13 changes: 6 additions & 7 deletions src/engine/strat_engine/backstore/backstore/v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ use either::Either;
use serde_json::Value;

use devicemapper::{
Bytes, CacheDev, CacheDevTargetTable, CacheTargetParams, DevId, Device, DmDevice, DmFlags,
DmOptions, LinearDev, LinearDevTargetParams, LinearTargetParams, Sectors, TargetLine,
TargetTable,
CacheDev, CacheDevTargetTable, CacheTargetParams, DevId, Device, DmDevice, DmFlags, DmOptions,
LinearDev, LinearDevTargetParams, LinearTargetParams, Sectors, TargetLine, TargetTable,
};

use crate::{
Expand All @@ -34,8 +33,8 @@ use crate::{
writing::wipe_sectors,
},
types::{
ActionAvailability, BlockDevTier, DevUuid, EncryptionInfo, KeyDescription, PoolUuid,
SizedKeyMemory, UnlockMethod,
ActionAvailability, BlockDevTier, DevUuid, EncryptionInfo, IntegrityTagSpec,
KeyDescription, PoolUuid, SizedKeyMemory, UnlockMethod,
},
},
stratis::{StratisError, StratisResult},
Expand Down Expand Up @@ -439,12 +438,12 @@ impl Backstore {
mda_data_size: MDADataSize,
encryption_info: Option<&EncryptionInfo>,
integrity_journal_size: Option<Sectors>,
integrity_tag_size: Option<Bytes>,
integrity_tag_spec: Option<IntegrityTagSpec>,
) -> StratisResult<Backstore> {
let data_tier = DataTier::<StratBlockDev>::new(
BlockDevMgr::<StratBlockDev>::initialize(pool_uuid, devices, mda_data_size)?,
integrity_journal_size,
integrity_tag_size,
integrity_tag_spec,
);

let mut backstore = Backstore {
Expand Down
18 changes: 12 additions & 6 deletions src/engine/strat_engine/backstore/blockdev/v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ use crate::{
types::BDAResult,
},
types::{
Compare, DevUuid, DevicePath, Diff, PoolUuid, StateDiff, StratBlockDevDiff,
StratSigblockVersion,
Compare, DevUuid, DevicePath, Diff, IntegrityTagSpec, PoolUuid, StateDiff,
StratBlockDevDiff, StratSigblockVersion,
},
},
stratis::{StratisError, StratisResult},
Expand All @@ -51,11 +51,17 @@ pub fn integrity_meta_space(
total_space: Sectors,
journal_size: Sectors,
block_size: Bytes,
tag_size: Bytes,
tag_size: IntegrityTagSpec,
) -> Sectors {
Bytes(4096).sectors()
+ journal_size
+ Bytes::from(((*total_space.bytes() / *block_size) * *tag_size + 4095) & !4095).sectors()
+ Bytes::from(
(((((total_space.bytes() / block_size) * u128::from(tag_size.as_bits())) / 8 + 7)
& !7)
+ 4095)
& !4095,
)
.sectors()
}

#[derive(Debug)]
Expand Down Expand Up @@ -221,7 +227,7 @@ impl StratBlockDev {
&mut self,
integrity_journal_size: Sectors,
integrity_block_size: Bytes,
integrity_tag_size: Bytes,
integrity_tag_spec: IntegrityTagSpec,
) -> StratisResult<bool> {
let size = BlockdevSize::new(Self::scan_blkdev_size(self.devnode())?);
let metadata_size = self.bda.dev_size();
Expand Down Expand Up @@ -253,7 +259,7 @@ impl StratBlockDev {
size.sectors(),
integrity_journal_size,
integrity_block_size,
integrity_tag_size,
integrity_tag_spec,
) - self
.integrity_meta_allocs
.iter()
Expand Down
6 changes: 3 additions & 3 deletions src/engine/strat_engine/backstore/blockdevmgr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::{
serde_structs::{BaseBlockDevSave, Recordable},
shared::bds_to_bdas,
},
types::{DevUuid, EncryptionInfo, Name, PoolEncryptionInfo, PoolUuid},
types::{DevUuid, EncryptionInfo, IntegrityTagSpec, Name, PoolEncryptionInfo, PoolUuid},
},
stratis::{StratisError, StratisResult},
};
Expand Down Expand Up @@ -248,7 +248,7 @@ impl BlockDevMgr<v2::StratBlockDev> {
dev: DevUuid,
integrity_journal_size: Sectors,
integrity_block_size: Bytes,
integrity_tag_size: Bytes,
integrity_tag_spec: IntegrityTagSpec,
) -> StratisResult<bool> {
let bd = self
.block_devs
Expand All @@ -258,7 +258,7 @@ impl BlockDevMgr<v2::StratBlockDev> {
bd.grow(
integrity_journal_size,
integrity_block_size,
integrity_tag_size,
integrity_tag_spec,
)
}

Expand Down
Loading

0 comments on commit 18b1495

Please sign in to comment.