From efe4481b2a745bc2f5f8850dcc35c68104d05c4c Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 4 Mar 2015 14:54:11 +1100 Subject: [PATCH 01/26] Added a CreateFile method. --- file_system.go | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/file_system.go b/file_system.go index fc6259d..998c828 100644 --- a/file_system.go +++ b/file_system.go @@ -69,10 +69,32 @@ type FileSystem interface { // Create a directory inode as a child of an existing directory inode. The // kernel sends this in response to a mkdir(2) call. + // + // The kernel appears to verify the name doesn't already exist (mkdir calls + // mkdirat calls user_path_create calls filename_create, which verifies: + // http://goo.gl/FZpLu5). But volatile file systems and paranoid non-volatile + // file systems should check for the reasons described below on CreateFile. MkDir( ctx context.Context, req *MkDirRequest) (*MkDirResponse, error) + // Create a file inode and open it. + // + // The kernel calls this method when the user asks to open a file with the + // O_CREAT flag and the kernel has observed that the file doesn't exist. (See + // for example lookup_open, http://goo.gl/PlqE9d). + // + // However it's impossible to tell for sure that all kernels make this check + // in all cases and the official fuse documentation is less than encouraging + // (" the file does not exist, first create it with the specified mode, and + // then open it"). Therefore file systems would be smart to be paranoid and + // check themselves, returning EEXIST when the file already exists. This of + // course particularly applies to file systems that are volatile from the + // kernel's point of view. + CreateFile( + ctx context.Context, + req *CreateFileRequest) (*CreateFileResponse, error) + /////////////////////////////////// // Inode destruction /////////////////////////////////// @@ -389,6 +411,47 @@ type MkDirResponse struct { Entry ChildInodeEntry } +type CreateFileRequest struct { + Header RequestHeader + + // The ID of parent directory inode within which to create the child file. + Parent InodeID + + // The name of the child to create, and the mode with which to create it. + Name string + Mode os.FileMode + + // Flags for the open operation. + Flags bazilfuse.OpenFlags +} + +type CreateFileResponse struct { + // Information about the inode that was created. + // + // The file system is responsible for initializing and recording (where + // supported) attributes like time information, ownership information, etc. + // + // Ownership information in particular must be set to something reasonable or + // by default root will own everything and unprivileged users won't be able + // to do anything useful. In traditional file systems in the kernel, the + // function inode_init_owner (http://goo.gl/5qavg8) contains the + // standards-compliant logic for this. + Entry ChildInodeEntry + + // An opaque ID that will be echoed in follow-up calls for this file using + // the same struct file in the kernel. In practice this usually means + // follow-up calls using the file descriptor returned by open(2). + // + // The handle may be supplied to the following methods: + // + // * ReadFile + // * ReleaseFileHandle + // + // The file system must ensure this ID remains valid until a later call to + // ReleaseFileHandle. + Handle HandleID +} + type RmDirRequest struct { Header RequestHeader From fd9dbe872e13a609e0771991e2d79e5ccb813f52 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 4 Mar 2015 14:55:16 +1100 Subject: [PATCH 02/26] Refactored ChildInodeEntry comments. --- file_system.go | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/file_system.go b/file_system.go index 998c828..5d8e787 100644 --- a/file_system.go +++ b/file_system.go @@ -257,8 +257,8 @@ type RequestHeader struct { } // Information about a child inode within its parent directory. Shared by the -// responses for LookUpInode, MkDir, etc. Consumed by the kernel in order to -// set up a dcache entry. +// responses for LookUpInode, MkDir, CreateFile, etc. Consumed by the kernel in +// order to set up a dcache entry. type ChildInodeEntry struct { // The ID of the child inode. The file system must ensure that the returned // inode ID remains valid until a later call to ForgetInode. @@ -269,6 +269,16 @@ type ChildInodeEntry struct { Generation GenerationNumber // Current attributes for the child inode. + // + // When creating a new inode, the file system is responsible for initializing + // and recording (where supported) attributes like time information, + // ownership information, etc. + // + // Ownership information in particular must be set to something reasonable or + // by default root will own everything and unprivileged users won't be able + // to do anything useful. In traditional file systems in the kernel, the + // function inode_init_owner (http://goo.gl/5qavg8) contains the + // standards-compliant logic for this. Attributes InodeAttributes // The FUSE VFS layer in the kernel maintains a cache of file attributes, @@ -399,15 +409,6 @@ type MkDirRequest struct { type MkDirResponse struct { // Information about the inode that was created. - // - // The file system is responsible for initializing and recording (where - // supported) attributes like time information, ownership information, etc. - // - // Ownership information in particular must be set to something reasonable or - // by default root will own everything and unprivileged users won't be able - // to do anything useful. In traditional file systems in the kernel, the - // function inode_init_owner (http://goo.gl/5qavg8) contains the - // standards-compliant logic for this. Entry ChildInodeEntry } @@ -427,15 +428,6 @@ type CreateFileRequest struct { type CreateFileResponse struct { // Information about the inode that was created. - // - // The file system is responsible for initializing and recording (where - // supported) attributes like time information, ownership information, etc. - // - // Ownership information in particular must be set to something reasonable or - // by default root will own everything and unprivileged users won't be able - // to do anything useful. In traditional file systems in the kernel, the - // function inode_init_owner (http://goo.gl/5qavg8) contains the - // standards-compliant logic for this. Entry ChildInodeEntry // An opaque ID that will be echoed in follow-up calls for this file using From 1aba7cb8a8442d0871827a8b689092404b7eca44 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 4 Mar 2015 14:55:50 +1100 Subject: [PATCH 03/26] Added NotImplementedFileSystem.CreateFile. --- fuseutil/not_implemented_file_system.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fuseutil/not_implemented_file_system.go b/fuseutil/not_implemented_file_system.go index 55697b9..4a741c6 100644 --- a/fuseutil/not_implemented_file_system.go +++ b/fuseutil/not_implemented_file_system.go @@ -57,6 +57,12 @@ func (fs *NotImplementedFileSystem) MkDir( return nil, fuse.ENOSYS } +func (fs *NotImplementedFileSystem) CreateFile( + ctx context.Context, + req *fuse.CreateFileRequest) (*fuse.CreateFileResponse, error) { + return nil, fuse.ENOSYS +} + func (fs *NotImplementedFileSystem) RmDir( ctx context.Context, req *fuse.RmDirRequest) (*fuse.RmDirResponse, error) { From bdefba382234a5412e3376051b9cbca74a6fd05e Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 4 Mar 2015 14:57:53 +1100 Subject: [PATCH 04/26] Added server support for CreateFile. --- server.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/server.go b/server.go index 14c8f15..37ef2ac 100644 --- a/server.go +++ b/server.go @@ -195,6 +195,35 @@ func (s *server) handleFuseRequest(fuseReq bazilfuse.Request) { s.logger.Println("Responding:", fuseResp) typed.Respond(fuseResp) + case *bazilfuse.CreateRequest: + // Convert the request. + req := &CreateFileRequest{ + Header: convertHeader(typed.Header), + Parent: InodeID(typed.Header.Node), + Name: typed.Name, + Mode: typed.Mode, + Flags: typed.Flags, + } + + // Call the file system. + resp, err := s.fs.CreateFile(ctx, req) + if err != nil { + s.logger.Println("Responding:", err) + typed.RespondError(err) + return + } + + // Convert the response. + fuseResp := &bazilfuse.CreateResponse{ + OpenResponse: bazilfuse.OpenResponse{ + Handle: bazilfuse.HandleID(resp.Handle), + }, + } + convertChildInodeEntry(s.clock, &resp.Entry, &fuseResp.LookupResponse) + + s.logger.Println("Responding:", fuseResp) + typed.Respond(fuseResp) + case *bazilfuse.RemoveRequest: // We don't yet support files. if !typed.Dir { From 1a975d9824d2b2a0e5996821fad6c8817474084d Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 4 Mar 2015 15:03:44 +1100 Subject: [PATCH 05/26] Declared some test names. --- samples/memfs/memfs_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index 1b848ef..f0369f9 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -336,6 +336,10 @@ func (t *MemFSTest) UnlinkFile_NonExistent() { AssertTrue(false, "TODO") } +func (t *MemFSTest) UnlinkFile_StillOpen() { + AssertTrue(false, "TODO") +} + func (t *MemFSTest) Rmdir_NonEmpty() { var err error @@ -468,3 +472,11 @@ func (t *MemFSTest) CaseSensitive() { AssertThat(err, Error(HasSubstr("no such file or directory"))) } } + +func (t *MemFSTest) FileReadsAndWrites() { + AssertTrue(false, "TODO") +} + +func (t *MemFSTest) FileReadsAndWrites_BeyondEOF() { + AssertTrue(false, "TODO") +} From c9298a943a13a24de3d742a7163061a916b82f8b Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 4 Mar 2015 15:05:58 +1100 Subject: [PATCH 06/26] Implemented memFS.CreateFile. --- samples/memfs/fs.go | 48 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/samples/memfs/fs.go b/samples/memfs/fs.go index eceed2b..07e3885 100644 --- a/samples/memfs/fs.go +++ b/samples/memfs/fs.go @@ -279,7 +279,7 @@ func (fs *memFS) MkDir( parent := fs.getInodeForModifyingOrDie(req.Parent) defer parent.mu.Unlock() - // Set up attributes from the child, using the credientials of the calling + // Set up attributes from the child, using the credentials of the calling // process as owner (matching inode_init_owner, cf. http://goo.gl/5qavg8). now := fs.clock.Now() childAttrs := fuse.InodeAttributes{ @@ -311,6 +311,52 @@ func (fs *memFS) MkDir( return } +func (fs *memFS) CreateFile( + ctx context.Context, + req *fuse.CreateFileRequest) (resp *fuse.CreateFileResponse, err error) { + resp = &fuse.CreateFileResponse{} + + fs.mu.Lock() + defer fs.mu.Unlock() + + // Grab the parent, which we will update shortly. + parent := fs.getInodeForModifyingOrDie(req.Parent) + defer parent.mu.Unlock() + + // Set up attributes from the child, using the credentials of the calling + // process as owner (matching inode_init_owner, cf. http://goo.gl/5qavg8). + now := fs.clock.Now() + childAttrs := fuse.InodeAttributes{ + Mode: req.Mode, + Atime: now, + Mtime: now, + Ctime: now, + Crtime: now, + Uid: req.Header.Uid, + Gid: req.Header.Gid, + } + + // Allocate a child. + childID, child := fs.allocateInode(childAttrs) + defer child.mu.Unlock() + + // Add an entry in the parent. + parent.AddChild(childID, req.Name, fuseutil.DT_File) + + // Fill in the response entry. + resp.Entry.Child = childID + resp.Entry.Attributes = child.attributes + + // We don't spontaneously mutate, so the kernel can cache as long as it wants + // (since it also handles invalidation). + resp.Entry.AttributesExpiration = fs.clock.Now().Add(365 * 24 * time.Hour) + resp.Entry.EntryExpiration = resp.Entry.EntryExpiration + + // We have nothing interesting to put in the Handle field. + + return +} + func (fs *memFS) RmDir( ctx context.Context, req *fuse.RmDirRequest) (resp *fuse.RmDirResponse, err error) { From 9c585268b0159527117358b36168a22e9fb5fb2a Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 4 Mar 2015 15:06:40 +1100 Subject: [PATCH 07/26] Performed a TODO. --- samples/memfs/memfs_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index f0369f9..4351648 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -280,7 +280,7 @@ func (t *MemFSTest) Mkdir_IntermediateIsFile() { err = os.Mkdir(dirName, 0754) AssertNe(nil, err) - ExpectThat(err, Error(HasSubstr("TODO"))) + ExpectThat(err, Error(HasSubstr("not a directory"))) } func (t *MemFSTest) Mkdir_IntermediateIsNonExistent() { From 87f8ec9eadd70e811e760a1de0e6a261fb4f0485 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 4 Mar 2015 15:10:26 +1100 Subject: [PATCH 08/26] MemFSTest.CreateNewFile_InRoot --- samples/memfs/memfs_test.go | 40 ++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index 4351648..8b2afd9 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -309,7 +309,45 @@ func (t *MemFSTest) Mkdir_PermissionDenied() { } func (t *MemFSTest) CreateNewFile_InRoot() { - AssertTrue(false, "TODO") + var err error + var fi os.FileInfo + var stat *syscall.Stat_t + + fileName := path.Join(t.mfs.Dir(), "foo") + const contents = "Hello\x00world" + + // Write a file. + createTime := t.clock.Now() + err = ioutil.WriteFile(fileName, []byte(contents), 0400) + AssertEq(nil, err) + + // Simulate time advancing. + t.clock.AdvanceTime(time.Second) + + // Stat it. + fi, err = os.Stat(fileName) + stat = fi.Sys().(*syscall.Stat_t) + + AssertEq(nil, err) + ExpectEq("foo", fi.Name()) + ExpectEq(len(contents), fi.Size()) + ExpectEq(0400, fi.Mode()) + ExpectEq(0, fi.ModTime().Sub(createTime)) + ExpectFalse(fi.IsDir()) + + ExpectNe(0, stat.Ino) + ExpectEq(1, stat.Nlink) + ExpectEq(currentUid(), stat.Uid) + ExpectEq(currentGid(), stat.Gid) + ExpectEq(len(contents), stat.Size) + ExpectEq(0, timespecToTime(stat.Atimespec).Sub(createTime)) + ExpectEq(0, timespecToTime(stat.Mtimespec).Sub(createTime)) + ExpectEq(0, timespecToTime(stat.Ctimespec).Sub(createTime)) + + // Read it back. + slice, err := ioutil.ReadFile(fileName) + AssertEq(nil, err) + ExpectEq(contents, string(slice)) } func (t *MemFSTest) CreateNewFile_InSubDir() { From dcf4c93dfd25483778e2f353ec12028617b5b7cf Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 4 Mar 2015 15:16:29 +1100 Subject: [PATCH 09/26] Added a WriteFile method. --- file_system.go | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/file_system.go b/file_system.go index 5d8e787..df208bc 100644 --- a/file_system.go +++ b/file_system.go @@ -150,11 +150,16 @@ type FileSystem interface { ctx context.Context, req *OpenFileRequest) (*OpenFileResponse, error) - // Read data from a file previously opened with OpenFile. + // Read data from a file previously opened with CreateFile or OpenFile. ReadFile( ctx context.Context, req *ReadFileRequest) (*ReadFileResponse, error) + // Write data to a file previously opened with CreateFile or OpenFile. + WriteFile( + ctx context.Context, + req *WriteFileRequest) (*WriteFileResponse, error) + // Release a previously-minted file handle. The kernel calls this when there // are no more references to an open file: all file descriptors are closed // and all memory mappings are unmapped. @@ -437,6 +442,7 @@ type CreateFileResponse struct { // The handle may be supplied to the following methods: // // * ReadFile + // * WriteFile // * ReleaseFileHandle // // The file system must ensure this ID remains valid until a later call to @@ -602,6 +608,7 @@ type OpenFileResponse struct { // The handle may be supplied to the following methods: // // * ReadFile + // * WriteFile // * ReleaseFileHandle // // The file system must ensure this ID remains valid until a later call to @@ -613,7 +620,7 @@ type ReadFileRequest struct { Header RequestHeader // The file inode that we are reading, and the handle previously returned by - // OpenFile when opening that inode. + // CreateFile or OpenFile when opening that inode. Inode InodeID Handle HandleID @@ -634,6 +641,30 @@ type ReadFileResponse struct { Data []byte } +type WriteFileRequest struct { + Header RequestHeader + + // The file inode that we are modifying, and the handle previously returned + // by CreateFile or OpenFile when opening that inode. + Inode InodeID + Handle HandleID + + // The data to write, and the offset at which to write it. + // + // The FUSE documentation requires that exactly the number of bytes supplied + // be written, except on error (http://goo.gl/KUpwwn). This appears to be + // because it uses file mmapping machinery (http://goo.gl/SGxnaN) to write a + // page at a time. + // + // TODO(jacobsa): Figure out what the posix semantics are for extending the + // file, and document them here. + Data []byte + Offset int64 +} + +type WriteFileResponse struct { +} + type ReleaseFileHandleRequest struct { Header RequestHeader From 0e4bdb538ab7700f2fe39a6a9fa7e11be0f7ba5a Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 4 Mar 2015 15:16:55 +1100 Subject: [PATCH 10/26] Added NotImplementedFileSystem.WriteFile. --- fuseutil/not_implemented_file_system.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fuseutil/not_implemented_file_system.go b/fuseutil/not_implemented_file_system.go index 4a741c6..7cf7e2d 100644 --- a/fuseutil/not_implemented_file_system.go +++ b/fuseutil/not_implemented_file_system.go @@ -99,6 +99,12 @@ func (fs *NotImplementedFileSystem) ReadFile( return nil, fuse.ENOSYS } +func (fs *NotImplementedFileSystem) WriteFile( + ctx context.Context, + req *fuse.WriteFileRequest) (*fuse.WriteFileResponse, error) { + return nil, fuse.ENOSYS +} + func (fs *NotImplementedFileSystem) ReleaseFileHandle( ctx context.Context, req *fuse.ReleaseFileHandleRequest) (*fuse.ReleaseFileHandleResponse, error) { From 5455e35b354f0f4659c073dc99990bd59b469257 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 4 Mar 2015 15:18:09 +1100 Subject: [PATCH 11/26] Added server support for WriteFile. --- server.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/server.go b/server.go index 37ef2ac..cc343b9 100644 --- a/server.go +++ b/server.go @@ -355,6 +355,32 @@ func (s *server) handleFuseRequest(fuseReq bazilfuse.Request) { typed.Respond(fuseResp) } + case *bazilfuse.WriteRequest: + // Convert the request. + req := &WriteFileRequest{ + Header: convertHeader(typed.Header), + Inode: InodeID(typed.Header.Node), + Handle: HandleID(typed.Handle), + Data: typed.Data, + Offset: typed.Offset, + } + + // Call the file system. + _, err := s.fs.WriteFile(ctx, req) + if err != nil { + s.logger.Println("Responding:", err) + typed.RespondError(err) + return + } + + // Convert the response. + fuseResp := &bazilfuse.WriteResponse{ + Size: len(typed.Data), + } + + s.logger.Println("Responding:", fuseResp) + typed.Respond(fuseResp) + default: s.logger.Println("Unhandled type. Returning ENOSYS.") typed.RespondError(ENOSYS) From d8fa4817cc65097149027310792027db634e2279 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 4 Mar 2015 15:23:31 +1100 Subject: [PATCH 12/26] Documented pwrite semantics. --- file_system.go | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/file_system.go b/file_system.go index df208bc..506aec2 100644 --- a/file_system.go +++ b/file_system.go @@ -649,17 +649,35 @@ type WriteFileRequest struct { Inode InodeID Handle HandleID - // The data to write, and the offset at which to write it. + // The offset at which to write the data below. + // + // The man page for pwrite(2) implies that aside from changing the file + // handle's offset, using pwrite is equivalent to using lseek(2) and then + // write(2). The man page for lseek(2) says the following: + // + // "The lseek() function allows the file offset to be set beyond the end of + // the file (but this does not change the size of the file). If data is later + // written at this point, subsequent reads of the data in the gap (a "hole") + // return null bytes (aq\0aq) until data is actually written into the gap." + // + // It is therefore reasonable to assume that the kernel is looking for + // the following semantics: + // + // * If the offset is less than or equal to the current size, extend the + // file as necessary to fit any data that goes past the end of the file. + // + // * If the offset is greater than the current size, extend the file + // with null bytes until it is not, then do the above. + // + Offset int64 + + // The data to write. // // The FUSE documentation requires that exactly the number of bytes supplied // be written, except on error (http://goo.gl/KUpwwn). This appears to be // because it uses file mmapping machinery (http://goo.gl/SGxnaN) to write a // page at a time. - // - // TODO(jacobsa): Figure out what the posix semantics are for extending the - // file, and document them here. - Data []byte - Offset int64 + Data []byte } type WriteFileResponse struct { From 9fd18f53f4273492537da03975b2b78727c646ff Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 5 Mar 2015 19:06:43 +1100 Subject: [PATCH 13/26] Began on a test to help confirm posix file behavior. --- samples/memfs/posix_test.go | 51 +++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 samples/memfs/posix_test.go diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go new file mode 100644 index 0000000..a804999 --- /dev/null +++ b/samples/memfs/posix_test.go @@ -0,0 +1,51 @@ +// Copyright 2015 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Tests for the behavior of os.File objects on plain old posix file systems, +// for use in verifying the intended behavior of memfs. + +package memfs_test + +import ( + "testing" + + . "github.com/jacobsa/ogletest" +) + +func TestPosix(t *testing.T) { RunTests(t) } + +//////////////////////////////////////////////////////////////////////// +// Boilerplate +//////////////////////////////////////////////////////////////////////// + +type PosixTest struct { + dir string +} + +var _ SetUpInterface = &PosixTest{} +var _ TearDownInterface = &PosixTest{} + +func init() { RegisterTestSuite(&PosixTest{}) } + +func (t *PosixTest) SetUp(ti *TestInfo) + +func (t *PosixTest) TearDown() + +//////////////////////////////////////////////////////////////////////// +// Test functions +//////////////////////////////////////////////////////////////////////// + +func (t *PosixTest) DoesFoo() { + AssertTrue(false, "TODO") +} From e435f6916cfaa96e914c6c613bd5ddf5b7106248 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 5 Mar 2015 19:08:48 +1100 Subject: [PATCH 14/26] Added test setup and tear-down code. --- samples/memfs/posix_test.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index a804999..803a7ab 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -18,6 +18,8 @@ package memfs_test import ( + "io/ioutil" + "os" "testing" . "github.com/jacobsa/ogletest" @@ -38,9 +40,23 @@ var _ TearDownInterface = &PosixTest{} func init() { RegisterTestSuite(&PosixTest{}) } -func (t *PosixTest) SetUp(ti *TestInfo) +func (t *PosixTest) SetUp(ti *TestInfo) { + var err error -func (t *PosixTest) TearDown() + // Create a temporary directory. + t.dir, err = ioutil.TempDir("", "posix_test") + if err != nil { + panic(err) + } +} + +func (t *PosixTest) TearDown() { + // Remove the temporary directory. + err := os.RemoveAll(t.dir) + if err != nil { + panic(err) + } +} //////////////////////////////////////////////////////////////////////// // Test functions From 541980129dc0ee8d4a522673c548aeb7d36bcba2 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 5 Mar 2015 19:10:25 +1100 Subject: [PATCH 15/26] Added some test names. --- samples/memfs/posix_test.go | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index 803a7ab..c9c849c 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -62,6 +62,34 @@ func (t *PosixTest) TearDown() { // Test functions //////////////////////////////////////////////////////////////////////// -func (t *PosixTest) DoesFoo() { +func (t *PosixTest) WriteOverlapsEndOfFile() { + AssertTrue(false, "TODO") +} + +func (t *PosixTest) WriteStartsAtEndOfFile() { + AssertTrue(false, "TODO") +} + +func (t *PosixTest) WriteStartsPastEndOfFile() { + AssertTrue(false, "TODO") +} + +func (t *PosixTest) WriteAtEffectOnOffset_NotAppendMode() { + AssertTrue(false, "TODO") +} + +func (t *PosixTest) WriteAtEffectOnOffset_AppendMode() { + AssertTrue(false, "TODO") +} + +func (t *PosixTest) ReadOverlapsEndOfFile() { + AssertTrue(false, "TODO") +} + +func (t *PosixTest) ReadStartsAtEndOfFile() { + AssertTrue(false, "TODO") +} + +func (t *PosixTest) ReadStartsPastEndOfFile() { AssertTrue(false, "TODO") } From 5dab658073136a428f5049da18d674c486ad9135 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 5 Mar 2015 19:17:57 +1100 Subject: [PATCH 16/26] PosixTest.WriteOverlapsEndOfFile --- samples/memfs/posix_test.go | 40 ++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index c9c849c..f483505 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -18,8 +18,10 @@ package memfs_test import ( + "io" "io/ioutil" "os" + "path" "testing" . "github.com/jacobsa/ogletest" @@ -32,7 +34,11 @@ func TestPosix(t *testing.T) { RunTests(t) } //////////////////////////////////////////////////////////////////////// type PosixTest struct { + // A temporary directory. dir string + + // Files to close when tearing down. Nil entries are skipped. + toClose []io.Closer } var _ SetUpInterface = &PosixTest{} @@ -51,6 +57,18 @@ func (t *PosixTest) SetUp(ti *TestInfo) { } func (t *PosixTest) TearDown() { + // Close any files we opened. + for _, c := range t.toClose { + if c == nil { + continue + } + + err := c.Close() + if err != nil { + panic(err) + } + } + // Remove the temporary directory. err := os.RemoveAll(t.dir) if err != nil { @@ -63,7 +81,27 @@ func (t *PosixTest) TearDown() { //////////////////////////////////////////////////////////////////////// func (t *PosixTest) WriteOverlapsEndOfFile() { - AssertTrue(false, "TODO") + var err error + var n int + + // Create a file. + f, err := os.Create(path.Join(t.dir, "foo")) + t.toClose = append(t.toClose, f) + AssertEq(nil, err) + + // Make it 4 bytes long. + err = f.Truncate(4) + AssertEq(nil, err) + + // Write the range [2, 6). + n, err = f.WriteAt([]byte("taco"), 2) + AssertEq(nil, err) + AssertEq(4, n) + + // Read the full contents of the file. + contents, err := ioutil.ReadAll(f) + AssertEq(nil, err) + ExpectEq("\x00\x00taco", string(contents)) } func (t *PosixTest) WriteStartsAtEndOfFile() { From 526ec9a9682856329e8b133927c4f89e5d74be6d Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 5 Mar 2015 19:18:18 +1100 Subject: [PATCH 17/26] PosixTest.WriteStartsAtEndOfFile --- samples/memfs/posix_test.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index f483505..701e973 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -105,7 +105,27 @@ func (t *PosixTest) WriteOverlapsEndOfFile() { } func (t *PosixTest) WriteStartsAtEndOfFile() { - AssertTrue(false, "TODO") + var err error + var n int + + // Create a file. + f, err := os.Create(path.Join(t.dir, "foo")) + t.toClose = append(t.toClose, f) + AssertEq(nil, err) + + // Make it 2 bytes long. + err = f.Truncate(2) + AssertEq(nil, err) + + // Write the range [2, 6). + n, err = f.WriteAt([]byte("taco"), 2) + AssertEq(nil, err) + AssertEq(4, n) + + // Read the full contents of the file. + contents, err := ioutil.ReadAll(f) + AssertEq(nil, err) + ExpectEq("\x00\x00taco", string(contents)) } func (t *PosixTest) WriteStartsPastEndOfFile() { From 0c5ea8464f988dddd6af5b063eb0f881d3072ecc Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 5 Mar 2015 19:18:34 +1100 Subject: [PATCH 18/26] PosixTest.WriteStartsPastEndOfFile --- samples/memfs/posix_test.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index 701e973..93564e9 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -129,7 +129,23 @@ func (t *PosixTest) WriteStartsAtEndOfFile() { } func (t *PosixTest) WriteStartsPastEndOfFile() { - AssertTrue(false, "TODO") + var err error + var n int + + // Create a file. + f, err := os.Create(path.Join(t.dir, "foo")) + t.toClose = append(t.toClose, f) + AssertEq(nil, err) + + // Write the range [2, 6). + n, err = f.WriteAt([]byte("taco"), 2) + AssertEq(nil, err) + AssertEq(4, n) + + // Read the full contents of the file. + contents, err := ioutil.ReadAll(f) + AssertEq(nil, err) + ExpectEq("\x00\x00taco", string(contents)) } func (t *PosixTest) WriteAtEffectOnOffset_NotAppendMode() { From 49bead83fa38cb8e67459fb6a479122e38388e34 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 5 Mar 2015 19:21:20 +1100 Subject: [PATCH 19/26] PosixTest.WriteAtDoesntChangeOffset_NotAppendMode --- samples/memfs/posix_test.go | 40 ++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index 93564e9..ab78985 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -29,6 +29,16 @@ import ( func TestPosix(t *testing.T) { RunTests(t) } +//////////////////////////////////////////////////////////////////////// +// Helpers +//////////////////////////////////////////////////////////////////////// + +func getFileOffset(f *os.File) (offset int64, err error) { + const relativeToCurrent = 1 + offset, err = f.Seek(0, relativeToCurrent) + return +} + //////////////////////////////////////////////////////////////////////// // Boilerplate //////////////////////////////////////////////////////////////////////// @@ -148,11 +158,35 @@ func (t *PosixTest) WriteStartsPastEndOfFile() { ExpectEq("\x00\x00taco", string(contents)) } -func (t *PosixTest) WriteAtEffectOnOffset_NotAppendMode() { - AssertTrue(false, "TODO") +func (t *PosixTest) WriteAtDoesntChangeOffset_NotAppendMode() { + var err error + var n int + + // Create a file. + f, err := os.Create(path.Join(t.dir, "foo")) + t.toClose = append(t.toClose, f) + AssertEq(nil, err) + + // Make it 16 bytes long. + err = f.Truncate(16) + AssertEq(nil, err) + + // Seek to offset 4. + _, err = f.Seek(4, 0) + AssertEq(nil, err) + + // Write the range [10, 14). + n, err = f.WriteAt([]byte("taco"), 2) + AssertEq(nil, err) + AssertEq(4, n) + + // We should still be at offset 4. + offset, err := getFileOffset(f) + AssertEq(nil, err) + ExpectEq(4, offset) } -func (t *PosixTest) WriteAtEffectOnOffset_AppendMode() { +func (t *PosixTest) WriteAtDoesntChangeOffset_AppendMode() { AssertTrue(false, "TODO") } From 95a89e5302c923142526af54eb247f610512d140 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 5 Mar 2015 19:22:32 +1100 Subject: [PATCH 20/26] PosixTest.WriteAtDoesntChangeOffset_AppendMode --- samples/memfs/posix_test.go | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index ab78985..4f4d68f 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -187,7 +187,35 @@ func (t *PosixTest) WriteAtDoesntChangeOffset_NotAppendMode() { } func (t *PosixTest) WriteAtDoesntChangeOffset_AppendMode() { - AssertTrue(false, "TODO") + var err error + var n int + + // Create a file in append mode. + f, err := os.OpenFile( + path.Join(t.dir, "foo"), + os.O_RDWR|os.O_APPEND|os.O_CREATE, + 0600) + + t.toClose = append(t.toClose, f) + AssertEq(nil, err) + + // Make it 16 bytes long. + err = f.Truncate(16) + AssertEq(nil, err) + + // Seek to offset 4. + _, err = f.Seek(4, 0) + AssertEq(nil, err) + + // Write the range [10, 14). + n, err = f.WriteAt([]byte("taco"), 2) + AssertEq(nil, err) + AssertEq(4, n) + + // We should still be at offset 4. + offset, err := getFileOffset(f) + AssertEq(nil, err) + ExpectEq(4, offset) } func (t *PosixTest) ReadOverlapsEndOfFile() { From fcf3be2896640909005925b907b6b56bfd177f05 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 5 Mar 2015 19:26:24 +1100 Subject: [PATCH 21/26] PosixTest.ReadsPastEndOfFile --- samples/memfs/posix_test.go | 38 +++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index 4f4d68f..f48be48 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -218,14 +218,36 @@ func (t *PosixTest) WriteAtDoesntChangeOffset_AppendMode() { ExpectEq(4, offset) } -func (t *PosixTest) ReadOverlapsEndOfFile() { - AssertTrue(false, "TODO") -} +func (t *PosixTest) ReadsPastEndOfFile() { + var err error + var n int + buf := make([]byte, 1024) -func (t *PosixTest) ReadStartsAtEndOfFile() { - AssertTrue(false, "TODO") -} + // Create a file. + f, err := os.Create(path.Join(t.dir, "foo")) + t.toClose = append(t.toClose, f) + AssertEq(nil, err) -func (t *PosixTest) ReadStartsPastEndOfFile() { - AssertTrue(false, "TODO") + // Give it some contents. + n, err = f.Write([]byte("taco")) + AssertEq(nil, err) + AssertEq(4, n) + + // Read a range overlapping EOF. + n, err = f.ReadAt(buf[:4], 2) + AssertEq(io.EOF, err) + ExpectEq(2, n) + ExpectEq("co", string(buf[:n])) + + // Read a range starting at EOF. + n, err = f.ReadAt(buf[:4], 4) + AssertEq(io.EOF, err) + ExpectEq(0, n) + ExpectEq("", string(buf[:n])) + + // Read a range starting past EOF. + n, err = f.ReadAt(buf[:4], 100) + AssertEq(io.EOF, err) + ExpectEq(0, n) + ExpectEq("", string(buf[:n])) } From 163b30373110044032707f415f83f2ad51d849cb Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 5 Mar 2015 19:33:10 +1100 Subject: [PATCH 22/26] Implemented memFS.WriteFile. --- samples/memfs/fs.go | 18 ++++++++++++++++++ samples/memfs/inode.go | 27 +++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/samples/memfs/fs.go b/samples/memfs/fs.go index 07e3885..36b3630 100644 --- a/samples/memfs/fs.go +++ b/samples/memfs/fs.go @@ -437,3 +437,21 @@ func (fs *memFS) ReadDir( return } + +func (fs *memFS) WriteFile( + ctx context.Context, + req *fuse.WriteFileRequest) (resp *fuse.WriteFileResponse, err error) { + resp = &fuse.WriteFileResponse{} + + fs.mu.RLock() + defer fs.mu.RUnlock() + + // Find the inode in question. + inode := fs.getInodeForModifyingOrDie(req.Inode) + defer inode.mu.Unlock() + + // Serve the request. + _, err = inode.WriteAt(req.Data, req.Offset) + + return +} diff --git a/samples/memfs/inode.go b/samples/memfs/inode.go index 163a22f..bfb2244 100644 --- a/samples/memfs/inode.go +++ b/samples/memfs/inode.go @@ -279,3 +279,30 @@ func (inode *inode) ReadDir(offset int, size int) (data []byte, err error) { return } + +// Write to the file's contents. See documentation for ioutil.WriterAt. +// +// REQUIRES: !inode.dir +// EXCLUSIVE_LOCKS_REQUIRED(inode.mu) +func (inode *inode) WriteAt(p []byte, off int64) (n int, err error) { + if inode.dir { + panic("WriteAt called on directory.") + } + + // Ensure that the contents slice is long enough. + newLen := int(off) + len(p) + if len(inode.contents) < newLen { + padding := make([]byte, newLen-len(inode.contents)) + inode.contents = append(inode.contents, padding...) + } + + // Copy in the data. + n = copy(inode.contents[off:], p) + + // Sanity check. + if n != len(p) { + panic(fmt.Sprintf("Unexpected short copy: %v", n)) + } + + return +} From 7bb6fe37d2a1bd6753a7be2525f59490e94ffe13 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 5 Mar 2015 19:35:32 +1100 Subject: [PATCH 23/26] Added an invariant for inode size. --- samples/memfs/inode.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/samples/memfs/inode.go b/samples/memfs/inode.go index bfb2244..07583ab 100644 --- a/samples/memfs/inode.go +++ b/samples/memfs/inode.go @@ -54,6 +54,7 @@ type inode struct { // INVARIANT: No non-permission mode bits are set besides os.ModeDir // INVARIANT: If dir, then os.ModeDir is set // INVARIANT: If !dir, then os.ModeDir is not set + // INVARIANT: attributes.Size == len(contents) attributes fuse.InodeAttributes // GUARDED_BY(mu) // For directories, entries describing the children of the directory. Unused @@ -142,6 +143,15 @@ func (inode *inode) checkInvariants() { panic("Non-nil entries in a file.") } } + + // Check the size. + if inode.attributes.Size != uint64(len(inode.contents)) { + panic( + fmt.Sprintf( + "Unexpected size: %v vs. %v", + inode.attributes.Size, + len(inode.contents))) + } } // Return the index of the child within inode.entries, if it exists. From 26d91ad9d7c020b65f62fc17fe2b924c05ca65cf Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 5 Mar 2015 19:36:09 +1100 Subject: [PATCH 24/26] Fixed a broken invariant. --- samples/memfs/inode.go | 1 + 1 file changed, 1 insertion(+) diff --git a/samples/memfs/inode.go b/samples/memfs/inode.go index 07583ab..647614c 100644 --- a/samples/memfs/inode.go +++ b/samples/memfs/inode.go @@ -304,6 +304,7 @@ func (inode *inode) WriteAt(p []byte, off int64) (n int, err error) { if len(inode.contents) < newLen { padding := make([]byte, newLen-len(inode.contents)) inode.contents = append(inode.contents, padding...) + inode.attributes.Size = uint64(newLen) } // Copy in the data. From 4e169a060225a1b80c6934bff4add26a6e6cc159 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 5 Mar 2015 19:37:33 +1100 Subject: [PATCH 25/26] Implemented memFS.OpenFile. --- samples/memfs/fs.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/samples/memfs/fs.go b/samples/memfs/fs.go index 36b3630..b0fe38c 100644 --- a/samples/memfs/fs.go +++ b/samples/memfs/fs.go @@ -438,6 +438,27 @@ func (fs *memFS) ReadDir( return } +func (fs *memFS) OpenFile( + ctx context.Context, + req *fuse.OpenFileRequest) (resp *fuse.OpenFileResponse, err error) { + resp = &fuse.OpenFileResponse{} + + fs.mu.RLock() + defer fs.mu.RUnlock() + + // We don't mutate spontaneosuly, so if the VFS layer has asked for an + // inode that doesn't exist, something screwed up earlier (a lookup, a + // cache invalidation, etc.). + inode := fs.getInodeForReadingOrDie(req.Inode) + defer inode.mu.RUnlock() + + if inode.dir { + panic("Found directory.") + } + + return +} + func (fs *memFS) WriteFile( ctx context.Context, req *fuse.WriteFileRequest) (resp *fuse.WriteFileResponse, err error) { From a612b8b8d6e5e49b2beffbda74b045e6b6384fe8 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 5 Mar 2015 20:25:40 +1100 Subject: [PATCH 26/26] Added notes about the page cache. --- file_system.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/file_system.go b/file_system.go index 506aec2..0588966 100644 --- a/file_system.go +++ b/file_system.go @@ -151,11 +151,40 @@ type FileSystem interface { req *OpenFileRequest) (*OpenFileResponse, error) // Read data from a file previously opened with CreateFile or OpenFile. + // + // Note that this method is not called for every call to read(2) by the end + // user; some reads may be served by the page cache. See notes on Write for + // more. ReadFile( ctx context.Context, req *ReadFileRequest) (*ReadFileResponse, error) // Write data to a file previously opened with CreateFile or OpenFile. + // + // When the user writes data using write(2), the write goes into the page + // cache and the page is marked dirty. Later the kernel may write back the + // page via the FUSE VFS layer, causing this method to be called: + // + // * The kernel calls address_space_operations::writepage when a dirty page + // needs to be written to backing store (see vfs.txt). Fuse sets this to + // fuse_writepage (see file.c). + // + // * fuse_writepage calls fuse_writepage_locked. + // + // * fuse_writepage_locked makes a write request to the userspace server. + // + // Note that writes *will* be received before a call to Flush when closing + // the file descriptor to which they were written: + // + // * fuse_flush calls write_inode_now, which appears to start a writeback + // in the background (it talks about a "flusher thread"). + // + // * fuse_flush then calls fuse_sync_writes, which "[waits] for all pending + // writepages on the inode to finish". + // + // * Only then does fuse_flush finally send the flush request. + // + // TODO(jacobsa): Add links for all of the references above. WriteFile( ctx context.Context, req *WriteFileRequest) (*WriteFileResponse, error)