From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neil Brown Subject: Re: NFS4 crack Date: Wed, 21 Sep 2005 04:37:36 +1000 Message-ID: <17200.22256.314596.486248@cse.unsw.edu.au> References: <20050918102100.GA23463@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: akpm@osdl.org, andros@citi.umich.edu, bfields@citi.umich.edu, linux-fsdevel@vger.kernel.org, Olaf Kirch Return-path: Received: from cantor2.suse.de ([195.135.220.15]:22951 "EHLO mx2.suse.de") by vger.kernel.org with ESMTP id S1750741AbVIUHay (ORCPT ); Wed, 21 Sep 2005 03:30:54 -0400 To: Christoph Hellwig In-Reply-To: message from Christoph Hellwig on Sunday September 18 Sender: linux-fsdevel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Sunday September 18, hch@lst.de wrote: > I've recently turned on NFS4 server support accidentally, just to get > error messages like: > > "NFSD: recovery directory /var/lib/nfs/v4recovery doesn't exist" > > To my horror I found out that this comes from kernel code, which messes > with a hardcoded directory, completelyu ingoring any namespace or other > uses issues. The fs handling in fs/nfs/nfs4recovery.c is rather broken > in addition. > > All this comes from "[PATCH] knfsd: nfsd4: initialize recovery directory", > commit ID 190e4fbf96037e5e526ba3210f2bcc2a3b6fe964. I confess that I am having trouble finding a convincing basis for your position, which is why I allowed the patch through in the first place (despite not particularly liking it). My problem is: where do you draw the line? It should be noted first that nfsd is unlike most (all?) other kernel code. It is an application that is running in-kernel. It is a consumer of kernel services, and provides no (significant) services to user-space, or to other parts of the kernel. Now, this in-kernel-application needs to store stable application-specific data somewhere. May it: 1/ open a directory and create files in it and write to them 2/ open a directory and create files provided that the name of the directory is given by userspace 3/ create files in a directory that was created by userspace and given to the knfsd application as a filedescriptor 4/ write data to files which were created and opened by used-space based on filenames provided by knfsd (hostnames or equivalents in this case). 5/ pass the data to userspace and let it worry completely. 6/ sorry, you cannot have application-specific state. I'm sure you will see a progression here. I ask again: "where do you draw the line?" You seem to rule out 1, and probably 2, and possibly 3 based on other comments in the thread. It cannot set a rational place to draw the line other than before-1 or after-4. i.e. if you allow 4, you may as well allow 1 too. If you have give a clear argument for some particular place to draw the line, I'd love to hear it, together with your justification. While considering it, you might also like to consider: - is it ok for knfsd to bind to port 2049 ? - is it ok if userspace tells it the number '2049' ? - does user-space have to create/bind the socket and pass it to knfsd? - does user-space have to receive the packets and pass them to knfsd? (ok, that one is really silly). and "why?" The reality is that NFS service is an application. Currently parts of it are in-kernel (nfsd, lockd) and parts are in user-space (portmap, statd(*), mountd). There are two positions on what-goes-where that make sense to me: 1- pragmatism: put code where it works best. I believe that the current code fits pragmatism quite well (modulo bugs). 2- "rightness": If you want to argue from a what-belongs-where perspective, you have to say that knfsd doesn't belong in the kernel at all. The kernel should just supply the core services (e.g. file-handle <-> fd mapping) and let userspace do the rest. Were I starting to write knfsd today, I would pick 2. Given where we actually are today, I pick 1. > we really need someone sane review NFS patches I thinkg. yes please.. pretty please :-) > > (not cc'ed to the nfs list because of its stupid subsribers only > policy) Sad, isn't it. Both nfs@lists.sourceforge.net and nfsv4@linux-nfs.org are like that, and nfs-devel@linux.kernel.org died long ago. :-( NeilBrown (*) There are patches in existence which move statd implementation into the kernel. The final conclusion here may well affect those patches, so I hope Olaf has been listening in....