Re: [RFC PATCH 1/2] exportfs: pass export option flags to the kernel when testing export

[Steve Dickson <SteveD@redhat.com>]

On 11/18/2015 02:16 PM, Jeff Layton wrote:
> On Wed, 18 Nov 2015 11:50:29 -0500
> Jeff Layton <jlayton@poochiereds.net> wrote:
> 
>> test_export takes a flag that just says whether to pass in the fsid or
>> not. This means that all we can test is the NFSEXP_FSIDi flag.
>>
>> Instead of doing that, pass in the actual export options that we
>> intend to use (sometimes or'ed with NFSEXP_FSID to test whether that
>> might be the reason for the failure).
>>
>> This allows us to test the actual export options that we plan to use,
>> and can allow the kernel to vet them properly before mount time.
>>
>> The rationale here is a patch that I have that will add the ability
>> for filesystems to opt out of subtree checking when they can't
>> properly support it.
>>
>> Cc: Neil Brown <neilb@suse.de>
>> Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
>> ---
>>  utils/exportfs/exportfs.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
>> index c7a79a69b5d6..cac01fc25f9a 100644
>> --- a/utils/exportfs/exportfs.c
>> +++ b/utils/exportfs/exportfs.c
>> @@ -531,7 +531,7 @@ static int can_test(void)
>>  	return 1;
>>  }
>>  
>> -static int test_export(char *path, int with_fsid)
>> +static int test_export(char *path, int opts)
>>  {
>>  	/* beside max path, buf size should take protocol str into account */
>>  	char buf[NFS_MAXPATHLEN+1+64] = { 0 };
>> @@ -545,7 +545,7 @@ static int test_export(char *path, int with_fsid)
>>  	qword_add(&bp, &len, path);
>>  	if (len < 1)
>>  		return 0;
>> -	snprintf(bp, len, " 3 %d 65534 65534 0\n", with_fsid ? NFSEXP_FSID : 0);
>> +	snprintf(bp, len, " 3 %d 65534 65534 0\n", opts);
>>  	fd = open("/proc/net/rpc/nfsd.export/channel", O_WRONLY);
>>  	if (fd < 0)
>>  		return 0;
>> @@ -587,12 +587,12 @@ validate_export(nfs_export *exp)
>>  
>>  	if ((exp->m_export.e_flags & NFSEXP_FSID) || exp->m_export.e_uuid ||
>>  	    fs_has_fsid) {
>> -		if ( !test_export(path, 1)) {
>> +		if ( !test_export(path, exp->m_export.e_flags)) {
>>  			xlog(L_ERROR, "%s does not support NFS export", path);
>>  			return;
>>  		}
>> -	} else if ( ! test_export(path, 0)) {
>> -		if (test_export(path, 1))
>> +	} else if ( ! test_export(path, exp->m_export.e_flags)) {
>> +		if (test_export(path, exp->m_export.e_flags & NFSEXP_FSID))
> 
> Duh, that should of course be "exp->m_export.e_flags | NFSEXP_FSID".
> I'll plan to resend after I wait a bit for others to comment. Please
> don't merge this as-is though -- it obviously needs better testing.
> 
> We might be able to simplify the above logic a little too...hmmm...
That logic did look a bit odd... I'll wait for the next version....

steved.

> 
>>  			xlog(L_ERROR, "%s requires fsid= for NFS export", path);
>>  		else
>>  			xlog(L_ERROR, "%s does not support NFS export", path);
> 
>