On Thu, 2 May 2013 11:16:34 -0400 "J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote: > On Thu, May 02, 2013 at 05:05:11PM +1000, NeilBrown wrote: > > > > > > Hi Steve, > > I just noticed > > > > commit ebe2826ca571a3959c3b5c8e29686c621f2775cf > > Author: Steve Dickson <steved@xxxxxxxxxx> > > Date: Sat Mar 23 10:30:17 2013 -0400 > > > > mountd: regression in crossmounts > > > > Sorry I missed the email you presumably sent me to let me know > > that I had caused a regression. > > Yup. The strategy around here is to bury people in email and trust they > have industrial-strength filtering. > > And I could have added Neil on the cc: too and forgot, oops: > > http://marc.info/?l=linux-nfs&m=136404930104976&w=2 > > > Here is the proper fix. > > I'm dense. I still had to scratch my head a while: > > So: in a case like the one steved gives: > > /home *(rw,fsid=0,crossmnt) > /home/fs1 *(rw,crossmnt) > /home/fs1/fs2/fs3 *(rw,nohide) > > where nfsd_fh is asked to look for an export with fsid=0, it gets a > match on the export of /home. But that loop there keeps going in case > it finds some better match. In particular it considers any mountpoints > under /home as candidates, since /home has "crossmnt". And they all > match too. So it considers in sequence the possibilities: > > (found=/export of /home, found_path=/home) > (found=/export of /home, found_path=/home/fs1) > (found=/export of /home, found_path=/home/fs1/fs2/fs3) > > The subexport() test--designed to favor crossmnt exports which are > "closer" to the target filesystem--passes in each case, so we end up > taking the last. > > So we pass down the longest path to the kernel, it does an export upcall > for that path, the hilarity ensues. > > Anyway: > > Acked-by: J. Bruce Fields <bfields@xxxxxxxxxx> Thanks. > > But this all still gives me a mild headache: > > - it only works if we iterate through the submounts in the > correct order? Does it? nfsd_fh() is trying to find the export for a given file handle. For every export it tries that filesystem and (if CROSSMOUNT) ever filesystem mounted below there. It only considers filesystem for which the fsid matches. Of those, it chooses the exp which is 'lowest' in the hierarchy. I don't think the result of that can be dependant on order. > - should fsid= be inherited by crossmnt anyway? No... If you have an export with fsid=N,crossmnt and a filehandle arrives for fsid=N, then when we first look at that export match_fsid() will report a match and 'found' will be set. nfsd_fh() will loop around and try again for the next entry in /etc/mtab which is under that same filesystem. It will get a match_fsid() as well but as subexport() will report "no" (as it is the same export), no change will be made to found. It also will not report that "X and Y have same file handle for Z, using first" as the e_paths will match. So it works as is. Maybe the code could be made clearer though. > - nfsd_fh() is too long, and the logic (with those extra loop > control variables declared static inside the CROSSMOUNT case) > could be more straightforward. You have a history of taking my too-long functions and breaking them up into manageable bits (kernel commit 3211af1119174fbe8b). Maybe you could try again :-) NeilBrown
Attachment:
signature.asc
Description: PGP signature
