Return-Path: Received: from acsinet15.oracle.com ([141.146.126.227]:45434 "EHLO acsinet15.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752946Ab1HGOvf convert rfc822-to-8bit (ORCPT ); Sun, 7 Aug 2011 10:51:35 -0400 Subject: Re: [PATCH] mount.nfs: Preserve any explicit port=2049 option Content-Type: text/plain; charset=us-ascii From: Chuck Lever In-Reply-To: <4E3D74A9.6070105@debian.org> Date: Sun, 7 Aug 2011 10:51:16 -0400 Cc: Steve Dickson , linux-nfs@vger.kernel.org Message-Id: <74C91E2E-05E4-4919-AD4D-BE735BE3BE8E@oracle.com> References: <1312625496-2496-1-git-send-email-luk@debian.org> <309705ED-3215-493C-BA2B-5CF19CA0BF91@oracle.com> <4E3D74A9.6070105@debian.org> To: Luk Claes Sender: linux-nfs-owner@vger.kernel.org List-ID: MIME-Version: 1.0 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 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 >>> --- >>> 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