diff mbox series

mount.nfs: Fix the sloppy option processing

Message ID 20210720212611.332856-1-steved@redhat.com (mailing list archive)
State New, archived
Headers show
Series mount.nfs: Fix the sloppy option processing | expand

Commit Message

Steve Dickson July 20, 2021, 9:26 p.m. UTC
The new mount API broke how the sloppy option is parsed.
So the option processing needs to be moved up in
the mount.nfs command.

The option needs to be the first option in the string
that is passed into the kernel with the -s mount(8)
and/or the -o sloppy is used.

Commit 92b664ef fixed the process of the -s flag
and this version fixes the -o sloppy processing
as well works when libmount-mount is and is not
enabled plus cleans up the mount options passed
to the kernel.

Signed-off-by: Steve Dickson <steved@redhat.com>
---
 utils/mount/nfs.man   |  7 +++++++
 utils/mount/stropts.c | 14 +++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

Comments

David Wysochanski July 22, 2021, 8:02 p.m. UTC | #1
On Tue, Jul 20, 2021 at 5:27 PM Steve Dickson <steved@redhat.com> wrote:
>
> The new mount API broke how the sloppy option is parsed.
> So the option processing needs to be moved up in
> the mount.nfs command.
>
> The option needs to be the first option in the string
> that is passed into the kernel with the -s mount(8)
> and/or the -o sloppy is used.
>
> Commit 92b664ef fixed the process of the -s flag
> and this version fixes the -o sloppy processing
> as well works when libmount-mount is and is not
> enabled plus cleans up the mount options passed
> to the kernel.
>
> Signed-off-by: Steve Dickson <steved@redhat.com>
> ---
>  utils/mount/nfs.man   |  7 +++++++
>  utils/mount/stropts.c | 14 +++++++++++---
>  2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/utils/mount/nfs.man b/utils/mount/nfs.man
> index f98cb47d..f1b76936 100644
> --- a/utils/mount/nfs.man
> +++ b/utils/mount/nfs.man
> @@ -555,6 +555,13 @@ using the FS-Cache facility. See cachefilesd(8)
>  and <kernel_source>/Documentation/filesystems/caching
>  for detail on how to configure the FS-Cache facility.
>  Default value is nofsc.
> +.TP 1.5i
> +.B sloppy
> +The
> +.B sloppy
> +option is an alternative to specifying
> +.BR mount.nfs " -s " option.
> +
>  .SS "Options for NFS versions 2 and 3 only"
>  Use these options, along with the options in the above subsection,
>  for NFS versions 2 and 3 only.
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 82b054a5..fa67a66f 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -339,11 +339,19 @@ static int nfs_verify_lock_option(struct mount_options *options)
>
>  static int nfs_insert_sloppy_option(struct mount_options *options)
>  {
> -       if (!sloppy || linux_version_code() < MAKE_VERSION(2, 6, 27))
> +       if (linux_version_code() < MAKE_VERSION(2, 6, 27))
>                 return 1;
>
> -       if (po_insert(options, "sloppy") == PO_FAILED)
> -               return 0;
> +       if (po_contains(options, "sloppy")) {
> +               po_remove_all(options, "sloppy");
> +               sloppy++;
> +       }
> +
> +       if (sloppy) {
> +               if (po_insert(options, "sloppy") == PO_FAILED)
> +                       return 0;
> +       }
> +
>         return 1;
>  }
>
> --
> 2.31.1
>

Looks good.

Tested with 1-liner, failed before this patch.

# FAIL=0; for o in blah,vers=4.1,sec=sys,sloppy
blah,vers=4.1,sloppy,sec=sys sloppy,blah,vers=4.1,sec=sys
vers=4.1,blah,sec=sys,sloppy vers=4.1,blah,sloppy,sec=sys
vers=4.1,sloppy,blah,sec=sys sloppy,vers=4.1,blah,sec=sys
sloppy,vers=4.1,sec=sys,blah; do ./utils/mount/mount.nfs -o $o
127.0.0.1:/exports /mnt/test; if [ $? -ne 0 ]; then FAIL=1; echo
ERROR: on options $o; break; fi; umount /mnt/test; done; if [ $FAIL
-eq 0 ]; then echo SUCCESS on kernel $(uname -r) nfs-utils at $(git
log --oneline | head -1); fi
mount.nfs: an incorrect mount option was specified
ERROR: on options blah,vers=4.1,sec=sys,sloppy

