diff mbox

exportfs: Make sure pass all valid export flags to nfsd

Message ID c0826298-545e-64a3-b6bb-86413bf9eb39@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kinglong Mee Dec. 31, 2016, 1:05 p.m. UTC
test_export pass a export flags only marks NFSEXP_FSID,
nfsd may want other flags for export checking.
This patch make sure exportfs pass all other flags to nfsd.

Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
---
 utils/exportfs/exportfs.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Steve Dickson Jan. 4, 2017, 4:56 p.m. UTC | #1
On 12/31/2016 08:05 AM, Kinglong Mee wrote:
> test_export pass a export flags only marks NFSEXP_FSID,
> nfsd may want other flags for export checking.
> This patch make sure exportfs pass all other flags to nfsd.
> 
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
Committed!

steved.

> ---
>  utils/exportfs/exportfs.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> index 15a1583..bacf106 100644
> --- a/utils/exportfs/exportfs.c
> +++ b/utils/exportfs/exportfs.c
> @@ -473,8 +473,10 @@ static int can_test(void)
>  	return 1;
>  }
>  
> -static int test_export(char *path, int with_fsid)
> +static int test_export(nfs_export *exp, int with_fsid)
>  {
> +	char *path = exp->m_export.e_path;
> +	int flags = exp->m_export.e_flags | (with_fsid ? NFSEXP_FSID : 0);
>  	/* beside max path, buf size should take protocol str into account */
>  	char buf[NFS_MAXPATHLEN+1+64] = { 0 };
>  	char *bp = buf;
> @@ -487,7 +489,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", flags);
>  	fd = open("/proc/net/rpc/nfsd.export/channel", O_WRONLY);
>  	if (fd < 0)
>  		return 0;
> @@ -529,12 +531,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(exp, 1)) {
>  			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(exp, 0)) {
> +		if (test_export(exp, 1))
>  			xlog(L_ERROR, "%s requires fsid= for NFS export", path);
>  		else
>  			xlog(L_ERROR, "%s does not support NFS export", path);
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields Jan. 6, 2017, 9:05 p.m. UTC | #2
On Sat, Dec 31, 2016 at 09:05:11PM +0800, Kinglong Mee wrote:
> test_export pass a export flags only marks NFSEXP_FSID,
> nfsd may want other flags for export checking.

Why?  What problem does this fix?

--b.

> This patch make sure exportfs pass all other flags to nfsd.
> 
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> ---
>  utils/exportfs/exportfs.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> index 15a1583..bacf106 100644
> --- a/utils/exportfs/exportfs.c
> +++ b/utils/exportfs/exportfs.c
> @@ -473,8 +473,10 @@ static int can_test(void)
>  	return 1;
>  }
>  
> -static int test_export(char *path, int with_fsid)
> +static int test_export(nfs_export *exp, int with_fsid)
>  {
> +	char *path = exp->m_export.e_path;
> +	int flags = exp->m_export.e_flags | (with_fsid ? NFSEXP_FSID : 0);
>  	/* beside max path, buf size should take protocol str into account */
>  	char buf[NFS_MAXPATHLEN+1+64] = { 0 };
>  	char *bp = buf;
> @@ -487,7 +489,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", flags);
>  	fd = open("/proc/net/rpc/nfsd.export/channel", O_WRONLY);
>  	if (fd < 0)
>  		return 0;
> @@ -529,12 +531,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(exp, 1)) {
>  			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(exp, 0)) {
> +		if (test_export(exp, 1))
>  			xlog(L_ERROR, "%s requires fsid= for NFS export", path);
>  		else
>  			xlog(L_ERROR, "%s does not support NFS export", path);
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kinglong Mee Jan. 7, 2017, 11:57 a.m. UTC | #3
On 1/7/2017 05:05, J. Bruce Fields wrote:
> On Sat, Dec 31, 2016 at 09:05:11PM +0800, Kinglong Mee wrote:
>> test_export pass a export flags only marks NFSEXP_FSID,
>> nfsd may want other flags for export checking.
> 
> Why?  What problem does this fix?

When testing the patch "NFSD: Only support readonly export for
!fsync and readonly filesystem", I found exportfs don't pass 
all valid export flags to nfsd. So, make this patch.

thanks,
Kinglong Mee

> 
> --b.
> 
>> This patch make sure exportfs pass all other flags to nfsd.
>>
>> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
>> ---
>>  utils/exportfs/exportfs.c | 12 +++++++-----
>>  1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
>> index 15a1583..bacf106 100644
>> --- a/utils/exportfs/exportfs.c
>> +++ b/utils/exportfs/exportfs.c
>> @@ -473,8 +473,10 @@ static int can_test(void)
>>  	return 1;
>>  }
>>  
>> -static int test_export(char *path, int with_fsid)
>> +static int test_export(nfs_export *exp, int with_fsid)
>>  {
>> +	char *path = exp->m_export.e_path;
>> +	int flags = exp->m_export.e_flags | (with_fsid ? NFSEXP_FSID : 0);
>>  	/* beside max path, buf size should take protocol str into account */
>>  	char buf[NFS_MAXPATHLEN+1+64] = { 0 };
>>  	char *bp = buf;
>> @@ -487,7 +489,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", flags);
>>  	fd = open("/proc/net/rpc/nfsd.export/channel", O_WRONLY);
>>  	if (fd < 0)
>>  		return 0;
>> @@ -529,12 +531,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(exp, 1)) {
>>  			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(exp, 0)) {
>> +		if (test_export(exp, 1))
>>  			xlog(L_ERROR, "%s requires fsid= for NFS export", path);
>>  		else
>>  			xlog(L_ERROR, "%s does not support NFS export", path);
>> -- 
>> 2.9.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields Jan. 11, 2017, 2:16 p.m. UTC | #4
On Sat, Jan 07, 2017 at 07:57:28PM +0800, Kinglong Mee wrote:
> On 1/7/2017 05:05, J. Bruce Fields wrote:
> > On Sat, Dec 31, 2016 at 09:05:11PM +0800, Kinglong Mee wrote:
> >> test_export pass a export flags only marks NFSEXP_FSID,
> >> nfsd may want other flags for export checking.
> > 
> > Why?  What problem does this fix?
> 
> When testing the patch "NFSD: Only support readonly export for
> !fsync and readonly filesystem", I found exportfs don't pass 
> all valid export flags to nfsd. So, make this patch.

This function is meant to test whether a filesystem supports nfs export
or not.  It doesn't need the full set of export flags.  Off the top of
my head I can't see reason this would cause problems, but I'm not
convinced it's safe, either.  (New nfs-utils against old kernels might
be a case to check, e.g. to see how unsupported flags are handled.)

