| Message ID | 20210106184028.150925-1-steved@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mount: parse default values correctly | expand |
On 1/6/21 1:40 PM, Steve Dickson wrote: > Commit 88c22f92 converted the configfile.c routines > to use the parse_opt interfaces which broke how > default values from nfsmount.conf are managed. > > Default values can not be added to the mount string > handed to the kernel. They must be interpreted into > the correct mount options then passed to the kernel. > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1912877 > > Signed-off-by: Steve Dickson <steved@redhat.com> Committed... (tag: nfs-utils-2-5-3-rc4) steved. > --- > utils/mount/configfile.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c > index 7934f4f..e865998 100644 > --- a/utils/mount/configfile.c > +++ b/utils/mount/configfile.c > @@ -277,8 +277,10 @@ conf_parse_mntopts(char *section, char *arg, struct mount_options *options) > } > if (buf[0] == '\0') > continue; > + if (default_value(buf)) > + continue; > + > po_append(options, buf); > - default_value(buf); > } > conf_free_list(list); > } >
On Wed, Jan 06 2021, Steve Dickson wrote: > Commit 88c22f92 converted the configfile.c routines > to use the parse_opt interfaces which broke how > default values from nfsmount.conf are managed. > > Default values can not be added to the mount string > handed to the kernel. They must be interpreted into > the correct mount options then passed to the kernel. > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1912877 > > Signed-off-by: Steve Dickson <steved@redhat.com> > --- > utils/mount/configfile.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c > index 7934f4f..e865998 100644 > --- a/utils/mount/configfile.c > +++ b/utils/mount/configfile.c > @@ -277,8 +277,10 @@ conf_parse_mntopts(char *section, char *arg, struct mount_options *options) > } > if (buf[0] == '\0') > continue; > + if (default_value(buf)) > + continue; > + > po_append(options, buf); > - default_value(buf); > } > conf_free_list(list); > } > -- > 2.29.2 Hi, unfortunately this is incorrect. As the referenced patch said: The default_value() call is now made as soon as the option has been parsed. It is left on the options list so that new instances of the value are ignored. The new code doesn't leave it on the option list, so new instances are not ignored. conf_get_mntopts() processes options for the mount point, then options for the server, then global options. If there is e.g. a defaultvers for both the mountpoint and as a global option, the global option will now not be ignored, so it takes precedence. The real problem as identified in the bugzilla discussion, is that po_remove_all(options, "default"); doesn't remove all the "default*" options as was the intent. Clearly I never tested that. This is easily fixed with - po_remove_all(options, "default"); + while (po_contains_prefix(options, "default", &ptr, 0) == PO_FOUND) + po_remove_all(options, ptr); which I have tested :-) I'll follow up with a patch which reverts the incorrect fix, and adds this one. Thanks, NeilBrown
diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c index 7934f4f..e865998 100644 --- a/utils/mount/configfile.c +++ b/utils/mount/configfile.c @@ -277,8 +277,10 @@ conf_parse_mntopts(char *section, char *arg, struct mount_options *options) } if (buf[0] == '\0') continue; + if (default_value(buf)) + continue; + po_append(options, buf); - default_value(buf); } conf_free_list(list); }
Commit 88c22f92 converted the configfile.c routines to use the parse_opt interfaces which broke how default values from nfsmount.conf are managed. Default values can not be added to the mount string handed to the kernel. They must be interpreted into the correct mount options then passed to the kernel. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1912877 Signed-off-by: Steve Dickson <steved@redhat.com> --- utils/mount/configfile.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)