From e44d40e7eee373879c1a6109335d88bf1d8f9b40 Mon Sep 17 00:00:00 2001 From: mulhern Date: Tue, 16 Jul 2024 17:06:37 -0400 Subject: [PATCH] Add MergeRequested property to filesystem Use it to set and unset the filesystem metadata. Signed-off-by: mulhern --- src/dbus_api/consts.rs | 1 + src/dbus_api/filesystem/filesystem_3_7/api.rs | 18 +++++++- src/dbus_api/filesystem/filesystem_3_7/mod.rs | 2 +- .../filesystem/filesystem_3_7/props.rs | 46 +++++++++++++++++-- src/dbus_api/filesystem/mod.rs | 6 ++- src/dbus_api/filesystem/shared.rs | 15 ++++++ src/dbus_api/tree.rs | 24 ++++++++++ src/dbus_api/types.rs | 14 ++++++ src/engine/engine.rs | 9 ++++ src/engine/sim_engine/filesystem.rs | 17 +++++++ src/engine/sim_engine/pool.rs | 16 +++++++ src/engine/strat_engine/pool/shared.rs | 11 +++++ src/engine/strat_engine/pool/v1.rs | 16 +++++++ src/engine/strat_engine/pool/v2.rs | 16 +++++++ .../strat_engine/thinpool/filesystem.rs | 21 ++++++++- src/engine/strat_engine/thinpool/thinpool.rs | 21 +++++++++ src/engine/types/actions.rs | 13 ++++++ 17 files changed, 257 insertions(+), 9 deletions(-) diff --git a/src/dbus_api/consts.rs b/src/dbus_api/consts.rs index 16ea2377d5..333470b12c 100644 --- a/src/dbus_api/consts.rs +++ b/src/dbus_api/consts.rs @@ -67,6 +67,7 @@ pub const FILESYSTEM_CREATED_PROP: &str = "Created"; pub const FILESYSTEM_SIZE_PROP: &str = "Size"; pub const FILESYSTEM_SIZE_LIMIT_PROP: &str = "SizeLimit"; pub const FILESYSTEM_ORIGIN_PROP: &str = "Origin"; +pub const FILESYSTEM_MERGE_SCHEDULED_PROP: &str = "MergeScheduled"; pub const BLOCKDEV_INTERFACE_NAME_3_0: &str = "org.storage.stratis3.blockdev.r0"; pub const BLOCKDEV_INTERFACE_NAME_3_1: &str = "org.storage.stratis3.blockdev.r1"; diff --git a/src/dbus_api/filesystem/filesystem_3_7/api.rs b/src/dbus_api/filesystem/filesystem_3_7/api.rs index b8864240e9..54f7cde132 100644 --- a/src/dbus_api/filesystem/filesystem_3_7/api.rs +++ b/src/dbus_api/filesystem/filesystem_3_7/api.rs @@ -4,7 +4,13 @@ use dbus_tree::{Access, EmitsChangedSignal, Factory, MTSync, Property}; -use crate::dbus_api::{consts, filesystem::filesystem_3_7::props::get_fs_origin, types::TData}; +use crate::dbus_api::{ + consts, + filesystem::filesystem_3_7::props::{ + get_fs_merge_scheduled, get_fs_origin, set_fs_merge_scheduled, + }, + types::TData, +}; pub fn origin_property(f: &Factory, TData>) -> Property, TData> { f.property::<(bool, String), _>(consts::FILESYSTEM_ORIGIN_PROP, ()) @@ -12,3 +18,13 @@ pub fn origin_property(f: &Factory, TData>) -> Property, TData>, +) -> Property, TData> { + f.property::(consts::FILESYSTEM_MERGE_SCHEDULED_PROP, ()) + .access(Access::ReadWrite) + .emits_changed(EmitsChangedSignal::True) + .on_get(get_fs_merge_scheduled) + .on_set(set_fs_merge_scheduled) +} diff --git a/src/dbus_api/filesystem/filesystem_3_7/mod.rs b/src/dbus_api/filesystem/filesystem_3_7/mod.rs index 4028a40d67..ca79e7c6c1 100644 --- a/src/dbus_api/filesystem/filesystem_3_7/mod.rs +++ b/src/dbus_api/filesystem/filesystem_3_7/mod.rs @@ -5,4 +5,4 @@ mod api; mod props; -pub use api::origin_property; +pub use api::{merge_scheduled_property, origin_property}; diff --git a/src/dbus_api/filesystem/filesystem_3_7/props.rs b/src/dbus_api/filesystem/filesystem_3_7/props.rs index 5fa35ee585..ec27d1cae9 100644 --- a/src/dbus_api/filesystem/filesystem_3_7/props.rs +++ b/src/dbus_api/filesystem/filesystem_3_7/props.rs @@ -2,12 +2,16 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at http://mozilla.org/MPL/2.0/. -use dbus::arg::IterAppend; +use dbus::arg::{Iter, IterAppend}; use dbus_tree::{MTSync, MethodErr, PropInfo}; -use crate::dbus_api::{ - filesystem::shared::{self, get_filesystem_property}, - types::TData, +use crate::{ + dbus_api::{ + consts, + filesystem::shared::{self, get_filesystem_property}, + types::TData, + }, + engine::PropChangeAction, }; pub fn get_fs_origin( @@ -16,3 +20,37 @@ pub fn get_fs_origin( ) -> Result<(), MethodErr> { get_filesystem_property(i, p, |(_, _, f)| Ok(shared::fs_origin_prop(f))) } + +pub fn get_fs_merge_scheduled( + i: &mut IterAppend<'_>, + p: &PropInfo<'_, MTSync, TData>, +) -> Result<(), MethodErr> { + get_filesystem_property(i, p, |(_, _, f)| Ok(shared::fs_merge_scheduled_prop(f))) +} + +/// Set the merge scheduled property on a filesystem +pub fn set_fs_merge_scheduled( + i: &mut Iter<'_>, + p: &PropInfo<'_, MTSync, TData>, +) -> Result<(), MethodErr> { + let merge_scheduled: bool = i + .get() + .ok_or_else(|| MethodErr::failed("Value required as argument to set property"))?; + + let res = shared::set_fs_property_to_display( + p, + consts::FILESYSTEM_MERGE_SCHEDULED_PROP, + |(_, uuid, p)| shared::set_fs_merge_scheduled_prop(uuid, p, merge_scheduled), + ); + + match res { + Ok(PropChangeAction::NewValue(v)) => { + p.tree + .get_data() + .push_fs_merge_scheduled_change(p.path.get_name(), v); + Ok(()) + } + Ok(PropChangeAction::Identity) => Ok(()), + Err(e) => Err(e), + } +} diff --git a/src/dbus_api/filesystem/mod.rs b/src/dbus_api/filesystem/mod.rs index 095a436cea..bbf0c7e109 100644 --- a/src/dbus_api/filesystem/mod.rs +++ b/src/dbus_api/filesystem/mod.rs @@ -126,7 +126,8 @@ pub fn create_dbus_filesystem<'a>( .add_p(filesystem_3_0::size_property(&f)) .add_p(filesystem_3_0::used_property(&f)) .add_p(filesystem_3_6::size_limit_property(&f)) - .add_p(filesystem_3_7::origin_property(&f)), + .add_p(filesystem_3_7::origin_property(&f)) + .add_p(filesystem_3_7::merge_scheduled_property(&f)), ); let path = object_path.get_name().to_owned(); @@ -217,7 +218,8 @@ pub fn get_fs_properties( consts::FILESYSTEM_SIZE_PROP => shared::fs_size_prop(fs), consts::FILESYSTEM_USED_PROP => shared::fs_used_prop(fs), consts::FILESYSTEM_SIZE_LIMIT_PROP => shared::fs_size_limit_prop(fs), - consts::FILESYSTEM_ORIGIN_PROP => shared::fs_origin_prop(fs) + consts::FILESYSTEM_ORIGIN_PROP => shared::fs_origin_prop(fs), + consts::FILESYSTEM_MERGE_SCHEDULED_PROP => shared::fs_merge_scheduled_prop(fs) } } } diff --git a/src/dbus_api/filesystem/shared.rs b/src/dbus_api/filesystem/shared.rs index 8efd2d72df..2fbe3d6388 100644 --- a/src/dbus_api/filesystem/shared.rs +++ b/src/dbus_api/filesystem/shared.rs @@ -207,3 +207,18 @@ pub fn fs_used_prop(fs: &dyn Filesystem) -> (bool, String) { pub fn fs_origin_prop(fs: &dyn Filesystem) -> (bool, String) { prop_conv::fs_origin_to_prop(fs.origin()) } + +/// Generate D-Bus representation of merge scheduled property +pub fn fs_merge_scheduled_prop(fs: &dyn Filesystem) -> bool { + fs.merge_scheduled() +} + +#[inline] +pub fn set_fs_merge_scheduled_prop( + uuid: FilesystemUuid, + pool: &mut dyn Pool, + schedule: bool, +) -> Result, String> { + pool.set_fs_merge_scheduled(uuid, schedule) + .map_err(|e| e.to_string()) +} diff --git a/src/dbus_api/tree.rs b/src/dbus_api/tree.rs index 1fadb300a6..38a2a28531 100644 --- a/src/dbus_api/tree.rs +++ b/src/dbus_api/tree.rs @@ -873,6 +873,26 @@ impl DbusTreeHandler { } } + /// Send a signal indicating that the filesystem merge scheduled value has + /// changed. + fn handle_fs_merge_scheduled_change(&self, path: Path<'static>, new_scheduled: bool) { + if let Err(e) = self.property_changed_invalidated_signal( + &path, + prop_hashmap!( + consts::FILESYSTEM_INTERFACE_NAME_3_7 => { + Vec::new(), + consts::FILESYSTEM_MERGE_SCHEDULED_PROP.to_string() => + box_variant!(new_scheduled) + } + ), + ) { + warn!( + "Failed to send a signal over D-Bus indicating filesystem merge scheduled value change: {}", + e + ); + } + } + /// Send a signal indicating that the blockdev user info has changed. fn handle_blockdev_user_info_change(&self, path: Path<'static>, new_user_info: Option) { let user_info_prop = blockdev_user_info_to_prop(new_user_info); @@ -1254,6 +1274,10 @@ impl DbusTreeHandler { self.handle_fs_size_limit_change(path, new_limit); Ok(true) } + DbusAction::FsMergeScheduledChange(path, new_scheduled) => { + self.handle_fs_merge_scheduled_change(path, new_scheduled); + Ok(true) + } DbusAction::PoolOverprovModeChange(path, new_mode) => { self.handle_pool_overprov_mode_change(path, new_mode); Ok(true) diff --git a/src/dbus_api/types.rs b/src/dbus_api/types.rs index af345c152c..cedfa16c7b 100644 --- a/src/dbus_api/types.rs +++ b/src/dbus_api/types.rs @@ -108,6 +108,7 @@ pub enum DbusAction { BlockdevTotalPhysicalSizeChange(Path<'static>, Sectors), FsOriginChange(Path<'static>), FsSizeLimitChange(Path<'static>, Option), + FsMergeScheduledChange(Path<'static>, bool), FsBackgroundChange( FilesystemUuid, SignalChange>, @@ -498,6 +499,19 @@ impl DbusContext { ) } } + + /// Send changed signal for pool MergeScheduled property. + pub fn push_fs_merge_scheduled_change(&self, item: &Path<'static>, new_merge_scheduled: bool) { + if let Err(e) = self.sender.send(DbusAction::FsMergeScheduledChange( + item.clone(), + new_merge_scheduled, + )) { + warn!( + "D-Bus filesystem merge scheduled change event could not be sent to the processing thread; no signal will be sent out for the merge scheduled change of filesystem with path {}: {}", + item, e, + ) + } + } } #[derive(Debug)] diff --git a/src/engine/engine.rs b/src/engine/engine.rs index f9eefe6628..d3c8a31712 100644 --- a/src/engine/engine.rs +++ b/src/engine/engine.rs @@ -96,6 +96,8 @@ pub trait Filesystem: Debug { /// Get filesystem snapshot origin. fn origin(&self) -> Option; + + fn merge_scheduled(&self) -> bool; } pub trait BlockDev: Debug { @@ -350,6 +352,13 @@ pub trait Pool: Debug + Send + Sync { /// Get the metadata version for a given pool. fn metadata_version(&self) -> StratSigblockVersion; + + /// Set whether a merge of the filesystem is scheduled. + fn set_fs_merge_scheduled( + &mut self, + fs: FilesystemUuid, + new_scheduled: bool, + ) -> StratisResult>; } pub type HandleEvents