So until we have a reason we *need* this, just to be safe, I'd prefer to
revert this patch.

--b.

> 
> thanks,
> Kinglong Mee
> 
> > 
> > --b.
> > 
> >> This patch make sure exportfs pass all other flags to nfsd.
> >>
> >> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> >> ---
> >>  utils/exportfs/exportfs.c | 12 +++++++-----
> >>  1 file changed, 7 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> >> index 15a1583..bacf106 100644
> >> --- a/utils/exportfs/exportfs.c
> >> +++ b/utils/exportfs/exportfs.c
> >> @@ -473,8 +473,10 @@ static int can_test(void)
> >>  	return 1;
> >>  }
> >>  
> >> -static int test_export(char *path, int with_fsid)
> >> +static int test_export(nfs_export *exp, int with_fsid)
> >>  {
> >> +	char *path = exp->m_export.e_path;
> >> +	int flags = exp->m_export.e_flags | (with_fsid ? NFSEXP_FSID : 0);
> >>  	/* beside max path, buf size should take protocol str into account */
> >>  	char buf[NFS_MAXPATHLEN+1+64] = { 0 };
> >>  	char *bp = buf;
> >> @@ -487,7 +489,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", flags);
> >>  	fd = open("/proc/net/rpc/nfsd.export/channel", O_WRONLY);
> >>  	if (fd < 0)
> >>  		return 0;
> >> @@ -529,12 +531,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(exp, 1)) {
> >>  			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(exp, 0)) {
> >> +		if (test_export(exp, 1))
> >>  			xlog(L_ERROR, "%s requires fsid= for NFS export", path);
> >>  		else
> >>  			xlog(L_ERROR, "%s does not support NFS export", path);
> >> -- 
> >> 2.9.3
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kinglong Mee Jan. 15, 2017, 7:43 a.m. UTC | #5
On 1/11/2017 22:16, J. Bruce Fields wrote:
> On Sat, Jan 07, 2017 at 07:57:28PM +0800, Kinglong Mee wrote:
>> On 1/7/2017 05:05, J. Bruce Fields wrote:
>>> On Sat, Dec 31, 2016 at 09:05:11PM +0800, Kinglong Mee wrote:
>>>> test_export pass a export flags only marks NFSEXP_FSID,
>>>> nfsd may want other flags for export checking.
>>>
>>> Why?  What problem does this fix?
>>
>> When testing the patch "NFSD: Only support readonly export for
>> !fsync and readonly filesystem", I found exportfs don't pass 
>> all valid export flags to nfsd. So, make this patch.
> 
> This function is meant to test whether a filesystem supports nfs export
> or not.  It doesn't need the full set of export flags.  Off the top of
> my head I can't see reason this would cause problems, but I'm not
> convinced it's safe, either.  (New nfs-utils against old kernels might
> be a case to check, e.g. to see how unsupported flags are handled.)

