Add open flags to OpenFileOp #63
Conversation
fuseops/ops.go
Outdated
| // advance, for example, because contents are generated on the fly. | ||
| UseDirectIO bool | ||
|
|
||
| // Open flags. See os.OpenFile |
There was a problem hiding this comment.
| // Open flags. See os.OpenFile | |
| // User-provided flags to open(2). See http://go/godoc/os/#OpenFile. |
samples/memfs/memfs.go
Outdated
| if !inode.isFile() { | ||
| panic("Found non-file.") | ||
| } | ||
| // What perms do we need? |
There was a problem hiding this comment.
I had trouble understanding what you were doing here at first. Could you make
the code read something like:
// Check that the open mode is legal according to the file
// permissions.
var permissionBits int
switch (...) {
...
}
if (...) {
err = EACCESS
return
}
There was a problem hiding this comment.
we should not be checking attributes, since they can change behind our back.
samples/memfs/memfs.go
Outdated
| case unix.O_RDWR: | ||
| perm = 0600 | ||
| // This is legal, at least on linux | ||
| case unix.O_ACCMODE: |
There was a problem hiding this comment.
This case is weird, and I think it's just an artifact of using a switch
statement, right? Why not something like:
if flags & unix.O_RDONLY && !(inode.attrs.Mode()&unix.O_RDONLY) {
return EACCESS
}
// Ditto with O_WRONLY and O_RDWR
...
There was a problem hiding this comment.
oh yeah, and that case is very deliberate: I tested it on linux before I added it. Again, I broke it out b/c it's not obvious.
There was a problem hiding this comment.
oops, I was thinking about the fs I am writing, not memfs, sorry. nvm.
There was a problem hiding this comment.
but if you want it to work your proposed way that's fine too.
There was a problem hiding this comment.
Sorry, I'm totally lost at this point. All I want you to do in memfs_test is somehow confirm that the open(2) flags correctly come through to the file system. Maybe you are attempting to do that here, but I don't understand how. Could you please improve the commenting, here or in the test, to explain how you're confirming this?
The open flag indicates the type of access we need. Add a test to the memfs server to make sure an open is blocked if the flags and permissions don't match. Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
rminnich
force-pushed
the
addflagtoopencreate
branch
from
f13b68c to
c004db6
Compare
2 participants
The open flag indicates the type of access we need.
Add a test to the memfs server to make sure an open
is blocked if the flags and permissions don't match.
Signed-off-by: Ronald G. Minnich rminnich@gmail.com