= ( diff --git a/src/engine/sim_engine/filesystem.rs b/src/engine/sim_engine/filesystem.rs index 5198f234bf..87e0ac975a 100644 --- a/src/engine/sim_engine/filesystem.rs +++ b/src/engine/sim_engine/filesystem.rs @@ -21,6 +21,7 @@ pub struct SimFilesystem { size: Sectors, size_limit: Option, origin: Option, + merge_scheduled: bool, } impl SimFilesystem { @@ -42,6 +43,7 @@ impl SimFilesystem { size, size_limit, origin, + merge_scheduled: false, }) } @@ -73,6 +75,17 @@ impl SimFilesystem { self.origin = None; changed } + + /// Set the merge scheduled value for the filesystem. + pub fn set_merge_scheduled(&mut self, scheduled: bool) -> StratisResult { + if self.merge_scheduled == scheduled { + Ok(false) + } else { + // TODO: reject if conflict or no origin + self.merge_scheduled = scheduled; + Ok(true) + } + } } impl Filesystem for SimFilesystem { @@ -105,6 +118,10 @@ impl Filesystem for SimFilesystem { fn origin(&self) -> Option { self.origin } + + fn merge_scheduled(&self) -> bool { + self.merge_scheduled + } } impl<'a> Into for &'a SimFilesystem { diff --git a/src/engine/sim_engine/pool.rs b/src/engine/sim_engine/pool.rs index 45f45eb0f6..eac9eb08a6 100644 --- a/src/engine/sim_engine/pool.rs +++ b/src/engine/sim_engine/pool.rs @@ -758,6 +758,22 @@ impl Pool for SimPool { fn metadata_version(&self) -> StratSigblockVersion { StratSigblockVersion::V2 } + + fn set_fs_merge_scheduled( + &mut self, + fs_uuid: FilesystemUuid, + schedule: bool, + ) -> StratisResult> { + let (_, fs) = self.filesystems.get_mut_by_uuid(fs_uuid).ok_or_else(|| { + StratisError::Msg(format!("Filesystem with UUID {fs_uuid} not found")) + })?; + let changed = fs.set_merge_scheduled(schedule)?; + if changed { + Ok(PropChangeAction::NewValue(schedule)) + } else { + Ok(PropChangeAction::Identity) + } + } } #[cfg(test)] diff --git a/src/engine/strat_engine/pool/shared.rs b/src/engine/strat_engine/pool/shared.rs index 3c17b2e361..272bbcfbec 100644 --- a/src/engine/strat_engine/pool/shared.rs +++ b/src/engine/strat_engine/pool/shared.rs @@ -348,4 +348,15 @@ impl Pool for AnyPool { AnyPool::V2(p) => p.metadata_version(), } } + + fn set_fs_merge_scheduled( + &mut self, + fs_uuid: FilesystemUuid, + new_scheduled: bool, + ) -> StratisResult> { + match self { + AnyPool::V1(p) => p.set_fs_merge_scheduled(fs_uuid, new_scheduled), + AnyPool::V2(p) => p.set_fs_merge_scheduled(fs_uuid, new_scheduled), + } + } } diff --git a/src/engine/strat_engine/pool/v1.rs b/src/engine/strat_engine/pool/v1.rs index 932dbe3849..d6a6f5a94f 100644 --- a/src/engine/strat_engine/pool/v1.rs +++ b/src/engine/strat_engine/pool/v1.rs @@ -1293,6 +1293,22 @@ impl Pool for StratPool { fn metadata_version(&self) -> StratSigblockVersion { StratSigblockVersion::V1 } + + #[pool_mutating_action("NoRequests")] + fn set_fs_merge_scheduled( + &mut self, + fs_uuid: FilesystemUuid, + new_scheduled: bool, + ) -> StratisResult> { + if self + .thin_pool + .set_fs_merge_scheduled(fs_uuid, new_scheduled)? + { + Ok(PropChangeAction::NewValue(new_scheduled)) + } else { + Ok(PropChangeAction::Identity) + } + } } pub struct StratPoolState { diff --git a/src/engine/strat_engine/pool/v2.rs b/src/engine/strat_engine/pool/v2.rs index e82c6cfbca..c0eda67dfb 100644 --- a/src/engine/strat_engine/pool/v2.rs +++ b/src/engine/strat_engine/pool/v2.rs @@ -1200,6 +1200,22 @@ impl Pool for StratPool { fn metadata_version(&self) -> StratSigblockVersion { StratSigblockVersion::V2 } + + #[pool_mutating_action("NoRequests")] + fn set_fs_merge_scheduled( + &mut self, + fs_uuid: FilesystemUuid, + new_scheduled: bool, + ) -> StratisResult> { + if self + .thin_pool + .set_fs_merge_scheduled(fs_uuid, new_scheduled)? + { + Ok(PropChangeAction::NewValue(new_scheduled)) + } else { + Ok(PropChangeAction::Identity) + } + } } pub struct StratPoolState { diff --git a/src/engine/strat_engine/thinpool/filesystem.rs b/src/engine/strat_engine/thinpool/filesystem.rs index e325958f07..78ebf97164 100644 --- a/src/engine/strat_engine/thinpool/filesystem.rs +++ b/src/engine/strat_engine/thinpool/filesystem.rs @@ -52,6 +52,7 @@ pub struct StratFilesystem { used: Option, size_limit: Option, origin: Option, + merge_scheduled: bool, } fn init_used(thin_dev: &ThinDev) -> Option { @@ -116,6 +117,7 @@ impl StratFilesystem { created: Utc::now(), size_limit, origin: None, + merge_scheduled: false, }, )) } @@ -143,6 +145,7 @@ impl StratFilesystem { created, size_limit: fssave.fs_size_limit, origin: fssave.origin, + merge_scheduled: fssave.merge.unwrap_or_default(), }) } @@ -250,6 +253,7 @@ impl StratFilesystem { created: Utc::now(), size_limit: self.size_limit, origin: Some(origin_uuid), + merge_scheduled: false, }) } Err(e) => Err(StratisError::Msg(format!( @@ -390,7 +394,7 @@ impl StratFilesystem { created: self.created.timestamp() as u64, fs_size_limit: self.size_limit, origin: self.origin, - merge: None, + merge: self.origin.map(|_| self.merge_scheduled), } } @@ -451,6 +455,17 @@ impl StratFilesystem { self.origin = None; changed } + + /// Set the merge scheduled value for the filesystem. + pub fn set_merge_scheduled(&mut self, scheduled: bool) -> StratisResult { + if self.merge_scheduled == scheduled { + Ok(false) + } else { + // TODO: reject if conflict or no origin + self.merge_scheduled = scheduled; + Ok(true) + } + } } impl Filesystem for StratFilesystem { @@ -494,6 +509,10 @@ impl Filesystem for StratFilesystem { fn origin(&self) -> Option { self.origin } + + fn merge_scheduled(&self) -> bool { + self.merge_scheduled + } } /// Represents the state of the Stratis filesystem at a given moment in time. diff --git a/src/engine/strat_engine/thinpool/thinpool.rs b/src/engine/strat_engine/thinpool/thinpool.rs index 1dcfb5e389..912daa7998 100644 --- a/src/engine/strat_engine/thinpool/thinpool.rs +++ b/src/engine/strat_engine/thinpool/thinpool.rs @@ -1715,6 +1715,27 @@ where } Ok(changed) } + + /// Set the filesystem merge scheduled value for filesystem with given UUID. + pub fn set_fs_merge_scheduled( + &mut self, + fs_uuid: FilesystemUuid, + scheduled: bool, + ) -> StratisResult { + let changed = { + let (_, fs) = self.get_mut_filesystem_by_uuid(fs_uuid).ok_or_else(|| { + StratisError::Msg(format!("No filesystem with UUID {fs_uuid} found")) + })?; + fs.set_merge_scheduled(scheduled)? + }; + let (name, fs) = self + .get_filesystem_by_uuid(fs_uuid) + .expect("Looked up above"); + if changed { + self.mdv.save_fs(&name, fs_uuid, fs)?; + } + Ok(changed) + } } impl<'a, B> Into for &'a ThinPool { diff --git a/src/engine/types/actions.rs b/src/engine/types/actions.rs index ca8088404a..3a764e6b53 100644 --- a/src/engine/types/actions.rs +++ b/src/engine/types/actions.rs @@ -796,6 +796,19 @@ where } } +impl ToDisplay for PropChangeAction { + type Display = PropChangeAction; + + fn to_display(&self) -> PropChangeAction { + match self { + PropChangeAction::Identity => PropChangeAction::Identity, + PropChangeAction::NewValue(v) => { + PropChangeAction::NewValue(format!("a value of {}", v)) + } + } + } +} + impl Display for PropChangeAction where T: Display,