Message ID | 1312625496-2496-1-git-send-email-luk@debian.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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
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
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 --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)
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(-)