2010-01-05 01:34:11

by Dan McGee

[permalink] [raw]
Subject: [PATCH] showmount: try v3 before falling back to v1

A lot of people don't have anything below v3 enabled, so showmount is
completely unusable. Try v3 {tcp, udp} first; if they don't work, fall back
to v1 {tcp, udp}; if those don't work then just fail as before.

Signed-off-by: Dan McGee <[email protected]>
---

First (and quick) attempt at a patch here for showmount. Let me know if you see
serious problems with it or the approach. It seemed relatively sane to me and
fixed my problems after brief testing.

See a report like this for some proof this is an issue in the wild:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=557954

Thanks,

-Dan

utils/showmount/showmount.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/utils/showmount/showmount.c b/utils/showmount/showmount.c
index 418e8b9..716c06d 100644
--- a/utils/showmount/showmount.c
+++ b/utils/showmount/showmount.c
@@ -96,6 +96,14 @@ static CLIENT *nfs_get_mount_client(const char *hostname)
rpcprog_t program = nfs_getrpcbyname(MOUNTPROG, nfs_sm_pgmtbl);
CLIENT *client;

+ client = clnt_create(hostname, program, MOUNTVERS_NFSV3, "tcp");
+ if (client)
+ return client;
+
+ client = clnt_create(hostname, program, MOUNTVERS_NFSV3, "udp");
+ if (client)
+ return client;
+
client = clnt_create(hostname, program, MOUNTVERS, "tcp");
if (client)
return client;
--
1.6.6



2010-01-07 17:11:48

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] showmount: try v3 before falling back to v1

Addressed the not freeing of the client handle when the RPC_PROGVERSMISMATCH
error occurs. Also discovered there no hang when the server does not
support TCP connections due to the fact the clnt_create() quires the
server on what transports are supported...

steved.


Author: Steve Dickson <[email protected]>
Date: Thu Jan 7 11:30:03 2010 -0500

showmount: Try the highest mount version then fall back to lower ones

Showmount should try the highest mount version first then fall
back to the lower ones when the server returns a RPC_PROGVERSMISMATCH
error. The idea being not using the lower mount versions will begin
the process of moving away from NFSv2 support.

Signed-off-by: Steve Dickson <[email protected]>

diff --git a/utils/showmount/showmount.c b/utils/showmount/showmount.c
index 418e8b9..80fd47a 100644
--- a/utils/showmount/showmount.c
+++ b/utils/showmount/showmount.c
@@ -78,29 +78,36 @@ static void usage(FILE *fp, int n)
exit(n);
}

-static const char *nfs_sm_pgmtbl[] = {
+static const char *mount_pgm_tbl[] = {
"showmount",
"mount",
"mountd",
NULL,
};

+static const rpcvers_t mount_vers_tbl[] = {
+ MOUNTVERS_NFSV3,
+ MOUNTVERS_POSIX,
+ MOUNTVERS,
+};
+static const int max_vers_tblsz =
+ (sizeof(mount_vers_tbl)/sizeof(mount_vers_tbl[0]));
+
/*
* Generate an RPC client handle connected to the mountd service
* at @hostname, or die trying.
*
* Supports both AF_INET and AF_INET6 server addresses.
*/
-static CLIENT *nfs_get_mount_client(const char *hostname)
+static CLIENT *nfs_get_mount_client(const char *hostname, rpcvers_t vers)
{
- rpcprog_t program = nfs_getrpcbyname(MOUNTPROG, nfs_sm_pgmtbl);
+ rpcprog_t program = nfs_getrpcbyname(MOUNTPROG, mount_pgm_tbl);
CLIENT *client;

- client = clnt_create(hostname, program, MOUNTVERS, "tcp");
+ client = clnt_create(hostname, program, vers, "tcp");
if (client)
return client;
-
- client = clnt_create(hostname, program, MOUNTVERS, "udp");
+ client = clnt_create(hostname, program, vers, "udp");
if (client)
return client;

@@ -123,6 +130,7 @@ int main(int argc, char **argv)
int i;
int n;
int maxlen;
+ int unsigned vers=0;
char **dumpv;

program_name = argv[0];
@@ -185,7 +193,8 @@ int main(int argc, char **argv)
break;
}

- mclient = nfs_get_mount_client(hostname);
+again:
+ mclient = nfs_get_mount_client(hostname, mount_vers_tbl[vers]);
mclient->cl_auth = authunix_create_default();
total_timeout.tv_sec = TOTAL_TIMEOUT;
total_timeout.tv_usec = 0;
@@ -197,6 +206,12 @@ int main(int argc, char **argv)
(xdrproc_t) xdr_void, NULL,
(xdrproc_t) xdr_exports, (caddr_t) &exportlist,
total_timeout);
+ if (clnt_stat == RPC_PROGVERSMISMATCH) {
+ if (++vers < max_vers_tblsz) {
+ clnt_destroy(mclient);
+ goto again;
+ }
+ }
if (clnt_stat != RPC_SUCCESS) {
clnt_perror(mclient, "rpc mount export");
clnt_destroy(mclient);
@@ -232,6 +247,12 @@ int main(int argc, char **argv)
(xdrproc_t) xdr_void, NULL,
(xdrproc_t) xdr_mountlist, (caddr_t) &dumplist,
total_timeout);
+ if (clnt_stat == RPC_PROGVERSMISMATCH) {
+ if (++vers < max_vers_tblsz) {
+ clnt_destroy(mclient);
+ goto again;
+ }
+ }
if (clnt_stat != RPC_SUCCESS) {
clnt_perror(mclient, "rpc mount dump");
clnt_destroy(mclient);


2010-01-05 17:36:28

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH] showmount: try v3 before falling back to v1


