From baffc23ee6c7b9c863224cb7d335b92916ae0a4e Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 09:58:07 +1100 Subject: [PATCH 01/21] MemFSTest.Rmdir_Empty --- samples/memfs/memfs_test.go | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index 683413f..21524f7 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -328,7 +328,32 @@ func (t *MemFSTest) Rmdir_NonEmpty() { } func (t *MemFSTest) Rmdir_Empty() { - AssertTrue(false, "TODO") + var err error + var entries []os.FileInfo + + // Create two levels of directories. + err = os.MkdirAll(path.Join(t.mfs.Dir(), "foo/bar"), 0754) + AssertEq(nil, err) + + // Remove the leaf. + err = os.Remove(path.Join(t.mfs.Dir(), "foo/bar")) + AssertEq(nil, err) + + // There should be nothing left in the parent. + entries, err = ioutil.ReadDir(path.Join(t.mfs.Dir(), "foo")) + + AssertEq(nil, err) + ExpectThat(entries, ElementsAre()) + + // Remove the parent. + err = os.Remove(path.Join(t.mfs.Dir(), "foo")) + AssertEq(nil, err) + + // Now the root directory should be empty, too. + entries, err = ioutil.ReadDir(t.mfs.Dir()) + + AssertEq(nil, err) + ExpectThat(entries, ElementsAre()) } func (t *MemFSTest) Rmdir_NotADirectory() { From 9f984e1aa652d942fa86aa68ba6ed6e0bc255d7d Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 11:12:50 +1100 Subject: [PATCH 02/21] Declared a test. --- samples/memfs/memfs_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index 21524f7..0f83259 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -363,3 +363,7 @@ func (t *MemFSTest) Rmdir_NotADirectory() { func (t *MemFSTest) Rmdir_NonExistent() { AssertTrue(false, "TODO") } + +func (t *MemFSTest) Rmdir_OpenedForReading() { + AssertTrue(false, "TODO") +} From 63408f6a0db1a4230e644d15094bbfe576741e09 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 11:25:50 +1100 Subject: [PATCH 03/21] Added an RmDir method. --- file_system.go | 30 +++++++++++++++++++++++++ fuseutil/not_implemented_file_system.go | 6 +++++ server.go | 27 ++++++++++++++++++++++ 3 files changed, 63 insertions(+) diff --git a/file_system.go b/file_system.go index cf6276c..e6c04fd 100644 --- a/file_system.go +++ b/file_system.go @@ -62,6 +62,24 @@ type FileSystem interface { ctx context.Context, req *MkDirRequest) (*MkDirResponse, error) + /////////////////////////////////// + // Inode destruction + /////////////////////////////////// + + // Unlink a directory from its parent. Because directories cannot have a link + // count above one, this means the directory inode should be deleted as well + // once the kernel calls ForgetInode. + // + // The file system is responsible for checking that the directory is empty. + // + // Sample implementation in ext2: ext2_rmdir (http://goo.gl/B9QmFf) + // + // TODO(jacobsa): Add tests for the assertion about directory link counts + // above (on a real file system and on memfs). + RmDir( + ctx context.Context, + req *RmDirRequest) (*RmDirResponse, error) + /////////////////////////////////// // Directory handles /////////////////////////////////// @@ -360,6 +378,18 @@ type MkDirResponse struct { Entry ChildInodeEntry } +type RmDirRequest struct { + Header RequestHeader + + // The ID of parent directory inode, and the name of the directory being + // removed within it. + Parent InodeID + Name string +} + +type RmDirResponse struct { +} + type OpenDirRequest struct { Header RequestHeader diff --git a/fuseutil/not_implemented_file_system.go b/fuseutil/not_implemented_file_system.go index 8453d97..e48d449 100644 --- a/fuseutil/not_implemented_file_system.go +++ b/fuseutil/not_implemented_file_system.go @@ -46,6 +46,12 @@ func (fs *NotImplementedFileSystem) MkDir( return nil, fuse.ENOSYS } +func (fs *NotImplementedFileSystem) RmDir( + ctx context.Context, + req *fuse.RmDirRequest) (*fuse.RmDirResponse, error) { + return nil, fuse.ENOSYS +} + func (fs *NotImplementedFileSystem) OpenDir( ctx context.Context, req *fuse.OpenDirRequest) (*fuse.OpenDirResponse, error) { diff --git a/server.go b/server.go index 762795e..c79ff6d 100644 --- a/server.go +++ b/server.go @@ -184,6 +184,33 @@ func (s *server) handleFuseRequest(fuseReq bazilfuse.Request) { s.logger.Println("Responding:", fuseResp) typed.Respond(fuseResp) + case *bazilfuse.RemoveRequest: + // We don't yet support files. + if !typed.Dir { + s.logger.Println("Not supported for files. Returning ENOSYS.") + typed.RespondError(ENOSYS) + return + } + + // Convert the request. + req := &RmDirRequest{ + Header: convertHeader(typed.Header), + Parent: InodeID(typed.Header.Node), + Name: typed.Name, + } + + // Call the file system. + _, err := s.fs.RmDir(ctx, req) + if err != nil { + s.logger.Println("Responding:", err) + typed.RespondError(err) + return + } + + // Respond successfully. + s.logger.Println("Responding OK.") + typed.Respond() + case *bazilfuse.OpenRequest: // Directory or file? if typed.Dir { From 57cb4ccda3997583e2e24733c360f4edbf5f0e0e Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 11:28:41 +1100 Subject: [PATCH 04/21] Outlined memFS.RmDir. --- samples/memfs/fs.go | 23 +++++++++++++++++++++++ samples/memfs/inode.go | 7 +++++++ 2 files changed, 30 insertions(+) diff --git a/samples/memfs/fs.go b/samples/memfs/fs.go index 27906ac..6e8b216 100644 --- a/samples/memfs/fs.go +++ b/samples/memfs/fs.go @@ -294,6 +294,29 @@ func (fs *memFS) MkDir( return } +func (fs *memFS) RmDir( + ctx context.Context, + req *fuse.RmDirRequest) (resp *fuse.RmDirResponse, err error) { + resp = &fuse.RmDirResponse{} + + fs.mu.Lock() + defer fs.mu.Unlock() + + // Grab the parent, which we will update shortly. + parent := fs.getInodeForModifyingOrDie(req.Parent) + defer parent.mu.Unlock() + + // TODO(jacobsa): Check for empty. (Make sure we have a failing test first.) + + // Remove the entry within the parent. + parent.RemoveChild(req.Name) + + // TODO(jacobsa): Remove the child when it's forgotten. (Can we get a failing + // test by looking at inode ID allocation?) + + return +} + func (fs *memFS) OpenDir( ctx context.Context, req *fuse.OpenDirRequest) (resp *fuse.OpenDirResponse, err error) { diff --git a/samples/memfs/inode.go b/samples/memfs/inode.go index f17306e..af641ea 100644 --- a/samples/memfs/inode.go +++ b/samples/memfs/inode.go @@ -158,6 +158,13 @@ func (inode *inode) AddChild( inode.entries = append(inode.entries, e) } +// Remove an entry for a child. +// +// REQUIRES: inode.dir +// REQUIRES: An entry for the given name exists. +// EXCLUSIVE_LOCKS_REQUIRED(inode.mu) +func (inode *inode) RemoveChild(name string) + // Serve a ReadDir request. // // REQUIRED: inode.dir From 621df57d8056fcdddd28874e39ce0374f544e1e0 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 14:23:46 +1100 Subject: [PATCH 05/21] Refactored inode.LookUpChild. --- samples/memfs/inode.go | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/samples/memfs/inode.go b/samples/memfs/inode.go index af641ea..f953a91 100644 --- a/samples/memfs/inode.go +++ b/samples/memfs/inode.go @@ -116,6 +116,26 @@ func (inode *inode) checkInvariants() { } } +// Return the index of the child within inode.entries, if it exists. +// +// REQUIRES: inode.dir +// SHARED_LOCKS_REQUIRED(inode.mu) +func (inode *inode) findChild(name string) (i int, ok bool) { + if !inode.dir { + panic("findChild called on non-directory.") + } + + var e fuseutil.Dirent + for i, e = range inode.entries { + if e.Name == name { + ok = true + return + } + } + + return +} + //////////////////////////////////////////////////////////////////////// // Public methods //////////////////////////////////////////////////////////////////////// @@ -125,16 +145,9 @@ func (inode *inode) checkInvariants() { // REQUIRES: inode.dir // SHARED_LOCKS_REQUIRED(inode.mu) func (inode *inode) LookUpChild(name string) (id fuse.InodeID, ok bool) { - if !inode.dir { - panic("LookUpChild called on non-directory.") - } - - for _, e := range inode.entries { - if e.Name == name { - id = e.Inode - ok = true - return - } + index, ok := inode.findChild(name) + if ok { + id = inode.entries[index].Inode } return @@ -167,7 +180,7 @@ func (inode *inode) RemoveChild(name string) // Serve a ReadDir request. // -// REQUIRED: inode.dir +// REQUIRES: inode.dir // SHARED_LOCKS_REQUIRED(inode.mu) func (inode *inode) ReadDir(offset int, size int) (data []byte, err error) { if !inode.dir { From 400d74dbdddd7e01086824a258b190f24735fe3a Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 14:29:49 +1100 Subject: [PATCH 06/21] Implemented inode.RemoveChild. --- samples/memfs/inode.go | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/samples/memfs/inode.go b/samples/memfs/inode.go index f953a91..5017290 100644 --- a/samples/memfs/inode.go +++ b/samples/memfs/inode.go @@ -38,7 +38,8 @@ type inode struct { // INVARIANT: If !dir, then os.ModeDir is not set attributes fuse.InodeAttributes // GUARDED_BY(mu) - // For directories, entries describing the children of the directory. + // For directories, entries describing the children of the directory. Unused + // entries are of type DT_Unknown. // // This array can never be shortened, nor can its elements be moved, because // we use its indices for Dirent.Offset, which is exposed to the user who @@ -50,7 +51,7 @@ type inode struct { // // INVARIANT: If dir is false, this is nil. // INVARIANT: For each i, entries[i].Offset == i+1 - // INVARIANT: Contains no duplicate names. + // INVARIANT: Contains no duplicate names in used entries. entries []fuseutil.Dirent // GUARDED_BY(mu) // For files, the current contents of the file. @@ -100,11 +101,13 @@ func (inode *inode) checkInvariants() { panic(fmt.Sprintf("Unexpected offset: %v", e.Offset)) } - if _, ok := childNames[e.Name]; ok { - panic(fmt.Sprintf("Duplicate name: %s", e.Name)) - } + if e.Type != fuseutil.DT_Unknown { + if _, ok := childNames[e.Name]; ok { + panic(fmt.Sprintf("Duplicate name: %s", e.Name)) + } - childNames[e.Name] = struct{}{} + childNames[e.Name] = struct{}{} + } } } @@ -156,6 +159,7 @@ func (inode *inode) LookUpChild(name string) (id fuse.InodeID, ok bool) { // Add an entry for a child. // // REQUIRES: inode.dir +// REQUIRES: dt != fuseutil.DT_Unknown // EXCLUSIVE_LOCKS_REQUIRED(inode.mu) func (inode *inode) AddChild( id fuse.InodeID, @@ -169,6 +173,8 @@ func (inode *inode) AddChild( } inode.entries = append(inode.entries, e) + + // TODO(jacobsa): Re-use gaps. } // Remove an entry for a child. @@ -176,7 +182,18 @@ func (inode *inode) AddChild( // REQUIRES: inode.dir // REQUIRES: An entry for the given name exists. // EXCLUSIVE_LOCKS_REQUIRED(inode.mu) -func (inode *inode) RemoveChild(name string) +func (inode *inode) RemoveChild(name string) { + // Find the entry. + i, ok := inode.findChild(name) + if !ok { + panic(fmt.Sprintf("Unknown child: %s", name)) + } + + // Mark it as unused. + inode.entries[i] = fuseutil.Dirent{ + Type: fuseutil.DT_Unknown, + } +} // Serve a ReadDir request. // @@ -188,6 +205,13 @@ func (inode *inode) ReadDir(offset int, size int) (data []byte, err error) { } for i := offset; i < len(inode.entries); i++ { + e := inode.entries[i] + + // Skip unused entries. + if e.Type == fuseutil.DT_Unknown { + continue + } + data = fuseutil.AppendDirent(data, inode.entries[i]) // Trim and stop early if we've exceeded the requested size. From 622d7a176bf22f4b421d1b3a724eaf32945720b6 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 14:30:14 +1100 Subject: [PATCH 07/21] Preserve the offset invariant. --- samples/memfs/inode.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/samples/memfs/inode.go b/samples/memfs/inode.go index 5017290..c969767 100644 --- a/samples/memfs/inode.go +++ b/samples/memfs/inode.go @@ -191,7 +191,8 @@ func (inode *inode) RemoveChild(name string) { // Mark it as unused. inode.entries[i] = fuseutil.Dirent{ - Type: fuseutil.DT_Unknown, + Type: fuseutil.DT_Unknown, + Offset: fuse.DirOffset(i + 1), } } From 4d18942456df3fa5db9c8597fe0bcdd95ff6024c Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 14:31:15 +1100 Subject: [PATCH 08/21] Re-use dirent gaps. --- samples/memfs/inode.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/samples/memfs/inode.go b/samples/memfs/inode.go index c969767..2a3eb57 100644 --- a/samples/memfs/inode.go +++ b/samples/memfs/inode.go @@ -165,6 +165,7 @@ func (inode *inode) AddChild( id fuse.InodeID, name string, dt fuseutil.DirentType) { + // Set up the entry. e := fuseutil.Dirent{ Offset: fuse.DirOffset(len(inode.entries) + 1), Inode: id, @@ -172,9 +173,16 @@ func (inode *inode) AddChild( Type: dt, } - inode.entries = append(inode.entries, e) + // Look for a gap in which we can insert it. + for i := range inode.entries { + if inode.entries[i].Type == fuseutil.DT_Unknown { + inode.entries[i] = e + return + } + } - // TODO(jacobsa): Re-use gaps. + // Append it to the end. + inode.entries = append(inode.entries, e) } // Remove an entry for a child. From ea5370d6f1f8d003849390065eaa3c3f424c8d8b Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 14:33:20 +1100 Subject: [PATCH 09/21] MemFSTest.Rmdir_NonExistent --- samples/memfs/memfs_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index 0f83259..12318ae 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -356,12 +356,11 @@ func (t *MemFSTest) Rmdir_Empty() { ExpectThat(entries, ElementsAre()) } -func (t *MemFSTest) Rmdir_NotADirectory() { - AssertTrue(false, "TODO") -} - func (t *MemFSTest) Rmdir_NonExistent() { - AssertTrue(false, "TODO") + err := os.Remove(path.Join(t.mfs.Dir(), "blah")) + + AssertNe(nil, err) + ExpectThat(err, Error(HasSubstr("no such file or directory"))) } func (t *MemFSTest) Rmdir_OpenedForReading() { From 98f868c319c34e585ac8625fce524380fe24ba8a Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 14:34:28 +1100 Subject: [PATCH 10/21] MemFSTest.Rmdir_NonEmpty --- samples/memfs/memfs_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index 12318ae..01afb60 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -324,7 +324,17 @@ func (t *MemFSTest) UnlinkFile_NonExistent() { } func (t *MemFSTest) Rmdir_NonEmpty() { - AssertTrue(false, "TODO") + var err error + + // Create two levels of directories. + err = os.MkdirAll(path.Join(t.mfs.Dir(), "foo/bar"), 0754) + AssertEq(nil, err) + + // Attempt to remove the parent. + err = os.Remove(path.Join(t.mfs.Dir(), "foo")) + + AssertNe(nil, err) + ExpectThat(err, Error(HasSubstr("non-empty"))) } func (t *MemFSTest) Rmdir_Empty() { From 01371975df17e01b01d8546fc9584dd22890818a Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 14:39:29 +1100 Subject: [PATCH 11/21] Check for empty in rmdir. --- errors.go | 13 +++++++++---- samples/memfs/fs.go | 17 ++++++++++++++++- samples/memfs/inode.go | 14 ++++++++++++++ samples/memfs/memfs_test.go | 2 +- 4 files changed, 40 insertions(+), 6 deletions(-) diff --git a/errors.go b/errors.go index 7ed27cb..860d7ec 100644 --- a/errors.go +++ b/errors.go @@ -3,12 +3,17 @@ package fuse -import bazilfuse "bazil.org/fuse" +import ( + "syscall" + + bazilfuse "bazil.org/fuse" +) const ( // Errors corresponding to kernel error numbers. These may be treated // specially when returned by a FileSystem method. - ENOSYS = bazilfuse.ENOSYS - ENOENT = bazilfuse.ENOENT - EIO = bazilfuse.EIO + EIO = bazilfuse.EIO + ENOENT = bazilfuse.ENOENT + ENOSYS = bazilfuse.ENOSYS + ENOTEMPTY = bazilfuse.Errno(syscall.ENOTEMPTY) ) diff --git a/samples/memfs/fs.go b/samples/memfs/fs.go index 6e8b216..31fe9f4 100644 --- a/samples/memfs/fs.go +++ b/samples/memfs/fs.go @@ -306,7 +306,22 @@ func (fs *memFS) RmDir( parent := fs.getInodeForModifyingOrDie(req.Parent) defer parent.mu.Unlock() - // TODO(jacobsa): Check for empty. (Make sure we have a failing test first.) + // Find the child within the parent. + childID, ok := parent.LookUpChild(req.Name) + if !ok { + err = fuse.ENOENT + return + } + + // Grab the child. + child := fs.getInodeForModifyingOrDie(childID) + defer child.mu.Unlock() + + // Make sure the child is empty. + if child.Len() != 0 { + err = fuse.ENOTEMPTY + return + } // Remove the entry within the parent. parent.RemoveChild(req.Name) diff --git a/samples/memfs/inode.go b/samples/memfs/inode.go index 2a3eb57..255dfe0 100644 --- a/samples/memfs/inode.go +++ b/samples/memfs/inode.go @@ -143,6 +143,20 @@ func (inode *inode) findChild(name string) (i int, ok bool) { // Public methods //////////////////////////////////////////////////////////////////////// +// Return the number of children of the directory. +// +// REQUIRES: inode.dir +// SHARED_LOCKS_REQUIRED(inode.mu) +func (inode *inode) Len() (n int) { + for _, e := range inode.entries { + if e.Type != fuseutil.DT_Unknown { + n++ + } + } + + return +} + // Find an entry for the given child name and return its inode ID. // // REQUIRES: inode.dir diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index 01afb60..456681b 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -334,7 +334,7 @@ func (t *MemFSTest) Rmdir_NonEmpty() { err = os.Remove(path.Join(t.mfs.Dir(), "foo")) AssertNe(nil, err) - ExpectThat(err, Error(HasSubstr("non-empty"))) + ExpectThat(err, Error(HasSubstr("not empty"))) } func (t *MemFSTest) Rmdir_Empty() { From 27731334485471003cca48c201ad66f27caad84c Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 14:43:06 +1100 Subject: [PATCH 12/21] MemFSTest.Rmdir_ReusesInodeID --- samples/memfs/memfs_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index 456681b..0c9e194 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -373,6 +373,32 @@ func (t *MemFSTest) Rmdir_NonExistent() { ExpectThat(err, Error(HasSubstr("no such file or directory"))) } +func (t *MemFSTest) Rmdir_ReusesInodeID() { + var err error + var fi os.FileInfo + + // Create a directory and record its inode ID. + err = os.Mkdir(path.Join(t.mfs.Dir(), "dir"), 0700) + AssertEq(nil, err) + + fi, err = os.Stat(path.Join(t.mfs.Dir(), "dir")) + AssertEq(nil, err) + inodeID := fi.Sys().(*syscall.Stat_t).Ino + + // Remove the directory. + err = os.Remove(path.Join(t.mfs.Dir(), "dir")) + AssertEq(nil, err) + + // Create a new directory. It should receive the most recently de-allocated + // inode ID. + err = os.Mkdir(path.Join(t.mfs.Dir(), "blah"), 0700) + AssertEq(nil, err) + + fi, err = os.Stat(path.Join(t.mfs.Dir(), "blah")) + AssertEq(nil, err) + ExpectEq(inodeID, fi.Sys().(*syscall.Stat_t).Ino) +} + func (t *MemFSTest) Rmdir_OpenedForReading() { AssertTrue(false, "TODO") } From 3076da562eb57e8e025c045700306505b529d314 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 14:45:39 +1100 Subject: [PATCH 13/21] Fixed an inode invariants bug. --- samples/memfs/inode.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/samples/memfs/inode.go b/samples/memfs/inode.go index 255dfe0..615b1e4 100644 --- a/samples/memfs/inode.go +++ b/samples/memfs/inode.go @@ -179,23 +179,31 @@ func (inode *inode) AddChild( id fuse.InodeID, name string, dt fuseutil.DirentType) { + var index int + + // No matter where we place the entry, make sure it has the correct Offset + // field. + defer func() { + inode.entries[index].Offset = fuse.DirOffset(index + 1) + }() + // Set up the entry. e := fuseutil.Dirent{ - Offset: fuse.DirOffset(len(inode.entries) + 1), - Inode: id, - Name: name, - Type: dt, + Inode: id, + Name: name, + Type: dt, } // Look for a gap in which we can insert it. - for i := range inode.entries { - if inode.entries[i].Type == fuseutil.DT_Unknown { - inode.entries[i] = e + for index = range inode.entries { + if inode.entries[index].Type == fuseutil.DT_Unknown { + inode.entries[index] = e return } } // Append it to the end. + index = len(inode.entries) inode.entries = append(inode.entries, e) } From d8905b0d05d41e48d02533aa44a256cb11c1f03f Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 14:48:21 +1100 Subject: [PATCH 14/21] Deallocate removed directories. --- samples/memfs/fs.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/samples/memfs/fs.go b/samples/memfs/fs.go index 31fe9f4..840f7a5 100644 --- a/samples/memfs/fs.go +++ b/samples/memfs/fs.go @@ -169,6 +169,12 @@ func (fs *memFS) allocateInode( return } +// EXCLUSIVE_LOCKS_REQUIRED(fs.mu) +func (fs *memFS) deallocateInode(id fuse.InodeID) { + fs.freeInodes = append(fs.freeInodes, id) + fs.inodes[id] = nil +} + //////////////////////////////////////////////////////////////////////// // FileSystem methods //////////////////////////////////////////////////////////////////////// @@ -326,8 +332,9 @@ func (fs *memFS) RmDir( // Remove the entry within the parent. parent.RemoveChild(req.Name) - // TODO(jacobsa): Remove the child when it's forgotten. (Can we get a failing - // test by looking at inode ID allocation?) + // TODO(jacobsa): Don't remove the child until it's forgotten. Can we get a + // failing test by continuing to read from an opened dir handle? + fs.deallocateInode(childID) return } From 8d9962ee62b8effd9901e039517e53e55ab70aff Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 14:52:03 +1100 Subject: [PATCH 15/21] MemFSTest.Rmdir_OpenedForReading --- samples/memfs/memfs_test.go | 38 ++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index 0c9e194..0a4530a 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -400,5 +400,41 @@ func (t *MemFSTest) Rmdir_ReusesInodeID() { } func (t *MemFSTest) Rmdir_OpenedForReading() { - AssertTrue(false, "TODO") + var err error + + // Create a directory. + err = os.Mkdir(path.Join(t.mfs.Dir(), "dir"), 0700) + AssertEq(nil, err) + + // Open the directory for reading. + f, err := os.Open(path.Join(t.mfs.Dir(), "dir")) + defer func() { + if f != nil { + ExpectEq(nil, f.Close()) + } + }() + + AssertEq(nil, err) + + // Remove the directory. + err = os.Remove(path.Join(t.mfs.Dir(), "dir")) + AssertEq(nil, err) + + // Create a new directory, with the same name even, and add some contents + // within it. + err = os.MkdirAll(path.Join(t.mfs.Dir(), "foo"), 0700) + AssertEq(nil, err) + + err = os.MkdirAll(path.Join(t.mfs.Dir(), "bar"), 0700) + AssertEq(nil, err) + + err = os.MkdirAll(path.Join(t.mfs.Dir(), "baz"), 0700) + AssertEq(nil, err) + + // Attempt to read from the directory. This should succeed even though it has + // been unlinked, and we shouldn't see any junk from the new directory. + entries, err := f.Readdir(0) + + AssertEq(nil, err) + ExpectThat(entries, ElementsAre()) } From 140bd1886386af277ab50d47bade818a3c2e907e Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 14:54:22 +1100 Subject: [PATCH 16/21] Fixed a silly test bug. --- samples/memfs/memfs_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index 0a4530a..38c017e 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -422,13 +422,13 @@ func (t *MemFSTest) Rmdir_OpenedForReading() { // Create a new directory, with the same name even, and add some contents // within it. - err = os.MkdirAll(path.Join(t.mfs.Dir(), "foo"), 0700) + err = os.MkdirAll(path.Join(t.mfs.Dir(), "dir/foo"), 0700) AssertEq(nil, err) - err = os.MkdirAll(path.Join(t.mfs.Dir(), "bar"), 0700) + err = os.MkdirAll(path.Join(t.mfs.Dir(), "dir/bar"), 0700) AssertEq(nil, err) - err = os.MkdirAll(path.Join(t.mfs.Dir(), "baz"), 0700) + err = os.MkdirAll(path.Join(t.mfs.Dir(), "dir/baz"), 0700) AssertEq(nil, err) // Attempt to read from the directory. This should succeed even though it has From 651984dc1ad8617f8d3547900afb89aabb8fb6c8 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 14:56:55 +1100 Subject: [PATCH 17/21] Only mark as unlinked in rmdir. --- samples/memfs/fs.go | 5 ++--- samples/memfs/inode.go | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/samples/memfs/fs.go b/samples/memfs/fs.go index 840f7a5..0a570a1 100644 --- a/samples/memfs/fs.go +++ b/samples/memfs/fs.go @@ -332,9 +332,8 @@ func (fs *memFS) RmDir( // Remove the entry within the parent. parent.RemoveChild(req.Name) - // TODO(jacobsa): Don't remove the child until it's forgotten. Can we get a - // failing test by continuing to read from an opened dir handle? - fs.deallocateInode(childID) + // Mark the child as unlinked. + child.linkCount-- return } diff --git a/samples/memfs/inode.go b/samples/memfs/inode.go index 615b1e4..45633b4 100644 --- a/samples/memfs/inode.go +++ b/samples/memfs/inode.go @@ -31,6 +31,13 @@ type inode struct { mu syncutil.InvariantMutex + // The number of times this inode is linked into a parent directory. This may + // be zero if the inode has been unlinked but not yet forgotten, because some + // process still has an open file handle. + // + // INVARIANT: linkCount >= 0 + linkCount int // GUARDED_BY(mu) + // The current attributes of this inode. // // INVARIANT: No non-permission mode bits are set besides os.ModeDir @@ -64,8 +71,10 @@ type inode struct { // Helpers //////////////////////////////////////////////////////////////////////// +// Initially the link count is one. func newInode(attrs fuse.InodeAttributes) (in *inode) { in = &inode{ + linkCount: 1, dir: (attrs.Mode&os.ModeDir != 0), attributes: attrs, } @@ -75,6 +84,11 @@ func newInode(attrs fuse.InodeAttributes) (in *inode) { } func (inode *inode) checkInvariants() { + // Check the link count. + if inode.linkCount < 0 { + panic(fmt.Sprintf("Negative link count: %v", inode.linkCount)) + } + // No non-permission mode bits should be set besides os.ModeDir. if inode.attributes.Mode & ^(os.ModePerm|os.ModeDir) != 0 { panic(fmt.Sprintf("Unexpected mode: %v", inode.attributes.Mode)) From 147be0630dae856cc19e06157ad057ad08eb9a5e Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 15:04:49 +1100 Subject: [PATCH 18/21] Added tests for the Nlink field. --- samples/memfs/memfs_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index 38c017e..dd2c1a6 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -160,6 +160,7 @@ func (t *MemFSTest) Mkdir_OneLevel() { ExpectEq(0, fi.ModTime().Sub(createTime)) ExpectTrue(fi.IsDir()) + ExpectNe(0, stat.Ino) ExpectEq(1, stat.Nlink) ExpectEq(currentUid(), stat.Uid) ExpectEq(currentGid(), stat.Gid) @@ -214,6 +215,7 @@ func (t *MemFSTest) Mkdir_TwoLevels() { ExpectEq(0, fi.ModTime().Sub(createTime)) ExpectTrue(fi.IsDir()) + ExpectNe(0, stat.Ino) ExpectEq(1, stat.Nlink) ExpectEq(currentUid(), stat.Uid) ExpectEq(currentGid(), stat.Gid) @@ -403,9 +405,13 @@ func (t *MemFSTest) Rmdir_OpenedForReading() { var err error // Create a directory. + createTime := t.clock.Now() err = os.Mkdir(path.Join(t.mfs.Dir(), "dir"), 0700) AssertEq(nil, err) + // Simulate time advancing. + t.clock.AdvanceTime(time.Second) + // Open the directory for reading. f, err := os.Open(path.Join(t.mfs.Dir(), "dir")) defer func() { @@ -431,6 +437,14 @@ func (t *MemFSTest) Rmdir_OpenedForReading() { err = os.MkdirAll(path.Join(t.mfs.Dir(), "dir/baz"), 0700) AssertEq(nil, err) + // We should still be able to stat the open file handle. It should show up as + // unlinked. + fi, err := f.Stat() + + ExpectEq("dir", fi.Name()) + ExpectEq(0, fi.ModTime().Sub(createTime)) + ExpectEq(0, fi.Sys().(*syscall.Stat_t).Nlink) + // Attempt to read from the directory. This should succeed even though it has // been unlinked, and we shouldn't see any junk from the new directory. entries, err := f.Readdir(0) From eec88dd31a669700616f61be34c79afeca69a37f Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 15:15:58 +1100 Subject: [PATCH 19/21] Disabled a test that cannot pass. --- samples/memfs/memfs_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index dd2c1a6..94caba6 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -443,7 +443,10 @@ func (t *MemFSTest) Rmdir_OpenedForReading() { ExpectEq("dir", fi.Name()) ExpectEq(0, fi.ModTime().Sub(createTime)) - ExpectEq(0, fi.Sys().(*syscall.Stat_t).Nlink) + + // TODO(jacobsa): Re-enable this assertion if the following issue is fixed: + // https://github.com/bazillion/fuse/issues/66 + // ExpectEq(0, fi.Sys().(*syscall.Stat_t).Nlink) // Attempt to read from the directory. This should succeed even though it has // been unlinked, and we shouldn't see any junk from the new directory. From dd66a1d2bb2fb0579235470e57cc604e5354ee18 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 15:17:04 +1100 Subject: [PATCH 20/21] Expanded an example. --- file_system.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/file_system.go b/file_system.go index e6c04fd..3bbdb7b 100644 --- a/file_system.go +++ b/file_system.go @@ -46,8 +46,8 @@ type FileSystem interface { ctx context.Context, req *GetInodeAttributesRequest) (*GetInodeAttributesResponse, error) - // Forget an inode ID previously issued (e.g. by LookUpInode). The kernel - // calls this when removing an inode from its internal caches. + // Forget an inode ID previously issued (e.g. by LookUpInode or MkDir). The + // kernel calls this when removing an inode from its internal caches. ForgetInode( ctx context.Context, req *ForgetInodeRequest) (*ForgetInodeResponse, error) From a956166291fa6f7059b76167c151a924eb30decc Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 3 Mar 2015 16:14:37 +1100 Subject: [PATCH 21/21] Deleted MemFSTest.Rmdir_ReusesInodeID. At least on OS X, the kernel doesn't relaibly send a Forget request right after RmDir, even if the directory is not open. (This is contrary to what the fuse low-level ops documentation says.) So there's no way to reliably test this. --- samples/memfs/memfs_test.go | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index 94caba6..8428647 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -375,32 +375,6 @@ func (t *MemFSTest) Rmdir_NonExistent() { ExpectThat(err, Error(HasSubstr("no such file or directory"))) } -func (t *MemFSTest) Rmdir_ReusesInodeID() { - var err error - var fi os.FileInfo - - // Create a directory and record its inode ID. - err = os.Mkdir(path.Join(t.mfs.Dir(), "dir"), 0700) - AssertEq(nil, err) - - fi, err = os.Stat(path.Join(t.mfs.Dir(), "dir")) - AssertEq(nil, err) - inodeID := fi.Sys().(*syscall.Stat_t).Ino - - // Remove the directory. - err = os.Remove(path.Join(t.mfs.Dir(), "dir")) - AssertEq(nil, err) - - // Create a new directory. It should receive the most recently de-allocated - // inode ID. - err = os.Mkdir(path.Join(t.mfs.Dir(), "blah"), 0700) - AssertEq(nil, err) - - fi, err = os.Stat(path.Join(t.mfs.Dir(), "blah")) - AssertEq(nil, err) - ExpectEq(inodeID, fi.Sys().(*syscall.Stat_t).Ino) -} - func (t *MemFSTest) Rmdir_OpenedForReading() { var err error