On Mon, Mar 01, 2010 at 10:13:06PM +0100, Iustin Pop wrote: > On Mon, Mar 01, 2010 at 03:40:10PM -0500, J. Bruce Fields wrote: > > On Sun, Feb 28, 2010 at 11:06:43AM +0100, Iustin Pop wrote: > > > Since nfs-utils 1.2.1, there are some problems with crossmnt usage. See > > > Debian bug http://bugs.debian.org/567546, but in short the problem seems > > > to be that sub-mounts (/a/b) take the top-level mount (/a) options > > > instead of their own, due to a bug in how mountd generates the crossmnt > > > subexports. > > > > > > I checked that reverting the write_secinfo changes in commit > > > bc0a6ab03089fc1ea4fea26ed9635c2cc918b01b fix the issue, but that might > > > only be a side effect, not the actual cause. > > > > > > A short test: > > > - have /a and /a/b exported, with different flags (e.g. ro on /a, rw on > > > /a/b) > > > - restart the mountd, clear exports, etc. > > > - try a mount on the client of /a/b, it gets readonly > > > - umount & remount, it's now r/w > > > - however, clearing the kernel export table (exportfs -f), makes the > > > next mount again get read-only > > > > > > Disabling crossmnt fixes the issue completely, so I would venture to > > > guess that the subexports creation code has some issue, but I don't know > > > enough of this to be able to debug it. > > > > Thanks for the report. What's the latest nfs-utils version you've > > tested? > > 1.2.2 - which is broken, as is 1.2.1. 1.2.0 works, because of the above > commit which, again, I presume un-hides the problem. > > Also there are reports of older versions being broken. > > > On a quick skim of the latest code I see one clear bug > > (path[strlen(path)] will never be '/'!), but that should have caused > > crossmnt to never get enforced at all rather than to override anything. > > > > Could you retest the latest from the upstream git, with this patch > > applied, and see if the problem is still present? > > I've tested right now with > git://git.linux-nfs.org/projects/steved/nfs-utils.git (I hope this is the right > upstream repo) plus the patch - still happens, no change. > > One thing that is interesting is that only the first mount gets the > wrong options, if the client unmounts and remounts (at this point the > exports are in the kernel's cache) the options are right. As long as the > kernel cache is populated, the options are right, if one clears the > cache, the first mount will be wrong. Maybe this helps, I find it an > interesting behaviour. > > > Then if it is I'll try to go reproduce it myself.... > > For reference, here is the exports file I use for tests: > /srv/nfs client(sec=sys,ro,sync,fsid=0,subtree_check,crossmnt) > /srv/nfs/homes client(sec=sys,rw,sync,subtree_check) > > And here is the fstab entry on the client: > server:/srv/nfs/homes /home nfs noauto,hard,bg,sec=sys,proto=tcp 0 0 > > The rest of the tools on server and client are Debian's nfs-utils 1.2.1 > (they have just a few, very trivial and unrelated, patches). The problem > manifests (in my case) with both nfsv3/sec=sys and nfsv4/sec=krb*. The loop in utils/mountd/cache.c:cache_export_ent() looks extremely suspicious: it seems to be populating the kernel's export cache with parameters taken from the parent export. I can't see why that's necessary at all--those exports can be looked up on demand later when they're needed. Something as simple as the following might help? (Totally untested!) --b. diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c index d63e10a..7d87812 100644 --- a/utils/mountd/cache.c +++ b/utils/mountd/cache.c @@ -619,7 +619,7 @@ static int is_subdirectory(char *subpath, char *path) int l = strlen(path); return strcmp(subpath, path) == 0 - || (strncmp(subpath, path, l) == 0 && path[l] == '/'); + || (strncmp(subpath, path, l) == 0 && subpath[l] == '/'); } static int path_matches(nfs_export *exp, char *path) @@ -817,46 +817,6 @@ int cache_export_ent(char *domain, struct exportent *exp, char *path) " fsid= required", exp->e_path); } - while (err == 0 && (exp->e_flags & NFSEXP_CROSSMOUNT) && path) { - /* really an 'if', but we can break out of - * a 'while' more easily */ - /* Look along 'path' for other filesystems - * and export them with the same options - */ - struct stat stb; - int l = strlen(exp->e_path); - int dev; - - if (strlen(path) <= l || path[l] != '/' || - strncmp(exp->e_path, path, l) != 0) - break; - if (stat(exp->e_path, &stb) != 0) - break; - dev = stb.st_dev; - while(path[l] == '/') { - char c; - /* errors for submount should fail whole filesystem */ - int err2; - - l++; - while (path[l] != '/' && path[l]) - l++; - c = path[l]; - path[l] = 0; - err2 = lstat(path, &stb); - path[l] = c; - if (err2 < 0) - break; - if (stb.st_dev == dev) - continue; - dev = stb.st_dev; - path[l] = 0; - dump_to_cache(f, domain, path, exp); - path[l] = c; - } - break; - } - fclose(f); return err; } -- 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