On Jan 4, 2010, at 8:34 PM, Dan McGee wrote:

> A lot of people don't have anything below v3 enabled, so showmount is
> completely unusable. Try v3 {tcp, udp} first; if they don't work,
> fall back
> to v1 {tcp, udp}; if those don't work then just fail as before.

I don't see any immediate problems with this.

It might be nice to add a command line option to select which
transport to use, but that's entirely optional.

> Signed-off-by: Dan McGee <[email protected]>
> ---
>
> First (and quick) attempt at a patch here for showmount. Let me know
> if you see
> serious problems with it or the approach. It seemed relatively sane
> to me and
> fixed my problems after brief testing.
>
> See a report like this for some proof this is an issue in the wild:
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=557954
>
> Thanks,
>
> -Dan
>
> utils/showmount/showmount.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/utils/showmount/showmount.c b/utils/showmount/showmount.c
> index 418e8b9..716c06d 100644
> --- a/utils/showmount/showmount.c
> +++ b/utils/showmount/showmount.c
> @@ -96,6 +96,14 @@ static CLIENT *nfs_get_mount_client(const char
> *hostname)
> rpcprog_t program = nfs_getrpcbyname(MOUNTPROG, nfs_sm_pgmtbl);
> CLIENT *client;
>
> + client = clnt_create(hostname, program, MOUNTVERS_NFSV3, "tcp");
> + if (client)
> + return client;
> +
> + client = clnt_create(hostname, program, MOUNTVERS_NFSV3, "udp");
> + if (client)
> + return client;
> +
> client = clnt_create(hostname, program, MOUNTVERS, "tcp");
> if (client)
> return client;
> --
> 1.6.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





2010-01-05 18:38:22

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] showmount: try v3 before falling back to v1



On 01/05/2010 12:31 PM, Chuck Lever wrote:
>
> On Jan 4, 2010, at 8:34 PM, Dan McGee wrote:
>
>> A lot of people don't have anything below v3 enabled, so showmount is
>> completely unusable. Try v3 {tcp, udp} first; if they don't work, fall
>> back
>> to v1 {tcp, udp}; if those don't work then just fail as before.
>
> I don't see any immediate problems with this.
>
Well I do have a bz request that we stop using the lower mount
version so we can move away from NFSv2 support... so here is
my version of this patch...

Comments??

steved.


Author: Steve Dickson <[email protected]>
Date: Tue Jan 5 13:29:07 2010 -0500

showmount: Try the highest mount version then fall back to lower ones

Showmount should try the highest mount version first then fall
back to the lower ones when the server returns a RPC_PROGVERSMISMATCH
error. The idea being not using the lower mount versions will begin
the process of moving away from NFSv2 support.

Signed-off-by: Steve Dickson <[email protected]>

diff --git a/utils/showmount/showmount.c b/utils/showmount/showmount.c
index 418e8b9..17224a6 100644
--- a/utils/showmount/showmount.c
+++ b/utils/showmount/showmount.c
@@ -85,22 +85,29 @@ static const char *nfs_sm_pgmtbl[] = {
NULL,
};

+static const int mount_vers[] = {
+ MOUNTVERS_NFSV3,
+ MOUNTVERS_POSIX,
+ MOUNTVERS,
+};
+static const int max_vers = (sizeof(mount_vers)/sizeof(mount_vers[0]));
+
/*
* Generate an RPC client handle connected to the mountd service
* at @hostname, or die trying.
*
* Supports both AF_INET and AF_INET6 server addresses.
*/
-static CLIENT *nfs_get_mount_client(const char *hostname)
+static CLIENT *nfs_get_mount_client(const char *hostname, int vers)
{
rpcprog_t program = nfs_getrpcbyname(MOUNTPROG, nfs_sm_pgmtbl);
CLIENT *client;

- client = clnt_create(hostname, program, MOUNTVERS, "tcp");
+ client = clnt_create(hostname, program, vers, "tcp");
if (client)
return client;

- client = clnt_create(hostname, program, MOUNTVERS, "udp");
+ client = clnt_create(hostname, program, vers, "udp");
if (client)
return client;

@@ -122,7 +129,7 @@ int main(int argc, char **argv)
mountlist list;
int i;
int n;
- int maxlen;
+ int maxlen, vers=0;
char **dumpv;

program_name = argv[0];
@@ -185,7 +192,8 @@ int main(int argc, char **argv)
break;
}