There are two cases that passing the flags to nfsd, 
1, exportfs checks the export entry, only NFSEXP_FSID without this patch,
2, mountd pass the validate export entry, all validate export flags.

When the new nfs-utils against old kernels,
user specifies an unsupported flags(Not NFSEXP_FSID),
exportfs doesn't warning that unsupported flags (without test),
mountd doesn't get the export entry for the flags.

If exportfs warning the unsupported flags, user can modify it and
do the right export.
Do I have an exact understanding?

thanks,
Kinglong Mee

> 
> So until we have a reason we *need* this, just to be safe, I'd prefer to
> revert this patch.
> 
> --b.
> 
>>
>> thanks,
>> Kinglong Mee
>>
>>>
>>> --b.
>>>
>>>> This patch make sure exportfs pass all other flags to nfsd.
>>>>
>>>> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
>>>> ---
>>>>  utils/exportfs/exportfs.c | 12 +++++++-----
>>>>  1 file changed, 7 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
>>>> index 15a1583..bacf106 100644
>>>> --- a/utils/exportfs/exportfs.c
>>>> +++ b/utils/exportfs/exportfs.c
>>>> @@ -473,8 +473,10 @@ static int can_test(void)
>>>>  	return 1;
>>>>  }
>>>>  
>>>> -static int test_export(char *path, int with_fsid)
>>>> +static int test_export(nfs_export *exp, int with_fsid)
>>>>  {
>>>> +	char *path = exp->m_export.e_path;
>>>> +	int flags = exp->m_export.e_flags | (with_fsid ? NFSEXP_FSID : 0);
>>>>  	/* beside max path, buf size should take protocol str into account */
>>>>  	char buf[NFS_MAXPATHLEN+1+64] = { 0 };
>>>>  	char *bp = buf;
>>>> @@ -487,7 +489,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", flags);
>>>>  	fd = open("/proc/net/rpc/nfsd.export/channel", O_WRONLY);
>>>>  	if (fd < 0)
>>>>  		return 0;
>>>> @@ -529,12 +531,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(exp, 1)) {
>>>>  			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(exp, 0)) {
>>>> +		if (test_export(exp, 1))
>>>>  			xlog(L_ERROR, "%s requires fsid= for NFS export", path);
>>>>  		else
>>>>  			xlog(L_ERROR, "%s does not support NFS export", path);
>>>> -- 
>>>> 2.9.3
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields Jan. 16, 2017, 4:54 p.m. UTC | #6
On Sun, Jan 15, 2017 at 03:43:21PM +0800, Kinglong Mee wrote:
> On 1/11/2017 22:16, J. Bruce Fields wrote:
> > On Sat, Jan 07, 2017 at 07:57:28PM +0800, Kinglong Mee wrote:
> >> On 1/7/2017 05:05, J. Bruce Fields wrote:
> >>> On Sat, Dec 31, 2016 at 09:05:11PM +0800, Kinglong Mee wrote:
> >>>> test_export pass a export flags only marks NFSEXP_FSID,
> >>>> nfsd may want other flags for export checking.
> >>>
> >>> Why?  What problem does this fix?
> >>
> >> When testing the patch "NFSD: Only support readonly export for
> >> !fsync and readonly filesystem", I found exportfs don't pass 
> >> all valid export flags to nfsd. So, make this patch.
> > 
> > This function is meant to test whether a filesystem supports nfs export
> > or not.  It doesn't need the full set of export flags.  Off the top of
> > my head I can't see reason this would cause problems, but I'm not
> > convinced it's safe, either.  (New nfs-utils against old kernels might
> > be a case to check, e.g. to see how unsupported flags are handled.)
> 
> There are two cases that passing the flags to nfsd, 
> 1, exportfs checks the export entry, only NFSEXP_FSID without this patch,
> 2, mountd pass the validate export entry, all validate export flags.
> 
> When the new nfs-utils against old kernels,
> user specifies an unsupported flags(Not NFSEXP_FSID),
> exportfs doesn't warning that unsupported flags (without test),
> mountd doesn't get the export entry for the flags.
> 
> If exportfs warning the unsupported flags, user can modify it and
> do the right export.
> Do I have an exact understanding?

