github/kata-containers
1
0
Fork 1
mirror of https://github.com/kata-containers/kata-containers.git synced 2025-06-26 15:32:30 +00:00
Code Issues Projects Releases Wiki Activity

agent: refine StorageDeviceGeneric::cleanup()

Browse Source 
Refine StorageDeviceGeneric::cleanup() to improve safety.

Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>
This commit is contained in:
Jiang Liu 2023-09-01 10:09:41 +08:00
parent 53edb19374
commit 57e7bf14a6
1 changed files with 28 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
src/agent/src/storage/mod.rs
View File

@ -68,7 +68,7 @@ impl StorageDeviceGeneric {
impl StorageDevice for StorageDeviceGeneric { impl StorageDevice for StorageDeviceGeneric {
fn path(&self) -> Option<&str> { fn path(&self) -> Option<&str> {
self.path.as_ref().map(|v| v.as_str()) self.path.as_deref()
} }
fn cleanup(&self) -> Result<()> { fn cleanup(&self) -> Result<()> {
@ -91,13 +91,27 @@ impl StorageDevice for StorageDeviceGeneric {
let mounts = vec![path.to_string()]; let mounts = vec![path.to_string()];
remove_mounts(&mounts)?; remove_mounts(&mounts)?;
} }
if matches!(is_mounted(path), Ok(true)) {
return Err(anyhow!("failed to umount mountpoint {}", path));
}
let p = Path::new(path);
if p.is_dir() {
let is_empty = p.read_dir()?.next().is_none();
if !is_empty {
return Err(anyhow!("directory is not empty when clean up storage"));
}
// "remove_dir" will fail if the mount point is backed by a read-only filesystem. // "remove_dir" will fail if the mount point is backed by a read-only filesystem.
// This is the case with the device mapper snapshotter, where we mount the block device // This is the case with the device mapper snapshotter, where we mount the block device
// directly at the underlying sandbox path which was provided from the base RO kataShared // directly at the underlying sandbox path which was provided from the base RO kataShared
// path from the host. // path from the host.
if let Err(err) = fs::remove_dir(path) { let _ = fs::remove_dir(p);
//warn!(self.logger, "failed to remove dir {}, {:?}", path, err); } else if !p.is_file() {
// TODO: should we remove the file for bind mount?
return Err(anyhow!(
"storage path {} is neither directory nor file",
path
));
} }
Ok(()) Ok(())
@ -723,6 +737,8 @@ mod tests {
.tempdir_in(tmpdir_path) .tempdir_in(tmpdir_path)
.unwrap(); .unwrap();
let srcdir_path = srcdir.path().to_str().unwrap(); let srcdir_path = srcdir.path().to_str().unwrap();
let empty_file = Path::new(srcdir_path).join("emptyfile");
fs::write(&empty_file, "test").unwrap();
let destdir = Builder::new() let destdir = Builder::new()
.prefix("dest") .prefix("dest")
@ -755,7 +771,12 @@ mod tests {
let s = StorageDeviceGeneric::new(destdir_path.to_string()); let s = StorageDeviceGeneric::new(destdir_path.to_string());
assert!(s.cleanup().is_ok()); assert!(s.cleanup().is_ok());
// fail to remove non-empty directory
let s = StorageDeviceGeneric::new(srcdir_path.to_string());
s.cleanup().unwrap_err();
// remove a directory without umount // remove a directory without umount
fs::remove_file(&empty_file).unwrap();
s.cleanup().unwrap(); s.cleanup().unwrap();
} }