On Fri, 2019-02-08 at 11:03 -0500, Jeff Layton wrote: > On Fri, 2019-02-08 at 16:45 +0200, Amir Goldstein wrote: > > On Fri, Feb 8, 2019 at 3:10 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > > On Fri, 2019-02-08 at 13:20 +0200, Amir Goldstein wrote: > > > > Hi Bruce, > > > > > > > > I have been following you discussion with Volker Lendecke > > > > on the samba technical mailing list [1] and have had discussed > > > > this issue with Volker myself as well. > > > > > > > > I decided to start this new thread to bring some kernel developers > > > > in the loop and to propose an idea that takes a somewhat > > > > different approach to the "interop" approaches I have seen > > > > so far. "interop" in this context often means consistency of file > > > > lock states between samba and nfs server, but I am referring > > > > to the stronger sense of interop with local filesystem on the server. > > > > > > > > You pointed to Pavel Shilovsky's O_DENY* patches [2] as a possible > > > > solution to interop of NFS Share Reservation and SMB Share Mode > > > > with local filesystems. > > > > Some of the complaints on this approach were (rightfully) concerned > > > > about DoS and the prospect of plaguing Linux with Windows server > > > > "files left open" issues. > > > > > > > > My idea comes from the observation that Windows server > > > > administrators can release locked files that were left open by clients. > > > > I suppose that an NFS server admin can do the same? > > > > > > The Linux kernel has no mechanism for this (aside from sending a SIGKILL > > > to lockd, which makes it drop all locks). Solaris did have a tool for > > > this at one point (and probably still does). > > > > > > It's a little less of a problem now than it used to be with NFS, given > > > the move to NFSv4 (which has lease-based locking). If you have > > > misbehaving clients, you just kick them out and their locks eventually > > > go away. v3 locks can stick around in perpetuity however, so people have > > > long wanted such a tool on Linux as well. > > > > > > > In a nut shell, I think my proposal is that samba will do something > > similar and request leases from the kernel instead of trying to > > enforce real mandatory locks. > > > > > > That realization makes "share access" locks (a.k.a. MAND_LOCK) > > > > not so very different from oplocks (leases/delegations). > > > > As long as samba and nfsd cooperate nicely with MAND_LOCK > > > > semantics, we don't really have to force local filesystems > > > > to obay MAND_LOCK semantics. If the file servers take leases > > > > on local filesystems, they will not get exclusive write access for > > > > files already open for write on local filesytem and same for read. > > > > > > > > > > I think this last statement isn't correct (if I'm parsing it correctly). > > > If a file is already open for write, then you just don't get a lease > > > when you try to request one. Ditto for write leases if it's already open > > > for read. > > > > > > > I think you miss read what I miss wrote ;-) > > As the title of this thread states, I am talking about the first case > > of acquiring an exclusive or read shared access to file at open time. > > It may be the fact that samba currently calls flock(LOCK_MAND) > > that is the source for confusion. > > > > Open failure is the expected behavior if file is already open for > > write (or read) on local filesystem, so my suggestion is: > > - Server opens the file and request a lease based of desired share mode > > - If file server got the lease, client gets the file handle > > - Otherwise, client gets an open failure > > > > On local file access on the server that violates the share mode, > > > > the file server acts as a grumpy washed out administrator that > > > > automatically grants any lock revoke ticket after timeout. > > > > > > > > > > Devil's advocate: > > > > > > Is this situation any better than just teaching the NFS/SMB servers to > > > track these locks out of band? Both samba and most NFS servers respect > > > share/deny mode locks, but only internally -- they aren't aware of the > > > others'. We could (in principle) come up with a mechanism to track these > > > that doesn't involve plumbing them into the kernel. > > > > > > > That would be a prerequisite to my suggested solution, as I wrote: > > "As long as samba and nfsd cooperate nicely with LOCK_MAND..." > > That means the two file servers cooperate on the share mode locks > > and try to figure out if there are outstanding leases before opening > > a file that will break those leases. > > > > > That said, coherent locking is best done in the kernel, IMO... > > > > > > > Indeed... > > > > > > This model may not fit use cases where "real" interop with > > > > local filesystem is needed, but compared to the existing > > > > solution (no interop at all) it is quite an improvement. > > > > > > > > Furthermore, short of SMB DENY_DELETE, we may not even > > > > need to change any kernel APIs. > > > > The addition of O_DENY* open flags can make programming > > > > easier, but taking a lease on an open file is still safe enough > > > > to implement share reservation (no?). > > > > > > > > Satisfying DENY_DELETE could be more tricky, but perhaps > > > > the existing SILLYRENAME interface of==between knfsd and vfs > > > > could be somehow utilized for this purpose? > > > > > > > > I though of bringing this up as a TOPIC for LSF/MM, but wanted > > > > to consult with you first. I am sure that you or Jeff can do a better > > > > job than me in enumerating the "interop" file lock issues that > > > > could be discussed in filesystems track forum. > > > > > > > > Thoughts? Explanation why this idea is idiotic? > > > > > > I think it's not a single idea. There are really two different aspects > > > to this given that we're really talking about two different types of > > > locks in SMB. I think you have to consider solving these problems > > > separately: > > > > > > 1) the ability to set a (typically whole-file) share/deny lock > > > atomically when you open a file. This is necessary for coherent > > > share/deny lock semantics. Note that these are only enforced open() > > > time. > > > > > > 2) mandatory locking (forbidding reads and writes on a byte range when > > > there is a conflicting lock set). > > > > > > > I was only trying to address the first problem (small steps...). > > > > > The first could (probably) be solved with something like what Pavel > > > proposed a few years ago...or maybe we just wire up O_EXLOCK and > > > O_SHLOCK: > > > > > > https://www.gnu.org/software/libc/manual/html_node/Open_002dtime-Flags.html > > > > > > > Nice. I wasn't aware of those BSD flags. > > > > Share/deny open semantics are pretty similar across NFS and SMB (by > design, really). If you intend to solve that use-case, what you really > want is whole-file, shared/exclusive locks that are set atomically with > the open call. O_EXLOCK and O_SHLOCK seem like a reasonable fit there. > > Then you could have SMB and NFS servers set these flags when opening > files, and deal with the occasional denial at open time. Other > applications won't be aware of them of course, but that's probably fine > for most use-cases where you want this sort of protocol interop. > > DENY_DELETE is a bit harder to deal with however, but that's probably > something that could be addressed separately. > > > > This seems like a fine idea (in principle) but it needs someone to drive > > > the work forward. You'll also likely be consuming a couple of O_* flags, > > > which could be tough sell (unless you come up with another way to do > > > it). > > > > > > > Once I know the obstacles to watch out from, I can drive this work. > > Thing is, I am not convinced myself that any new O_ flags are needed. > > > > How about this (for samba, knfsd is simpler): > > - pfd = open(filename, O_PATH) > > - flock(pfd, LOCK_MAND) (for file servers interop) > > - vfs checks no conflicting LOCK_MAND locks (like patch you once posted) > > - open(filename, O_RDWR) (and verify st_ino like samba does) > > - Request lease (for local fs interop) > > - check_conflicting_open() is changed to use inode_is_open_for_read()gi > > - we already have i_readcount, just need to remove ifdef CONFIG_IMA > > - On lease break (from local fs), break client oplocks and invalidate > > file handle on server > > > > Now that I look at the handling of flock LOCK_MAND, I'm not sure how > it's supposed to work. In particular, flock_locks_conflict basically > says that a LOCK_MAND lock can never conflict with anything. I'm not > sure what good that does. > > The flock manpage does not document LOCK_MAND. It's in /usr/include/asm- > generic/fcntl.h on my machine, but it looks like it just got taken right > out of the kernel headers long ago. > > I think we need to have a hard look at what this flag is doing today > (seems like not much). What are samba's expectations with that flag? > Yeah, in fact, I rolled this program and ran it in two different shells on the same machine against the same file, and they both acquired a lock: ---------------------------[snip]------------------------------ #define _GNU_SOURCE #include <stdio.h> #include <sys/types.h> #include <sys/stat.h> #include <sys/file.h> #include <fcntl.h> int main(int argc, char **argv) { int fd, ret; fd = open(argv[1], O_RDWR|O_CREAT, 0644); if (fd < 0) perror("open"); ret = flock(fd, LOCK_EX|LOCK_MAND); if (ret) perror("flock"); printf("Lock acquired"); getchar(); return 0; } ---------------------------[snip]------------------------------ I move that LOCK_MAND be nuked from orbit...or someone step forward to propose reasonable semantics for it. :) -- Jeffrey Layton <jlayton@xxxxxxxxx>
