Return-Path: Subject: Re: [PATCH] Trim extra slash in v4 nfs_path To: Benjamin Coddington References: <1cb38ee043176bca8ded7e18120776f8cf6a722d.1466017326.git.bcodding@redhat.com> <01b31d32-ffbf-673c-4838-67e2205f75aa@Netapp.com> <4671E84D-E1E1-4799-97A2-BE5BD5C224DE@redhat.com> CC: , Trond Myklebust From: Anna Schumaker Message-ID: <65a4c862-996b-9f4f-367e-8f72779cd59c@Netapp.com> Date: Mon, 24 Oct 2016 12:07:03 -0400 MIME-Version: 1.0 In-Reply-To: <4671E84D-E1E1-4799-97A2-BE5BD5C224DE@redhat.com> Content-Type: text/plain; charset="utf-8" List-ID: 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 >>>>>> --- >>>>>> 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 >>