OK, so when somebody specifies unsupported flags, the failures is silent
and client mounts just fail.  It would be better to warn when exportfs
runs.

I agree that that would be helpful.  Have you tested those cases?  (new
nfs-utils, old kernel).

It would be worth looking at validate_export() to see how it could
improve error messages in this case.  Currently it will say only:

	/example/export does not support NFS export

or

	/example/export requires fsid= for export

With your patch (forgive me if I misremember), I believe those are still
the only error messages, so it will be confusing if, for example, you
get

	/example/export does not support NFS export

when the real problem is that an export flag is unsupported.  (But maybe
you had kernel messages to help there, I don't remember.)

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kinglong Mee Jan. 18, 2017, 10:16 a.m. UTC | #7
On 1/17/2017 00:54, J. Bruce Fields wrote:
> On Sun, Jan 15, 2017 at 03:43:21PM +0800, Kinglong Mee wrote:
>> On 1/11/2017 22:16, J. Bruce Fields wrote:
>>> On Sat, Jan 07, 2017 at 07:57:28PM +0800, Kinglong Mee wrote:
>>>> On 1/7/2017 05:05, J. Bruce Fields wrote:
>>>>> On Sat, Dec 31, 2016 at 09:05:11PM +0800, Kinglong Mee wrote:
>>>>>> test_export pass a export flags only marks NFSEXP_FSID,
>>>>>> nfsd may want other flags for export checking.
>>>>>
>>>>> Why?  What problem does this fix?
>>>>
>>>> When testing the patch "NFSD: Only support readonly export for
>>>> !fsync and readonly filesystem", I found exportfs don't pass 
>>>> all valid export flags to nfsd. So, make this patch.
>>>
>>> This function is meant to test whether a filesystem supports nfs export
>>> or not.  It doesn't need the full set of export flags.  Off the top of
>>> my head I can't see reason this would cause problems, but I'm not
>>> convinced it's safe, either.  (New nfs-utils against old kernels might
>>> be a case to check, e.g. to see how unsupported flags are handled.)
>>
>> There are two cases that passing the flags to nfsd, 
>> 1, exportfs checks the export entry, only NFSEXP_FSID without this patch,
>> 2, mountd pass the validate export entry, all validate export flags.
>>
>> When the new nfs-utils against old kernels,
>> user specifies an unsupported flags(Not NFSEXP_FSID),
>> exportfs doesn't warning that unsupported flags (without test),
>> mountd doesn't get the export entry for the flags.
>>
>> If exportfs warning the unsupported flags, user can modify it and
>> do the right export.
>> Do I have an exact understanding?
> 
> OK, so when somebody specifies unsupported flags, the failures is silent
> and client mounts just fail.  It would be better to warn when exportfs
> runs.
> 
> I agree that that would be helpful.  Have you tested those cases?  (new
> nfs-utils, old kernel).

