winfsp / cgofuse

Cross-platform FUSE library for Go - Works on Windows, macOS, Linux, FreeBSD, NetBSD, OpenBSD
https://winfsp.dev
MIT License
527 stars 84 forks source link

Unknown flags being delivered to Create method #19

Closed ncw closed 6 years ago

ncw commented 6 years ago

I'm trying to track down a problem with my new mount code and rclone. I've changed the way the open flags are handled to (optionally) enable on disk caching which will make the file system a whole lot more compatible.

What seems to be happening is that when cgofuse calls the Create method, it calls it with some flags that rclone doesn't understand - in particular the 0x100 flag doesn't seem to match anything.

The flags which were supplied by the go program writing to the mount were from ioutil.WriteFile so should have been os.O_WRONLY|os.O_CREATE|os.O_TRUNC. So it is as if O_TRUNC is being delivered as 0x100.

Can you confirm? Am I making a mistake assuming that the open flags are the same as the go flags?

I haven't managed to find definitions for these flags anywhere in your code so I'm not sure where the constants come from!

If you want to play with the code it is on the vfs-rw branch and I've been running in cmd/cmount the command go test -v -tags cmount.

Any help much appreciated - thanks!

2017/11/15 01:10:51 DEBUG : /a file: Create: flags=0x102, mode=0700
2017/11/15 01:10:51 DEBUG : a file: Open: flags=O_RDWR|0x100
2017/11/15 01:10:51 ERROR : a file: Can't open for read and write without cache
2017/11/15 01:10:51 DEBUG : a file: >Open: fd=<nil>, err=permission denied
2017/11/15 01:10:51 DEBUG : /a file: >Create: errc=-1, fh=0xFFFFFFFFFFFFFFFF
billziss-gh commented 6 years ago

I think the relevant code is in the WinFsp-FUSE layer [link]:

    if ('C' == f->env->environment) /* Cygwin */
        fi.flags = 0x0200 | 2 /*O_CREAT|O_RDWR*/;
    else
        fi.flags = 0x0100 | 2 /*O_CREAT|O_RDWR*/;

WinFsp-FUSE supports both the MSVC and Cygwin environments. In MSVC O_CREAT|O_RDWR translates to 0x0102. In Cygwin it translates to 0x0202. Cgofuse uses the MSVC environment, which is the reason you see 0x0102.

From the MSVC <fcntl.h>:

#define _O_RDONLY      0x0000  // open for reading only
#define _O_WRONLY      0x0001  // open for writing only
#define _O_RDWR        0x0002  // open for reading and writing
#define _O_CREAT       0x0100  // create and open file

The go constant syscall.O_CREAT has the wrong value under Windows. IMO it should be the same as MSVC, which is the primary development environment on Windows. From godoc under Windows:

const (
    // Invented values to support what package os expects.
    O_RDONLY   = 0x00000
    O_WRONLY   = 0x00001
    O_RDWR     = 0x00002
    O_CREAT    = 0x00040
    O_EXCL     = 0x00080
    O_NOCTTY   = 0x00100
    O_TRUNC    = 0x00200
    O_NONBLOCK = 0x00800
    O_APPEND   = 0x00400
    O_SYNC     = 0x01000
    O_ASYNC    = 0x02000
    O_CLOEXEC  = 0x80000
)

From the perspective of a cgofuse client, this is a bug in cgofuse. I will have it fixed (and also examine Open while we are it).

ncw commented 6 years ago

That is very interesting - thanks for working that out.

So that mystery 0x100 should be O_CREAT...

From the perspective of that code the O_TRUNC has gone missing too. I read that (linux) FUSE doesn't normally supply O_TRUNC to Open - it opens the file and then truncates it.

From rclone's perspective I'd much rather know about the truncation when I open the file (so I don't needlessly fetch it just to truncate it), so I've been using -o atomic_o_trunc to enforce this. I guess that WinFSP doesn't support this?

If not I can jiggle the code around to make a pending truncate without too many problems to avoid the fetch then truncate dance.

billziss-gh commented 6 years ago

You are correct, WinFsp does not support atomic truncation. You will always get Open and then Truncate.

The reasons for this are complicated and it has to do both with WinFsp and Windows design. The primary reason is that after the user mode file system Open's a file, the FSD (File System Driver) may still choose to Close the file instead of handing it to the originating process for a number of reasons (e.g. file share check failure). So file truncation must be a two step process because it is destructive: imagine a case where the user mode file system truncates a file during Open, but then the FSD fails the Open; the originating process sees a failed Open(O_TRUNC) but the file contents are gone anyway!

The native WinFsp API includes a hint during Open that the file is going to be truncated (although the file system is not supposed to perform the truncation at that point for the reasons explained above). Unfortunately there is no provision in the FUSE API to pass the truncation hint: you must either pass O_TRUNC in which case the file will be truncated or not. There is no O_MAYBE_WILL_TRUNC.

ncw commented 6 years ago

Thanks for clearing that up - I'll go back to plan A and remove the -o atomic_o_trunc and make a pending truncate.

billziss-gh commented 6 years ago

BTW, as I am looking into resolving this I note that cgofuse already defines its own O_* constants:

https://github.com/billziss-gh/cgofuse/blob/master/fuse/fsop.go#L118-L128 https://github.com/billziss-gh/cgofuse/blob/master/fuse/fsop.go#L232-L241

It looks to me as if the intent was that constants such as fuse.O_CREAT rather than syscall.O_CREAT should be used by a file system (because they may have different values). What do you think?

ncw commented 6 years ago

BTW, as I am looking into resolving this I note that cgofuse already defines its own O_* constants:

Ah ha! So if I use the fuse.O_CREAT then would all be well?

I'd need to write a translator for fuse constants -> go constants which would be quite straight forward.

I note that your passthrough code makes the assumption that you can pass Open flags straight to syscall.Open. I don't think this code runs on Windows though does it?

billziss-gh commented 6 years ago

BTW, as I am looking into resolving this I note that cgofuse already defines its own O_* constants: Ah ha! So if I use the fuse.O_CREAT then would all be well?

I'd need to write a translator for fuse constants -> go constants which would be quite straight forward.

Yes. That would do it.

In retrospect we should have probably used the syscall constants instead of inventing our own. [Actually we did discuss this in #3.]

I note that your passthrough code makes the assumption that you can pass Open flags straight to syscall.Open. I don't think this code runs on Windows though does it?

You are right. I now wish I had spent the time to get it to run on Windows, because it would have probably caught this problem.

billziss-gh commented 6 years ago

Given our discussion in #3 and the fact the cgofuse has its ownfuse.O_* constants, I am inclined to consider this as "works as intended" and not a "bug". Would you agree?

ncw commented 6 years ago

Yes that is fine.

It should probably say in the Open docs that the flags a combination of the fuse.O_* constants. Or maybe here. Or perhaps best of all a comment above the definition that these flags should be passed into Open() etc would look nice in the docs.

billziss-gh commented 6 years ago

Agreed. I will have this added some time later today.

billziss-gh commented 6 years ago

Commit 487e2baa5611bab252a906d7f9b869f944607305 improves the documentation.