diff --git a/Cargo.toml b/Cargo.toml index aa1379d..556239a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,7 @@ rev = "4b23d3962c1baff6483caabf1ca15068216a7506" [dependencies.nix] # TODO(jmmv): Replace this with 0.12 once released. git = "https://github.com/nix-rust/nix.git" -rev = "eef3a432d57e8f830e05fede6e3099dcb689aa6b" +rev = "8c3e43ccd4fc83583c16848a35410022f5a8efc9" [dev-dependencies] tempdir = "0.3" diff --git a/WORKSPACE b/WORKSPACE index 77f1c9e..8187155 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -57,7 +57,7 @@ go_repository( go_repository( name = "org_golang_x_sys", importpath = "golang.org/x/sys", - commit = "4b45465282a4624cf39876842a017334f13b8aff", + commit = "9b800f95dbbc54abff0acf7ee32d88ba4e328c89", ) gazelle_dependencies() 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/integration/read_write_test.go b/integration/read_write_test.go index ca7d743..f1c064f 100644 --- a/integration/read_write_test.go +++ b/integration/read_write_test.go @@ -24,6 +24,8 @@ import ( "testing" "time" + "golang.org/x/sys/unix" + "github.com/bazelbuild/sandboxfs/integration/utils" "github.com/bazelbuild/sandboxfs/internal/sandbox" ) @@ -565,20 +567,11 @@ func TestReadWrite_Chmod(t *testing.T) { } }) - t.Run("DanglingSymlink", func(t *testing.T) { - utils.MustSymlink(t, "missing", state.RootPath("dangling-symlink")) - - path := state.MountPath("dangling-symlink") - if err := os.Chmod(path, 0555); err == nil { - t.Errorf("Want chmod to fail on dangling link, got success") - } - }) - - t.Run("GoodSymlink", func(t *testing.T) { + t.Run("Symlink", func(t *testing.T) { utils.MustWriteFile(t, state.RootPath("target"), 0644, "") - utils.MustSymlink(t, "target", state.RootPath("good-symlink")) + utils.MustSymlink(t, "target", state.RootPath("symlink")) - path := state.MountPath("good-symlink") + path := state.MountPath("symlink") linkFileInfo, err := os.Lstat(path) if err != nil { t.Fatalf("Failed to stat %s: %v", path, err) @@ -588,11 +581,11 @@ func TestReadWrite_Chmod(t *testing.T) { t.Fatalf("Failed to chmod %s: %v", path, err) } - if err := checkPerm("good-symlink", linkFileInfo.Mode()&os.ModePerm); err != nil { + if err := checkPerm("symlink", linkFileInfo.Mode()&os.ModePerm); err != nil { t.Error(err) } if err := checkPerm("target", 0200); err != nil { - t.Error(err) + t.Errorf("Mode of symlink target was modified but shouldn't have been: %v", err) } }) } @@ -664,9 +657,8 @@ func TestReadWrite_Chown(t *testing.T) { utils.MustMkdirAll(t, state.RootPath("dir"), 0755) utils.MustWriteFile(t, state.RootPath("file"), 0644, "new content") - utils.MustSymlink(t, "missing", state.RootPath("dangling-symlink")) utils.MustWriteFile(t, state.RootPath("target"), 0644, "") - utils.MustSymlink(t, "target", state.RootPath("good-symlink")) + utils.MustSymlink(t, "target", state.RootPath("symlink")) targetFileInfo, err := os.Lstat(state.RootPath("target")) if err != nil { @@ -683,8 +675,7 @@ func TestReadWrite_Chown(t *testing.T) { }{ {"Dir", "dir", 1, 2}, {"File", "file", 3, 4}, - {"DanglingSymlink", "dangling-symlink", 5, 6}, - {"GoodSymlink", "good-symlink", 7, 8}, + {"Symlink", "symlink", 7, 8}, } for _, d := range testData { t.Run(d.name, func(t *testing.T) { @@ -821,35 +812,22 @@ func TestReadWrite_Chtimes(t *testing.T) { } }) - t.Run("DanglingSymlink", func(t *testing.T) { - utils.MustSymlink(t, "missing", state.RootPath("dangling-symlink")) - - if _, err := chtimes("dangling-symlink", time.Unix(0, 0), time.Unix(0, 0)); err == nil { - t.Errorf("Want chtimes to fail on dangling link, got success") - } - }) - - t.Run("GoodSymlink", func(t *testing.T) { + t.Run("Symlink", func(t *testing.T) { utils.MustWriteFile(t, state.RootPath("target"), 0644, "") - utils.MustSymlink(t, "target", state.RootPath("good-symlink")) - path := state.MountPath("good-symlink") + utils.MustSymlink(t, "target", state.RootPath("symlink")) + path := state.MountPath("symlink") - linkFileInfo, err := os.Lstat(path) + atimeTimespec, err := unix.TimeToTimespec(someAtime) if err != nil { - t.Fatalf("Failed to stat %s: %v", path, err) + t.Fatalf("Failed to convert %v to a timespec: %v", someAtime, err) } - linkStat := linkFileInfo.Sys().(*syscall.Stat_t) - - wantMinCtime, err := chtimes(path, someAtime, someMtime) + mtimeTimespec, err := unix.TimeToTimespec(someMtime) if err != nil { - t.Fatal(err) + t.Fatalf("Failed to convert %v to a timespec: %v", someMtime, err) } - if err := checkTimes("good-symlink", time.Unix(0, 0), linkFileInfo.ModTime(), sandbox.Ctime(linkStat)); err != nil { - t.Error(err) - } - if err := checkTimes("target", someAtime, someMtime, wantMinCtime); err != nil { - t.Error(err) + if err = unix.UtimesNanoAt(unix.AT_FDCWD, path, []unix.Timespec{atimeTimespec, mtimeTimespec}, unix.AT_SYMLINK_NOFOLLOW); err == nil || err != syscall.EOPNOTSUPP { + t.Fatalf("Expected EOPNOTSUPP changing the times of a symlink; got %v", err) } }) } diff --git a/internal/sandbox/node.go b/internal/sandbox/node.go index ed80fdb..0b28617 100644 --- a/internal/sandbox/node.go +++ b/internal/sandbox/node.go @@ -342,6 +342,13 @@ func (n *BaseNode) setattrTimes(req *fuse.SetattrRequest) error { } if underlyingPath, isMapped := n.UnderlyingPath(); isMapped && !n.deleted { + if n.attr.Mode&os.ModeType == os.ModeSymlink { + // TODO(https://github.com/bazelbuild/sandboxfs/issues/46): Should use + // futimensat to support changing the times of a symlink if requested to + // do so. + return fuse.Errno(syscall.EOPNOTSUPP) + } + if err := os.Chtimes(underlyingPath, atime, mtime); err != nil { return err } diff --git a/src/lib.rs b/src/lib.rs index e5bccc2..dd04f9c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,6 +12,13 @@ // License for the specific language governing permissions and limitations // under the License. +// For portability reasons, we need to be able to cast integer values to system-level opaque +// types such as "mode_t". Because we don't know the size of those integers on the platform we +// are building for, sometimes the casts do widen the values but other times they are no-ops. +#![cfg_attr(feature = "cargo-clippy", allow(identity_conversion))] + +// We construct complex structures in multiple places, and allowing for redundant field names +// increases readability. #![cfg_attr(feature = "cargo-clippy", allow(redundant_field_names))] #[macro_use] extern crate failure; @@ -482,9 +489,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 +553,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/conv.rs b/src/nodes/conv.rs index cc6bd34..cbbabed 100644 --- a/src/nodes/conv.rs +++ b/src/nodes/conv.rs @@ -60,6 +60,18 @@ pub fn timespec_to_timeval(spec: Timespec) -> sys::time::TimeVal { TimeVal::seconds(spec.sec) + TimeVal::nanoseconds(spec.nsec.into()) } +/// Converts a `sys::time::TimeVal` object into a `time::Timespec`. +// TODO(jmmv): Consider upstreaming this function as a TimeVal method. +pub fn timeval_to_timespec(val: sys::time::TimeVal) -> Timespec { + let usec = if val.tv_usec() > sys::time::suseconds_t::from(std::i32::MAX) { + warn!("Cannot represent too-long usec quantity {} in timespec; using 0", val.tv_usec()); + 0 + } else { + val.tv_usec() as i32 + }; + Timespec::new(val.tv_sec() as sys::time::time_t, usec) +} + /// Converts a file type as returned by the file system to a FUSE file type. /// /// `path` is the file from which the file type was originally extracted and is only for debugging @@ -206,6 +218,14 @@ mod tests { assert_eq!(45, val.tv_usec()); } + #[test] + fn test_timeval_to_timespec() { + let val = sys::time::TimeVal::seconds(654) + sys::time::TimeVal::nanoseconds(123_456); + let spec = timeval_to_timespec(val); + assert_eq!(654, spec.sec); + assert_eq!(123, spec.nsec); + } + #[test] fn test_system_time_to_timespec_ok() { let sys_time = SystemTime::UNIX_EPOCH + Duration::new(12345, 6789); diff --git a/src/nodes/dir.rs b/src/nodes/dir.rs index f0a1edf..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,12 +518,16 @@ 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(); - if let Some(path) = &state.underlying_path { - setattr(path, &state.attr, delta)?; - } - Dir::getattr_locked(self.inode, &mut state) + state.attr = setattr(state.underlying_path.as_ref(), &state.attr, delta)?; + Ok(state.attr) } fn symlink(&self, name: &OsStr, link: &Path, ids: &IdGenerator, cache: &Cache) @@ -511,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 8901ede..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(); - setattr(&state.underlying_path, &state.attr, delta)?; - File::getattr_locked(self.inode, &mut state) + 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 6526f4f..565c97d 100644 --- a/src/nodes/mod.rs +++ b/src/nodes/mod.rs @@ -88,57 +88,143 @@ pub struct AttrDelta { /// Generic result type for of all node operations. pub type NodeResult = Result; -/// Updates the metadata of a file given a delta of attributes. -/// -/// This is a helper function to implement `Node::setattr` for the various node types. The caller -/// is responsible for obtaining the updated metadata of the underlying file to return it to the -/// kernel, as computing the updated metadata here would be inaccurate. The reason is that we don't -/// track ctimes ourselves so any modifications to the file cause the backing ctime to be updated -/// and we need to obtain it. -/// TODO(https://github.com/bazelbuild/sandboxfs/issues/43): Compute the modified fuse::FileAttr and -/// return it once we track ctimes. -/// -/// This tries to apply as many properties as possible in case of errors. When errors occur, -/// returns the first that was encountered. -pub fn setattr(path: &Path, attr: &fuse::FileAttr, delta: &AttrDelta) -> Result<(), nix::Error> { - let mut result = Ok(()); +/// Applies an operation to a path if present, or otherwise returns Ok. +fn try_path nix::Result<()>>(path: Option<&PathBuf>, op: O) -> nix::Result<()> { + match path { + Some(path) => op(path), + None => Ok(()), + } +} - if let Some(mode) = delta.mode { - result = result.and( - sys::stat::fchmodat(None, path, mode, sys::stat::FchmodatFlags::NoFollowSymlink)); +/// Helper function for `setattr` to apply only the mode changes. +fn setattr_mode(attr: &mut fuse::FileAttr, path: Option<&PathBuf>, mode: Option) + -> Result<(), nix::Error> { + if mode.is_none() { + return Ok(()) } + let mode = mode.unwrap(); - if delta.uid.is_some() || delta.gid.is_some() { - result = result.and( - unistd::fchownat(None, path, delta.uid, delta.gid, - unistd::FchownatFlags::NoFollowSymlink)); + if mode.bits() > sys::stat::mode_t::from(std::u16::MAX) { + warn!("Got setattr with mode {:?} for {:?} (inode {}), which is too large; ignoring", + mode, path, attr.ino); + return Err(nix::Error::from_errno(Errno::EIO)); } + let perm = mode.bits() as u16; - if delta.atime.is_some() || delta.mtime.is_some() { - let atime = delta.atime.unwrap_or_else(|| conv::timespec_to_timeval(attr.atime)); - let mtime = delta.mtime.unwrap_or_else(|| conv::timespec_to_timeval(attr.mtime)); - if attr.kind == fuse::FileType::Symlink { - // TODO(jmmv): Should use futimensat to avoid following symlinks but this function is - // not available on macOS El Capitan, which we currently require for CI builds. - warn!("Asked to update atime/mtime for symlink {} but following symlink instead", - path.display()); - } - result = result.and(sys::stat::utimes(path, &atime, &mtime)); + let result = try_path(path, |p| + sys::stat::fchmodat(None, p, mode, sys::stat::FchmodatFlags::NoFollowSymlink)); + if result.is_ok() { + attr.perm = perm; + } + result +} + +/// Helper function for `setattr` to apply only the UID and GID changes. +fn setattr_owners(attr: &mut fuse::FileAttr, path: Option<&PathBuf>, uid: Option, + gid: Option) -> Result<(), nix::Error> { + if uid.is_none() && gid.is_none() { + return Ok(()) + } + + let result = try_path(path, |p| + unistd::fchownat(None, p, uid, gid, unistd::FchownatFlags::NoFollowSymlink)); + if result.is_ok() { + attr.uid = uid.map_or(attr.uid, u32::from); + attr.gid = uid.map_or(attr.gid, u32::from); + } + result +} + +/// Helper function for `setattr` to apply only the atime and mtime changes. +fn setattr_times(attr: &mut fuse::FileAttr, path: Option<&PathBuf>, + atime: Option, mtime: Option) + -> Result<(), nix::Error> { + if atime.is_none() && mtime.is_none() { + return Ok(()); + } + + if attr.kind == fuse::FileType::Symlink { + // TODO(https://github.com/bazelbuild/sandboxfs/issues/46): Should use futimensat to support + // changing the times of a symlink if requested to do so. + return Err(nix::Error::from_errno(Errno::EOPNOTSUPP)); + } + + let atime = atime.unwrap_or_else(|| conv::timespec_to_timeval(attr.atime)); + let mtime = mtime.unwrap_or_else(|| conv::timespec_to_timeval(attr.mtime)); + let result = try_path(path, |p| sys::stat::utimes(p, &atime, &mtime)); + if result.is_ok() { + attr.atime = conv::timeval_to_timespec(atime); + attr.mtime = conv::timeval_to_timespec(mtime); + } + result +} + +/// Helper function for `setattr` to apply only the size changes. +fn setattr_size(attr: &mut fuse::FileAttr, path: Option<&PathBuf>, size: Option) + -> Result<(), nix::Error> { + if size.is_none() { + return Ok(()); + } + let size = size.unwrap(); + + let result = if size > ::std::i64::MAX as u64 { + warn!("truncate request got size {}, which is too large (exceeds i64's MAX)", size); + Err(nix::Error::invalid_argument()) + } else { + try_path(path, |p| unistd::truncate(p, size as i64)) + }; + if result.is_ok() { + attr.size = size; } + result +} + +/// Updates the metadata of a file given a delta of attributes. +/// +/// This is a helper function to implement `Node::setattr` for the various node types. +/// +/// This tries to apply as many properties as possible in case of errors. When errors occur, +/// returns the first that was encountered. +pub fn setattr(path: Option<&PathBuf>, attr: &fuse::FileAttr, delta: &AttrDelta) + -> Result { + // Compute the potential new ctime for these updates. We want to avoid picking a ctime that is + // larger than what the operations below can result in (so as to prevent a future getattr from + // moving the ctime back) which is tricky because we don't know the time resolution of the + // underlying file system. Some, like HFS+, only have 1-second resolution... so pick that in + // the worst case. + // + // This is not perfectly accurate because we don't actually know what ctime the underlying file + // has gotten and thus a future getattr on it will cause the ctime to change. But as long as we + // avoid going back on time, we can afford to do this -- which is a requirement for supporting + // setting attributes on deleted files. + // + // TODO(https://github.com/bazelbuild/sandboxfs/issues/43): Revisit this when we track + // ctimes purely on our own. + let updated_ctime = { + let mut now = time::now().to_timespec(); + now.nsec = 0; + if attr.ctime > now { + attr.ctime + } else { + now + } + }; - if let Some(size) = delta.size { + let mut new_attr = *attr; + // Intentionally using and() instead of and_then() to try to apply all changes but to only + // keep the first error result. + let result = Ok(()) + .and(setattr_mode(&mut new_attr, path, delta.mode)) + .and(setattr_owners(&mut new_attr, path, delta.uid, delta.gid)) + .and(setattr_times(&mut new_attr, path, delta.atime, delta.mtime)) // Updating the size only makes sense on files, but handling it here is much simpler than // doing so on a node type basis. Plus, who knows, if the kernel asked us to change the // size of anything other than a file, maybe we have to obey and try to do it. - if size > ::std::i64::MAX as u64 { - warn!("truncate request got size {}, which is too large (exceeds i64's MAX)", size); - result = result.and(Err(nix::Error::invalid_argument())); - } else { - result = result.and(unistd::truncate(path, size as i64)); - } + .and(setattr_size(&mut new_attr, path, delta.size)); + if *attr != new_attr { + new_attr.ctime = updated_ctime; } - - result + result.and(Ok(new_attr)) } /// Abstract representation of an open file handle. @@ -204,6 +290,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 @@ -280,6 +372,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; @@ -294,4 +391,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 19d77e5..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(); - setattr(&state.underlying_path, &state.attr, delta)?; - Symlink::getattr_locked(self.inode, &mut state) + state.attr = setattr(state.underlying_path.as_ref(), &state.attr, delta)?; + Ok(state.attr) } }