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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxxxxxxxx
>>> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
