Skip to content

Commit

Permalink
Reject merge scheduling if invalid
Browse files Browse the repository at this point in the history
Signed-off-by: mulhern <amulhern@redhat.com>
  • Loading branch information
mulkieran committed Aug 17, 2024
1 parent 34daa5b commit 1d5489f
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 20 deletions.
5 changes: 4 additions & 1 deletion src/engine/sim_engine/filesystem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,11 @@ impl SimFilesystem {
pub fn set_merge_scheduled(&mut self, scheduled: bool) -> StratisResult<bool> {
if self.merge_scheduled == scheduled {
Ok(false)
} else if scheduled && self.origin.is_none() {
Err(StratisError::Msg(
"Filesystem has no origin filesystem; can not schedule a merge".into(),
))
} else {
// TODO: reject if conflict or no origin
self.merge_scheduled = scheduled;
Ok(true)
}
Expand Down
50 changes: 42 additions & 8 deletions src/engine/sim_engine/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -798,17 +798,51 @@ impl Pool for SimPool {
fn set_fs_merge_scheduled(
&mut self,
fs_uuid: FilesystemUuid,
schedule: bool,
scheduled: bool,
) -> StratisResult<PropChangeAction<bool>> {
let (_, fs) = self.filesystems.get_mut_by_uuid(fs_uuid).ok_or_else(|| {
StratisError::Msg(format!("Filesystem with UUID {fs_uuid} not found"))
let (_, fs) = self
.filesystems
.get_by_uuid(fs_uuid)
.ok_or_else(|| StratisError::Msg(format!("No filesystem with UUID {fs_uuid} found")))?;

let origin = fs.origin().ok_or_else(|| {
StratisError::Msg(format!(
"Filesystem {fs_uuid} has no origin, revert cannot be scheduled"
))
})?;
let changed = fs.set_merge_scheduled(schedule)?;
if changed {
Ok(PropChangeAction::NewValue(schedule))
} else {
Ok(PropChangeAction::Identity)

if fs.merge_scheduled() == scheduled {
return Ok(PropChangeAction::Identity);
}

if scheduled {
let others_scheduled = self
.filesystems
.iter()
.filter(|(_, _, f)| {
f.origin().map(|o| o == origin).unwrap_or(false) && f.merge_scheduled()
})
.collect::<Vec<_>>();

assert!(others_scheduled.len() < 2);

if let Some((n, u, _)) = others_scheduled.first() {
return Err(StratisError::Msg(format!(
"Filesystem {:} with UUID {:} is already scheduled to be reverted into origin filesystem {:}",
n, u, origin,
)));
}
}

assert!(self
.filesystems
.get_mut_by_uuid(fs_uuid)
.expect("Looked up above")
.1
.set_merge_scheduled(scheduled)
.expect("fs.origin() is not None"));

Ok(PropChangeAction::NewValue(scheduled))
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/engine/strat_engine/thinpool/filesystem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,8 +460,11 @@ impl StratFilesystem {
pub fn set_merge_scheduled(&mut self, scheduled: bool) -> StratisResult<bool> {
if self.merge_scheduled == scheduled {
Ok(false)
} else if scheduled && self.origin.is_none() {
Err(StratisError::Msg(
"Filesystem has no origin filesystem; can not schedule a merge".into(),
))
} else {
// TODO: reject if conflict or no origin
self.merge_scheduled = scheduled;
Ok(true)
}
Expand Down
55 changes: 45 additions & 10 deletions src/engine/strat_engine/thinpool/thinpool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1811,24 +1811,59 @@ where
}

/// Set the filesystem merge scheduled value for filesystem with given UUID.
/// Returns true if the value was changed from the filesystem's, previous
/// value, otherwise false.
pub fn set_fs_merge_scheduled(
&mut self,
fs_uuid: FilesystemUuid,
scheduled: bool,
) -> StratisResult<bool> {
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 (_, fs) = self
.get_filesystem_by_uuid(fs_uuid)
.ok_or_else(|| StratisError::Msg(format!("No filesystem with UUID {fs_uuid} found")))?;

let origin = fs.origin().ok_or_else(|| {
StratisError::Msg(format!(
"Filesystem {fs_uuid} has no origin, revert cannot be scheduled or unscheduled"
))
})?;

if fs.merge_scheduled() == scheduled {
return Ok(false);
}

if scheduled {
let others_scheduled = self
.filesystems
.iter()
.filter(|(_, _, f)| {
f.origin().map(|o| o == origin).unwrap_or(false) && f.merge_scheduled()
})
.collect::<Vec<_>>();

assert!(others_scheduled.len() < 2);

if let Some((n, u, _)) = others_scheduled.first() {
return Err(StratisError::Msg(format!(
"Filesystem {:} with UUID {:} is already scheduled to be reverted into origin filesystem {:}",
n, u, origin,
)));
}
}

assert!(self
.get_mut_filesystem_by_uuid(fs_uuid)
.expect("Looked up above")
.1
.set_merge_scheduled(scheduled)
.expect("fs.origin() is not None"));

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)

self.mdv.save_fs(&name, fs_uuid, fs)?;
Ok(true)
}
}

Expand Down

0 comments on commit 1d5489f

Please sign in to comment.