On Mon, Sep 13, 2010 at 01:17:36PM -0400, Chuck Lever wrote:
> Clean up.
>
> I'm beginning to agree with Bruce and Steve's assessment that the
> fallthrough switch case in nfs_try_mount() is more difficult to read
> and understand than it needs to be. The logic that manages
> negotiating NFS version and protocol settings is getting more complex
> over time anyway.
>
> So let's split the autonegotiation piece out of nfs_try_mount().
>
> We can reduce indenting, and use cleaner switch-based logic. Also,
> adding more comments can only help.
Looks OK to me, thanks.--b.
>
> Neil also suggested replacing the pre-call "errno = 0" trick. The
> lower-level functions may try to mount several times (given a list of
> addresses to try). errno could be set by any of those. The mount
> request will succeed at some point, and "success" is returned, but
> errno is still set to some non-zero value.
>
> The kernel version check in nfs_try_mount() is more or less loop
> invariant: it's impossible for the result of that test to change
> between retries. So we should be able to safely move it to the logic
> that sets the initial value of mi->version.
>
> This patch is not supposed to cause a behavioral change.
>
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
>
> utils/mount/stropts.c | 67 +++++++++++++++++++++++++++++++++++++++----------
> 1 files changed, 53 insertions(+), 14 deletions(-)
>
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index c82ddfe..c5c4ba1 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -304,6 +304,16 @@ static int nfs_set_version(struct nfsmount_info *mi)
> mi->version = 4;
>
> /*
> + * Before 2.6.32, the kernel NFS client didn't
> + * support "-t nfs vers=4" mounts, so NFS version
> + * 4 cannot be included when autonegotiating
> + * while running on those kernels.
> + */
> + if (mi->version == 0 &&
> + linux_version_code() <= MAKE_VERSION(2, 6, 31))
> + mi->version = 3;
> +
> + /*
> * If we still don't know, check for version-specific
> * mount options.
> */
> @@ -746,6 +756,47 @@ static int nfs_try_mount_v4(struct nfsmount_info *mi)
> }
>
> /*
> + * Handle NFS version and transport protocol
> + * autonegotiation.
> + *
> + * When no version or protocol is specified on the
> + * command line, mount.nfs negotiates with the server
> + * to determine appropriate settings for the new
> + * mount point.
> + *
> + * Returns TRUE if successful, otherwise FALSE.
> + * "errno" is set to reflect the individual error.
> + */
> +static int nfs_autonegotiate(struct nfsmount_info *mi)
> +{
> + int result;
> +
> + result = nfs_try_mount_v4(mi);
> + if (result)
> + return result;
> +
> + switch (errno) {
> + case EPROTONOSUPPORT:
> + /* A clear indication that the server or our
> + * client does not support NFS version 4. */
> + goto fall_back;
> + case ENOENT:
> + /* Legacy Linux servers don't export an NFS
> + * version 4 pseudoroot. */
> + goto fall_back;
> + case EPERM:
> + /* Linux servers prior to 2.6.25 may return
> + * EPERM when NFS version 4 is not supported. */
> + goto fall_back;
> + default:
> + return result;
> + }
> +
> +fall_back:
> + return nfs_try_mount_v3v2(mi);
> +}
> +
> +/*
> * This is a single pass through the fg/bg loop.
> *
> * Returns TRUE if successful, otherwise FALSE.
> @@ -757,20 +808,8 @@ static int nfs_try_mount(struct nfsmount_info *mi)
>
> switch (mi->version) {
> case 0:
> - if (linux_version_code() > MAKE_VERSION(2, 6, 31)) {
> - errno = 0;
> - result = nfs_try_mount_v4(mi);
> - if (errno != EPROTONOSUPPORT) {
> - /*
> - * To deal with legacy Linux servers that don't
> - * automatically export a pseudo root, retry
> - * ENOENT errors using version 3. And for
> - * Linux servers prior to 2.6.25, retry EPERM
> - */
> - if (errno != ENOENT && errno != EPERM)
> - break;
> - }
> - }
> + result = nfs_autonegotiate(mi);
> + break;
> case 2:
> case 3:
> result = nfs_try_mount_v3v2(mi);
>
> --
> 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
--
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
