Skip to content
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

lock() & flock() methods not implemented #20

Open
julian-hj opened this issue Apr 19, 2017 · 8 comments
Open

lock() & flock() methods not implemented #20

julian-hj opened this issue Apr 19, 2017 · 8 comments

Comments

@julian-hj
Copy link
Contributor

One of our users pointed out to us that flock applied on fuse-nfs mounts doesn't enforce locking between different clients. After confirming that

  1. It's true and
  2. kernel NFS mounts do enforce flock locks between clients
    We took a look at the fuse-nfs implementation and noticed that fuse-nfs just treats lock() and flock() as no-ops.

So, we are wondering if you left the locking implementation out because you didn't get to it, or if you left it out because NFS locking is flaky and inconsistent between servers and could cause stuff to hang or fail or is otherwise a bad idea.

If it is the former, are you open to a PR to include a locking implementation? And if so do you have an opinion about where the synchronous implementation of rpc_nlm4_lock_async() should live? (presumably either in libnfs or in fuse-nfs)

@sahlberg
Copy link
Owner

sahlberg commented Apr 19, 2017 via email

@julian-hj
Copy link
Contributor Author

That all makes sense. Thanks very much for the quick and thoughtful feedback. We will have a think about how we want to proceed.

@sahlberg
Copy link
Owner

sahlberg commented Apr 20, 2017 via email

@julian-hj
Copy link
Contributor Author

Interesting....so, how does the server know what "this client" means? In our use case, we may have numerous fuse-nfs processes starting and stopping on the same host, potentially mounting the same nfs share with different options.

Also, I noticed in the packet trace that libnfs just sends "libnfs" as the client machine name in the credentials block. Could that result in the nfs server getting confused when different fuse processes go up and down?

Thanks!

@sahlberg
Copy link
Owner

Client that NSM is using should be the string that you provide in the nlm lock request as
NLM4_LOCKargs.lock.caller_name

The credentials block is part of the ONC-RPC header and should not affect (or even be visible) to the NLM layer, but it may be a good idea to change this from "libnfs" to something more descriptive.
If for no other reason making it easier to analyze and filter on network traces.

As few things really look at or depend on the cred->host field, having "libnfs" as the default might be ok-ish. Possibly this should be changed to something more descriptive such as libnfs- or something.
In your case you probably want to have something more specific to your use case and to easily distinguish between different instances of the fuse module. You can actually override and replace cred->host with whatever you want using the nfs_set_auth and libnfs_authunix_create calls.
Maybe fuse-nfs.c should use these functions and set the credentials explicitely to "fuse-nfs-" or something ?
Even better might be to be able to set the hostname string from the command line via the nfs url.
The NFS url already allow you to set uid and gid. Maybe just add an argument to also set the hostname.

@sahlberg
Copy link
Owner

sahlberg commented Aug 9, 2017

FYI.
NFSv4 support in libnfs is coming along and should be reasonably complete and to the state where
one can use it for for example fuse-nfs. This is likely going to be ready towards the end of the year.

At that stage, when we can run libnfs in a NFSv4 mode, it should be fairly straightforward to do the plumbing to add locking as we no longer need to rely on NLM or NSM.

@julian-hj
Copy link
Contributor Author

That's excellent to hear! NFSv4 and file locking have both been on our roadmap for a while. If we can add support for v4, then I think support for locking in v3 might be quite a bit less critical.

@sahlberg
Copy link
Owner

Libnfs has support for both lockf() and fcntl() locking now but only for nfs v4.

With this, it should be fairly straightforward to add locking support to fuse-nfs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants