* [PATCH] NFS: Trim extra slash in v4 nfs_path @ 2016-06-15 19:02 Benjamin Coddington 2016-06-20 15:08 ` Anna Schumaker 0 siblings, 1 reply; 7+ messages in thread From: Benjamin Coddington @ 2016-06-15 19:02 UTC (permalink / raw) To: linux-nfs; +Cc: Trond Myklebust, Anna Schumaker A NFSv4 mount of a subdirectory will show an extra slash (as in 'server://path') in proc's mountinfo which will not match the device name and path. This can cause problems for programs searching for the mount. Fix this by checking for a leading slash in the dentry path, if so trim away any trailing slashes in the device name. Signed-off-by: Benjamin Coddington <bcodding@redhat.com> --- fs/nfs/namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c index c8162c660c44..5551e8ef67fd 100644 --- a/fs/nfs/namespace.c +++ b/fs/nfs/namespace.c @@ -98,7 +98,7 @@ rename_retry: return end; } namelen = strlen(base); - if (flags & NFS_PATH_CANONICAL) { + if (*end == '/') { /* Strip off excess slashes in base string */ while (namelen > 0 && base[namelen - 1] == '/') namelen--; -- 2.5.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] NFS: Trim extra slash in v4 nfs_path 2016-06-15 19:02 [PATCH] NFS: Trim extra slash in v4 nfs_path Benjamin Coddington @ 2016-06-20 15:08 ` Anna Schumaker 2016-06-20 15:38 ` [PATCH] " Benjamin Coddington 0 siblings, 1 reply; 7+ messages in thread From: Anna Schumaker @ 2016-06-20 15:08 UTC (permalink / raw) To: Benjamin Coddington, linux-nfs; +Cc: Trond Myklebust, Anna Schumaker Hi Ben, On 06/15/2016 03:02 PM, Benjamin Coddington wrote: > A NFSv4 mount of a subdirectory will show an extra slash (as in > 'server://path') in proc's mountinfo which will not match the device name > and path. This can cause problems for programs searching for the mount. > Fix this by checking for a leading slash in the dentry path, if so trim > away any trailing slashes in the device name. > > Signed-off-by: Benjamin Coddington <bcodding@redhat.com> > --- > fs/nfs/namespace.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c > index c8162c660c44..5551e8ef67fd 100644 > --- a/fs/nfs/namespace.c > +++ b/fs/nfs/namespace.c > @@ -98,7 +98,7 @@ rename_retry: > return end; > } > namelen = strlen(base); > - if (flags & NFS_PATH_CANONICAL) { > + if (*end == '/') { Are we getting here through nfs_show_devname()? Because it looks like that function is passing 0 for flags instead of NFS_PATH_CANONICAL. Earlier in this function we have a check for: if ((flags & NFS_PATH_CANONICAL) && *end != '/') Should we be checking if NFS_PATH_CANONICAL is set here, too? Thanks, Anna > /* Strip off excess slashes in base string */ > while (namelen > 0 && base[namelen - 1] == '/') > namelen--; > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Trim extra slash in v4 nfs_path 2016-06-20 15:08 ` Anna Schumaker @ 2016-06-20 15:38 ` Benjamin Coddington 2016-07-26 10:58 ` Benjamin Coddington 0 siblings, 1 reply; 7+ messages in thread From: Benjamin Coddington @ 2016-06-20 15:38 UTC (permalink / raw) To: Anna Schumaker; +Cc: linux-nfs, Trond Myklebust On 20 Jun 2016, at 11:08, Anna Schumaker wrote: > Hi Ben, > > On 06/15/2016 03:02 PM, Benjamin Coddington wrote: >> A NFSv4 mount of a subdirectory will show an extra slash (as in >> 'server://path') in proc's mountinfo which will not match the device >> name >> and path. This can cause problems for programs searching for the >> mount. >> Fix this by checking for a leading slash in the dentry path, if so >> trim >> away any trailing slashes in the device name. >> >> Signed-off-by: Benjamin Coddington <bcodding@redhat.com> >> --- >> fs/nfs/namespace.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c >> index c8162c660c44..5551e8ef67fd 100644 >> --- a/fs/nfs/namespace.c >> +++ b/fs/nfs/namespace.c >> @@ -98,7 +98,7 @@ rename_retry: >> return end; >> } >> namelen = strlen(base); >> - if (flags & NFS_PATH_CANONICAL) { >> + if (*end == '/') { > > Are we getting here through nfs_show_devname()? Because it looks like > that function is passing 0 for flags instead of NFS_PATH_CANONICAL. We can get here with or withount NFS_PATH_CANONICAL, through nfs_show_devname() or nfs4_path() or nfs_do_submount(). > > Earlier in this function we have a check for: > if ((flags & NFS_PATH_CANONICAL) && *end != '/') > > Should we be checking if NFS_PATH_CANONICAL is set here, too? I don't think we need it. The "if (flags & NFS_PATH_CANONICAL) {" was meant to cover the case where NFS_PATH_CANONICAL had earlier added a leading '/' to the path, and so would trim away any trailing '/' from the base. So, don't combine server:/ and /path into server://path after adding a leading '/' to the path. The logic here is simplified such that we should _never_ concat these two: a base:/ and a /path, otherwise we'll have server://path. Further confusing the matter is that for v3 mounts, the base is server:/path and the path is '/' (which is added by NFS_PATH_CANONICAL), but for v4 mounts the base is server:/ and the path is /path/to/export Ben > > Thanks, > Anna > >> /* Strip off excess slashes in base string */ >> while (namelen > 0 && base[namelen - 1] == '/') >> namelen--; >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Trim extra slash in v4 nfs_path 2016-06-20 15:38 ` [PATCH] " Benjamin Coddington @ 2016-07-26 10:58 ` Benjamin Coddington 2016-08-01 19:11 ` Anna Schumaker 0 siblings, 1 reply; 7+ messages in thread From: Benjamin Coddington @ 2016-07-26 10:58 UTC (permalink / raw) To: Anna Schumaker; +Cc: linux-nfs, Trond Myklebust Hi Anna, On 20 Jun 2016, at 11:38, Benjamin Coddington wrote: > On 20 Jun 2016, at 11:08, Anna Schumaker wrote: > >> Hi Ben, >> >> On 06/15/2016 03:02 PM, Benjamin Coddington wrote: >>> A NFSv4 mount of a subdirectory will show an extra slash (as in >>> 'server://path') in proc's mountinfo which will not match the device >>> name >>> and path. This can cause problems for programs searching for the >>> mount. >>> Fix this by checking for a leading slash in the dentry path, if so >>> trim >>> away any trailing slashes in the device name. >>> >>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com> >>> --- >>> fs/nfs/namespace.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c >>> index c8162c660c44..5551e8ef67fd 100644 >>> --- a/fs/nfs/namespace.c >>> +++ b/fs/nfs/namespace.c >>> @@ -98,7 +98,7 @@ rename_retry: >>> return end; >>> } >>> namelen = strlen(base); >>> - if (flags & NFS_PATH_CANONICAL) { >>> + if (*end == '/') { >> >> Are we getting here through nfs_show_devname()? Because it looks >> like that function is passing 0 for flags instead of >> NFS_PATH_CANONICAL. > > We can get here with or withount NFS_PATH_CANONICAL, through > nfs_show_devname() or nfs4_path() or nfs_do_submount(). > >> >> Earlier in this function we have a check for: >> if ((flags & NFS_PATH_CANONICAL) && *end != '/') >> >> Should we be checking if NFS_PATH_CANONICAL is set here, too? > > I don't think we need it. > > The "if (flags & NFS_PATH_CANONICAL) {" was meant to cover the case > where > NFS_PATH_CANONICAL had earlier added a leading '/' to the path, and so > would > trim away any trailing '/' from the base. So, don't combine server:/ > and > /path into server://path after adding a leading '/' to the path. > > The logic here is simplified such that we should _never_ concat these > two: > a base:/ and a /path, otherwise we'll have server://path. > > Further confusing the matter is that for v3 mounts, the base is > server:/path > and the path is '/' (which is added by NFS_PATH_CANONICAL), but for v4 > mounts the base is server:/ and the path is /path/to/export Did this make any sense, or should I try to make it clearer or rework this? This patch seems like just a minor fix, but the double slashes breaks some mount parsing in tests we have. Ben ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Trim extra slash in v4 nfs_path 2016-07-26 10:58 ` Benjamin Coddington @ 2016-08-01 19:11 ` Anna Schumaker 2016-10-21 16:16 ` Benjamin Coddington 0 siblings, 1 reply; 7+ messages in thread From: Anna Schumaker @ 2016-08-01 19:11 UTC (permalink / raw) To: Benjamin Coddington, Anna Schumaker; +Cc: linux-nfs, Trond Myklebust Hi Ben, On 07/26/2016 06:58 AM, Benjamin Coddington wrote: > Hi Anna, > > On 20 Jun 2016, at 11:38, Benjamin Coddington wrote: > >> On 20 Jun 2016, at 11:08, Anna Schumaker wrote: >> >>> Hi Ben, >>> >>> On 06/15/2016 03:02 PM, Benjamin Coddington wrote: >>>> A NFSv4 mount of a subdirectory will show an extra slash (as in >>>> 'server://path') in proc's mountinfo which will not match the device name >>>> and path. This can cause problems for programs searching for the mount. >>>> Fix this by checking for a leading slash in the dentry path, if so trim >>>> away any trailing slashes in the device name. >>>> >>>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com> >>>> --- >>>> fs/nfs/namespace.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c >>>> index c8162c660c44..5551e8ef67fd 100644 >>>> --- a/fs/nfs/namespace.c >>>> +++ b/fs/nfs/namespace.c >>>> @@ -98,7 +98,7 @@ rename_retry: >>>> return end; >>>> } >>>> namelen = strlen(base); >>>> - if (flags & NFS_PATH_CANONICAL) { >>>> + if (*end == '/') { >>> >>> Are we getting here through nfs_show_devname()? Because it looks like that function is passing 0 for flags instead of NFS_PATH_CANONICAL. >> >> We can get here with or withount NFS_PATH_CANONICAL, through >> nfs_show_devname() or nfs4_path() or nfs_do_submount(). >> >>> >>> Earlier in this function we have a check for: >>> if ((flags & NFS_PATH_CANONICAL) && *end != '/') >>> >>> Should we be checking if NFS_PATH_CANONICAL is set here, too? >> >> I don't think we need it. >> >> The "if (flags & NFS_PATH_CANONICAL) {" was meant to cover the case where >> NFS_PATH_CANONICAL had earlier added a leading '/' to the path, and so would >> trim away any trailing '/' from the base. So, don't combine server:/ and >> /path into server://path after adding a leading '/' to the path. >> >> The logic here is simplified such that we should _never_ concat these two: >> a base:/ and a /path, otherwise we'll have server://path. >> >> Further confusing the matter is that for v3 mounts, the base is server:/path >> and the path is '/' (which is added by NFS_PATH_CANONICAL), but for v4 >> mounts the base is server:/ and the path is /path/to/export > > Did this make any sense, or should I try to make it clearer or rework this? Your explanation made sense to me, and if tests are failing without this patch then we should probably go with it. Thanks, Anna > > This patch seems like just a minor fix, but the double slashes breaks some mount parsing in tests we have. > > Ben > -- > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Trim extra slash in v4 nfs_path 2016-08-01 19:11 ` Anna Schumaker @ 2016-10-21 16:16 ` Benjamin Coddington 2016-10-24 16:07 ` Anna Schumaker 0 siblings, 1 reply; 7+ messages in thread From: Benjamin Coddington @ 2016-10-21 16:16 UTC (permalink / raw) To: Anna Schumaker; +Cc: linux-nfs, Trond Myklebust Hi Anna, will this one go in for rc1 as well? Ben On 1 Aug 2016, at 15:11, Anna Schumaker wrote: > Hi Ben, > > On 07/26/2016 06:58 AM, Benjamin Coddington wrote: >> Hi Anna, >> >> On 20 Jun 2016, at 11:38, Benjamin Coddington wrote: >> >>> On 20 Jun 2016, at 11:08, Anna Schumaker wrote: >>> >>>> Hi Ben, >>>> >>>> On 06/15/2016 03:02 PM, Benjamin Coddington wrote: >>>>> A NFSv4 mount of a subdirectory will show an extra slash (as in >>>>> 'server://path') in proc's mountinfo which will not match the >>>>> device name >>>>> and path. This can cause problems for programs searching for the >>>>> mount. >>>>> Fix this by checking for a leading slash in the dentry path, if so >>>>> trim >>>>> away any trailing slashes in the device name. >>>>> >>>>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com> >>>>> --- >>>>> fs/nfs/namespace.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c >>>>> index c8162c660c44..5551e8ef67fd 100644 >>>>> --- a/fs/nfs/namespace.c >>>>> +++ b/fs/nfs/namespace.c >>>>> @@ -98,7 +98,7 @@ rename_retry: >>>>> return end; >>>>> } >>>>> namelen = strlen(base); >>>>> - if (flags & NFS_PATH_CANONICAL) { >>>>> + if (*end == '/') { >>>> >>>> Are we getting here through nfs_show_devname()? Because it looks >>>> like that function is passing 0 for flags instead of >>>> NFS_PATH_CANONICAL. >>> >>> We can get here with or withount NFS_PATH_CANONICAL, through >>> nfs_show_devname() or nfs4_path() or nfs_do_submount(). >>> >>>> >>>> Earlier in this function we have a check for: >>>> if ((flags & NFS_PATH_CANONICAL) && *end != '/') >>>> >>>> Should we be checking if NFS_PATH_CANONICAL is set here, too? >>> >>> I don't think we need it. >>> >>> The "if (flags & NFS_PATH_CANONICAL) {" was meant to cover the case >>> where >>> NFS_PATH_CANONICAL had earlier added a leading '/' to the path, and >>> so would >>> trim away any trailing '/' from the base. So, don't combine >>> server:/ and >>> /path into server://path after adding a leading '/' to the path. >>> >>> The logic here is simplified such that we should _never_ concat >>> these two: >>> a base:/ and a /path, otherwise we'll have server://path. >>> >>> Further confusing the matter is that for v3 mounts, the base is >>> server:/path >>> and the path is '/' (which is added by NFS_PATH_CANONICAL), but for >>> v4 >>> mounts the base is server:/ and the path is /path/to/export >> >> Did this make any sense, or should I try to make it clearer or rework >> this? > > Your explanation made sense to me, and if tests are failing without > this patch then we should probably go with it. > > Thanks, > Anna > >> >> This patch seems like just a minor fix, but the double slashes breaks >> some mount parsing in tests we have. >> >> Ben >> -- >> 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Trim extra slash in v4 nfs_path 2016-10-21 16:16 ` Benjamin Coddington @ 2016-10-24 16:07 ` Anna Schumaker 0 siblings, 0 replies; 7+ messages in thread From: Anna Schumaker @ 2016-10-24 16:07 UTC (permalink / raw) To: Benjamin Coddington; +Cc: linux-nfs, Trond Myklebust Hi Ben, On 10/21/2016 12:16 PM, Benjamin Coddington wrote: > Hi Anna, will this one go in for rc1 as well? Sorry I didn't see this before sending the pull request! I'll make sure it's included in the next one. Thanks, Anna > > Ben > > > On 1 Aug 2016, at 15:11, Anna Schumaker wrote: > >> Hi Ben, >> >> On 07/26/2016 06:58 AM, Benjamin Coddington wrote: >>> Hi Anna, >>> >>> On 20 Jun 2016, at 11:38, Benjamin Coddington wrote: >>> >>>> On 20 Jun 2016, at 11:08, Anna Schumaker wrote: >>>> >>>>> Hi Ben, >>>>> >>>>> On 06/15/2016 03:02 PM, Benjamin Coddington wrote: >>>>>> A NFSv4 mount of a subdirectory will show an extra slash (as in >>>>>> 'server://path') in proc's mountinfo which will not match the device name >>>>>> and path. This can cause problems for programs searching for the mount. >>>>>> Fix this by checking for a leading slash in the dentry path, if so trim >>>>>> away any trailing slashes in the device name. >>>>>> >>>>>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com> >>>>>> --- >>>>>> fs/nfs/namespace.c | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c >>>>>> index c8162c660c44..5551e8ef67fd 100644 >>>>>> --- a/fs/nfs/namespace.c >>>>>> +++ b/fs/nfs/namespace.c >>>>>> @@ -98,7 +98,7 @@ rename_retry: >>>>>> return end; >>>>>> } >>>>>> namelen = strlen(base); >>>>>> - if (flags & NFS_PATH_CANONICAL) { >>>>>> + if (*end == '/') { >>>>> >>>>> Are we getting here through nfs_show_devname()? Because it looks like that function is passing 0 for flags instead of NFS_PATH_CANONICAL. >>>> >>>> We can get here with or withount NFS_PATH_CANONICAL, through >>>> nfs_show_devname() or nfs4_path() or nfs_do_submount(). >>>> >>>>> >>>>> Earlier in this function we have a check for: >>>>> if ((flags & NFS_PATH_CANONICAL) && *end != '/') >>>>> >>>>> Should we be checking if NFS_PATH_CANONICAL is set here, too? >>>> >>>> I don't think we need it. >>>> >>>> The "if (flags & NFS_PATH_CANONICAL) {" was meant to cover the case where >>>> NFS_PATH_CANONICAL had earlier added a leading '/' to the path, and so would >>>> trim away any trailing '/' from the base. So, don't combine server:/ and >>>> /path into server://path after adding a leading '/' to the path. >>>> >>>> The logic here is simplified such that we should _never_ concat these two: >>>> a base:/ and a /path, otherwise we'll have server://path. >>>> >>>> Further confusing the matter is that for v3 mounts, the base is server:/path >>>> and the path is '/' (which is added by NFS_PATH_CANONICAL), but for v4 >>>> mounts the base is server:/ and the path is /path/to/export >>> >>> Did this make any sense, or should I try to make it clearer or rework this? >> >> Your explanation made sense to me, and if tests are failing without this patch then we should probably go with it. >> >> Thanks, >> Anna >> >>> >>> This patch seems like just a minor fix, but the double slashes breaks some mount parsing in tests we have. >>> >>> Ben >>> -- >>> 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 >> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-10-24 16:07 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-06-15 19:02 [PATCH] NFS: Trim extra slash in v4 nfs_path Benjamin Coddington 2016-06-20 15:08 ` Anna Schumaker 2016-06-20 15:38 ` [PATCH] " Benjamin Coddington 2016-07-26 10:58 ` Benjamin Coddington 2016-08-01 19:11 ` Anna Schumaker 2016-10-21 16:16 ` Benjamin Coddington 2016-10-24 16:07 ` Anna Schumaker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).