8000 sshfs_mount: Recover mounts when sshfs exits for unexpected reasons by townsend2010 · Pull Request #937 · canonical/multipass · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

sshfs_mount: Recover mounts when sshfs exits for unexpected reasons #937

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 7, 2019
Merged

sshfs_mount: Recover mounts when sshfs exits for unexpected reasons #937

merged 2 commits into from
Aug 7, 2019

Conversation

townsend2010
Copy link
Contributor

Fixes #509

@Saviq
Copy link
Collaborator
Saviq commented Jul 31, 2019

Does that relate to #936 do you think?

@townsend2010 townsend2010 force-pushed the resilient-sshfs-failures branch from 633c4a1 to b6f384f Compare July 31, 2019 17:38
@codecov
Copy link
codecov bot commented Jul 31, 2019 •
edited
Loading

Codecov Report

Merging #937 into master will increase coverage by 0.03%.
The diff coverage is 86.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #937      +/-   ##
==========================================
+ Coverage   69.29%   69.33%   +0.03%     
==========================================
  Files         193      192       -1     
  Lines        6716     6734      +18     
==========================================
+ Hits         4654     4669      +15     
- Misses       2062     2065       +3
Impacted Files Coverage Δ
src/ssh/ssh_process.cpp 100% <100%> (ø) ⬆️
src/sshfs_mount/sshfs_mount.cpp 100% <100%> (ø) ⬆️
src/sshfs_mount/sftp_server.cpp 87.5% <84.37%> (-0.28%) ⬇️
include/multipass/ssh/ssh_process.h 100% <0%> (+100%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d21c22...b6f384f. Read the comment docs.

@townsend2010
Copy link
Contributor Author

@Saviq

Does that relate to #936 do you think?

It might...

gerboland
gerboland reviewed Aug 6, 2019
View reviewed changes
Copy link
Contributor
@gerboland gerboland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of polling the existence of the sshfs_server process in the VM on each sshfs event, did you consider taking the return code from the handle_* functions in SftpServer::process_message and on failure, try to determine if server dead, resurrect and retry?

libssh has a SSH_FX_CONECTION_LOST error code that can be returned by sftp_get_error() too.

I'm just curious why you took this approach.

@townsend2010
Copy link
Contributor Author

@gerboland,

I'm not sure where you think I'm polling for the sshfs process inside the VM on each sshfs event. I only check for the sshfs process if sftp_get_client_message returns a NULL client message, indicating that something bad happened to the connection.

@gerboland
Copy link
Contributor

@gerboland,

I'm not sure where you think I'm polling for the sshfs process inside the VM on each sshfs event. I only check for the sshfs process if sftp_get_client_message returns a NULL client message, indicating that something bad happened to the connection.

Ah ok, I hadn't noticed that null check was so important.

@townsend2010
Copy link
Contributor Author

Ah ok, I hadn't noticed that null check was so important.

Ok, good, I hoped I wasn't missing something 😄

gerboland
gerboland approved these changes Aug 7, 2019
View reviewed changes
Copy link
Contributor
@gerboland gerboland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep it appears to do the job. I could kill sshfs_server in the VM and this restarted it ok.

I do still think we could do things to make the sshfs_server in the VM more robust, perhaps set the oom killer score to make it less likely to be killed in a low memory situation. And might it be crashing?

Still, this is worth having
bors r+

bors bot added a commit that referenced this pull request Aug 7, 2019
@bors
Merge #937
b618ecf 
937:  sshfs_mount: Recover mounts when sshfs exits for unexpected reasons  r=gerboland a=townsend2010

Fixes #509 

Co-authored-by: Chris Townsend <christopher.townsend@canonical.com>
@townsend2010
Copy link
Contributor Author

@gerboland,

I'm not sure of the exact reasons why sshfs goes away in the instance, but I suspect it's etiher crashing or erroring out.

The archive version of libfuse and sshfs are quite old, so I have another project at https://github.com/CanonicalLtd/multipass-sshfs that builds a snap package using upstream libfuse and sshfs. This works quite well and should resolve many crashing/error issues. I'm just waiting for the bare base to be promoted to stable and then support for that to be merged into snapcraft. That way, the whole core18 snap will not need to be downloaded and installed when installing the multipass-sshfs snap.

@bors
Copy link
Contributor
bors bot commented Aug 7, 2019

Build succeeded

@bors bors bot merged commit b6f384f into master Aug 7, 2019
@bors bors bot deleted the resilient-sshfs-failures branch August 7, 2019 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gerboland gerboland gerboland approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[mount] multipassd fails to notice sshfs going away inside the instance
3 participants
@townsend2010 @Saviq @gerboland
0