[RFC,PATCH 1/4] Dynamic Pseudo Root

[Steve Dickson]

Neil Brown wrote:
> On Friday December 7, SteveD at redhat.com wrote:
>> +
>> +	for (i = 0; i < MCL_MAXTYPES; i++) {
>> +		for (exp = exportlist[i]; exp; exp = nxt) {
>> +			nxt = exp->m_next;
>> +			if (strcmp(exp->m_export.e_path, "/")) 
>> +				snprintf(path, BUFSIZ, "%s/%s", _PATH_PSEUDO_ROOT, 
>> +					exp->m_export.e_path);
>> +			if (!is_mountpoint(path)) {
>> +				exec_mkpath(path);
>> +				bind_mount(exp->m_export.e_path, path);
>> +			}
>> +		}
>> +	}
> 
> The 
> 			if (strcmp(exp->m_export.e_path, "/")) 
> 
> test is odd, and not just because I find that idiom hard to read (it
> is testing if e_path is not "/", but there is no "!" or "!=" in the
> test).
Okay... 'if (strcmp(exp->m_export.e_path, "/") != 0)' it is...
I'm indifferent but if simple things like this make the code easier
to read... I'm all for it...


> If e_path is "/", then path is left uninitialised for the following
> code.
Point..

> 
> And is the "!is_mountpoint" test just to avoid binding the same
> filesystem twice?
Yes.

> 
> And there is a case (explicitly mentioned in the RFC I believe) that
> this doesn't cover.
> If I have 3 filesystems:   /a, /a/b and /a/b/c, and I export /a and
> /a/b/c, then we need to mount a second tmpfs on /a/b before mkpath("/a/b/c").
> 
> This could be done fairly easily if we wrote our own mkpath instead of
> using "mkdir -p".
> To create a path, keep truncating the name until we find something
> that exists.  If it is a tmpfs, simply do a mkdir, else do a mount,
> then a mkdir.
I used "mkdir -p" to avoid the complicity rolling our own mkpath()
but let investigate you example a little further to if rolling
our own does indeed make more sense.  

> 
> I suspect you could also get rid of the special case of '/' being
> exported. 
> You don't create an initial 'tmpfs' mounted on .../nfs4root.  You only
> mount tmpfs on demand.
> Then "mkpath" for "/" finds that the directory already exists and
> simply does a bind mount of "/" to ".../nfs4root".
I think I tried something similar to this, but I keep running into
the "BUG_ON(IS_ROOT(dentry))" in nfs_follow_mountpoint() on client
when I did an 'ls' on one of the directories... 

And now that I know other implementations just use '/' as the 
pseudo root, I feel better about enforcing the fact that
when '/' is exported, that export is the pseudo root...
 

> 
>> +inline int
>> +exec_rmdir(char *dir)
>> +{
>> +	char *const myargv[] = {_RMDIR_CMD, dir, NULL};
>> +
>> +	return execute_cmd(_RMDIR_CMD, myargv);
>> +}
> 
> what is wrong with calling rmdir(dir) directly?
No good reason... I think this came from the fact early on I was
calling "rm -rf" on a directory which turnout to be a *really* bad
idea! 8-) 

steved.