-
Notifications
You must be signed in to change notification settings - Fork 180
add-go-fuse-to-inject-filesystem-error #583
base: master
Are you sure you want to change the base?
Changes from 80 commits
b97c06f
29b7209
394b39a
2472c78
ec0a5e0
75d7e04
c44cf44
a9ed232
81f7dd5
8ce9574
13374dd
62ae86f
5037567
406adcc
37157e9
b8478d4
d469dbf
261d9c5
f3808fe
b00d5bd
c1a3061
e11678e
4ac7384
bb5f22d
c8b6c6f
097610a
b634c3e
ccf974d
6aa7043
7ba8847
0795eb8
5de9085
ca04b2b
c607310
ddcc50d
9753d96
54774f1
5f15701
0417ed6
f3f984d
4bda5c0
8d09cc0
ac9068f
8c485d6
d9c5506
90f8077
80edb85
797257b
de99273
26b3fb8
0c0a19d
e16e3a7
ad6151f
59c387b
687fd7f
20534be
42b7859
9f79aa9
6d56002
9b0e24f
0e390ac
33e8785
0ac1b40
a545d05
43c2c03
f1cf68f
5859805
f7be8b1
0ebd0a7
9408f0d
8833d11
f9af261
0e6f527
e670888
f7c0590
719257b
213d516
bbc3ee7
33b31b8
6320020
0680777
e59acfa
998ef76
429f7b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,37 @@ | ||
dist: trusty | ||
language: go | ||
os: | ||
- windows | ||
- linux | ||
- osx | ||
|
||
go: | ||
- 1.12.x | ||
|
||
go_import_path: github.com/prometheus/tsdb | ||
matrix: | ||
include: | ||
- os: linux | ||
dist: trusty | ||
sudo: required | ||
go: 1.12.x | ||
install: | ||
- make deps | ||
script: | ||
- sudo -E apt-get -yq --no-install-suggests --no-install-recommends --force-yes install fuse | ||
- sudo modprobe fuse | ||
- sudo chmod a+rw /etc/fuse.conf | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need these?
I think just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to write I remove following lines, but the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are you sure that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are you sure that sudo echo user_allow_other >> /etc/fuse.conf doesn't work without sudo chmod a+rw /etc/fuse.conf ? |
||
- sudo echo user_allow_other >> /etc/fuse.conf | ||
- make all | ||
- os: osx | ||
go: 1.12.x | ||
sudo: required | ||
osx_image: xcode8.3 | ||
script: | ||
- brew update | ||
- brew tap homebrew/cask | ||
- brew cask install osxfuse | ||
- sudo /Library/Filesystems/osxfuse.fs/Contents/Resources/load_osxfuse | ||
- sudo sysctl -w vfs.generic.osxfuse.tunables.allow_other=1 | ||
- make all | ||
- os: windows | ||
go: 1.12.x | ||
before_install: | ||
- choco install make | ||
install: | ||
- make deps | ||
script: | ||
- make test | ||
|
||
before_install: | ||
- if [[ "$TRAVIS_OS_NAME" == "windows" ]]; then choco install make; fi | ||
|
||
install: | ||
- make deps | ||
|
||
script: | ||
- if [[ "$TRAVIS_OS_NAME" == "windows" ]]; then make test; else make all; fi |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,160 @@ | ||
// +build linux darwin | ||
|
||
// Copyright 2019 The Prometheus Authors | ||
// 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. | ||
|
||
package fuse | ||
|
||
import ( | ||
"fmt" | ||
"path/filepath" | ||
"runtime" | ||
"syscall" | ||
"time" | ||
|
||
"github.com/hanwen/go-fuse/fuse" | ||
"github.com/hanwen/go-fuse/fuse/nodefs" | ||
"github.com/hanwen/go-fuse/fuse/pathfs" | ||
) | ||
|
||
type loopBackFs struct { | ||
pathfs.FileSystem | ||
} | ||
|
||
// hookFs implements the fuse FileSystem interface and allows injecting hooks for different operations. | ||
// Its main purpose is to provide a way to inject file system errors. | ||
type hookFs struct { | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
original string | ||
mountpoint string | ||
fsName string | ||
loopBackFs | ||
hook Hook | ||
} | ||
|
||
// String returns the string representation of the mounted points. | ||
func (h *hookFs) String() string { | ||
return fmt.Sprintf("HookFs{Original=%s, Mountpoint=%s, FsName=%s, Underlying fs=%s, hook=%s}", | ||
h.original, h.mountpoint, h.fsName, h.loopBackFs.String(), h.hook) | ||
} | ||
|
||
// Rename calls the injected rename hooks if those exist or the underlying loopback fs Rename api. | ||
func (h *hookFs) Rename(oldName string, newName string, context *fuse.Context) fuse.Status { | ||
preHooked, err := h.hook.PreRename(oldName, newName) | ||
if preHooked { | ||
if err != nil { | ||
return fuse.ToStatus(err) | ||
} | ||
} | ||
|
||
status := h.loopBackFs.Rename(oldName, newName, context) | ||
|
||
postHooked, err := h.hook.PostRename(oldName, newName) | ||
if postHooked { | ||
if err != nil { | ||
return fuse.ToStatus(err) | ||
} | ||
} | ||
return status | ||
} | ||
|
||
func (h *hookFs) newServer() (*fuse.Server, error) { | ||
opts := &nodefs.Options{ | ||
NegativeTimeout: time.Second, | ||
AttrTimeout: time.Second, | ||
EntryTimeout: time.Second, | ||
} | ||
pathFsOpts := &pathfs.PathNodeFsOptions{ClientInodes: true} | ||
pathFs := pathfs.NewPathNodeFs(h, pathFsOpts) | ||
conn := nodefs.NewFileSystemConnector(pathFs.Root(), opts) | ||
originalAbs, _ := filepath.Abs(h.original) | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
mOpts := &fuse.MountOptions{ | ||
AllowOther: true, | ||
Name: h.fsName, | ||
FsName: originalAbs, | ||
} | ||
return fuse.NewServer(conn.RawFS(), h.mountpoint, mOpts) | ||
} | ||
|
||
// Server is a fuse server proxy. | ||
type Server struct { | ||
server *fuse.Server | ||
original string | ||
mountpoint string | ||
} | ||
|
||
// NewServer creates a fuse server and attaches it to the given `mountpoint` directory. | ||
// The caller should not forget to close the server to release the mountpoints. | ||
func NewServer(original, mountpoint string, hook Hook) (*Server, error) { | ||
fs := &hookFs{ | ||
original: original, | ||
mountpoint: mountpoint, | ||
fsName: "hookfs", | ||
loopBackFs: loopBackFs{pathfs.NewLoopbackFileSystem(original)}, | ||
hook: hook, | ||
} | ||
|
||
server, err := fs.newServer() | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// Async start fuse server, and it will be stopped when calling `fuse.Unmount()` method. | ||
go func() { | ||
server.Serve() | ||
}() | ||
|
||
return &Server{ | ||
server: server, | ||
mountpoint: mountpoint, | ||
original: original, | ||
}, server.WaitMount() | ||
} | ||
|
||
// Close releases the mountpoints and return false if the unmount fails. | ||
// When the mountpoint has open files it tries to force unmount. | ||
func (s *Server) Close() error { | ||
err := s.server.Unmount() | ||
if err != nil { | ||
return s.forceUnmount() | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// forceUnmount forces to unmount even when there are still open files. | ||
func (s *Server) forceUnmount() (err error) { | ||
delay := time.Duration(0) | ||
for try := 0; try < 5; try++ { | ||
err = syscall.Unmount(s.mountpoint, unmountFlag()) | ||
if err == nil { | ||
break | ||
} | ||
|
||
// Sleep for a bit. This is not pretty, but there is | ||
// no way we can be certain that the kernel thinks all | ||
// open files have already been closed. | ||
delay = 2*delay + 10*time.Millisecond | ||
time.Sleep(delay) | ||
} | ||
|
||
return err | ||
} | ||
|
||
// unmountFlag returns platform dependent force unmount flag. | ||
func unmountFlag() int { | ||
if runtime.GOOS == "darwin" { | ||
return -1 | ||
} | ||
|
||
return 0x1 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
// Copyright 2019 The Prometheus Authors | ||
// 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. | ||
krasi-georgiev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
package fuse | ||
|
||
import "errors" | ||
|
||
// Server implements a mock server to allow skipping tests for windows platform | ||
// which doesn't support fuse. | ||
type Server struct { | ||
} | ||
|
||
// NewServer always returns an error because fuse does not support Windows platform. | ||
func NewServer(original, mountpoint string, hook Hook) (*Server, error) { | ||
return nil, errors.New("fuse not supported under Windows") | ||
} | ||
|
||
// Close is a noop method. | ||
func (s *Server) Close() error { | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry maybe I wasn't clear. We still need to run all but the file system tests for Windows.
For example maybe put all unix OS specific tests in
block_unix_test.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@krasi-georgiev
Currently, go fuse library do not support windows platform, so there is a compile error in windows platform
I am trying to solve it and do you have any suggestions?
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you think the library is loaded when running the tests under windows?
When you have something like
block_unix_test.go
the file should be loaded only on unix OS-esThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should rename all fuse utils files to
fuse_..._unix.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For linux and macos, i add the build flag in golang files
Then if the platform is linux or osx, these go files will be automatic applied.
And for windows platform, the fuse_utils_windows.go will be automatic applied.
The advantage is that other developers can write fuse cases in everywhere and do not need to put fuse cases in a special directory.
From the build flag, i see that there is only linux、darwin、windows、freebsd suffix to support. I am not sure the file which suffix is unix can work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, but looking at the code seems that these tests will fail under windows anyway as they will not use the fuse server mounts so can't inject errors and when the tests expects an error, no error will be returned.
yes inside you can put:
// +build !windows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@krasi-georgiev
They do not fail under windows platform because fuse related cases will automatic skipped in windows platform(Other developer who want to inject file system error do not care it because it is automatic skipped in windows platform). In windows platform, it only run other cases which do not include fuse related cases.
For linux and macosx platform, it will run all cases. They works well.
I think the tests which need to inject filesystem errors only run in linux and macosx platform.
And in windows platform, it only run other cases, the fuse related cases will automatic skipped in windows platform. Because windows platform do not support fuse.
Every thing looks well, do you think it is ok?