Hey Ben,
On 11/17/2014 02:43 PM, Benjamin Coddington wrote:
> Update nfsmount.conf to allow minor version specification, and rearrange
> the autonegotiation logic to agreed upon best behavior. Depending upon the
> combination of values specified within nfsmount.conf and given to mount.nfs,
> the retry behavior of mount.nfs varies according to the general rule of
> defaulting to the most specific setting, with some exceptions. A general
> guide to the expected behavior follows:
Would you mind breaking this patch up into a more digestible patch
series defined by functionality. 199 insertions and 150 deletions
in one patch makes it a bit tough to digest it... at least for me. :-)
>
> ┌────────────────┐
> │ nfsmount.conf ├──────────────┬───────────────────┐
> │ Defaultvers= │ arg option │ attempts: │
> ├────────────────┼──────────────┼───────────────────┤
> │ 4.2 │ not set │ v4.2 v4.1 v4.0 v3 │
> │ 4.2 │ v4 │ v4.2 v4.1 v4.0 │
> │ 4.1 │ not set │ v4.1 v4.0 v3 │
> │ 4.1 │ v4 │ v4.1 v4.0 │
> │ 4 │ not set │ v4.0 v3 │
> │ 4 │ v4 │ v4.0 │
> │ 3 │ not set │ v3 │
> │ any set │ v4.2 │ v4.2 │
> │ any set │ v4.1 │ v4.1 │
> │ any set │ v4 │ v4.0 │
> │ any set │ v3 │ v3 │
> │ not set │ not set │ v4.0 v3 │
> └────────────────┴──────────────┴───────────────────┘
Why is "not set" and "not set" not the same as Defaultvers=4.2?
>
> If built without --enable-mountconfig, then the behavior is the same as if
> nfsmount.conf did not set Defaultvers.
We probably should switch this around so mountconfig is always enabled and
let people disable if they want.
>
> The last case in the above table is a special case of disagreement about how
> to determine the upper bound for a minor number of the v4 protocol. It
> could be argued that mount.nfs should start at the highest available
> protocol version, if that version could be discovered at mount time. For
> now, the upper bound on the value can be modified by setting the values
> within nfs_default_version().
For now lets just hard code the upper bound a 4.2. Minor version don't come
along very often so the recompiling of mount.nfs will not be that
big of deal...
steved.
>
> Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
> ---
> utils/mount/configfile.c | 36 +-----------
> utils/mount/network.c | 122 +++++++++++++++++++--------------------
> utils/mount/network.h | 15 +++++-
> utils/mount/nfsumount.c | 4 +-
> utils/mount/parse_opt.c | 26 ++++++++-
> utils/mount/parse_opt.h | 2 +
> utils/mount/stropts.c | 144 ++++++++++++++++++++++++++++++----------------
> 7 files changed, 199 insertions(+), 150 deletions(-)
>
> diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c
> index 39d3741..0a4cc04 100644
> --- a/utils/mount/configfile.c
> +++ b/utils/mount/configfile.c
> @@ -228,37 +228,8 @@ void free_all(void)
> free(entry);
> }
> }
> -static char *versions[] = {"v2", "v3", "v4", "vers", "nfsvers", NULL};
> -static int
> -check_vers(char *mopt, char *field)
> -{
> - int i, found=0;
> -
> - /*
> - * First check to see if the config setting is one
> - * of the many version settings
> - */
> - for (i=0; versions[i]; i++) {
> - if (strcasestr(field, versions[i]) != NULL) {
> - found++;
> - break;
> - }
> - }
> - if (!found)
> - return 0;
> - /*
> - * It appears the version is being set, now see
> - * if the version appears on the command
> - */
> - for (i=0; versions[i]; i++) {
> - if (strcasestr(mopt, versions[i]) != NULL)
> - return 1;
> - }
> -
> - return 0;
> -}
>
> -unsigned long config_default_vers;
> +struct nfs_version config_default_vers;
> unsigned long config_default_proto;
> extern sa_family_t config_default_family;
>
> @@ -331,11 +302,6 @@ conf_parse_mntopts(char *section, char *arg, char *opts)
> snprintf(buf, BUFSIZ, "%s=", node->field);
> if (opts && strcasestr(opts, buf) != NULL)
> continue;
> - /*
> - * Protocol verions can be set in a number of ways
> - */
> - if (opts && check_vers(opts, node->field))
> - continue;
>
> if (lookup_entry(node->field) != NULL)
> continue;
> diff --git a/utils/mount/network.c b/utils/mount/network.c
> index 4f8c15c..b5ed850 100644
> --- a/utils/mount/network.c
> +++ b/utils/mount/network.c
> @@ -92,9 +92,6 @@ static const char *nfs_version_opttbl[] = {
> "v4",
> "vers",
> "nfsvers",
> - "v4.0",
> - "v4.1",
> - "v4.2",
> NULL,
> };
>
> @@ -1242,71 +1239,69 @@ nfs_nfs_program(struct mount_options *options, unsigned long *program)
> * or FALSE if the option was specified with an invalid value.
> */
> int
> -nfs_nfs_version(struct mount_options *options, unsigned long *version)
> +nfs_nfs_version(struct mount_options *options, struct nfs_version *version)
> {
> - long tmp;
> + char *version_key, *version_val, *cptr;
> + int i, found = 0;
>
> - switch (po_rightmost(options, nfs_version_opttbl)) {
> - case 0: /* v2 */
> - *version = 2;
> - return 1;
> - case 1: /* v3 */
> - *version = 3;
> - return 1;
> - case 2: /* v4 */
> - *version = 4;
> - return 1;
> - case 3: /* vers */
> - switch (po_get_numeric(options, "vers", &tmp)) {
> - case PO_FOUND:
> - if (tmp >= 2 && tmp <= 4) {
> - *version = tmp;
> - return 1;
> - }
> - nfs_error(_("%s: parsing error on 'vers=' option\n"),
> - progname);
> - return 0;
> - case PO_NOT_FOUND:
> - nfs_error(_("%s: parsing error on 'vers=' option\n"),
> - progname);
> - return 0;
> - case PO_BAD_VALUE:
> - nfs_error(_("%s: invalid value for 'vers=' option"),
> - progname);
> - return 0;
> - }
> - case 4: /* nfsvers */
> - switch (po_get_numeric(options, "nfsvers", &tmp)) {
> - case PO_FOUND:
> - if (tmp >= 2 && tmp <= 4) {
> - *version = tmp;
> - return 1;
> - }
> - nfs_error(_("%s: parsing error on 'nfsvers=' option\n"),
> - progname);
> - return 0;
> - case PO_NOT_FOUND:
> - nfs_error(_("%s: parsing error on 'nfsvers=' option\n"),
> - progname);
> - return 0;
> - case PO_BAD_VALUE:
> - nfs_error(_("%s: invalid value for 'nfsvers=' option"),
> - progname);
> - return 0;
> + version->v_mode = V_DEFAULT;
> +
> + for (i = 0; nfs_version_opttbl[i]; i++) {
> + if (po_contains_prefix(options, nfs_version_opttbl[i],
> + &version_key) == PO_FOUND) {
> + found++;
> + break;
> }
> - case 5: /* v4.0 */
> - case 6: /* v4.1 */
> - case 7: /* v4.2 */
> - *version = 4;
> + }
> +
> + if (!found)
> return 1;
> +
> + if (i <= 2 ) {
> + /* v2, v3, v4 */
> + version_val = version_key + 1;
> + version->v_mode = V_SPECIFIC;
> + } else if (i > 2 ) {
> + /* vers=, nfsvers= */
> + version_val = po_get(options, version_key);
> }
>
> - /*
> - * NFS version wasn't specified. The pmap version value
> - * will be filled in later by an rpcbind query in this case.
> - */
> - *version = 0;
> + if (!version_val)
> + goto ret_error;
> +
> + if (!(version->major = strtol(version_val, &cptr, 10)))
> + goto ret_error;
> +
> + if (version->major < 4)
> + version->v_mode = V_SPECIFIC;
> +
> + if (*cptr == '.') {
> + version_val = ++cptr;
> + if (!(version->minor = strtol(version_val, &cptr, 10)) && cptr == version_val)
> + goto ret_error;
> + version->v_mode = V_SPECIFIC;
> + } else if (version->major > 3 && *cptr == '\0')
> + version->v_mode = V_GENERAL;
> +
> + if (*cptr != '\0')
> + goto ret_error;
> +
> return 1;
> +
> +ret_error:
> + if (i <= 2 ) {
> + nfs_error(_("%s: parsing error on 'v' option"),
> + progname);
> + } else if (i == 3 ) {
> + nfs_error(_("%s: parsing error on 'vers=' option"),
> + progname);
> + } else if (i == 4) {
> + nfs_error(_("%s: parsing error on 'nfsvers=' option"),
> + progname);
> + }
> + version->v_mode = V_PARSE_ERR;
> + errno = EINVAL;
> + return 0;
> }
>
> /*
> @@ -1625,10 +1620,13 @@ out_err:
> int nfs_options2pmap(struct mount_options *options,
> struct pmap *nfs_pmap, struct pmap *mnt_pmap)
> {
> + struct nfs_version version;
> +
> if (!nfs_nfs_program(options, &nfs_pmap->pm_prog))
> return 0;
> - if (!nfs_nfs_version(options, &nfs_pmap->pm_vers))
> + if (!nfs_nfs_version(options, &version))
> return 0;
> + nfs_pmap->pm_vers = version.major;
> if (!nfs_nfs_protocol(options, &nfs_pmap->pm_prot))
> return 0;
> if (!nfs_nfs_port(options, &nfs_pmap->pm_port))
> diff --git a/utils/mount/network.h b/utils/mount/network.h
> index d7636d7..9cc5dec 100644
> --- a/utils/mount/network.h
> +++ b/utils/mount/network.h
> @@ -57,9 +57,22 @@ int clnt_ping(struct sockaddr_in *, const unsigned long,
>
> struct mount_options;
>
> +enum {
> + V_DEFAULT = 0,
> + V_GENERAL,
> + V_SPECIFIC,
> + V_PARSE_ERR,
> +};
> +
> +struct nfs_version {
> + unsigned long major;
> + unsigned long minor;
> + int v_mode;
> +};
> +
> int nfs_nfs_proto_family(struct mount_options *options, sa_family_t *family);
> int nfs_mount_proto_family(struct mount_options *options, sa_family_t *family);
> -int nfs_nfs_version(struct mount_options *options, unsigned long *version);
> +int nfs_nfs_version(struct mount_options *options, struct nfs_version *version);
> int nfs_nfs_protocol(struct mount_options *options, unsigned long *protocol);
>
> int nfs_options2pmap(struct mount_options *,
> diff --git a/utils/mount/nfsumount.c b/utils/mount/nfsumount.c
> index 3538d88..de284f2 100644
> --- a/utils/mount/nfsumount.c
> +++ b/utils/mount/nfsumount.c
> @@ -179,10 +179,10 @@ static int nfs_umount_is_vers4(const struct mntentchn *mc)
>
> options = po_split(pmc->m.mnt_opts);
> if (options != NULL) {
> - unsigned long version;
> + struct nfs_version version;
> int rc = nfs_nfs_version(options, &version);
> po_destroy(options);
> - if (rc && version == 4)
> + if (rc && version.major == 4)
> goto out_nfs4;
> }
>
> diff --git a/utils/mount/parse_opt.c b/utils/mount/parse_opt.c
> index 75a0daa..7ba61c4 100644
> --- a/utils/mount/parse_opt.c
> +++ b/utils/mount/parse_opt.c
> @@ -391,7 +391,7 @@ po_return_t po_append(struct mount_options *options, char *str)
> }
>
> /**
> - * po_contains - check for presense of an option in a group
> + * po_contains - check for presence of an option in a group
> * @options: pointer to mount options
> * @keyword: pointer to a C string containing option keyword for which to search
> *
> @@ -410,6 +410,30 @@ po_found_t po_contains(struct mount_options *options, char *keyword)
> }
>
> /**
> + * po_contains_prefix - check for presence of an option matching a prefix
> + * @options: pointer to mount options
> + * @prefix: pointer to prefix to match against a keyword
> + * @keyword: pointer to a C string containing the option keyword if found
> + *
> + * On success, *keyword contains the pointer of the matching option's keyword.
> + */
> +po_found_t po_contains_prefix(struct mount_options *options,
> + const char *prefix, char **keyword)
> +{
> + struct mount_option *option;
> +
> + if (options && prefix) {
> + for (option = options->head; option; option = option->next)
> + if (strncmp(option->keyword, prefix, strlen(prefix)) == 0) {
> + *keyword = option->keyword;
> + return PO_FOUND;
> + }
> + }
> +
> + return PO_NOT_FOUND;
> +}
> +
> +/**
> * po_get - return the value of the rightmost instance of an option
> * @options: pointer to mount options
> * @keyword: pointer to a C string containing option keyword for which to search
> diff --git a/utils/mount/parse_opt.h b/utils/mount/parse_opt.h
> index 5037207..0745e0f 100644
> --- a/utils/mount/parse_opt.h
> +++ b/utils/mount/parse_opt.h
> @@ -45,6 +45,8 @@ po_return_t po_join(struct mount_options *, char **);
>
> po_return_t po_append(struct mount_options *, char *);
> po_found_t po_contains(struct mount_options *, char *);
> +po_found_t po_contains_prefix(struct mount_options *options,
> + const char *prefix, char **keyword);
> char * po_get(struct mount_options *, char *);
> po_found_t po_get_numeric(struct mount_options *,
> char *, long *);
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 2d72d5b..936f65c 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -88,30 +88,44 @@ struct nfsmount_info {
> struct mount_options *options; /* parsed mount options */
> char **extra_opts; /* string for /etc/mtab */
>
> - unsigned long version; /* NFS version */
> + struct nfs_version version; /* NFS version */
> int flags, /* MS_ flags */
> fake, /* actually do the mount? */
> child; /* forked bg child? */
> };
>
> -#ifdef MOUNT_CONFIG
> -static void nfs_default_version(struct nfsmount_info *mi);
>
> static void nfs_default_version(struct nfsmount_info *mi)
> {
> - extern unsigned long config_default_vers;
> +#ifdef MOUNT_CONFIG
> + extern struct nfs_version config_default_vers;
> /*
> * Use the default value set in the config file when
> * the version has not been explicitly set.
> */
> - if (mi->version == 0 && config_default_vers) {
> - if (config_default_vers < 4)
> - mi->version = config_default_vers;
> + if (config_default_vers.v_mode == V_PARSE_ERR) {
> + mi->version.v_mode = V_PARSE_ERR;
> + return;
> }
> -}
> -#else
> -inline void nfs_default_version(__attribute__ ((unused)) struct nfsmount_info *mi) {}
> +
> + if (mi->version.v_mode == V_DEFAULT &&
> + config_default_vers.v_mode != V_DEFAULT) {
> + mi->version.major = config_default_vers.major;
> + mi->version.minor = config_default_vers.minor;
> + return;
> + }
> +
> + if (mi->version.v_mode == V_GENERAL &&
> + config_default_vers.v_mode != V_DEFAULT) {
> + if (mi->version.major == config_default_vers.major)
> + mi->version.minor = config_default_vers.minor;
> + return;
> + }
> +
> #endif /* MOUNT_CONFIG */
> + mi->version.major = 4;
> + mi->version.minor = 0;
> +}
>
> /*
> * Obtain a retry timeout value based on the value of the "retry=" option.
> @@ -300,7 +314,7 @@ static int nfs_set_version(struct nfsmount_info *mi)
> return 0;
>
> if (strncmp(mi->type, "nfs4", 4) == 0)
> - mi->version = 4;
> + mi->version.major = 4;
>
> /*
> * Before 2.6.32, the kernel NFS client didn't
> @@ -308,28 +322,44 @@ static int nfs_set_version(struct nfsmount_info *mi)
> * 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 (mi->version.v_mode == V_DEFAULT &&
> + linux_version_code() <= MAKE_VERSION(2, 6, 31)) {
> + mi->version.major = 3;
> + mi->version.v_mode = V_SPECIFIC;
> + }
>
> /*
> * If we still don't know, check for version-specific
> * mount options.
> */
> - if (mi->version == 0) {
> + if (mi->version.v_mode == V_DEFAULT) {
> if (po_contains(mi->options, "mounthost") ||
> po_contains(mi->options, "mountaddr") ||
> po_contains(mi->options, "mountvers") ||
> - po_contains(mi->options, "mountproto"))
> - mi->version = 3;
> + po_contains(mi->options, "mountproto")) {
> + mi->version.major = 3;
> + mi->version.v_mode = V_SPECIFIC;
> + }
> }
>
> /*
> * If enabled, see if the default version was
> * set in the config file
> */
> - nfs_default_version(mi);
> -
> + if (mi->version.v_mode != V_SPECIFIC) {
> + nfs_default_version(mi);
> + /*
> + * If the version was not specifically set, it will
> + * be set by autonegotiation later, so remove it now:
> + */
> + po_remove_all(mi->options, "v4");
> + po_remove_all(mi->options, "vers");
> + po_remove_all(mi->options, "nfsvers");
> + }
> +
> + if (mi->version.v_mode == V_PARSE_ERR)
> + return 0;
> +
> return 1;
> }
>
> @@ -684,6 +714,7 @@ static int nfs_do_mount_v4(struct nfsmount_info *mi,
> {
> struct mount_options *options = po_dup(mi->options);
> int result = 0;
> + char version_opt[16];
> char *extra_opts = NULL;
>
> if (!options) {
> @@ -691,20 +722,24 @@ static int nfs_do_mount_v4(struct nfsmount_info *mi,
> return result;
> }
>
> - if (mi->version == 0) {
> - if (po_contains(options, "mounthost") ||
> - po_contains(options, "mountaddr") ||
> - po_contains(options, "mountvers") ||
> - po_contains(options, "mountproto")) {
> - /*
> - * Since these mountd options are set assume version 3
> - * is wanted so error out with EPROTONOSUPPORT so the
> - * protocol negation starts with v3.
> - */
> - errno = EPROTONOSUPPORT;
> - goto out_fail;
> - }
> - if (po_append(options, "vers=4") == PO_FAILED) {
> + if (po_contains(options, "mounthost") ||
> + po_contains(options, "mountaddr") ||
> + po_contains(options, "mountvers") ||
> + po_contains(options, "mountproto")) {
> + /*
> + * Since these mountd options are set assume version 3
> + * is wanted so error out with EPROTONOSUPPORT so the
> + * protocol negation starts with v3.
> + */
> + errno = EPROTONOSUPPORT;
> + goto out_fail;
> + }
> +
> + if (mi->version.v_mode != V_SPECIFIC) {
> + snprintf(version_opt, sizeof(version_opt) - 1,
> + "vers=%lu.%lu", mi->version.major, mi->version.minor);
> +
> + if (po_append(options, version_opt) == PO_FAILED) {
> errno = EINVAL;
> goto out_fail;
> }
> @@ -792,14 +827,25 @@ static int nfs_autonegotiate(struct nfsmount_info *mi)
> int result;
>
> result = nfs_try_mount_v4(mi);
> +check_result:
> if (result)
> return result;
>
> -check_errno:
> switch (errno) {
> case EPROTONOSUPPORT:
> /* A clear indication that the server or our
> - * client does not support NFS version 4. */
> + * client does not support NFS version 4 and minor */
> + if (mi->version.v_mode == V_GENERAL &&
> + mi->version.minor == 0)
> + return result;
> + if (mi->version.v_mode != V_SPECIFIC) {
> + if (mi->version.minor > 0) {
> + mi->version.minor--;
> + result = nfs_try_mount_v4(mi);
> + goto check_result;
> + }
> + }
> +
> goto fall_back;
> case ENOENT:
> /* Legacy Linux servers don't export an NFS
> @@ -818,7 +864,7 @@ check_errno:
> /* v4 server seems to be registered now. */
> result = nfs_try_mount_v4(mi);
> if (result == 0 && errno != ECONNREFUSED)
> - goto check_errno;
> + goto check_result;
> }
> return result;
> default:
> @@ -839,19 +885,19 @@ static int nfs_try_mount(struct nfsmount_info *mi)
> {
> int result = 0;
>
> - switch (mi->version) {
> - case 0:
> - result = nfs_autonegotiate(mi);
> - break;
> - case 2:
> - case 3:
> - result = nfs_try_mount_v3v2(mi, FALSE);
> - break;
> - case 4:
> - result = nfs_try_mount_v4(mi);
> - break;
> - default:
> - errno = EIO;
> + switch (mi->version.major) {
> + case 2:
> + case 3:
> + result = nfs_try_mount_v3v2(mi, FALSE);
> + break;
> + case 4:
> + if (mi->version.v_mode != V_SPECIFIC)
> + result = nfs_autonegotiate(mi);
> + else
> + result = nfs_try_mount_v4(mi);
> + break;
> + default:
> + errno = EIO;
> }
>
> return result;
>
--
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
