-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Could not edit Microsoft Office files from mountpoint #54
Comments
@willscott
|
I wonder if #72 was the underlying issue behind this can you try again with the current version? |
@willscott |
Does anyone have a clear idea what is happening here? This is affecting rclone users in rclone/rclone#7973 in exactly the same way. Putting Is this a problem because the nfs server doesn't implement locking? If so could we implement a skeleton locking system? I notice that memfs has a stubbed out Lock and Unlock which don't appear to be called anywhere Lines 335 to 343 in 91bc389
|
@ncw - the reason memfs does not implement lock / unlock is because the accompanying 'Network Lock Manager (NLM) |
Thanks @willscott What do you think the best way forward with this issue is? Rclone can't support locking so how can we tell the clients not to try? |
|
What is the best way of doing this? The logging in the library doesn't seem to do a full rpc in/out trace or at least I haven't figured out how to make it do that if it can. I'm not 100% convinced that this problem is to do with locking so tracing sound like a great idea
It doesn't look too complicated... |
There's a log trace on each request: https://github.com/willscott/go-nfs/blob/master/conn.go#L62 |
There's a major issue with NLM in this context - which is that from what i can tell mount'ing of NFS (the clients in Mac and Linux at least) don't allow specification of an explicit NLM port. As such, the server needs to be running in the standard privileged context where portmap is running and used to discover the port the lock manager is running on. I don't see a way to allow interaction with the locking sideband protocol with the same explicit mount-time behavior as with mountd and nfs itself |
Here is a trace of those log statements I extracted these from the rclone log which gives a bit more context I did not notice this line before
I wonder if that is the problem? Can we just ignore the exclusive mode? That comes from here Lines 36 to 44 in 91bc389
|
I'm attempting an experiment using this patch. I've made an rclone binary with this patch and sent it to the user with the mac (not me!). diff --git a/nfs_oncreate.go b/nfs_oncreate.go
index 63d7901..262eb8f 100644
--- a/nfs_oncreate.go
+++ b/nfs_oncreate.go
@@ -39,9 +39,8 @@ func onCreate(ctx context.Context, w *response, userHandle Handler) error {
if err := xdr.Read(w.req.Body, &verf); err != nil {
return &NFSStatusError{NFSStatusInval, err}
}
- Log.Errorf("failing create to indicate lack of support for 'exclusive' mode.")
- // TODO: support 'exclusive' mode.
- return &NFSStatusError{NFSStatusNotSupp, os.ErrPermission}
+ // We ignore the createverf3 which is the key to the lock
+ Log.Errorf("ignoring 'exclusive' mode on create file.")
} else {
// invalid
return &NFSStatusError{NFSStatusNotSupp, os.ErrInvalid}
@@ -88,9 +87,11 @@ func onCreate(ctx context.Context, w *response, userHandle Handler) error {
fp := userHandle.ToHandle(fs, newFile)
changer := userHandle.Change(fs)
- if err := attrs.Apply(changer, fs, newFilePath); err != nil {
- Log.Errorf("Error applying attributes: %v\n", err)
- return &NFSStatusError{NFSStatusIO, err}
+ if attrs != nil {
+ if err := attrs.Apply(changer, fs, newFilePath); err != nil {
+ Log.Errorf("Error applying attributes: %v\n", err)
+ return &NFSStatusError{NFSStatusIO, err}
+ }
}
writer := bytes.NewBuffer([]byte{}) Which I think should work (cross fingers!) There was one thing I didn't understand in that code though - the code appears to assume that the file already exists (ie has a handle and can have Stat called on it without an error) whereas the RFC doesn't say anything about the file existing or not. I've probably mis-understood the code or the RFC as I'm not very familiar with either but I just thought I'd mention it. |
|
@ncw perhaps also try with my branch at https://github.com/willscott/go-nfs/tree/feat/exclusive-create and see if it makes a difference |
Alas this does not compile on Linux
Annoyingly the names of the Atimespec and Ctimespec change on a per OS basis
As for rclone, this locking scheme is unlikely to work as-is as backends don't support atime/ctime. Something based on mtime alone would work, but bear in mind that the precision might be truncated depending on the backend. Probably my preference would be to have a way to tell the library just to treat exclusive creates the same as non-exclusive ones. |
I think the approach i have should work in a degraded way - if there isn't an exact match, a temporary file is made to save and recover the verifier. In cases where atime/ctime aren't supported, or things like dos with lower time precision, the intention is to degrade. Thanks for flagging the build issue. I need some more os-specific casing |
Sounds good :-) As an aside while we are talking about how annoying I'd prefer you used a struct you defined specifically (probably your file.Fileinfo struct). You could make this change in a backwards compatible way by making file.GetInfo look for a |
I think that's what that method does, right? if the |
It does, you are quite right 😊 It should probably say that here - I will send a PR to fix. Lines 89 to 91 in 91bc389
|
Does the addition of exclusive create change office behavior? |
Sorry, didn't see you updated your branch. Will test tomorrow |
Just a FYI - you can't The branch compiles much better now, but it didn't compile for these GOOS/GOARCH pairs
With a selection of errors like
You can check out the full selection in rclone's build logs 1 2 It built on darwin though so I've sent that off to my testers I went through this pain in rclone so if you want to see what I came up with I needed 3 build files for bsd, linux and other unix. |
I've had a report back from the user doing the testing. Alas, it did not work. Here is the nfs logs from go-nfs set to Trace level debug fix-7973-nfs-server-ids-ff8ce1bef.nfs.log Which were grepped out of the rclone logs here which have more context as to what the backend is doing fix-7973-nfs-server-ids-ff8ce1bef.log I annotated some of the rclone log with the nfs transactions below. I got bored of annotating the stat calls to the root and I stopped after the create. The description of what the user did
There is just one file available. I think if we we could figure out why word thinks it is read only then we could probably fix the problem. Your new code is visible setting atime/mtime below, but the OS overwrites them immediately so I don't think this strategy is going to work. I think just ignoring the exclusive lock is probably the best strategy. Click to see annotated transactionsList root
Stat
Open
stat the root loads of times
Now stat random files which appear to have nothing to do with what is going on
Some more root stats for good measure
stat
and then the root
List the root - still just the one file
stat the root some more
Stat
And now the root some more
Stat
List the directory again, still only
By this point Word has concluded (somehow) that Check the file
Create the file
I think this is your new code attempting to set the atime and mtime - note odd 2042 date!
Does the lock file exist?
I think this is word or macOS setting the modification times (not sure)
more stats
more stats
Create the lock file - apparently successful
something loves stat calls!
stat a lock file
Write to the
List the root (root directory is "" in rclone world)
stat
stat the file
Rename
stat
check
stat
stat
Remove
|
|
Could this be something about 'word defaults to open files on remote servers in a protected view' https://answers.microsoft.com/en-us/msoffice/forum/all/word-for-mac-opens-docs-on-a-server-read-only-how/410df015-4b27-414e-9e0f-2a4ac233f580 ? |
We double checked the permissions of things and they all look OK. I think the clue here is that
I'm pretty sure these directories are created by the NFS client to implement locallocks. the So it is almost certainly the lack of some kind of locking which is causing the problem I think. |
seems like the main issue in being able to do something other than |
looking at the kernel handling of |
I wonder if another potential culprit could be that nfsv3 doesn't support extended file attributes. If the software is trying to make use of extended attributes, it will find that the filesystem doesn't support them. Solutions to both of these issues (userland locking protocol support, and extended attributes) seem to point to nfs v4 protocol rather than this library which offers v3. |
Everything works fine except editing Microsoft Office files(e.g. Word, Excel, PowerPoint) from mountpoint.
It can be edited if the file open directly from source folder, but when opening from mountpoint the Microsoft Office shows "readonly".
./example/osnfs
mount
The text was updated successfully, but these errors were encountered: