From 359e3f25c7f8e9b1e81b9085c97e285e806bc592 Mon Sep 17 00:00:00 2001 From: Julio Merino Date: Wed, 31 Oct 2018 16:24:41 -0400 Subject: [PATCH] Implement rmdir and unlink To support operations on deleted nodes, this changes File and Symlink so that their underlying path is optional (Dir already had this). When None we treat these nodes as deleted, which means that some operations become impossible on them and others, like setattr, only have affect in-memory node data. --- admin/travis-build.sh | 6 ------ src/lib.rs | 28 +++++++++++++++++++------ src/nodes/dir.rs | 48 +++++++++++++++++++++++++++++++++++-------- src/nodes/file.rs | 34 ++++++++++++++++++++---------- src/nodes/mod.rs | 16 +++++++++++++++ src/nodes/symlink.rs | 34 ++++++++++++++++++++---------- 6 files changed, 123 insertions(+), 43 deletions(-) diff --git a/admin/travis-build.sh b/admin/travis-build.sh index d1a03a0..5e86176 100755 --- a/admin/travis-build.sh +++ b/admin/travis-build.sh @@ -91,16 +91,10 @@ do_rust() { TestProfiling_FileProfiles TestProfiling_BadConfiguration TestReadOnly_Attributes - TestReadWrite_Remove TestReadWrite_InodeReassignedAfterRecreation - TestReadWrite_FstatOnDeletedNode - TestReadWrite_FtruncateOnDeletedFile TestReadWrite_NestedMappingsInheritDirectoryProperties TestReadWrite_RenameFile TestReadWrite_MoveFile - TestReadWrite_FchmodOnDeletedNode - TestReadWrite_FchownOnDeletedNode - TestReadWrite_FutimesOnDeletedNode TestReconfiguration_Streams TestReconfiguration_Steps TestReconfiguration_Unmap diff --git a/src/lib.rs b/src/lib.rs index e5bccc2..1025a4e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -482,9 +482,17 @@ impl fuse::Filesystem for SandboxFS { panic!("Required RW operation not yet implemented"); } - fn rmdir(&mut self, _req: &fuse::Request, _parent: u64, _name: &OsStr, - _reply: fuse::ReplyEmpty) { - panic!("Required RW operation not yet implemented"); + fn rmdir(&mut self, _req: &fuse::Request, parent: u64, name: &OsStr, reply: fuse::ReplyEmpty) { + let dir_node = self.find_node(parent); + if !dir_node.writable() { + reply.error(Errno::EPERM as i32); + return; + } + + match dir_node.rmdir(name) { + Ok(_) => reply.ok(), + Err(e) => reply.error(e.errno_as_i32()), + } } fn setattr(&mut self, _req: &fuse::Request, inode: u64, mode: Option, uid: Option, @@ -538,9 +546,17 @@ impl fuse::Filesystem for SandboxFS { } } - fn unlink(&mut self, _req: &fuse::Request, _parent: u64, _name: &OsStr, - _reply: fuse::ReplyEmpty) { - panic!("Required RW operation not yet implemented"); + fn unlink(&mut self, _req: &fuse::Request, parent: u64, name: &OsStr, reply: fuse::ReplyEmpty) { + let dir_node = self.find_node(parent); + if !dir_node.writable() { + reply.error(Errno::EPERM as i32); + return; + } + + match dir_node.unlink(name) { + Ok(_) => reply.ok(), + Err(e) => reply.error(e.errno_as_i32()), + } } fn write(&mut self, _req: &fuse::Request, _inode: u64, fh: u64, offset: i64, data: &[u8], diff --git a/src/nodes/dir.rs b/src/nodes/dir.rs index 5fb3fe6..8f37d08 100644 --- a/src/nodes/dir.rs +++ b/src/nodes/dir.rs @@ -254,12 +254,12 @@ impl Dir { if let Some(path) = &state.underlying_path { let fs_attr = fs::symlink_metadata(path)?; if !fs_attr.is_dir() { - warn!("Path {:?} backing a directory node is no longer a directory; got {:?}", - path, fs_attr.file_type()); + warn!("Path {} backing a directory node is no longer a directory; got {:?}", + path.display(), fs_attr.file_type()); return Err(KernelError::from_errno(errno::Errno::EIO)); } state.attr = conv::attr_fs_to_fuse(path, inode, &fs_attr); - }; + } Ok(state.attr) } @@ -348,6 +348,22 @@ impl Dir { } } } + + /// Common implementation for the `rmdir` and `unlink` operations. + /// + /// The behavior of these operations differs only in the syscall we invoke to delete the + /// underlying entry, which is passed in as the `remove` parameter. + fn remove_any(&self, name: &OsStr, remove: R) -> NodeResult<()> + where R: Fn(&PathBuf) -> io::Result<()> { + let mut state = self.state.lock().unwrap(); + let path = Dir::get_writable_path(&mut state, name)?; + + remove(&path)?; + + state.children[name].node.delete(); + state.children.remove(name); + Ok(()) + } } impl Node for Dir { @@ -363,6 +379,14 @@ impl Node for Dir { fuse::FileType::Directory } + fn delete(&self) { + let mut state = self.state.lock().unwrap(); + assert!( + state.underlying_path.is_some(), + "Delete already called or trying to delete an explicit mapping"); + state.underlying_path = None; + } + fn map(&self, components: &[Component], underlying_path: &Path, writable: bool, ids: &IdGenerator, cache: &Cache) -> Result<(), Error> { let (name, remainder) = split_components(components); @@ -494,14 +518,14 @@ impl Node for Dir { })) } + fn rmdir(&self, name: &OsStr) -> NodeResult<()> { + // TODO(jmmv): Figure out how to remove the redundant closure. + #[cfg_attr(feature = "cargo-clippy", allow(redundant_closure))] + self.remove_any(name, |p| fs::remove_dir(p)) + } + fn setattr(&self, delta: &AttrDelta) -> NodeResult { let mut state = self.state.lock().unwrap(); - - // setattr only gets called on writable nodes, which must have an underlying path. This - // assertion will go away when we have the ability to delete nodes as those may still - // exist in memory but have no corresponding underlying path. - debug_assert!(state.underlying_path.is_some()); - state.attr = setattr(state.underlying_path.as_ref(), &state.attr, delta)?; Ok(state.attr) } @@ -515,4 +539,10 @@ impl Node for Dir { Dir::post_create_lookup(self.writable, &mut state, &path, name, fuse::FileType::Symlink, ids, cache) } + + fn unlink(&self, name: &OsStr) -> NodeResult<()> { + // TODO(jmmv): Figure out how to remove the redundant closure. + #[cfg_attr(feature = "cargo-clippy", allow(redundant_closure))] + self.remove_any(name, |p| fs::remove_file(p)) + } } diff --git a/src/nodes/file.rs b/src/nodes/file.rs index 9b19f0d..deb1676 100644 --- a/src/nodes/file.rs +++ b/src/nodes/file.rs @@ -58,7 +58,7 @@ pub struct File { /// Holds the mutable data of a file node. struct MutableFile { - underlying_path: PathBuf, + underlying_path: Option, attr: fuse::FileAttr, } @@ -85,8 +85,8 @@ impl File { let attr = conv::attr_fs_to_fuse(underlying_path, inode, &fs_attr); let state = MutableFile { - underlying_path: PathBuf::from(underlying_path), - attr, + underlying_path: Some(PathBuf::from(underlying_path)), + attr: attr, }; Arc::new(File { inode, writable, state: Mutex::from(state) }) @@ -94,13 +94,15 @@ impl File { /// Same as `getattr` but with the node already locked. fn getattr_locked(inode: u64, state: &mut MutableFile) -> NodeResult { - let fs_attr = fs::symlink_metadata(&state.underlying_path)?; - if !File::supports_type(fs_attr.file_type()) { - warn!("Path {:?} backing a file node is no longer a file; got {:?}", - &state.underlying_path, fs_attr.file_type()); - return Err(KernelError::from_errno(errno::Errno::EIO)); + if let Some(path) = &state.underlying_path { + let fs_attr = fs::symlink_metadata(path)?; + if !File::supports_type(fs_attr.file_type()) { + warn!("Path {} backing a file node is no longer a file; got {:?}", + path.display(), fs_attr.file_type()); + return Err(KernelError::from_errno(errno::Errno::EIO)); + } + state.attr = conv::attr_fs_to_fuse(path, inode, &fs_attr); } - state.attr = conv::attr_fs_to_fuse(&state.underlying_path, inode, &fs_attr); Ok(state.attr) } @@ -120,6 +122,14 @@ impl Node for File { state.attr.kind } + fn delete(&self) { + let mut state = self.state.lock().unwrap(); + assert!( + state.underlying_path.is_some(), + "Delete already called or trying to delete an explicit mapping"); + state.underlying_path = None; + } + fn getattr(&self) -> NodeResult { let mut state = self.state.lock().unwrap(); File::getattr_locked(self.inode, &mut state) @@ -129,13 +139,15 @@ impl Node for File { let state = self.state.lock().unwrap(); let options = conv::flags_to_openoptions(flags, self.writable)?; - let file = options.open(&state.underlying_path)?; + let path = state.underlying_path.as_ref().expect( + "Don't know how to handle a request to reopen a deleted file"); + let file = options.open(path)?; Ok(Arc::from(file)) } fn setattr(&self, delta: &AttrDelta) -> NodeResult { let mut state = self.state.lock().unwrap(); - state.attr = setattr(Some(&state.underlying_path), &state.attr, delta)?; + state.attr = setattr(state.underlying_path.as_ref(), &state.attr, delta)?; Ok(state.attr) } } diff --git a/src/nodes/mod.rs b/src/nodes/mod.rs index a72bd9a..d9ed116 100644 --- a/src/nodes/mod.rs +++ b/src/nodes/mod.rs @@ -282,6 +282,12 @@ pub trait Node { /// `Symlink`s (on which we don't allow type changes at all), it's OK. fn file_type_cached(&self) -> fuse::FileType; + /// Marks the node as deleted (though the in-memory representation remains). + /// + /// The implication of this is that a node loses its backing underlying path once this method + /// is called. + fn delete(&self); + /// Maps a path onto a node and creates intermediate components as immutable directories. /// /// `_components` is the path to map, broken down into components, and relative to the current @@ -358,6 +364,11 @@ pub trait Node { panic!("Not implemented"); } + /// Deletes the empty directory `_name`. + fn rmdir(&self, _name: &OsStr) -> NodeResult<()> { + panic!("Not implemented"); + } + /// Sets one or more properties of the node's metadata. fn setattr(&self, _delta: &AttrDelta) -> NodeResult; @@ -372,4 +383,9 @@ pub trait Node { -> NodeResult<(Arc, fuse::FileAttr)> { panic!("Not implemented") } + + /// Deletes the file `_name`. + fn unlink(&self, _name: &OsStr) -> NodeResult<()> { + panic!("Not implemented"); + } } diff --git a/src/nodes/symlink.rs b/src/nodes/symlink.rs index 0e58eae..199c91d 100644 --- a/src/nodes/symlink.rs +++ b/src/nodes/symlink.rs @@ -30,7 +30,7 @@ pub struct Symlink { /// Holds the mutable data of a symlink node. struct MutableSymlink { - underlying_path: PathBuf, + underlying_path: Option, attr: fuse::FileAttr, } @@ -52,8 +52,8 @@ impl Symlink { let attr = conv::attr_fs_to_fuse(underlying_path, inode, &fs_attr); let state = MutableSymlink { - underlying_path: PathBuf::from(underlying_path), - attr, + underlying_path: Some(PathBuf::from(underlying_path)), + attr: attr, }; Arc::new(Symlink { inode, writable, state: Mutex::from(state) }) @@ -61,13 +61,15 @@ impl Symlink { /// Same as `getattr` but with the node already locked. fn getattr_locked(inode: u64, state: &mut MutableSymlink) -> NodeResult { - let fs_attr = fs::symlink_metadata(&state.underlying_path)?; - if !fs_attr.file_type().is_symlink() { - warn!("Path {:?} backing a symlink node is no longer a symlink; got {:?}", - &state.underlying_path, fs_attr.file_type()); - return Err(KernelError::from_errno(errno::Errno::EIO)); + if let Some(path) = &state.underlying_path { + let fs_attr = fs::symlink_metadata(path)?; + if !fs_attr.file_type().is_symlink() { + warn!("Path {} backing a symlink node is no longer a symlink; got {:?}", + path.display(), fs_attr.file_type()); + return Err(KernelError::from_errno(errno::Errno::EIO)); + } + state.attr = conv::attr_fs_to_fuse(path, inode, &fs_attr); } - state.attr = conv::attr_fs_to_fuse(&state.underlying_path, inode, &fs_attr); Ok(state.attr) } @@ -86,6 +88,14 @@ impl Node for Symlink { fuse::FileType::Symlink } + fn delete(&self) { + let mut state = self.state.lock().unwrap(); + assert!( + state.underlying_path.is_some(), + "Delete already called or trying to delete an explicit mapping"); + state.underlying_path = None; + } + fn getattr(&self) -> NodeResult { let mut state = self.state.lock().unwrap(); Symlink::getattr_locked(self.inode, &mut state) @@ -94,12 +104,14 @@ impl Node for Symlink { fn readlink(&self) -> NodeResult { let state = self.state.lock().unwrap(); - Ok(fs::read_link(&state.underlying_path)?) + let path = state.underlying_path.as_ref().expect( + "There is no known API to get the target of a deleted symlink"); + Ok(fs::read_link(path)?) } fn setattr(&self, delta: &AttrDelta) -> NodeResult { let mut state = self.state.lock().unwrap(); - state.attr = setattr(Some(&state.underlying_path), &state.attr, delta)?; + state.attr = setattr(state.underlying_path.as_ref(), &state.attr, delta)?; Ok(state.attr) } }