Jamie Heilman wrote:
> Jamie Heilman wrote:
> > Michael Tokarev wrote:
> > > On 04.06.2012 08:13, Jamie Heilman wrote:
> > > > Michael Tokarev wrote:
> > > >> Overall, the code quality is very very low, I'm not sure
> > > >> it is possible to maintain this package without very
> > > >> serious work with upstream first.
> > > >
> > > > I took a stab at a slightly less annoying workaround, and found that
> > > > with 5.0.6-2 using a map of "jamie -fstype=nfs4 canarsie:/home/jamie"
> > > > worked too. Reviewing the code, I'm actually kinda surprised it
> > > > worked, but the more I try to follow the logic, the more fragile
> > > > everything seems to get.
> > >
> > > I had a discussion with upstream about this. It looks like the code
> > > works correctly when specifying -fstype=nfs4 OR specifying -port=2049.
> > > In either case automount does not try to contact the portmapper, and
> > > works instantly.
> > >
> > > On the other hand, -vers=4 does NOT work, because automount only checks
> > > for -fstype and -port, but not -vers. This is a defect in autofs, but
> > > it is a small defect. I think it can be made to work with -vers=4 too
> > > the same way it works with -fstype=nfs4, that should be easy.
> >
> > Yeah, I'm testing a patch for this atm...
>
> OK, here's 3 patches, one for fixing the regression, one for fixing
> behavior of port= options, and some documentation updates.
... and here's a quick respin of patch 2 that doesn't introduce new
compiler warnings. :-P Sorry, I shoulda checked that more carefully.
--
Jamie Heilman http://audible.transient.net/~jamie/
>From bf745c18cc19f8eadffb7d236d17b1f6ac09a8cd Mon Sep 17 00:00:00 2001
From: Jamie Heilman <ja...@audible.transient.net>
Date: Sat, 16 Jun 2012 00:33:00 +0000
Subject: [PATCH] mount_nfs.so: fix port=0 option behavior v2
With NFS v4, the only time mount.nfs attempts to ask rpcbind for the
nfs server port is if the port option is explicitly set to 0, whereas,
with v3 and v2, consulting rpcbind is also done if no explicit port
was given. The automounter's behavior should mimic that of mount.
This also fixes a problem where specifying a mountport= option
(v2/v3) would be mistaken for the port= option by sloppy parsing.
---
include/replicated.h | 2 +-
modules/mount_nfs.c | 35 +++++++----
modules/replicated.c | 156 ++++++++++++++++++++++----------------------------
3 files changed, 93 insertions(+), 100 deletions(-)
diff --git a/include/replicated.h b/include/replicated.h
index a143ccf..ff0e7b9 100644
--- a/include/replicated.h
+++ b/include/replicated.h
@@ -70,7 +70,7 @@ struct host {
void seed_random(void);
void free_host_list(struct host **);
int parse_location(unsigned, struct host **, const char *, unsigned int);
-int prune_host_list(unsigned, struct host **, unsigned int, const char *);
+int prune_host_list(unsigned, struct host **, unsigned int, int);
void dump_host_list(struct host *);
#endif
diff --git a/modules/mount_nfs.c b/modules/mount_nfs.c
index 0d149e9..9b8e5f1 100644
--- a/modules/mount_nfs.c
+++ b/modules/mount_nfs.c
@@ -63,11 +63,13 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
struct host *this, *hosts = NULL;
unsigned int mount_default_proto, vers;
char *nfsoptions = NULL;
+ const char *port_opt = NULL;
unsigned int flags = ap->flags &
(MOUNT_FLAG_RANDOM_SELECT | MOUNT_FLAG_USE_WEIGHT_ONLY);
int nobind = ap->flags & MOUNT_FLAG_NOBIND;
int len, status, err, existed = 1;
int nosymlink = 0;
+ int port = -1;
int ro = 0; /* Set if mount bind should be read-only */
if (ap->flags & MOUNT_FLAG_REMOUNT)
@@ -134,6 +136,17 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
if (strncmp("vers=4", cp, o_len) == 0 ||
strncmp("nfsvers=4", cp, o_len) == 0)
vers = NFS4_VERS_MASK | TCP_SUPPORTED;
+ else if (strstr(cp, "port=") == cp &&
+ o_len - 5 < 25) {
+ char optport[25];
+
+ strncpy(optport, cp + 5, o_len - 5);
+ optport[o_len - 5] = '\0';
+ port = atoi(optport);
+ if (port < 0)
+ port = 0;
+ port_opt = cp;
+ }
/* Check for options that also make sense
with bind mounts */
else if (strncmp("ro", cp, o_len) == 0)
@@ -153,7 +166,7 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
info(ap->logopt, MODPREFIX "no hosts available");
return 1;
}
- prune_host_list(ap->logopt, &hosts, vers, nfsoptions);
+ prune_host_list(ap->logopt, &hosts, vers, port);
if (!hosts) {
info(ap->logopt, MODPREFIX "no hosts available");
@@ -186,18 +199,18 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
if (!status)
existed = 0;
+ /*
+ * If any *port= option is specified, then we don't want
+ * a bind mount. Use the "port" option if you want to
+ * avoid attempting a local bind mount, such as when
+ * tunneling NFS via localhost.
+ */
+ if (nfsoptions && *nfsoptions && !port_opt)
+ port_opt = strstr(nfsoptions, "port=");
+
this = hosts;
while (this) {
- char *loc, *port_opt = NULL;
-
- /*
- * If the "port" option is specified, then we don't want
- * a bind mount. Use the "port" option if you want to
- * avoid attempting a local bind mount, such as when
- * tunneling NFS via localhost.
- */
- if (nfsoptions && *nfsoptions)
- port_opt = strstr(nfsoptions, "port=");
+ char *loc;
/* Port option specified, don't try to bind */
if (!(nosymlink || nobind) &&
diff --git a/modules/replicated.c b/modules/replicated.c
index d80eda5..753121b 100644
--- a/modules/replicated.c
+++ b/modules/replicated.c
@@ -488,42 +488,10 @@ void free_host_list(struct host **list)
*list = NULL;
}
-static unsigned short get_port_option(const char *options)
-{
- const char *start;
- long port = 0;
-
- if (!options)
- return NFS_PORT;
-
- start = strstr(options, "port=");
- if (!start)
- port = NFS_PORT;
- else {
- char optport[30], *opteq, *end;
- int len;
-
- end = strchr(start, ',');
- len = end ? end - start : strlen(start);
- strncpy(optport, start, len);
- optport[len] = '\0';
- opteq = strchr(optport, '=');
- if (opteq)
- port = atoi(opteq + 1);
- }
-
- if (port < 0)
- port = 0;
-
- return (unsigned short) port;
-}
-
static unsigned int get_nfs_info(unsigned logopt, struct host *host,
struct conn_info *pm_info, struct conn_info *rpc_info,
- const char *proto, unsigned int version,
- const char *options)
+ const char *proto, unsigned int version, int port)
{
- char *have_port_opt = options ? strstr(options, "port=") : NULL;
unsigned int random_selection = host->options & MOUNT_FLAG_RANDOM_SELECT;
unsigned int use_weight_only = host->options & MOUNT_FLAG_USE_WEIGHT_ONLY;
socklen_t len = INET6_ADDRSTRLEN;
@@ -544,34 +512,64 @@ static unsigned int get_nfs_info(unsigned logopt, struct host *host,
"called for host %s proto %s version 0x%x",
host->name, proto, version);
- memset(&parms, 0, sizeof(struct pmap));
-
- parms.pm_prog = NFS_PROGRAM;
-
/* Try to prode UDP first to conserve socket space */
rpc_info->proto = getprotobyname(proto);
if (!rpc_info->proto)
return 0;
+ memset(&parms, 0, sizeof(struct pmap));
+
+ parms.pm_prog = NFS_PROGRAM;
+ parms.pm_prot = rpc_info->proto->p_proto;
+
+ if ((!(version & NFS4_REQUESTED) && port <= 0) ||
+ (version & NFS4_REQUESTED && port == 0)) {
+ status = rpc_portmap_getclient(pm_info,
+ host->name, host->addr, host->addr_len,
+ proto, RPC_CLOSE_DEFAULT);
+ if (status == -EHOSTUNREACH) {
+ supported = status;
+ goto done_ver;
+ } else if (status)
+ goto done_ver;
+ }
+
if (!(version & NFS4_REQUESTED))
goto v3_ver;
- if (!(rpc_info->port = get_port_option(options)))
- goto v3_ver;
+ if (port < 0)
+ rpc_info->port = NFS_PORT;
+ else if (port > 0)
+ rpc_info->port = port;
+ else {
+ parms.pm_vers = NFS4_VERSION;
+ status = rpc_portmap_getport(pm_info, &parms, &rpc_info->port);
+ if (status == -EHOSTUNREACH || status == -ETIMEDOUT) {
+ supported = status;
+ goto done_ver;
+ } else if (status < 0) {
+ if (version & NFS_VERS_MASK)
+ goto v3_ver; /* MOUNT_NFS_DEFAULT_PROTOCOL=4 */
+ else
+ goto done_ver;
+ }
+ }
if (rpc_info->proto->p_proto == IPPROTO_UDP)
status = rpc_udp_getclient(rpc_info, NFS_PROGRAM, NFS4_VERSION);
else
status = rpc_tcp_getclient(rpc_info, NFS_PROGRAM, NFS4_VERSION);
- if (status == -EHOSTUNREACH)
- return (unsigned int) status;
- else if (!status) {
+ if (status == -EHOSTUNREACH) {
+ supported = status;
+ goto done_ver;
+ } else if (!status) {
gettimeofday(&start, &tz);
status = rpc_ping_proto(rpc_info);
gettimeofday(&end, &tz);
- if (status == -ETIMEDOUT)
- return (unsigned int) status;
- else if (status > 0) {
+ if (status == -ETIMEDOUT) {
+ supported = status;
+ goto done_ver;
+ } else if (status > 0) {
double reply;
if (random_selection) {
/* Random value between 0 and 1 */
@@ -592,25 +590,12 @@ static unsigned int get_nfs_info(unsigned logopt, struct host *host,
goto done_ver;
v3_ver:
- if (!have_port_opt) {
- status = rpc_portmap_getclient(pm_info,
- host->name, host->addr, host->addr_len,
- proto, RPC_CLOSE_DEFAULT);
- if (status == -EHOSTUNREACH) {
- supported = status;
- goto done_ver;
- } else if (status)
- goto done_ver;
- }
-
if (!(version & NFS3_REQUESTED))
goto v2_ver;
- if (have_port_opt) {
- if (!(rpc_info->port = get_port_option(options)))
- goto done_ver;
- } else {
- parms.pm_prot = rpc_info->proto->p_proto;
+ if (port > 0)
+ rpc_info->port = port;
+ else {
parms.pm_vers = NFS3_VERSION;
status = rpc_portmap_getport(pm_info, &parms, &rpc_info->port);
if (status == -EHOSTUNREACH || status == -ETIMEDOUT) {
@@ -655,11 +640,9 @@ v2_ver:
if (!(version & NFS2_REQUESTED))
goto done_ver;
- if (have_port_opt) {
- if (!(rpc_info->port = get_port_option(options)))
- goto done_ver;
- } else {
- parms.pm_prot = rpc_info->proto->p_proto;
+ if (port > 0)
+ rpc_info->port = port;
+ else {
parms.pm_vers = NFS2_VERSION;
status = rpc_portmap_getport(pm_info, &parms, &rpc_info->port);
if (status == -EHOSTUNREACH || status == -ETIMEDOUT) {
@@ -730,7 +713,7 @@ done_ver:
}
static int get_vers_and_cost(unsigned logopt, struct host *host,
- unsigned int version, const char *options)
+ unsigned int version, int port)
{
struct conn_info pm_info, rpc_info;
time_t timeout = RPC_TIMEOUT;
@@ -757,7 +740,7 @@ static int get_vers_and_cost(unsigned logopt, struct host *host,
if (version & TCP_REQUESTED) {
supported = get_nfs_info(logopt, host,
- &pm_info, &rpc_info, "tcp", vers, options);
+ &pm_info, &rpc_info, "tcp", vers, port);
if (IS_ERR(supported)) {
if (ERR(supported) == EHOSTUNREACH ||
ERR(supported) == ETIMEDOUT)
@@ -770,7 +753,7 @@ static int get_vers_and_cost(unsigned logopt, struct host *host,
if (version & UDP_REQUESTED) {
supported = get_nfs_info(logopt, host,
- &pm_info, &rpc_info, "udp", vers, options);
+ &pm_info, &rpc_info, "udp", vers, port);
if (IS_ERR(supported)) {
if (ERR(supported) == ETIMEDOUT)
return ret;
@@ -784,22 +767,20 @@ static int get_vers_and_cost(unsigned logopt, struct host *host,
}
static int get_supported_ver_and_cost(unsigned logopt, struct host *host,
- unsigned int version, const char *options)
+ unsigned int version, int port)
{
- char *have_port_opt = options ? strstr(options, "port=") : NULL;
unsigned int random_selection = host->options & MOUNT_FLAG_RANDOM_SELECT;
unsigned int use_weight_only = host->options & MOUNT_FLAG_USE_WEIGHT_ONLY;
socklen_t len = INET6_ADDRSTRLEN;
char buf[len + 1];
struct conn_info pm_info, rpc_info;
- struct pmap parms;
const char *proto;
unsigned int vers;
struct timeval start, end;
struct timezone tz;
double taken = 0;
time_t timeout = RPC_TIMEOUT;
- int status;
+ int status = 0;
if (host->addr)
debug(logopt, "called with host %s(%s) version 0x%x",
@@ -811,7 +792,6 @@ static int get_supported_ver_and_cost(unsigned logopt, struct host *host,
memset(&pm_info, 0, sizeof(struct conn_info));
memset(&rpc_info, 0, sizeof(struct conn_info));
- memset(&parms, 0, sizeof(struct pmap));
if (host->proximity == PROXIMITY_NET)
timeout = RPC_TIMEOUT * 2;
@@ -826,8 +806,6 @@ static int get_supported_ver_and_cost(unsigned logopt, struct host *host,
rpc_info.close_option = RPC_CLOSE_DEFAULT;
rpc_info.client = NULL;
- parms.pm_prog = NFS_PROGRAM;
-
/*
* The version passed in is the version as defined in
* include/replicated.h. However, the version we want to send
@@ -859,29 +837,31 @@ static int get_supported_ver_and_cost(unsigned logopt, struct host *host,
if (!rpc_info.proto)
return 0;
- status = 0;
-
- parms.pm_vers = vers;
- if (have_port_opt || (vers & NFS4_VERSION)) {
- if (!(rpc_info.port = get_port_option(options)))
- return 0;
- } else {
+ if (port > 0)
+ rpc_info.port = port;
+ else if (vers & NFS4_VERSION && port < 0)
+ rpc_info.port = NFS_PORT;
+ else {
+ struct pmap parms;
int ret = rpc_portmap_getclient(&pm_info,
host->name, host->addr, host->addr_len,
proto, RPC_CLOSE_DEFAULT);
if (ret)
return 0;
+ memset(&parms, 0, sizeof(struct pmap));
+ parms.pm_prog = NFS_PROGRAM;
parms.pm_prot = rpc_info.proto->p_proto;
+ parms.pm_vers = vers;
ret = rpc_portmap_getport(&pm_info, &parms, &rpc_info.port);
if (ret < 0)
goto done;
}
if (rpc_info.proto->p_proto == IPPROTO_UDP)
- status = rpc_udp_getclient(&rpc_info, NFS_PROGRAM, parms.pm_vers);
+ status = rpc_udp_getclient(&rpc_info, NFS_PROGRAM, vers);
else
- status = rpc_tcp_getclient(&rpc_info, NFS_PROGRAM, parms.pm_vers);
+ status = rpc_tcp_getclient(&rpc_info, NFS_PROGRAM, vers);
if (status == -EHOSTUNREACH)
goto done;
else if (!status) {
@@ -928,7 +908,7 @@ done:
}
int prune_host_list(unsigned logopt, struct host **list,
- unsigned int vers, const char *options)
+ unsigned int vers, int port)
{
struct host *this, *last, *first;
struct host *new = NULL;
@@ -985,7 +965,7 @@ int prune_host_list(unsigned logopt, struct host **list,
break;
if (this->name) {
- status = get_vers_and_cost(logopt, this, vers, options);
+ status = get_vers_and_cost(logopt, this, vers, port);
if (!status) {
if (this == first) {
first = next;
@@ -1094,7 +1074,7 @@ int prune_host_list(unsigned logopt, struct host **list,
add_host(&new, this);
} else {
status = get_supported_ver_and_cost(logopt, this,
- selected_version, options);
+ selected_version, port);
if (status) {
this->version = selected_version;
remove_host(list, this);
--
1.7.10
