diff mbox

mount.nfs: Preserve any explicit port=2049 option

Message ID 1312625496-2496-1-git-send-email-luk@debian.org (mailing list archive)
State New, archived
Headers show

Commit Message

Luk Claes Aug. 6, 2011, 10:11 a.m. UTC
If NFS port (2049) is supplied explicitly, don't ignore this setting by requesting it to portmapper again. Thanks to Ben Hutchings <ben@decadent.org.uk> for the patch.

Signed-off-by: Luk Claes <luk@debian.org>
---
 utils/mount/stropts.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Comments

Chuck Lever Aug. 6, 2011, 5:01 p.m. UTC | #1
On Aug 6, 2011, at 6:11 AM, Luk Claes wrote:

> If NFS port (2049) is supplied explicitly, don't ignore this setting by requesting it to portmapper again. Thanks to Ben Hutchings <ben@decadent.org.uk> for the patch.

I'm not clear on what's broken.

> Signed-off-by: Luk Claes <luk@debian.org>
> ---
> utils/mount/stropts.c |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index f1aa503..8b2799c 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -437,8 +437,8 @@ static int nfs_construct_new_options(struct mount_options *options,
> 	if (po_append(options, new_option) == PO_FAILED)
> 		return 0;
> 
> -	po_remove_all(options, "port");
> -	if (nfs_pmap->pm_port != NFS_PORT) {
> +	if(po_remove_all(options, "port") == PO_FOUND ||
> +	   nfs_pmap->pm_port != NFS_PORT) {
> 		snprintf(new_option, sizeof(new_option) - 1,
> 			 "port=%lu", nfs_pmap->pm_port);
> 		if (po_append(options, new_option) == PO_FAILED)
> -- 
> 1.7.5.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luk Claes Aug. 6, 2011, 5:06 p.m. UTC | #2
On 08/06/2011 07:01 PM, Chuck Lever wrote:
> 
> On Aug 6, 2011, at 6:11 AM, Luk Claes wrote:
> 
>> If NFS port (2049) is supplied explicitly, don't ignore this setting by requesting it to portmapper again. Thanks to Ben Hutchings <ben@decadent.org.uk> for the patch.
> 
> I'm not clear on what's broken.

Without the patch, there will be a SunRPC GETPORT call to portmap when
the mount option would be port=2049, while it would not do that call
when port=2050 for instance.

Cheers

Luk

>> Signed-off-by: Luk Claes <luk@debian.org>
>> ---
>> utils/mount/stropts.c |    4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>> index f1aa503..8b2799c 100644
>> --- a/utils/mount/stropts.c
>> +++ b/utils/mount/stropts.c
>> @@ -437,8 +437,8 @@ static int nfs_construct_new_options(struct mount_options *options,
>> 	if (po_append(options, new_option) == PO_FAILED)
>> 		return 0;
>>
>> -	po_remove_all(options, "port");
>> -	if (nfs_pmap->pm_port != NFS_PORT) {
>> +	if(po_remove_all(options, "port") == PO_FOUND ||
>> +	   nfs_pmap->pm_port != NFS_PORT) {
>> 		snprintf(new_option, sizeof(new_option) - 1,
>> 			 "port=%lu", nfs_pmap->pm_port);
>> 		if (po_append(options, new_option) == PO_FAILED)
>> -- 
>> 1.7.5.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever Aug. 7, 2011, 2:51 p.m. UTC | #3
On Aug 6, 2011, at 1:06 PM, Luk Claes wrote:

> On 08/06/2011 07:01 PM, Chuck Lever wrote:
>> 
>> On Aug 6, 2011, at 6:11 AM, Luk Claes wrote:
>> 
>>> If NFS port (2049) is supplied explicitly, don't ignore this setting by requesting it to portmapper again. Thanks to Ben Hutchings <ben@decadent.org.uk> for the patch.
>> 
>> I'm not clear on what's broken.
> 
> Without the patch, there will be a SunRPC GETPORT call to portmap when
> the mount option would be port=2049, while it would not do that call
> when port=2050 for instance.

One nit: please use the standard white space convention for "if ( )".  Thanks!  Otherwise, fix seems OK, but I haven't thought too hard about it.  I wish we had a comprehensive unit test suite for this code.

> Cheers
> 
> Luk
> 
>>> Signed-off-by: Luk Claes <luk@debian.org>
>>> ---
>>> utils/mount/stropts.c |    4 ++--
>>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>>> index f1aa503..8b2799c 100644
>>> --- a/utils/mount/stropts.c
>>> +++ b/utils/mount/stropts.c
>>> @@ -437,8 +437,8 @@ static int nfs_construct_new_options(struct mount_options *options,
>>> 	if (po_append(options, new_option) == PO_FAILED)
>>> 		return 0;
>>> 
>>> -	po_remove_all(options, "port");
>>> -	if (nfs_pmap->pm_port != NFS_PORT) {
>>> +	if(po_remove_all(options, "port") == PO_FOUND ||
>>> +	   nfs_pmap->pm_port != NFS_PORT) {
>>> 		snprintf(new_option, sizeof(new_option) - 1,
>>> 			 "port=%lu", nfs_pmap->pm_port);
>>> 		if (po_append(options, new_option) == PO_FAILED)
>>> -- 
>>> 1.7.5.4
>>> 
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 
>> --
>> Chuck Lever
>> chuck[dot]lever[at]oracle[dot]com
>> 
>> 
>> 
> 

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



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index f1aa503..8b2799c 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -437,8 +437,8 @@  static int nfs_construct_new_options(struct mount_options *options,
 	if (po_append(options, new_option) == PO_FAILED)
 		return 0;
 
-	po_remove_all(options, "port");
-	if (nfs_pmap->pm_port != NFS_PORT) {
+	if(po_remove_all(options, "port") == PO_FOUND ||
+	   nfs_pmap->pm_port != NFS_PORT) {
 		snprintf(new_option, sizeof(new_option) - 1,
 			 "port=%lu", nfs_pmap->pm_port);
 		if (po_append(options, new_option) == PO_FAILED)