- mclient = nfs_get_mount_client(hostname);
+again:
+ mclient = nfs_get_mount_client(hostname, mount_vers[vers]);
mclient->cl_auth = authunix_create_default();
total_timeout.tv_sec = TOTAL_TIMEOUT;
total_timeout.tv_usec = 0;
@@ -197,6 +205,10 @@ int main(int argc, char **argv)
(xdrproc_t) xdr_void, NULL,
(xdrproc_t) xdr_exports, (caddr_t) &exportlist,
total_timeout);
+ if (clnt_stat == RPC_PROGVERSMISMATCH) {
+ if (++vers < max_vers)
+ goto again;
+ }
if (clnt_stat != RPC_SUCCESS) {
clnt_perror(mclient, "rpc mount export");
clnt_destroy(mclient);
@@ -232,6 +244,10 @@ int main(int argc, char **argv)
(xdrproc_t) xdr_void, NULL,
(xdrproc_t) xdr_mountlist, (caddr_t) &dumplist,
total_timeout);
+ if (clnt_stat == RPC_PROGVERSMISMATCH) {
+ if (++vers < max_vers)
+ goto again;
+ }
if (clnt_stat != RPC_SUCCESS) {
clnt_perror(mclient, "rpc mount dump");
clnt_destroy(mclient);

2010-01-05 20:41:21

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] showmount: try v3 before falling back to v1

Revised patch incorporating the code review comments...

steved.

commit a96b79f57f49ce5b4d05b6b9da79bdec03b13764
Author: Steve Dickson <[email protected]>
Date: Tue Jan 5 15:39:00 2010 -0500

showmount: Try the highest mount version then fall back to lower ones

Showmount should try the highest mount version first then fall
back to the lower ones when the server returns a RPC_PROGVERSMISMATCH
error. The idea being not using the lower mount versions will begin
the process of moving away from NFSv2 support.

Signed-off-by: Steve Dickson <[email protected]>

diff --git a/utils/showmount/showmount.c b/utils/showmount/showmount.c
index 418e8b9..74cf116 100644
--- a/utils/showmount/showmount.c
+++ b/utils/showmount/showmount.c
@@ -78,29 +78,36 @@ static void usage(FILE *fp, int n)
exit(n);
}

-static const char *nfs_sm_pgmtbl[] = {
+static const char *mount_pgm_tbl[] = {
"showmount",
"mount",
"mountd",
NULL,
};

+static const rpcvers_t mount_vers_tbl[] = {
+ MOUNTVERS_NFSV3,
+ MOUNTVERS_POSIX,
+ MOUNTVERS,
+};
+static const int max_vers_tblsz =
+ (sizeof(mount_vers_tbl)/sizeof(mount_vers_tbl[0]));
+
/*
* Generate an RPC client handle connected to the mountd service
* at @hostname, or die trying.
*
* Supports both AF_INET and AF_INET6 server addresses.
*/
-static CLIENT *nfs_get_mount_client(const char *hostname)
+static CLIENT *nfs_get_mount_client(const char *hostname, rpcvers_t vers)
{
- rpcprog_t program = nfs_getrpcbyname(MOUNTPROG, nfs_sm_pgmtbl);
+ rpcprog_t program = nfs_getrpcbyname(MOUNTPROG, mount_pgm_tbl);
CLIENT *client;

- client = clnt_create(hostname, program, MOUNTVERS, "tcp");
+ client = clnt_create(hostname, program, vers, "tcp");
if (client)
return client;
-
- client = clnt_create(hostname, program, MOUNTVERS, "udp");
+ client = clnt_create(hostname, program, vers, "udp");
if (client)
return client;

@@ -123,6 +130,7 @@ int main(int argc, char **argv)
int i;
int n;
int maxlen;
+ int unsigned vers=0;
char **dumpv;

program_name = argv[0];
@@ -185,7 +193,8 @@ int main(int argc, char **argv)
break;
}

- mclient = nfs_get_mount_client(hostname);
+again:
+ mclient = nfs_get_mount_client(hostname, mount_vers_tbl[vers]);
mclient->cl_auth = authunix_create_default();
total_timeout.tv_sec = TOTAL_TIMEOUT;
total_timeout.tv_usec = 0;
@@ -197,6 +206,10 @@ int main(int argc, char **argv)
(xdrproc_t) xdr_void, NULL,
(xdrproc_t) xdr_exports, (caddr_t) &exportlist,
total_timeout);
+ if (clnt_stat == RPC_PROGVERSMISMATCH) {
+ if (++vers < max_vers_tblsz)
+ goto again;
+ }
if (clnt_stat != RPC_SUCCESS) {
clnt_perror(mclient, "rpc mount export");
clnt_destroy(mclient);
@@ -232,6 +245,10 @@ int main(int argc, char **argv)
(xdrproc_t) xdr_void, NULL,
(xdrproc_t) xdr_mountlist, (caddr_t) &dumplist,
total_timeout);
+ if (clnt_stat == RPC_PROGVERSMISMATCH) {
+ if (++vers < max_vers_tblsz)
+ goto again;
+ }
if (clnt_stat != RPC_SUCCESS) {
clnt_perror(mclient, "rpc mount dump");
clnt_destroy(mclient);