From 40519818d512649d59ec66ed54065ee7d7b8fe5f Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 17 Mar 2015 16:14:37 +1100 Subject: [PATCH 1/7] Clarified the expectations in a couple of tests. --- samples/cachingfs/caching_fs_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/samples/cachingfs/caching_fs_test.go b/samples/cachingfs/caching_fs_test.go index d570a08..0e1bf96 100644 --- a/samples/cachingfs/caching_fs_test.go +++ b/samples/cachingfs/caching_fs_test.go @@ -446,15 +446,16 @@ func (t *AttributeCachingTest) StatRenumberStat() { ExpectEq(t.fs.BarID(), getInodeID(barAfter)) } -func (t *AttributeCachingTest) StatMtimeStat() { +func (t *AttributeCachingTest) StatMtimeStat_ViaPath() { newMtime := t.initialMtime.Add(time.Second) t.statAll() t.fs.SetMtime(newMtime) fooAfter, dirAfter, barAfter := t.statAll() - // We should see the new attributes, since the entry had to be looked up - // again and the new attributes were returned with the entry. + // Since we don't have entry caching enabled, the call above had to look up + // the entry again. With the lookup we returned new attributes, so we expect + // that the mtime will be fresh. ExpectThat(fooAfter.ModTime(), timeutil.TimeEq(newMtime)) ExpectThat(dirAfter.ModTime(), timeutil.TimeEq(newMtime)) ExpectThat(barAfter.ModTime(), timeutil.TimeEq(newMtime)) @@ -490,7 +491,7 @@ func (t *AttributeCachingTest) StatMtimeStat_ViaFileDescriptor() { ExpectThat(barAfter.ModTime(), timeutil.TimeEq(newMtime)) } -func (t *AttributeCachingTest) StatRenumberMtimeStat() { +func (t *AttributeCachingTest) StatRenumberMtimeStat_ViaPath() { newMtime := t.initialMtime.Add(time.Second) t.statAll() @@ -500,7 +501,7 @@ func (t *AttributeCachingTest) StatRenumberMtimeStat() { // We should see new everything, because this is the first time the new // inodes have been encountered. Entries for the old ones should not have - // been cached. + // been cached, because we have entry caching disabled. ExpectEq(t.fs.FooID(), getInodeID(fooAfter)) ExpectEq(t.fs.DirID(), getInodeID(dirAfter)) ExpectEq(t.fs.BarID(), getInodeID(barAfter)) From 2f64ad7f08c44910af745d14e823bb6eb8f133ea Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 17 Mar 2015 16:19:13 +1100 Subject: [PATCH 2/7] Added MountConfig.EnableVnodeCaching. --- mounted_file_system.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/mounted_file_system.go b/mounted_file_system.go index b1e52de..d456561 100644 --- a/mounted_file_system.go +++ b/mounted_file_system.go @@ -117,10 +117,25 @@ func (mfs *MountedFileSystem) mountAndServe( // Optional configuration accepted by Mount. type MountConfig struct { + // OS X only. + // + // Normally on OS X we mount with the novncache option + // (cf. http://goo.gl/1pTjuk), which disables entry caching in the kernel. + // This is because osxfuse does not support the entry expiration values we + // return to it (cf. http://goo.gl/8yR0Ie) and it is probably better to fail + // to cache than to cache for too long, since the latter is more likely to + // hide consistency bugs that are difficult to detect and diagnose. + // + // This field disables the use of novncache, restoring entry caching. Beware: + // the value of ChildInodeEntry.EntryExpiration is ignored by the kernel, and + // entries will be cached for an arbitrarily long time. + EnableVnodeCaching bool } // Convert to mount options to be passed to package bazilfuse. func (c *MountConfig) bazilfuseOptions() (opts []bazilfuse.MountOption) { + panic("TODO: Test and support MountConfig.EnableVnodeCaching") + opts = []bazilfuse.MountOption{ // Enable permissions checking in the kernel. See the comments on // InodeAttributes.Mode. From be35ff0ddec49a0b55468d5f9a046f5df16c14d3 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 17 Mar 2015 16:21:10 +1100 Subject: [PATCH 3/7] Set EnableVnodeCaching in the appropriate test. --- samples/cachingfs/caching_fs_test.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/samples/cachingfs/caching_fs_test.go b/samples/cachingfs/caching_fs_test.go index 0e1bf96..5f8e1e1 100644 --- a/samples/cachingfs/caching_fs_test.go +++ b/samples/cachingfs/caching_fs_test.go @@ -48,7 +48,8 @@ var _ TearDownInterface = &cachingFSTest{} func (t *cachingFSTest) setUp( lookupEntryTimeout time.Duration, - getattrTimeout time.Duration) { + getattrTimeout time.Duration, + config *fuse.MountConfig) { var err error // Set up a temporary directory for mounting. @@ -60,7 +61,7 @@ func (t *cachingFSTest) setUp( AssertEq(nil, err) // Mount it. - t.mfs, err = fuse.Mount(t.dir, t.fs, &fuse.MountConfig{}) + t.mfs, err = fuse.Mount(t.dir, t.fs, config) AssertEq(nil, err) err = t.mfs.WaitForReady(context.Background()) @@ -168,7 +169,7 @@ func (t *BasicsTest) SetUp(ti *TestInfo) { getattrTimeout = 0 ) - t.cachingFSTest.setUp(lookupEntryTimeout, getattrTimeout) + t.cachingFSTest.setUp(lookupEntryTimeout, getattrTimeout, &fuse.MountConfig{}) } func (t *BasicsTest) StatNonexistent() { @@ -241,7 +242,7 @@ func (t *NoCachingTest) SetUp(ti *TestInfo) { getattrTimeout = 0 ) - t.cachingFSTest.setUp(lookupEntryTimeout, getattrTimeout) + t.cachingFSTest.setUp(lookupEntryTimeout, getattrTimeout, &fuse.MountConfig{}) } func (t *NoCachingTest) StatStat() { @@ -318,7 +319,11 @@ func init() { RegisterTestSuite(&EntryCachingTest{}) } func (t *EntryCachingTest) SetUp(ti *TestInfo) { t.lookupEntryTimeout = 250 * time.Millisecond - t.cachingFSTest.setUp(t.lookupEntryTimeout, 0) + config := &fuse.MountConfig{ + EnableVnodeCaching: true, + } + + t.cachingFSTest.setUp(t.lookupEntryTimeout, 0, config) } func (t *EntryCachingTest) StatStat() { @@ -417,7 +422,7 @@ func init() { RegisterTestSuite(&AttributeCachingTest{}) } func (t *AttributeCachingTest) SetUp(ti *TestInfo) { t.getattrTimeout = 250 * time.Millisecond - t.cachingFSTest.setUp(0, t.getattrTimeout) + t.cachingFSTest.setUp(0, t.getattrTimeout, &fuse.MountConfig{}) } func (t *AttributeCachingTest) StatStat() { From b9b5a4f5fca99746b60659f790db9676571c400b Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 17 Mar 2015 16:22:19 +1100 Subject: [PATCH 4/7] Updated MountConfig.bazilfuseOptions. --- mounted_file_system.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/mounted_file_system.go b/mounted_file_system.go index d456561..4f3741b 100644 --- a/mounted_file_system.go +++ b/mounted_file_system.go @@ -16,6 +16,7 @@ package fuse import ( "errors" + "runtime" "github.com/jacobsa/bazilfuse" "golang.org/x/net/context" @@ -134,12 +135,13 @@ type MountConfig struct { // Convert to mount options to be passed to package bazilfuse. func (c *MountConfig) bazilfuseOptions() (opts []bazilfuse.MountOption) { - panic("TODO: Test and support MountConfig.EnableVnodeCaching") + // Enable permissions checking in the kernel. See the comments on + // InodeAttributes.Mode. + opts = append(opts, bazilfuse.SetOption("default_permissions", "")) - opts = []bazilfuse.MountOption{ - // Enable permissions checking in the kernel. See the comments on - // InodeAttributes.Mode. - bazilfuse.SetOption("default_permissions", ""), + // OS X only: set novncache when appropriate. + if runtime.GOOS == "darwin" && !c.EnableVnodeCaching { + opts = append(opts, bazilfuse.SetOption("novncache", "")) } return From 6fe7f548297aa8cf78421c434b371bc2aa26fe6d Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 17 Mar 2015 16:24:15 +1100 Subject: [PATCH 5/7] Updated a few tests for darwin's special entry cache behavior. --- samples/cachingfs/caching_fs_test.go | 37 ++++++++++++++++++---------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/samples/cachingfs/caching_fs_test.go b/samples/cachingfs/caching_fs_test.go index 5f8e1e1..ad9efd8 100644 --- a/samples/cachingfs/caching_fs_test.go +++ b/samples/cachingfs/caching_fs_test.go @@ -19,6 +19,7 @@ import ( "log" "os" "path" + "runtime" "strings" "syscall" "testing" @@ -353,12 +354,17 @@ func (t *EntryCachingTest) StatRenumberStat() { // But after waiting for the entry cache to expire, we should see the new // IDs. - time.Sleep(2 * t.lookupEntryTimeout) - fooAfter, dirAfter, barAfter = t.statAll() + // + // Note that the cache is not guaranteed to expire on darwin. See notes on + // fuse.MountConfig.EnableVnodeCaching. + if runtime.GOOS != "darwin" { + time.Sleep(2 * t.lookupEntryTimeout) + fooAfter, dirAfter, barAfter = t.statAll() - ExpectEq(t.fs.FooID(), getInodeID(fooAfter)) - ExpectEq(t.fs.DirID(), getInodeID(dirAfter)) - ExpectEq(t.fs.BarID(), getInodeID(barAfter)) + ExpectEq(t.fs.FooID(), getInodeID(fooAfter)) + ExpectEq(t.fs.DirID(), getInodeID(dirAfter)) + ExpectEq(t.fs.BarID(), getInodeID(barAfter)) + } } func (t *EntryCachingTest) StatMtimeStat() { @@ -395,16 +401,21 @@ func (t *EntryCachingTest) StatRenumberMtimeStat() { // After waiting for the entry cache to expire, we should see fresh // everything. - time.Sleep(2 * t.lookupEntryTimeout) - fooAfter, dirAfter, barAfter = t.statAll() + // + // Note that the cache is not guaranteed to expire on darwin. See notes on + // fuse.MountConfig.EnableVnodeCaching. + if runtime.GOOS != "darwin" { + time.Sleep(2 * t.lookupEntryTimeout) + fooAfter, dirAfter, barAfter = t.statAll() - ExpectEq(t.fs.FooID(), getInodeID(fooAfter)) - ExpectEq(t.fs.DirID(), getInodeID(dirAfter)) - ExpectEq(t.fs.BarID(), getInodeID(barAfter)) + ExpectEq(t.fs.FooID(), getInodeID(fooAfter)) + ExpectEq(t.fs.DirID(), getInodeID(dirAfter)) + ExpectEq(t.fs.BarID(), getInodeID(barAfter)) - ExpectThat(fooAfter.ModTime(), timeutil.TimeEq(newMtime)) - ExpectThat(dirAfter.ModTime(), timeutil.TimeEq(newMtime)) - ExpectThat(barAfter.ModTime(), timeutil.TimeEq(newMtime)) + ExpectThat(fooAfter.ModTime(), timeutil.TimeEq(newMtime)) + ExpectThat(dirAfter.ModTime(), timeutil.TimeEq(newMtime)) + ExpectThat(barAfter.ModTime(), timeutil.TimeEq(newMtime)) + } } //////////////////////////////////////////////////////////////////////// From 038f8e49a2d6952256781d144fa1365b12e4d6b9 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 17 Mar 2015 16:25:14 +1100 Subject: [PATCH 6/7] Added a note on the EntryExpiration field. --- file_system.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/file_system.go b/file_system.go index fe7922f..56fc07b 100644 --- a/file_system.go +++ b/file_system.go @@ -406,6 +406,9 @@ type ChildInodeEntry struct { // lookup request. // // Leave at the zero value to disable caching. + // + // Beware: this value is ignored on OS X, where entry caching is disabled by + // default. See notes on MountConfig.EnableVnodeCaching for more. EntryExpiration time.Time } From b851c516e33713e8c5d92cca16a096e058254380 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 17 Mar 2015 16:30:03 +1100 Subject: [PATCH 7/7] Fixed AttributeCachingTest.StatMtimeStat_ViaPath on OS X. --- samples/cachingfs/caching_fs_test.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/samples/cachingfs/caching_fs_test.go b/samples/cachingfs/caching_fs_test.go index ad9efd8..90aa803 100644 --- a/samples/cachingfs/caching_fs_test.go +++ b/samples/cachingfs/caching_fs_test.go @@ -28,6 +28,7 @@ import ( "github.com/jacobsa/fuse" "github.com/jacobsa/fuse/samples/cachingfs" "github.com/jacobsa/gcsfuse/timeutil" + . "github.com/jacobsa/oglematchers" . "github.com/jacobsa/ogletest" "golang.org/x/net/context" ) @@ -470,11 +471,13 @@ func (t *AttributeCachingTest) StatMtimeStat_ViaPath() { fooAfter, dirAfter, barAfter := t.statAll() // Since we don't have entry caching enabled, the call above had to look up - // the entry again. With the lookup we returned new attributes, so we expect - // that the mtime will be fresh. - ExpectThat(fooAfter.ModTime(), timeutil.TimeEq(newMtime)) - ExpectThat(dirAfter.ModTime(), timeutil.TimeEq(newMtime)) - ExpectThat(barAfter.ModTime(), timeutil.TimeEq(newMtime)) + // the entry again. With the lookup we returned new attributes, so it's + // possible that the mtime will be fresh. On Linux it appears to be, and on + // OS X it appears to not be. + m := AnyOf(timeutil.TimeEq(newMtime), timeutil.TimeEq(t.initialMtime)) + ExpectThat(fooAfter.ModTime(), m) + ExpectThat(dirAfter.ModTime(), m) + ExpectThat(barAfter.ModTime(), m) } func (t *AttributeCachingTest) StatMtimeStat_ViaFileDescriptor() {