-
Notifications
You must be signed in to change notification settings - Fork 385
Is SSHFS supported? #265
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
Comments
I don’t think it was thought about as a support point before. But, I think the use case is intended to be targeting SFTP clients specifically, rather than providing the complete generic VFS system itself, which SSHFS probably depends upon. |
SSHFS wasn't specifically targeted to be supported, but reading the docs it is supposed to be implemented on top of SFTP which would make it just another SFTP client. So I'll start looking into it from that POV. Thanks for the report. |
I think I've figured it out. It is the damn packet ordering again. The packet order code has 2 modes; a command mode where it has 1 worker and processes commands sequentially and a read/write mode where it has a pool of workers for handling file reads and writes faster. The mode starts in the command mode and switches to read/write mode when an Open packet is received (and switched back on the Close packet). In this case the client (sshfs) is sending the Open packet for a file immediately followed by a Stat packet. The Open packet flips it to r/w mode and passes the Open packet to a worker. The Stat packet comes in then and is given to another worker... the Stat then gets checked before the Open runs and finds no file yet. Thinking about the best way to fix. Initial idea is to look into changing the logic around flipping into R/W mode to flip based on seeing the (file) Handle packet packet (the response to the Open packet). But I'm not sure how to do this nicely right off. |
Ugh… yeah. Looks like a fencing issue, where when transitioning between states a fence need be written… but of course this is easier said than done. |
I think I have a good idea for a fix. Instead Open/Close packets switching state, ignore the Open and just send Read/Write packets to the worker pool and everything else to the single worker. Keeping the wg.Wait on the Close to make sure all RW is complete. I'll post a PR with the change once I'm happy with it and come up with a test (if I can). I was thinking about how to test it (how to insert the delay to trigger the race) when I came up with the above. |
I spent much of the day trying to think of a good way to inject the delay to trigger the race but have given up for now. I left in the test with a comment about this and about how I want to add a way to inject delays later. Pull request should be up in a couple. |
PR #266 is up w/ my fix. |
I have done a little test with your issue branch and it seems to work on my end :) Thanks for working on this! |
An Open packet would trigger the use of a worker pool, then the Stat packet would come in go to the pool and return faster than the Open (returning a file-not-found error). This fixes that be eliminating the pool/non-pool state switching. The include test doesn't really exercise it fully as it cannot inject a delay in the right place to trigger the race. I plan on adding a means to inject some logic into the packet handling in the future once I rewrite the old filesystem server code as a request-server backend. Fixes #265
Uh oh!
There was an error while loading. Please reload this page.
It seems that SSHFS is not supported, because mounting the SFTP server with SSHFS works, and most things will actually work, except for file creation, uploading, etc. Is this intended? Or is pkg/sftp supposed to support SSHFS?
The steps I have taken to test this are as follows:
https://github.com/pkg/sftp/tree/master/examples/sftp-server
The files that generate this error will have been created on the server, but the contents will be empty there (0 byte files).
Some files will actually copy, but generally there will always be a large amount that fail. (The randomness seems to me like a timing issue perhaps)
Is this a bug or is SSHFS something the package just doesn't support?
The text was updated successfully, but these errors were encountered: