8000 Is SSHFS supported? · Issue #265 · pkg/sftp · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Closed
vansante opened this issue Aug 14, 2018 · 8 comments · Fixed by #266
Closed

Is SSHFS supported? #265

vansante opened this issue Aug 14, 2018 · 8 comments · Fixed by #266
Assignees
Labels

Comments

@vansante
Copy link
Contributor
vansante commented Aug 14, 2018

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:

  • Start a test SFTP server with the example at https://github.com/pkg/sftp/tree/master/examples/sftp-server
  • Mount it with SSHFS like so:
paul@paul-VirtualBox:~$ sshfs -p 2022 "testuser"@192.168.56.1:/ /home/paul/sshfs
testuser@192.168.56.1's password: 
  • And then attempt to copy a few pictures like so:
paul@paul-VirtualBox:~$ cp plaatjes/*.png sshfs/mnt/d/sftp/
cp: cannot create regular file 'sshfs/mnt/d/sftp/5136c62b07be1.png': No such file or directory
cp: cannot create regular file 'sshfs/mnt/d/sftp/Azure-DNS.png': No such file or directory
cp: cannot create regular file 'sshfs/mnt/d/sftp/moreinfogone.png': No such file or directory

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?

@puellanivis
Copy link
Collaborator
puellanivis commented Aug 14, 2018

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.

@eikenb
Copy link
Member
eikenb commented Aug 17, 2018

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.

@eikenb eikenb self-assigned this Aug 17, 2018
@eikenb eikenb added the bug label Aug 17, 2018
@eikenb
Copy link
Member
eikenb commented Aug 17, 2018

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.
When that happens (not every time due to it being a race) you get the file not found error (from the Stat) w/ the file created (from the Open) but with no content (as the client stops the write due to the error).

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.

@puellanivis
Copy link
Collaborator

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.

@eikenb
Copy link
Member
eikenb commented Aug 19, 2018

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.

@eikenb
Copy link
Member
eikenb commented Aug 20, 2018

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.

@eikenb
Copy link
Member
eikenb commented Aug 20, 2018

PR #266 is up w/ my fix.

@vansante
Copy link
Contributor Author

I have done a little test with your issue branch and it seems to work on my end :)

Thanks for working on this!

7B21

eikenb added a commit that referenced this issue Aug 22, 2018
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
0