I test the latest nfs-utils with centos7 (3.10.0-514.2.2.el7.x86_64) which
not support "security_label" flag. nfsd runs without any errors.

# cat /etc/exports
/nfs	*(rw,insecure,no_root_squash,no_subtree_check,fsid=0,pnfs,security_label)

# cat /var/lib/nfs/etab 
/nfs	*(rw,sync,wdelay,hide,nocrossmnt,insecure,no_root_squash,
no_all_squash,no_subtree_check,secure_locks,acl,security_label,pnfs,fsid=0,
anonuid=65534,anongid=65534,sec=sys,insecure,no_root_squash,no_all_squash)

# cat /proc/fs/nfsd/exports 
# Version 1.1
# Path Client(Flags) # IPs
/nfs	*(rw,insecure,no_root_squash,sync,wdelay,no_subtree_check,pnfs,fsid=0,
uuid=767ef2f3:a8b34db9:b372b254:8cae731a,sec=1)

I add trace log for export flags in svc_export as,
svc_export_parse: exflags 0x224a2         "a" contains NFSEXP_SECURITY_LABEL
fh_verify: exflags 0x224a2       "a" contains NFSEXP_SECURITY_LABEL         

if I remove "security_label" from /etc/exports, 
svc_export_parse: exflags 0x22422
fh_verify: exflags 0x22422

nfsd has the right export flags (include unsupported flags), but
doesn't parse the unsupported flags, so that, runs without any errors.

> 
> It would be worth looking at validate_export() to see how it could
> improve error messages in this case.  Currently it will say only:
> 
> 	/example/export does not support NFS export
> 
> or
> 
> 	/example/export requires fsid= for export
> 
> With your patch (forgive me if I misremember), I believe those are still
> the only error messages, so it will be confusing if, for example, you
> get
> 
> 	/example/export does not support NFS export
> 
> when the real problem is that an export flag is unsupported.  (But maybe
> you had kernel messages to help there, I don't remember.)

Yes, those error messages are shortage.
I will check them and try to make them better.

thanks,
Kinglong Mee
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index 15a1583..bacf106 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -473,8 +473,10 @@  static int can_test(void)
 	return 1;
 }
 
-static int test_export(char *path, int with_fsid)
+static int test_export(nfs_export *exp, int with_fsid)
 {
+	char *path = exp->m_export.e_path;
+	int flags = exp->m_export.e_flags | (with_fsid ? NFSEXP_FSID : 0);
 	/* beside max path, buf size should take protocol str into account */
 	char buf[NFS_MAXPATHLEN+1+64] = { 0 };
 	char *bp = buf;
@@ -487,7 +489,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", flags);
 	fd = open("/proc/net/rpc/nfsd.export/channel", O_WRONLY);
 	if (fd < 0)
 		return 0;
@@ -529,12 +531,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(exp, 1)) {
 			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(exp, 0)) {
+		if (test_export(exp, 1))
 			xlog(L_ERROR, "%s requires fsid= for NFS export", path);
 		else
 			xlog(L_ERROR, "%s does not support NFS export", path);