On Mon, May 06, 2013 at 02:44:13PM +1000, NeilBrown wrote: > 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. Well, my only point was that we depend on first trying the main export and then the filesystems mounted below--as that's probably how it would work under any reasonable organization of the code, I suppose that's not so fragile. > > - 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. Oh, I see--you're right. > 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 :-) Yeah, yeah, yeah. Maybe the compulsion will take me some day. For the sake of several other pressing projects, hopefully not today. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