# FAIL=0; for o in blah,vers=4.1,sec=sys,sloppy
blah,vers=4.1,sloppy,sec=sys sloppy,blah,vers=4.1,sec=sys
vers=4.1,blah,sec=sys,sloppy vers=4.1,blah,sloppy,sec=sys
vers=4.1,sloppy,blah,sec=sys sloppy,vers=4.1,blah,sec=sys
sloppy,vers=4.1,sec=sys,blah; do ./utils/mount/mount.nfs -o $o
127.0.0.1:/exports /mnt/test; if [ $? -ne 0 ]; then FAIL=1; echo
ERROR: on options $o; break; fi; umount /mnt/test; done; if [ $FAIL
-eq 0 ]; then echo SUCCESS on kernel $(uname -r) nfs-utils at $(git
log --oneline | head -1); fi
SUCCESS on kernel 5.14.0-rc2 nfs-utils at d3e53193c6d6 mount.nfs: Fix
the sloppy option processing


Reviewed-and-tested-by: Dave Wysochanski <dwysocha@redhat.com>
Steve Dickson July 26, 2021, 4:31 p.m. UTC | #2
On 7/20/21 5:26 PM, Steve Dickson wrote:
> The new mount API broke how the sloppy option is parsed.
> So the option processing needs to be moved up in
> the mount.nfs command.
> 
> The option needs to be the first option in the string
> that is passed into the kernel with the -s mount(8)
> and/or the -o sloppy is used.
> 
> Commit 92b664ef fixed the process of the -s flag
> and this version fixes the -o sloppy processing
> as well works when libmount-mount is and is not
> enabled plus cleans up the mount options passed
> to the kernel.
> 
> Signed-off-by: Steve Dickson <steved@redhat.com>
Committed... (tag: nfs-utils-2-5-5-rc1)

steved
> ---
>   utils/mount/nfs.man   |  7 +++++++
>   utils/mount/stropts.c | 14 +++++++++++---
>   2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/utils/mount/nfs.man b/utils/mount/nfs.man
> index f98cb47d..f1b76936 100644
> --- a/utils/mount/nfs.man
> +++ b/utils/mount/nfs.man
> @@ -555,6 +555,13 @@ using the FS-Cache facility. See cachefilesd(8)
>   and <kernel_source>/Documentation/filesystems/caching
>   for detail on how to configure the FS-Cache facility.
>   Default value is nofsc.
> +.TP 1.5i
> +.B sloppy
> +The
> +.B sloppy
> +option is an alternative to specifying
> +.BR mount.nfs " -s " option.
> +
>   .SS "Options for NFS versions 2 and 3 only"
>   Use these options, along with the options in the above subsection,
>   for NFS versions 2 and 3 only.
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 82b054a5..fa67a66f 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -339,11 +339,19 @@ static int nfs_verify_lock_option(struct mount_options *options)
>   
>   static int nfs_insert_sloppy_option(struct mount_options *options)
>   {
> -	if (!sloppy || linux_version_code() < MAKE_VERSION(2, 6, 27))
> +	if (linux_version_code() < MAKE_VERSION(2, 6, 27))
>   		return 1;
>   
> -	if (po_insert(options, "sloppy") == PO_FAILED)
> -		return 0;
> +	if (po_contains(options, "sloppy")) {
> +		po_remove_all(options, "sloppy");
> +		sloppy++;
> +	}
> +
> +	if (sloppy) {
> +		if (po_insert(options, "sloppy") == PO_FAILED)
> +			return 0;
> +	}
> +
>   	return 1;
>   }
>   
>
diff mbox series

Patch

diff --git a/utils/mount/nfs.man b/utils/mount/nfs.man
index f98cb47d..f1b76936 100644
--- a/utils/mount/nfs.man
+++ b/utils/mount/nfs.man
@@ -555,6 +555,13 @@  using the FS-Cache facility. See cachefilesd(8)
 and <kernel_source>/Documentation/filesystems/caching
 for detail on how to configure the FS-Cache facility.
 Default value is nofsc.
+.TP 1.5i
+.B sloppy
+The
+.B sloppy
+option is an alternative to specifying
+.BR mount.nfs " -s " option.
+
 .SS "Options for NFS versions 2 and 3 only"
 Use these options, along with the options in the above subsection,
 for NFS versions 2 and 3 only.
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 82b054a5..fa67a66f 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -339,11 +339,19 @@  static int nfs_verify_lock_option(struct mount_options *options)
 
 static int nfs_insert_sloppy_option(struct mount_options *options)
 {
-	if (!sloppy || linux_version_code() < MAKE_VERSION(2, 6, 27))
+	if (linux_version_code() < MAKE_VERSION(2, 6, 27))
 		return 1;
 
-	if (po_insert(options, "sloppy") == PO_FAILED)
-		return 0;
+	if (po_contains(options, "sloppy")) {
+		po_remove_all(options, "sloppy");
+		sloppy++;
+	}
+
+	if (sloppy) {
+		if (po_insert(options, "sloppy") == PO_FAILED)
+			return 0;
+	}
+
 	return 1;
 }