Skip to content

Commit

Permalink
Change destroy_filesystems to work with merging
Browse files Browse the repository at this point in the history 
Move all the functionality into thinpool implementation.

Do more checks to avoid deletings filesystems that shouldn't be deleted.

Check for situations where multiple snapshots are referring to the same
deleted filesystem.

Remove a now unused function.

Signed-off-by: mulhern <amulhern@redhat.com>
  • Loading branch information
@mulkieran
mulkieran committed Aug 15, 2024
1 parent 2b37186 commit 03e19a3
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 103 deletions.
63 changes: 39 additions & 24 deletions src/engine/sim_engine/pool.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,6 @@ impl SimPool {
}
}

fn filesystems_mut(&mut self) -> Vec<(Name, FilesystemUuid, &mut SimFilesystem)> {
self.filesystems
.iter_mut()
.map(|(name, uuid, x)| (name.clone(), *uuid, x))
.collect()
}

pub fn record(&self, name: &str) -> PoolSave {
PoolSave {
name: name.to_owned(),
Expand Down Expand Up @@ -489,30 +482,52 @@ impl Pool for SimPool {
_pool_name: &str,
fs_uuids: &HashSet<FilesystemUuid>,
) -> StratisResult<SetDeleteAction<FilesystemUuid, FilesystemUuid>> {
let mut removed = Vec::new();
for &uuid in fs_uuids {
if self.filesystems.remove_by_uuid(uuid).is_some() {
removed.push(uuid);
}
}

let updated_origins: Vec<FilesystemUuid> = self
.filesystems_mut()
.iter_mut()
let snapshots = self
.filesystems()
.iter()
.filter_map(|(_, u, fs)| {
fs.origin().and_then(|x| {
if removed.contains(&x) {
if fs.unset_origin() {
Some(*u)
} else {
None
}
if fs_uuids.contains(&x) {
Some((x, (*u, fs.merge_scheduled())))
} else {
None
}
})
})
.collect();
.fold(HashMap::new(), |mut acc, (u, v)| {
acc.entry(u)
.and_modify(|e: &mut Vec<_>| e.push(v))
.or_insert(vec![v]);
acc
});

let scheduled_for_merge = snapshots
.iter()
.filter(|(_, snaps)| snaps.iter().any(|(_, scheduled)| *scheduled))
.collect::<Vec<_>>();
if !scheduled_for_merge.is_empty() {
let err_str = format!("The filesystem destroy operation can not be begun until the revert operations for the following filesystem snapshots have been cancelled: {}", scheduled_for_merge.iter().map(|(u, _)| u.to_string()).collect::<Vec<_>>().join(", "));
return Err(StratisError::Msg(err_str));
}

let (mut removed, mut updated_origins) = (Vec::new(), Vec::new());
for &uuid in fs_uuids {
if self.filesystems.remove_by_uuid(uuid).is_some() {
removed.push(uuid);

let snaps = snapshots
.get(&uuid)
.expect("snapshots map construction ensures key is present");
// The filesystems in snaps may have been removed; any one of
// them may also be a filesystem that was scheduled for removal.
for (sn_uuid, _) in snaps {
if let Some((_, sn)) = self.filesystems.get_mut_by_uuid(*sn_uuid) {
assert!(sn.unset_origin());
updated_origins.push(*sn_uuid);
};
}
}
}

Ok(SetDeleteAction::new(removed, updated_origins))
}
Expand Down
31 changes: 1 addition & 30 deletions src/engine/strat_engine/pool/v1.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1023,36 +1023,7 @@ impl Pool for StratPool {
pool_name: &str,
fs_uuids: &HashSet<FilesystemUuid>,
) -> StratisResult<SetDeleteAction<FilesystemUuid, FilesystemUuid>> {
let mut removed = Vec::new();
for &uuid in fs_uuids {
if let Some(uuid) = self.thin_pool.destroy_filesystem(pool_name, uuid)? {
removed.push(uuid);
}
}

let snapshots: Vec<FilesystemUuid> = self
.thin_pool
.filesystems()
.iter()
.filter_map(|(_, u, fs)| {
fs.origin()
.and_then(|x| if removed.contains(&x) { Some(*u) } else { None })
})
.collect();

let mut updated_origins = vec![];
for sn_uuid in snapshots {
if let Err(err) = self.thin_pool.unset_fs_origin(sn_uuid) {
warn!(
"Failed to write null origin to metadata for filesystem with UUID {}: {}",
sn_uuid, err
);
} else {
updated_origins.push(sn_uuid);
}
}

Ok(SetDeleteAction::new(removed, updated_origins))
self.thin_pool.destroy_filesystems(pool_name, fs_uuids)
}

#[pool_mutating_action("NoRequests")]
Expand Down
31 changes: 1 addition & 30 deletions src/engine/strat_engine/pool/v2.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -928,36 +928,7 @@ impl Pool for StratPool {
pool_name: &str,
fs_uuids: &HashSet<FilesystemUuid>,
) -> StratisResult<SetDeleteAction<FilesystemUuid, FilesystemUuid>> {
let mut removed = Vec::new();
for &uuid in fs_uuids {
if let Some(uuid) = self.thin_pool.destroy_filesystem(pool_name, uuid)? {
removed.push(uuid);
}
}

let snapshots: Vec<FilesystemUuid> = self
.thin_pool
.filesystems()
.iter()
.filter_map(|(_, u, fs)| {
fs.origin()
.and_then(|x| if removed.contains(&x) { Some(*u) } else { None })
})
.collect();

let mut updated_origins = vec![];
for sn_uuid in snapshots {
if let Err(err) = self.thin_pool.unset_fs_origin(sn_uuid) {
warn!(
"Failed to write null origin to metadata for filesystem with UUID {}: {}",
sn_uuid, err
);
} else {
updated_origins.push(sn_uuid);
}
}

Ok(SetDeleteAction::new(removed, updated_origins))
self.thin_pool.destroy_filesystems(pool_name, fs_uuids)
}

#[pool_mutating_action("NoRequests")]
Expand Down
83 changes: 64 additions & 19 deletions src/engine/strat_engine/thinpool/thinpool.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use devicemapper::{

use crate::{
engine::{
engine::{DumpState, StateDiff},
engine::{DumpState, Filesystem, StateDiff},
strat_engine::{
backstore::backstore::{v1, v2, InternalBackstore},
cmd::{thin_check, thin_metadata_size, thin_repair},
Expand All @@ -38,7 +38,10 @@ use crate::{
writing::wipe_sectors,
},
structures::Table,
types::{Compare, Diff, FilesystemUuid, Name, PoolUuid, StratFilesystemDiff, ThinPoolDiff},
types::{
Compare, Diff, FilesystemUuid, Name, PoolUuid, SetDeleteAction, StratFilesystemDiff,
ThinPoolDiff,
},
},
stratis::{StratisError, StratisResult},
};
Expand Down Expand Up @@ -603,7 +606,7 @@ impl<B> ThinPool<B> {
/// * Ok(Some(uuid)) provides the uuid of the destroyed filesystem
/// * Ok(None) is returned if the filesystem did not exist
/// * Err(_) is returned if the filesystem could not be destroyed
pub fn destroy_filesystem(
fn destroy_filesystem(
&mut self,
pool_name: &str,
uuid: FilesystemUuid,
Expand Down Expand Up @@ -765,6 +768,64 @@ impl<B> ThinPool<B> {
)
.map_err(|e| e.into())
}

pub fn destroy_filesystems(
&mut self,
pool_name: &str,
fs_uuids: &HashSet<FilesystemUuid>,
) -> StratisResult<SetDeleteAction<FilesystemUuid, FilesystemUuid>> {
let snapshots = self
.filesystems()
.iter()
.filter_map(|(_, u, fs)| {
fs.origin().and_then(|x| {
if fs_uuids.contains(&x) {
Some((x, (*u, fs.merge_scheduled())))
} else {
None
}
})
})
.fold(HashMap::new(), |mut acc, (u, v)| {
acc.entry(u)
.and_modify(|e: &mut Vec<_>| e.push(v))
.or_insert(vec![v]);
acc
});

let scheduled_for_merge = snapshots
.iter()
.filter(|(_, snaps)| snaps.iter().any(|(_, scheduled)| *scheduled))
.collect::<Vec<_>>();
if !scheduled_for_merge.is_empty() {
let err_str = format!("The filesystem destroy operation can not be begun until the revert operations for the following filesystem snapshots have been cancelled: {}", scheduled_for_merge.iter().map(|(u, _)| u.to_string()).collect::<Vec<_>>().join(", "));
return Err(StratisError::Msg(err_str));
}

let (mut removed, mut updated_origins) = (Vec::new(), Vec::new());
for &uuid in fs_uuids {
if let Some(uuid) = self.destroy_filesystem(pool_name, uuid)? {
removed.push(uuid);

let snaps = snapshots
.get(&uuid)
.expect("snapshots map construction ensures key is present");
// The filesystems in snaps may have been removed; any one of
// them may also be a filesystem that was scheduled for removal.
for (sn_uuid, _) in snaps {
if let Some((_, sn)) = self.get_mut_filesystem_by_uuid(*sn_uuid) {
assert!(sn.unset_origin());
updated_origins.push(*sn_uuid);

let (name, sn) = self.get_filesystem_by_uuid(*sn_uuid).expect("just got");
self.mdv.save_fs(&name, *sn_uuid, sn)?;
};
}
}
}

Ok(SetDeleteAction::new(removed, updated_origins))
}
}

impl ThinPool<v1::Backstore> {
Expand Down Expand Up @@ -1737,22 +1798,6 @@ where
Ok(changed)
}

pub fn unset_fs_origin(&mut self, fs_uuid: FilesystemUuid) -> 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.unset_origin()
};
if changed {
let (name, fs) = self
.get_filesystem_by_uuid(fs_uuid)
.expect("Looked up above.");
self.mdv.save_fs(&name, fs_uuid, fs)?;
}
Ok(changed)
}

/// Set the filesystem merge scheduled value for filesystem with given UUID.
pub fn set_fs_merge_scheduled(
&mut self,
Expand Down

0 comments on commit 03e19a3

Please sign in to comment.