* [PATCH] mount: parse default values correctly @ 2021-01-06 18:40 Steve Dickson 2021-01-07 16:03 ` Steve Dickson 2021-01-28 23:39 ` NeilBrown 0 siblings, 2 replies; 6+ messages in thread From: Steve Dickson @ 2021-01-06 18:40 UTC (permalink / raw) To: Linux NFS Mailing 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(-) 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mount: parse default values correctly 2021-01-06 18:40 [PATCH] mount: parse default values correctly Steve Dickson @ 2021-01-07 16:03 ` Steve Dickson 2021-01-28 23:39 ` NeilBrown 1 sibling, 0 replies; 6+ messages in thread From: Steve Dickson @ 2021-01-07 16:03 UTC (permalink / raw) To: Linux NFS Mailing list 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); > } > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mount: parse default values correctly 2021-01-06 18:40 [PATCH] mount: parse default values correctly Steve Dickson 2021-01-07 16:03 ` Steve Dickson @ 2021-01-28 23:39 ` NeilBrown 2021-01-28 23:40 ` [PATCH nfs-utils] mount: fix parsing of default options NeilBrown 1 sibling, 1 reply; 6+ messages in thread From: NeilBrown @ 2021-01-28 23:39 UTC (permalink / raw) To: Steve Dickson, Linux NFS Mailing list [-- Attachment #1: Type: text/plain, Size: 2193 bytes --] 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 853 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH nfs-utils] mount: fix parsing of default options 2021-01-28 23:39 ` NeilBrown @ 2021-01-28 23:40 ` NeilBrown 2021-02-01 5:00 ` [PATCH nfs-utils v2] " NeilBrown 0 siblings, 1 reply; 6+ messages in thread From: NeilBrown @ 2021-01-28 23:40 UTC (permalink / raw) To: Steve Dickson, Linux NFS Mailing list [-- Attachment #1: Type: text/plain, Size: 1804 bytes --] A recent patch to change configfile.c to use parse_opt.c contained code which was intended to remove all "default*" options from the list before that could be passed to the kernel. This code didn't work, so default* options WERE passed to the kernel, and the kernel complained and failed the mount attempt. A more recent patch attempted to fix this by not including the "default*" options in the option list at all. This resulting in global-default defaults over-riding per-mount or per-server defaults. This patch reverse the "more recent" patch, and fixes the original patch by providing correct code to remove all "default*" options before the kernel can see them. Fixes: 88c22f924f1b ("mount: convert configfile.c to use parse_opt.c") Fixes: 8142542bda28 ("mount: parse default values correctly") Signed-off-by: NeilBrown <neilb@suse.de> --- utils/mount/configfile.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c index e865998dd5a9..ab386d08b031 100644 --- a/utils/mount/configfile.c +++ b/utils/mount/configfile.c @@ -277,10 +277,9 @@ 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); } @@ -335,7 +334,8 @@ char *conf_get_mntopts(char *spec, char *mount_point, * Strip out defaults, which have already been handled, * then join the rest and return. */ - po_remove_all(options, "default"); + while (po_contains_prefix(options, "default", &ptr, 0) == PO_FOUND) + po_remove_all(options, ptr); po_join(options, &mount_opts); po_destroy(options); -- 2.30.0 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 853 bytes --] ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH nfs-utils v2] mount: fix parsing of default options 2021-01-28 23:40 ` [PATCH nfs-utils] mount: fix parsing of default options NeilBrown @ 2021-02-01 5:00 ` NeilBrown 2021-02-02 17:00 ` Steve Dickson 0 siblings, 1 reply; 6+ messages in thread From: NeilBrown @ 2021-02-01 5:00 UTC (permalink / raw) To: Steve Dickson, Linux NFS Mailing list [-- Attachment #1: Type: text/plain, Size: 2023 bytes --] A recent patch to change configfile.c to use parse_opt.c contained code which was intended to remove all "default*" options from the list before that could be passed to the kernel. This code didn't work, so default* options WERE passed to the kernel, and the kernel complained and failed the mount attempt. A more recent patch attempted to fix this by not including the "default*" options in the option list at all. This resulting in global-default defaults over-riding per-mount or per-server defaults. This patch reverse the "more recent" patch, and fixes the original patch by providing correct code to remove all "default*" options before the kernel can see them. Fixes: 88c22f924f1b ("mount: convert configfile.c to use parse_opt.c") Fixes: 8142542bda28 ("mount: parse default values correctly") Signed-off-by: NeilBrown <neilb@suse.de> --- I realized that po_remove_all() could free 'ptr' and then compare it against the next option, which would have undefined results. So best to strdup and free it. utils/mount/configfile.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c index e865998dd5a9..3d3684efa186 100644 --- a/utils/mount/configfile.c +++ b/utils/mount/configfile.c @@ -277,10 +277,9 @@ 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); } @@ -335,7 +334,11 @@ char *conf_get_mntopts(char *spec, char *mount_point, * Strip out defaults, which have already been handled, * then join the rest and return. */ - po_remove_all(options, "default"); + while (po_contains_prefix(options, "default", &ptr, 0) == PO_FOUND) { + ptr = strdup(ptr); + po_remove_all(options, ptr); + free(ptr); + } po_join(options, &mount_opts); po_destroy(options); -- 2.30.0 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 853 bytes --] ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH nfs-utils v2] mount: fix parsing of default options 2021-02-01 5:00 ` [PATCH nfs-utils v2] " NeilBrown @ 2021-02-02 17:00 ` Steve Dickson 0 siblings, 0 replies; 6+ messages in thread From: Steve Dickson @ 2021-02-02 17:00 UTC (permalink / raw) To: NeilBrown, Linux NFS Mailing list On 2/1/21 12:00 AM, NeilBrown wrote: > > A recent patch to change configfile.c to use parse_opt.c contained code > which was intended to remove all "default*" options from the list before > that could be passed to the kernel. This code didn't work, so default* > options WERE passed to the kernel, and the kernel complained and failed > the mount attempt. > > A more recent patch attempted to fix this by not including the > "default*" options in the option list at all. This resulting in > global-default defaults over-riding per-mount or per-server defaults. > > This patch reverse the "more recent" patch, and fixes the original patch > by providing correct code to remove all "default*" options before the > kernel can see them. > > Fixes: 88c22f924f1b ("mount: convert configfile.c to use parse_opt.c") > Fixes: 8142542bda28 ("mount: parse default values correctly") > Signed-off-by: NeilBrown <neilb@suse.de> Committed.... (tag: nfs-utils-2-5-3-rc5) steved. > --- > > I realized that po_remove_all() could free 'ptr' and then compare it > against the next option, which would have undefined results. > So best to strdup and free it. > > > utils/mount/configfile.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c > index e865998dd5a9..3d3684efa186 100644 > --- a/utils/mount/configfile.c > +++ b/utils/mount/configfile.c > @@ -277,10 +277,9 @@ 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); > } > @@ -335,7 +334,11 @@ char *conf_get_mntopts(char *spec, char *mount_point, > * Strip out defaults, which have already been handled, > * then join the rest and return. > */ > - po_remove_all(options, "default"); > + while (po_contains_prefix(options, "default", &ptr, 0) == PO_FOUND) { > + ptr = strdup(ptr); > + po_remove_all(options, ptr); > + free(ptr); > + } > > po_join(options, &mount_opts); > po_destroy(options); > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-02 17:04 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-06 18:40 [PATCH] mount: parse default values correctly Steve Dickson 2021-01-07 16:03 ` Steve Dickson 2021-01-28 23:39 ` NeilBrown 2021-01-28 23:40 ` [PATCH nfs-utils] mount: fix parsing of default options NeilBrown 2021-02-01 5:00 ` [PATCH nfs-utils v2] " NeilBrown 2021-02-02 17:00 ` Steve Dickson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.