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

@@ -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

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<u32>, uid: Option<u32>,
@@ -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],

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<R>(&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<fuse::FileAttr> {
         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))
+    }
 }

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<PathBuf>,
     attr: fuse::FileAttr,
 }
 
@@ -85,22 +85,24 @@ 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) })
     }
 
     /// Same as `getattr` but with the node already locked.
     fn getattr_locked(inode: u64, state: &mut MutableFile) -> NodeResult<fuse::FileAttr> {
-        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<fuse::FileAttr> {
         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<fuse::FileAttr> {
         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)
     }
 }

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<fuse::FileAttr>;
 
@@ -372,4 +383,9 @@ pub trait Node {
         -> NodeResult<(Arc<Node>, fuse::FileAttr)> {
         panic!("Not implemented")
     }
+
+    /// Deletes the file `_name`.
+    fn unlink(&self, _name: &OsStr) -> NodeResult<()> {
+        panic!("Not implemented");
+    }
 }

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<PathBuf>,
     attr: fuse::FileAttr,
 }
 
@@ -52,22 +52,24 @@ 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) })
     }
 
     /// Same as `getattr` but with the node already locked.
     fn getattr_locked(inode: u64, state: &mut MutableSymlink) -> NodeResult<fuse::FileAttr> {
-        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<fuse::FileAttr> {
         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<PathBuf> {
         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<fuse::FileAttr> {
         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)
     }
 }