2008-04-11 20:03:49

by Chuck Lever

[permalink] [raw]
Subject: [PATCH] NFS: Only warn on unrecognized mount options

To provide compatibility with automounters who use a common set of mount
options for all file systems, change the NFS in-kernel mount option parser
to ignore mount options it doesn't recognize.

Signed-off-by: Chuck Lever <[email protected]>
---
Yet another NFS mount patch! Build tested only. Comments?

fs/nfs/super.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index f921902..a7201f0 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1044,7 +1044,8 @@ static int nfs_parse_mount_options(char *raw,
break;

default:
- goto out_unknown;
+ printk(KERN_INFO "NFS: unrecognized mount option '%s'"
+ " ignored\n", p);
}
}

@@ -1070,10 +1071,6 @@ out_unrec_xprt:
out_unrec_sec:
printk(KERN_INFO "NFS: unrecognized security flavor\n");
return 0;
-
-out_unknown:
- printk(KERN_INFO "NFS: unknown mount option: %s\n", p);
- return 0;
}

/*



2008-04-11 20:08:08

by Peter Staubach

[permalink] [raw]
Subject: Re: [PATCH] NFS: Only warn on unrecognized mount options

Chuck Lever wrote:
> To provide compatibility with automounters who use a common set of mount
> options for all file systems, change the NFS in-kernel mount option parser
> to ignore mount options it doesn't recognize.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> Yet another NFS mount patch! Build tested only. Comments?
>
> fs/nfs/super.c | 7 ++-----
> 1 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index f921902..a7201f0 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1044,7 +1044,8 @@ static int nfs_parse_mount_options(char *raw,
> break;
>
> default:
> - goto out_unknown;
> + printk(KERN_INFO "NFS: unrecognized mount option '%s'"
> + " ignored\n", p);
> }
> }
>
> @@ -1070,10 +1071,6 @@ out_unrec_xprt:
> out_unrec_sec:
> printk(KERN_INFO "NFS: unrecognized security flavor\n");
> return 0;
> -
> -out_unknown:
> - printk(KERN_INFO "NFS: unknown mount option: %s\n", p);
> - return 0;
> }
>
> /*

This will potentially cause a very large number of messages to be
printed in a valid deployment. Do we really need the message?

ps

2008-04-11 20:14:38

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH] NFS: Only warn on unrecognized mount options

Peter Staubach wrote:
> Chuck Lever wrote:
>> To provide compatibility with automounters who use a common set of mount
>> options for all file systems, change the NFS in-kernel mount option
>> parser
>> to ignore mount options it doesn't recognize.
>>
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>> Yet another NFS mount patch! Build tested only. Comments?
>>
>> fs/nfs/super.c | 7 ++-----
>> 1 files changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index f921902..a7201f0 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -1044,7 +1044,8 @@ static int nfs_parse_mount_options(char *raw,
>> break;
>>
>> default:
>> - goto out_unknown;
>> + printk(KERN_INFO "NFS: unrecognized mount option '%s'"
>> + " ignored\n", p);
>> }
>> }
>>
>> @@ -1070,10 +1071,6 @@ out_unrec_xprt:
>> out_unrec_sec:
>> printk(KERN_INFO "NFS: unrecognized security flavor\n");
>> return 0;
>> -
>> -out_unknown:
>> - printk(KERN_INFO "NFS: unknown mount option: %s\n", p);
>> - return 0;
>> }
>>
>> /*
>
> This will potentially cause a very large number of messages to be
> printed in a valid deployment. Do we really need the message?

I was wondering about that.

I left it in because it's useful to know when a valid mount option is
misspelled. In that case it might cause an important option (such as
"noac") to be ignored.


Attachments:
chuck_lever.vcf (259.00 B)

2008-04-11 20:23:52

by Peter Staubach

[permalink] [raw]
Subject: Re: [PATCH] NFS: Only warn on unrecognized mount options

Chuck Lever wrote:
> Peter Staubach wrote:
>> Chuck Lever wrote:
>>> To provide compatibility with automounters who use a common set of
>>> mount
>>> options for all file systems, change the NFS in-kernel mount option
>>> parser
>>> to ignore mount options it doesn't recognize.
>>>
>>> Signed-off-by: Chuck Lever <[email protected]>
>>> ---
>>> Yet another NFS mount patch! Build tested only. Comments?
>>>
>>> fs/nfs/super.c | 7 ++-----
>>> 1 files changed, 2 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>> index f921902..a7201f0 100644
>>> --- a/fs/nfs/super.c
>>> +++ b/fs/nfs/super.c
>>> @@ -1044,7 +1044,8 @@ static int nfs_parse_mount_options(char *raw,
>>> break;
>>>
>>> default:
>>> - goto out_unknown;
>>> + printk(KERN_INFO "NFS: unrecognized mount option '%s'"
>>> + " ignored\n", p);
>>> }
>>> }
>>>
>>> @@ -1070,10 +1071,6 @@ out_unrec_xprt:
>>> out_unrec_sec:
>>> printk(KERN_INFO "NFS: unrecognized security flavor\n");
>>> return 0;
>>> -
>>> -out_unknown:
>>> - printk(KERN_INFO "NFS: unknown mount option: %s\n", p);
>>> - return 0;
>>> }
>>>
>>> /*
>>
>> This will potentially cause a very large number of messages to be
>> printed in a valid deployment. Do we really need the message?
>
> I was wondering about that.
>
> I left it in because it's useful to know when a valid mount option is
> misspelled. In that case it might cause an important option (such as
> "noac") to be ignored.

There do seem to be valid uses for the message. However, it
could also end up being a bad thing.

Perhaps we could just gather the unknown options and lump
them together in something that would be visible via /proc/mounts
or some such. Something like "unknown=..." in the options list.

ps

2008-04-11 20:25:01

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] NFS: Only warn on unrecognized mount options


On Fri, 2008-04-11 at 16:03 -0400, Chuck Lever wrote:
> To provide compatibility with automounters who use a common set of mount
> options for all file systems, change the NFS in-kernel mount option parser
> to ignore mount options it doesn't recognize.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> Yet another NFS mount patch! Build tested only. Comments?
>
> fs/nfs/super.c | 7 ++-----
> 1 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index f921902..a7201f0 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1044,7 +1044,8 @@ static int nfs_parse_mount_options(char *raw,
> break;
>
> default:
> - goto out_unknown;
> + printk(KERN_INFO "NFS: unrecognized mount option '%s'"
> + " ignored\n", p);
> }
> }
>
> @@ -1070,10 +1071,6 @@ out_unrec_xprt:
> out_unrec_sec:
> printk(KERN_INFO "NFS: unrecognized security flavor\n");
> return 0;
> -
> -out_unknown:
> - printk(KERN_INFO "NFS: unknown mount option: %s\n", p);
> - return 0;
> }
>
> /*

This isn't really a very good solution either. Spamming the syslog on
every option that is being ignored isn't going to help the folks with
the global automounter maps. Either the rules should be that 'all
options are allowed' or they should be that 'only recognised NFS options
are allowed'.

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com