From f073423b67678a083c3ef451749ae88789bbb90f Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 13:27:18 +1000 Subject: [PATCH 01/16] Added test names. --- samples/memfs/posix_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index b9197df..ec84dc9 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -418,3 +418,15 @@ func (t *PosixTest) RmdirWhileOpenedForReading() { ExpectThat(names, ElementsAre()) } } + +func (t *PosixTest) CreateInParallel_NoTruncate() { + AssertFalse(true, "TODO") +} + +func (t *PosixTest) CreateInParallel_Truncate() { + AssertFalse(true, "TODO") +} + +func (t *PosixTest) CreateInParallel_Exclusive() { + AssertFalse(true, "TODO") +} From acccaf1e28858609a1d3e3e0642ec28a12eef039 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 13:46:47 +1000 Subject: [PATCH 02/16] PosixTest.CreateInParallel_NoTruncate --- samples/memfs/posix_test.go | 65 ++++++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index ec84dc9..9b311c9 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -18,13 +18,18 @@ package memfs_test import ( + "fmt" "io" "io/ioutil" "os" "path" "runtime" "testing" + "time" + "golang.org/x/net/context" + + "github.com/jacobsa/gcloud/syncutil" . "github.com/jacobsa/oglematchers" . "github.com/jacobsa/ogletest" ) @@ -420,7 +425,65 @@ func (t *PosixTest) RmdirWhileOpenedForReading() { } func (t *PosixTest) CreateInParallel_NoTruncate() { - AssertFalse(true, "TODO") + // Ensure that we get parallelism for this test. + defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(runtime.NumCPU())) + + // Try for awhile to see if anything breaks. + const duration = 500 * time.Millisecond + startTime := time.Now() + for time.Since(startTime) < duration { + filename := path.Join(t.dir, "foo") + + // Set up a function that opens the file with O_CREATE and then appends a + // byte to it. + worker := func(id byte) (err error) { + f, err := os.OpenFile(filename, os.O_CREATE|os.O_APPEND, 0600) + if err != nil { + err = fmt.Errorf("Worker %d: Open: %v", id, err) + return + } + + _, err = f.Write([]byte{id}) + if err != nil { + err = fmt.Errorf("Worker %d: Write: %v", id, err) + return + } + + return + } + + // Run several workers in parallel. + const numWorkers = 16 + b := syncutil.NewBundle(context.Background()) + for i := 0; i < numWorkers; i++ { + id := byte(i) + b.Add(func(ctx context.Context) (err error) { + err = worker(id) + return + }) + } + + err := b.Join() + AssertEq(nil, err) + + // Read the contents of the file. We should see each worker's ID once. + contents, err := ioutil.ReadFile(filename) + AssertEq(nil, err) + + idsSeen := make(map[byte]struct{}) + for i, _ := range contents { + id := contents[i] + AssertLt(id, numWorkers) + + if _, ok := idsSeen[id]; ok { + AddFailure("Duplicate ID: %d", id) + } + + idsSeen[id] = struct{}{} + } + + AssertEq(numWorkers, len(idsSeen)) + } } func (t *PosixTest) CreateInParallel_Truncate() { From 113fabe93759dd6b11ec35b8ec72db7627c8635d Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 13:48:56 +1000 Subject: [PATCH 03/16] Don't forget to close. --- samples/memfs/posix_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index 9b311c9..7b6aaed 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -443,6 +443,8 @@ func (t *PosixTest) CreateInParallel_NoTruncate() { return } + defer f.Close() + _, err = f.Write([]byte{id}) if err != nil { err = fmt.Errorf("Worker %d: Write: %v", id, err) From e295ef6019bcf6c83fa601a6d936d73d8670971b Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 13:50:18 +1000 Subject: [PATCH 04/16] Fixed two test bugs. --- samples/memfs/posix_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index 7b6aaed..1a4f415 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -437,7 +437,7 @@ func (t *PosixTest) CreateInParallel_NoTruncate() { // Set up a function that opens the file with O_CREATE and then appends a // byte to it. worker := func(id byte) (err error) { - f, err := os.OpenFile(filename, os.O_CREATE|os.O_APPEND, 0600) + f, err := os.OpenFile(filename, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) if err != nil { err = fmt.Errorf("Worker %d: Open: %v", id, err) return @@ -485,6 +485,10 @@ func (t *PosixTest) CreateInParallel_NoTruncate() { } AssertEq(numWorkers, len(idsSeen)) + + // Delete the file. + err = os.Remove(filename) + AssertEq(nil, err) } } From e69cd9188e920b73b3fa2e77b0122bd355a69bc5 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 13:53:52 +1000 Subject: [PATCH 05/16] Split out into a reusable function. --- samples/memfs/posix_test.go | 140 +++++++++++++++++++----------------- 1 file changed, 75 insertions(+), 65 deletions(-) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index 1a4f415..9495d59 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -46,11 +46,83 @@ func getFileOffset(f *os.File) (offset int64, err error) { return } +func runCreateInParallelTest( + ctx context.Context, + dir string) { + // Ensure that we get parallelism for this test. + defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(runtime.NumCPU())) + + // Try for awhile to see if anything breaks. + const duration = 500 * time.Millisecond + startTime := time.Now() + for time.Since(startTime) < duration { + filename := path.Join(dir, "foo") + + // Set up a function that opens the file with O_CREATE and then appends a + // byte to it. + worker := func(id byte) (err error) { + f, err := os.OpenFile(filename, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) + if err != nil { + err = fmt.Errorf("Worker %d: Open: %v", id, err) + return + } + + defer f.Close() + + _, err = f.Write([]byte{id}) + if err != nil { + err = fmt.Errorf("Worker %d: Write: %v", id, err) + return + } + + return + } + + // Run several workers in parallel. + const numWorkers = 16 + b := syncutil.NewBundle(ctx) + for i := 0; i < numWorkers; i++ { + id := byte(i) + b.Add(func(ctx context.Context) (err error) { + err = worker(id) + return + }) + } + + err := b.Join() + AssertEq(nil, err) + + // Read the contents of the file. We should see each worker's ID once. + contents, err := ioutil.ReadFile(filename) + AssertEq(nil, err) + + idsSeen := make(map[byte]struct{}) + for i, _ := range contents { + id := contents[i] + AssertLt(id, numWorkers) + + if _, ok := idsSeen[id]; ok { + AddFailure("Duplicate ID: %d", id) + } + + idsSeen[id] = struct{}{} + } + + AssertEq(numWorkers, len(idsSeen)) + + // Delete the file. + err = os.Remove(filename) + AssertEq(nil, err) + } +} + //////////////////////////////////////////////////////////////////////// // Boilerplate //////////////////////////////////////////////////////////////////////// type PosixTest struct { + ctx context.Context + // A temporary directory. dir string @@ -66,6 +138,8 @@ func init() { RegisterTestSuite(&PosixTest{}) } func (t *PosixTest) SetUp(ti *TestInfo) { var err error + t.ctx = ti.Ctx + // Create a temporary directory. t.dir, err = ioutil.TempDir("", "posix_test") if err != nil { @@ -425,71 +499,7 @@ func (t *PosixTest) RmdirWhileOpenedForReading() { } func (t *PosixTest) CreateInParallel_NoTruncate() { - // Ensure that we get parallelism for this test. - defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(runtime.NumCPU())) - - // Try for awhile to see if anything breaks. - const duration = 500 * time.Millisecond - startTime := time.Now() - for time.Since(startTime) < duration { - filename := path.Join(t.dir, "foo") - - // Set up a function that opens the file with O_CREATE and then appends a - // byte to it. - worker := func(id byte) (err error) { - f, err := os.OpenFile(filename, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) - if err != nil { - err = fmt.Errorf("Worker %d: Open: %v", id, err) - return - } - - defer f.Close() - - _, err = f.Write([]byte{id}) - if err != nil { - err = fmt.Errorf("Worker %d: Write: %v", id, err) - return - } - - return - } - - // Run several workers in parallel. - const numWorkers = 16 - b := syncutil.NewBundle(context.Background()) - for i := 0; i < numWorkers; i++ { - id := byte(i) - b.Add(func(ctx context.Context) (err error) { - err = worker(id) - return - }) - } - - err := b.Join() - AssertEq(nil, err) - - // Read the contents of the file. We should see each worker's ID once. - contents, err := ioutil.ReadFile(filename) - AssertEq(nil, err) - - idsSeen := make(map[byte]struct{}) - for i, _ := range contents { - id := contents[i] - AssertLt(id, numWorkers) - - if _, ok := idsSeen[id]; ok { - AddFailure("Duplicate ID: %d", id) - } - - idsSeen[id] = struct{}{} - } - - AssertEq(numWorkers, len(idsSeen)) - - // Delete the file. - err = os.Remove(filename) - AssertEq(nil, err) - } + runCreateInParallelTest(t.ctx, t.dir) } func (t *PosixTest) CreateInParallel_Truncate() { From 25ded77e80ca01874d1dc82f618ebc1749214b2f Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 14:18:54 +1000 Subject: [PATCH 06/16] PosixTest.CreateInParallel_Truncate --- samples/memfs/posix_test.go | 82 +++++++++++++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 3 deletions(-) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index 9495d59..811a3c9 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -46,7 +46,7 @@ func getFileOffset(f *os.File) (offset int64, err error) { return } -func runCreateInParallelTest( +func runCreateInParallelTest_NoTruncate( ctx context.Context, dir string) { // Ensure that we get parallelism for this test. @@ -116,6 +116,82 @@ func runCreateInParallelTest( } } +func runCreateInParallelTest_Truncate( + ctx context.Context, + dir string) { + // Ensure that we get parallelism for this test. + defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(runtime.NumCPU())) + + // Try for awhile to see if anything breaks. + const duration = 500 * time.Millisecond + startTime := time.Now() + for time.Since(startTime) < duration { + filename := path.Join(dir, "foo") + + // Set up a function that opens the file with O_CREATE and O_TRUNC and then + // appends a byte to it. + worker := func(id byte) (err error) { + f, err := os.OpenFile( + filename, + os.O_CREATE|os.O_WRONLY|os.O_APPEND|os.O_TRUNC, + 0600) + + if err != nil { + err = fmt.Errorf("Worker %d: Open: %v", id, err) + return + } + + defer f.Close() + + _, err = f.Write([]byte{id}) + if err != nil { + err = fmt.Errorf("Worker %d: Write: %v", id, err) + return + } + + return + } + + // Run several workers in parallel. + const numWorkers = 16 + b := syncutil.NewBundle(ctx) + for i := 0; i < numWorkers; i++ { + id := byte(i) + b.Add(func(ctx context.Context) (err error) { + err = worker(id) + return + }) + } + + err := b.Join() + AssertEq(nil, err) + + // Read the contents of the file. We should see at least one ID (the last + // one that truncated), and at most all of them. + contents, err := ioutil.ReadFile(filename) + AssertEq(nil, err) + + idsSeen := make(map[byte]struct{}) + for i, _ := range contents { + id := contents[i] + AssertLt(id, numWorkers) + + if _, ok := idsSeen[id]; ok { + AddFailure("Duplicate ID: %d", id) + } + + idsSeen[id] = struct{}{} + } + + AssertGe(len(idsSeen), 1) + AssertLe(len(idsSeen), numWorkers) + + // Delete the file. + err = os.Remove(filename) + AssertEq(nil, err) + } +} + //////////////////////////////////////////////////////////////////////// // Boilerplate //////////////////////////////////////////////////////////////////////// @@ -499,11 +575,11 @@ func (t *PosixTest) RmdirWhileOpenedForReading() { } func (t *PosixTest) CreateInParallel_NoTruncate() { - runCreateInParallelTest(t.ctx, t.dir) + runCreateInParallelTest_NoTruncate(t.ctx, t.dir) } func (t *PosixTest) CreateInParallel_Truncate() { - AssertFalse(true, "TODO") + runCreateInParallelTest_Truncate(t.ctx, t.dir) } func (t *PosixTest) CreateInParallel_Exclusive() { From 63888f4ddd6f846f8b9f7a44d787623793359e2e Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 14:22:44 +1000 Subject: [PATCH 07/16] PosixTest.CreateInParallel_Exclusive --- samples/memfs/posix_test.go | 78 ++++++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index 811a3c9..1052814 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -24,6 +24,7 @@ import ( "os" "path" "runtime" + "sync/atomic" "testing" "time" @@ -192,6 +193,81 @@ func runCreateInParallelTest_Truncate( } } +func runCreateInParallelTest_Exclusive( + ctx context.Context, + dir string) { + // Ensure that we get parallelism for this test. + defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(runtime.NumCPU())) + + // Try for awhile to see if anything breaks. + const duration = 500 * time.Millisecond + startTime := time.Now() + for time.Since(startTime) < duration { + filename := path.Join(dir, "foo") + + // Set up a function that opens the file with O_CREATE and O_EXCL, and then + // appends a byte to it if it was successfully opened. + var openCount uint64 + worker := func(id byte) (err error) { + f, err := os.OpenFile( + filename, + os.O_CREATE|os.O_EXCL|os.O_WRONLY|os.O_APPEND, + 0600) + + // If we failed to open due to the file already existing, just leave. + if os.IsExist(err) { + err = nil + return + } + + // Propgate other errors. + if err != nil { + err = fmt.Errorf("Worker %d: Open: %v", id, err) + return + } + + atomic.AddUint64(&openCount, 1) + defer f.Close() + + _, err = f.Write([]byte{id}) + if err != nil { + err = fmt.Errorf("Worker %d: Write: %v", id, err) + return + } + + return + } + + // Run several workers in parallel. + const numWorkers = 16 + b := syncutil.NewBundle(ctx) + for i := 0; i < numWorkers; i++ { + id := byte(i) + b.Add(func(ctx context.Context) (err error) { + err = worker(id) + return + }) + } + + err := b.Join() + AssertEq(nil, err) + + // Exactly one worker should have opened successfully. + AssertEq(1, openCount) + + // Read the contents of the file. It should contain that one worker's ID. + contents, err := ioutil.ReadFile(filename) + AssertEq(nil, err) + + AssertEq(1, len(contents)) + AssertLt(contents[0], numWorkers) + + // Delete the file. + err = os.Remove(filename) + AssertEq(nil, err) + } +} + //////////////////////////////////////////////////////////////////////// // Boilerplate //////////////////////////////////////////////////////////////////////// @@ -583,5 +659,5 @@ func (t *PosixTest) CreateInParallel_Truncate() { } func (t *PosixTest) CreateInParallel_Exclusive() { - AssertFalse(true, "TODO") + runCreateInParallelTest_Exclusive(t.ctx, t.dir) } From 4269d7cd470d08ac6937b3335b8b946267d3558d Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 13:26:03 +1000 Subject: [PATCH 08/16] Check for already existing in memFS.CreateFile. --- samples/memfs/memfs.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/samples/memfs/memfs.go b/samples/memfs/memfs.go index 872b955..1aa4079 100644 --- a/samples/memfs/memfs.go +++ b/samples/memfs/memfs.go @@ -341,6 +341,14 @@ func (fs *memFS) CreateFile( parent := fs.getInodeForModifyingOrDie(op.Parent) defer parent.mu.Unlock() + // Ensure that the name doesn't alread exist, so we don't wind up with a + // duplicate. + _, exists := parent.LookUpChild(op.Name) + if exists { + err = fmt.Errorf("Name %q already exists in parent", op.Name) + return + } + // 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() From 0407fb769054e3718690b9bdf88be184adcadb1b Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 14:30:52 +1000 Subject: [PATCH 09/16] Added memfs create in parallel tests. --- 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 76567b4..44bee40 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -1251,3 +1251,15 @@ func (t *MemFSTest) DeleteSymlink() { AssertEq(nil, err) ExpectThat(entries, ElementsAre()) } + +func (t *MemFSTest) CreateInParallel_NoTruncate() { + runCreateInParallelTest_NoTruncate(t.Ctx, t.Dir) +} + +func (t *MemFSTest) CreateInParallel_Truncate() { + runCreateInParallelTest_Truncate(t.Ctx, t.Dir) +} + +func (t *MemFSTest) CreateInParallel_Exclusive() { + runCreateInParallelTest_Exclusive(t.Ctx, t.Dir) +} From 150440d500cd8ae0c3f3f179caf5cce3c2142561 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 15:14:16 +1000 Subject: [PATCH 10/16] Return EEXIST from create. This appears to work, on OS X at least. --- errors.go | 1 + samples/memfs/memfs.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/errors.go b/errors.go index 316234f..3fd3ba4 100644 --- a/errors.go +++ b/errors.go @@ -23,6 +23,7 @@ import ( const ( // Errors corresponding to kernel error numbers. These may be treated // specially by fuseops.Op.Respond methods. + EEXIST = bazilfuse.EEXIST EINVAL = bazilfuse.Errno(syscall.EINVAL) EIO = bazilfuse.EIO ENOENT = bazilfuse.ENOENT diff --git a/samples/memfs/memfs.go b/samples/memfs/memfs.go index 1aa4079..beacc4c 100644 --- a/samples/memfs/memfs.go +++ b/samples/memfs/memfs.go @@ -345,7 +345,7 @@ func (fs *memFS) CreateFile( // duplicate. _, exists := parent.LookUpChild(op.Name) if exists { - err = fmt.Errorf("Name %q already exists in parent", op.Name) + err = fuse.EEXIST return } From 2b71f8cbe83fb88f1a2de14d54898087181228c8 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 15:19:39 +1000 Subject: [PATCH 11/16] PosixTest.MkdirInParallel --- samples/memfs/posix_test.go | 59 +++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index 1052814..efb5da2 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -30,6 +30,7 @@ import ( "golang.org/x/net/context" + "github.com/jacobsa/fuse/fusetesting" "github.com/jacobsa/gcloud/syncutil" . "github.com/jacobsa/oglematchers" . "github.com/jacobsa/ogletest" @@ -268,6 +269,60 @@ func runCreateInParallelTest_Exclusive( } } +func runMkdirInParallelTest( + ctx context.Context, + dir string) { + // Ensure that we get parallelism for this test. + defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(runtime.NumCPU())) + + // Try for awhile to see if anything breaks. + const duration = 500 * time.Millisecond + startTime := time.Now() + for time.Since(startTime) < duration { + filename := path.Join(dir, "foo") + + // Set up a function that creates the directory, ignoring EEXIST errors. + worker := func(id byte) (err error) { + err = os.Mkdir(filename, 0700) + + if os.IsExist(err) { + err = nil + } + + if err != nil { + err = fmt.Errorf("Worker %d: Mkdir: %v", id, err) + return + } + + return + } + + // Run several workers in parallel. + const numWorkers = 16 + b := syncutil.NewBundle(ctx) + for i := 0; i < numWorkers; i++ { + id := byte(i) + b.Add(func(ctx context.Context) (err error) { + err = worker(id) + return + }) + } + + err := b.Join() + AssertEq(nil, err) + + // The directory should have been created, once. + entries, err := fusetesting.ReadDirPicky(dir) + AssertEq(nil, err) + AssertEq(1, len(entries)) + AssertEq("foo", entries[0].Name()) + + // Delete the directory. + err = os.Remove(filename) + AssertEq(nil, err) + } +} + //////////////////////////////////////////////////////////////////////// // Boilerplate //////////////////////////////////////////////////////////////////////// @@ -661,3 +716,7 @@ func (t *PosixTest) CreateInParallel_Truncate() { func (t *PosixTest) CreateInParallel_Exclusive() { runCreateInParallelTest_Exclusive(t.ctx, t.dir) } + +func (t *PosixTest) MkdirInParallel() { + runMkdirInParallelTest(t.ctx, t.dir) +} From e158bcfa174be4bc11aa6215a25917196fbd38a3 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 15:20:01 +1000 Subject: [PATCH 12/16] MemFSTest.MkdirInParallel --- 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 44bee40..41d9ad6 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -1263,3 +1263,7 @@ func (t *MemFSTest) CreateInParallel_Truncate() { func (t *MemFSTest) CreateInParallel_Exclusive() { runCreateInParallelTest_Exclusive(t.Ctx, t.Dir) } + +func (t *MemFSTest) MkdirInParallel() { + runMkdirInParallelTest(t.Ctx, t.Dir) +} From 7ec610e0be88c3a9f633dec21e33627a5b46bcc8 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 15:21:05 +1000 Subject: [PATCH 13/16] PosixTest.SymlinkInParallel --- samples/memfs/posix_test.go | 58 +++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/samples/memfs/posix_test.go b/samples/memfs/posix_test.go index efb5da2..e8fccb5 100644 --- a/samples/memfs/posix_test.go +++ b/samples/memfs/posix_test.go @@ -323,6 +323,60 @@ func runMkdirInParallelTest( } } +func runSymlinkInParallelTest( + ctx context.Context, + dir string) { + // Ensure that we get parallelism for this test. + defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(runtime.NumCPU())) + + // Try for awhile to see if anything breaks. + const duration = 500 * time.Millisecond + startTime := time.Now() + for time.Since(startTime) < duration { + filename := path.Join(dir, "foo") + + // Set up a function that creates the symlink, ignoring EEXIST errors. + worker := func(id byte) (err error) { + err = os.Symlink("blah", filename) + + if os.IsExist(err) { + err = nil + } + + if err != nil { + err = fmt.Errorf("Worker %d: Symlink: %v", id, err) + return + } + + return + } + + // Run several workers in parallel. + const numWorkers = 16 + b := syncutil.NewBundle(ctx) + for i := 0; i < numWorkers; i++ { + id := byte(i) + b.Add(func(ctx context.Context) (err error) { + err = worker(id) + return + }) + } + + err := b.Join() + AssertEq(nil, err) + + // The symlink should have been created, once. + entries, err := fusetesting.ReadDirPicky(dir) + AssertEq(nil, err) + AssertEq(1, len(entries)) + AssertEq("foo", entries[0].Name()) + + // Delete the directory. + err = os.Remove(filename) + AssertEq(nil, err) + } +} + //////////////////////////////////////////////////////////////////////// // Boilerplate //////////////////////////////////////////////////////////////////////// @@ -720,3 +774,7 @@ func (t *PosixTest) CreateInParallel_Exclusive() { func (t *PosixTest) MkdirInParallel() { runMkdirInParallelTest(t.ctx, t.dir) } + +func (t *PosixTest) SymlinkInParallel() { + runSymlinkInParallelTest(t.ctx, t.dir) +} From 49a5ff232824bf0a36bc45c89659cd6cd868dab0 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 15:21:26 +1000 Subject: [PATCH 14/16] MemFSTest.SymlinkInParallel --- 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 41d9ad6..41d676e 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -1267,3 +1267,7 @@ func (t *MemFSTest) CreateInParallel_Exclusive() { func (t *MemFSTest) MkdirInParallel() { runMkdirInParallelTest(t.Ctx, t.Dir) } + +func (t *MemFSTest) SymlinkInParallel() { + runSymlinkInParallelTest(t.Ctx, t.Dir) +} From bc969387cae8918cb8d84ef95e059cfcbc19f474 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 15:22:26 +1000 Subject: [PATCH 15/16] Fixed memfs bugs. --- samples/memfs/memfs.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/samples/memfs/memfs.go b/samples/memfs/memfs.go index beacc4c..588ec7d 100644 --- a/samples/memfs/memfs.go +++ b/samples/memfs/memfs.go @@ -301,6 +301,14 @@ func (fs *memFS) MkDir( parent := fs.getInodeForModifyingOrDie(op.Parent) defer parent.mu.Unlock() + // Ensure that the name doesn't already exist, so we don't wind up with a + // duplicate. + _, exists := parent.LookUpChild(op.Name) + if exists { + err = fuse.EEXIST + return + } + // Set up attributes from the child, using the credentials of the calling // process as owner (matching inode_init_owner, cf. http://goo.gl/5qavg8). childAttrs := fuseops.InodeAttributes{ @@ -341,7 +349,7 @@ func (fs *memFS) CreateFile( parent := fs.getInodeForModifyingOrDie(op.Parent) defer parent.mu.Unlock() - // Ensure that the name doesn't alread exist, so we don't wind up with a + // Ensure that the name doesn't already exist, so we don't wind up with a // duplicate. _, exists := parent.LookUpChild(op.Name) if exists { @@ -396,6 +404,14 @@ func (fs *memFS) CreateSymlink( parent := fs.getInodeForModifyingOrDie(op.Parent) defer parent.mu.Unlock() + // Ensure that the name doesn't already exist, so we don't wind up with a + // duplicate. + _, exists := parent.LookUpChild(op.Name) + if exists { + err = fuse.EEXIST + return + } + // 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() From bd4af3f5a58fc979a32d13ee7af255346d1bf740 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 21 May 2015 15:30:08 +1000 Subject: [PATCH 16/16] Updated advice on create, symlink, and mkdir. --- fuseops/ops.go | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/fuseops/ops.go b/fuseops/ops.go index 9da1877..5686ede 100644 --- a/fuseops/ops.go +++ b/fuseops/ops.go @@ -275,10 +275,14 @@ func (o *ForgetInodeOp) toBazilfuseResponse() (bfResp 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. +// The Linux 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). Indeed, the tests in samples/memfs that call in +// parallel appear to bear this out. But osxfuse does not appear to guarantee +// this (cf. https://goo.gl/PqzZDv). And if names may be created outside of the +// kernel's control, it doesn't matter what the kernel does anyway. +// +// Therefore the file system should return EEXIST if the name already exists. type MkDirOp struct { commonOp @@ -314,15 +318,12 @@ func (o *MkDirOp) toBazilfuseResponse() (bfResp interface{}) { // // The kernel sends this 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). +// example lookup_open, http://goo.gl/PlqE9d). However, osxfuse doesn't appear +// to make this check atomically (cf. https://goo.gl/PqzZDv). And if names may +// be created outside of the kernel's control, it doesn't matter what the +// kernel does anyway. // -// 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. +// Therefore the file system should return EEXIST if the name already exists. type CreateFileOp struct { commonOp @@ -371,7 +372,8 @@ func (o *CreateFileOp) toBazilfuseResponse() (bfResp interface{}) { return } -// Create a symlink inode. +// Create a symlink inode. If the name already exists, the file system should +// return EEXIST (cf. the notes on CreateFileOp and MkDirOp). type CreateSymlinkOp struct { commonOp