On Aug 13, 2013, at 6:45 PM, Scott Mayhew <smayhew@xxxxxxxxxx> wrote:
> On Tue, 13 Aug 2013, Chuck Lever wrote:
>
>> Hi Scott-
>>
>> On Aug 13, 2013, at 3:20 PM, Scott Mayhew <smayhew@xxxxxxxxxx> wrote:
>>
>>> conf_parse_mntopts() maintains a linked list of options that will
>>> ultimately be passed in the data field of the mount() syscall in order
>>> to avoid unnecessary duplicates and/or conflicts. This doesn't work
>>> very well for MNT_NOARG type options, since setting any of these to
>>> false in the nfsmount.conf doesn't correspond to any 'real' mount option
>>> (i.e. there are no such options 'nobg', 'nofg', and 'nosloppy').
>>
>> What's broken here, exactly?
>
> The problem here is that specifying something like bg=false, fg=false, or
> sloppy=false doesn't perform any negation if those options appeared
> somewhere in an earlier section. Take for example a simple config that
> ooks like this:
>
> [ NFSMount_Global_Options ]
> Nfsvers=4
>
> [ Server "nfs.smayhew.test" ]
> Background=True
>
> [ MountPoint "/mnt" ]
> Background=False
>
> If I try to perform a mount using that server and mountpoint, and the
> server happens to be unresponsive, then what should happen is that the
> mount should time out after 2 minutes. What actually happens is that
> we'll keep retrying for 10000 minutes.
"bg" and "fg" negate each other. "Background=True" should mean "bg" and "Background=False" should mean "fg."
"sloppy" is another matter… It was originally not a mount option, but a command line option on the mount.nfs command. When mount option parsing was moved into the kernel, we had to make "sloppy" a real mount option.
It probably requires the approach you took. However, that means that the mount command line cannot negate "sloppy" set in the config file.
> -Scott
>>
>>>
>>> This patch adds a set of internal variables for the three real MNT_NOARG
>>> options (bg, fg, sloppy) along with their pseudo options (nobg, nofg,
>>> nosloppy), and a helper function to track which of these options has
>>> been previously seen and to determine whether or not to append an option
>>> to the linked list.
>>>
>>> Signed-off-by: Scott Mayhew <smayhew@xxxxxxxxxx>
>>> ---
>>> utils/mount/configfile.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 50 insertions(+)
>>>
>>> diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c
>>> index 221436f..623c886 100644
>>> --- a/utils/mount/configfile.c
>>> +++ b/utils/mount/configfile.c
>>> @@ -73,6 +73,8 @@ struct mnt_alias {
>>> };
>>> int mnt_alias_sz = (sizeof(mnt_alias_tab)/sizeof(mnt_alias_tab[0]));
>>>
>>> +static int bg, nobg, fg, nofg, sloppy, nosloppy;
>>> +
>>> /*
>>> * See if the option is an alias, if so return the
>>> * real mount option along with the argument type.
>>> @@ -278,6 +280,46 @@ default_value(char *mopt)
>>>
>>> return 1;
>>> }
>>> +
>>> +int
>>> +should_add_noarg_opt(char *opt, char *val)
>>> +{
>>> + int ret = 0;
>>> +
>>> + if (strcasecmp(opt, "bg") == 0) {
>>> + if (strcasecmp(val, "true") == 0) {
>>> + if (bg == 0 && nobg == 0 && fg == 0) {
>>> + bg = 1;
>>> + ret = 1;
>>> + }
>>> + } else if (strcasecmp(val, "false") == 0) {
>>> + if (bg == 0 && nobg == 0)
>>> + nobg = 1;
>>> + }
>>> + } else if (strcasecmp(opt, "fg") == 0) {
>>> + if (strcasecmp(val, "true") == 0) {
>>> + if (fg == 0 && nofg == 0 && bg == 0) {
>>> + fg = 1;
>>> + ret = 1;
>>> + }
>>> + } else if (strcasecmp(val, "false") == 0) {
>>> + if (fg == 0 && nofg == 0)
>>> + nofg = 1;
>>> + }
>>> + } else if (strcasecmp(opt, "sloppy") == 0) {
>>> + if (sloppy == 0 && nosloppy == 0) {
>>> + if(strcasecmp(val, "true") == 0) {
>>> + sloppy = 1;
>>> + ret = 1;
>>> + } else
>>> + nosloppy = 1;
>>> + }
>>> + } else
>>> + xlog_warn("Invalid MNT_NOARG option: '%s'", opt);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> /*
>>> * Parse the given section of the configuration
>>> * file to if there are any mount options set.
>>> @@ -315,6 +357,13 @@ conf_parse_mntopts(char *section, char *arg, char *opts)
>>> field = mountopts_alias(node->field, &argtype);
>>> if (lookup_entry(field) != NULL)
>>> continue;
>>> + if (argtype == MNT_NOARG) {
>>> + if (should_add_noarg_opt(field, value)) {
>>> + list_size += strlen(field) + 1;
>>> + add_entry(field);
>>> + }
>>> + continue;
>>> + }
>>> if (argtype != MNT_NOARG) {
>>> snprintf(buf, BUFSIZ, "no%s", field);
>>> if (lookup_entry(buf) != NULL)
>>> @@ -359,6 +408,7 @@ char *conf_get_mntopts(char *spec, char *mount_point,
>>> char *ptr, *server, *config_opts;
>>> int optlen = 0;
>>>
>>> + bg = nobg = fg = nofg = sloppy = nosloppy = 0;
>>> SLIST_INIT(&head);
>>> list_size = 0;
>>> /*
>>> --
>>> 1.7.11.7
>>>
>>> --
>>> 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
>>
>> --
>> Chuck Lever
>> chuck[dot]lever[at]oracle[dot]com
>>
>>
>>
>>
>
> --
> Scott Mayhew, RHCE
> Software Maintenance Engineer
> Red Hat Global Support Services
> smayhew@xxxxxxxxxx
> 1-888-REDHAT1 x43741
> --
> 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
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
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
