diff mbox

mountd: regression in crossmounts

Message ID 1364049149-3751-1-git-send-email-steved@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Steve Dickson March 23, 2013, 2:32 p.m. UTC
commit 8e2fb3fc cause a regression in mount export
that are on different local file system.
Exports like (all on different filesystems)

/home *(rw,fsid=0,crossmnt)
/home/fs1 *(rw,crossmnt)
/home/fs1/fs2/fs3 *(rw,nohide)

and then a mount of the root 'mount /home /mnt'
would end up mounting /home/fs1/fs2/fs3 not /home

Reverting the logic of commit 8e2fb3fc until
a better solution can be found for the original
problem.

Signed-off-by: Steve Dickson <steved@redhat.com>
---
 utils/mountd/cache.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Steve Dickson March 25, 2013, 2:18 p.m. UTC | #1
On 23/03/13 10:32, Steve Dickson wrote:
> commit 8e2fb3fc cause a regression in mount export
> that are on different local file system.
> Exports like (all on different filesystems)
> 
> /home *(rw,fsid=0,crossmnt)
> /home/fs1 *(rw,crossmnt)
> /home/fs1/fs2/fs3 *(rw,nohide)
> 
> and then a mount of the root 'mount /home /mnt'
> would end up mounting /home/fs1/fs2/fs3 not /home
> 
> Reverting the logic of commit 8e2fb3fc until
> a better solution can be found for the original
> problem.
> 
> Signed-off-by: Steve Dickson <steved@redhat.com>
Committed...

steved.

> ---
>  utils/mountd/cache.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> index c8aa46f..978698d 100644
> --- a/utils/mountd/cache.c
> +++ b/utils/mountd/cache.c
> @@ -371,8 +371,11 @@ export_matches(nfs_export *exp, char *dom, char *path, struct addrinfo *ai)
>  static bool subexport(struct exportent *e1, struct exportent *e2)
>  {
>  	char *p1 = e1->e_path, *p2 = e2->e_path;
> +	size_t l2 = strlen(p2);
> +
>  	return e2->e_flags & NFSEXP_CROSSMOUNT
> -		&& is_subdirectory(p1, p2);
> +		&& strncmp(p1, p2, l2) == 0
> +		&& p1[l2] == '/';
>  }
>  
>  struct parsed_fsid {
> 
--
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
J. Bruce Fields March 25, 2013, 6:13 p.m. UTC | #2
On Sat, Mar 23, 2013 at 10:32:29AM -0400, Steve Dickson wrote:
> commit 8e2fb3fc cause a regression in mount export
> that are on different local file system.
> Exports like (all on different filesystems)
> 
> /home *(rw,fsid=0,crossmnt)
> /home/fs1 *(rw,crossmnt)
> /home/fs1/fs2/fs3 *(rw,nohide)
> 
> and then a mount of the root 'mount /home /mnt'
> would end up mounting /home/fs1/fs2/fs3 not /home

That's strange.

> Reverting the logic of commit 8e2fb3fc until
> a better solution can be found for the original
> problem.

I can't figure out why the patch below would help.  The only change I
can see is to the way that an export of "/" would be handled.

--b.

> 
> 
> Signed-off-by: Steve Dickson <steved@redhat.com>
> ---
>  utils/mountd/cache.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> index c8aa46f..978698d 100644
> --- a/utils/mountd/cache.c
> +++ b/utils/mountd/cache.c
> @@ -371,8 +371,11 @@ export_matches(nfs_export *exp, char *dom, char *path, struct addrinfo *ai)
>  static bool subexport(struct exportent *e1, struct exportent *e2)
>  {
>  	char *p1 = e1->e_path, *p2 = e2->e_path;
> +	size_t l2 = strlen(p2);
> +
>  	return e2->e_flags & NFSEXP_CROSSMOUNT
> -		&& is_subdirectory(p1, p2);
> +		&& strncmp(p1, p2, l2) == 0
> +		&& p1[l2] == '/';
>  }
>  
>  struct parsed_fsid {
> -- 
> 1.8.1.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
--
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
Steve Dickson March 25, 2013, 8:22 p.m. UTC | #3
On 25/03/13 14:13, J. Bruce Fields wrote:
> On Sat, Mar 23, 2013 at 10:32:29AM -0400, Steve Dickson wrote:
>> commit 8e2fb3fc cause a regression in mount export
>> that are on different local file system.
>> Exports like (all on different filesystems)
>>
>> /home *(rw,fsid=0,crossmnt)
>> /home/fs1 *(rw,crossmnt)
>> /home/fs1/fs2/fs3 *(rw,nohide)
>>
>> and then a mount of the root 'mount /home /mnt'
>> would end up mounting /home/fs1/fs2/fs3 not /home
> 
> That's strange.
Yeah... I had this guy reporting from day one that the 
latest nfs-utils broke his export world... 
 
> 
>> Reverting the logic of commit 8e2fb3fc until
>> a better solution can be found for the original
>> problem.
> 
> I can't figure out why the patch below would help.  The only change I
> can see is to the way that an export of "/" would be handled.
The reason I didn't debug this further was I did not know how to 
reproduce the original problem the patch was trying to fix... 

steved.
--
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
J. Bruce Fields March 25, 2013, 9:27 p.m. UTC | #4
On Mon, Mar 25, 2013 at 04:22:21PM -0400, Steve Dickson wrote:
> 
> 
> On 25/03/13 14:13, J. Bruce Fields wrote:
> > On Sat, Mar 23, 2013 at 10:32:29AM -0400, Steve Dickson wrote:
> >> commit 8e2fb3fc cause a regression in mount export
> >> that are on different local file system.
> >> Exports like (all on different filesystems)
> >>
> >> /home *(rw,fsid=0,crossmnt)
> >> /home/fs1 *(rw,crossmnt)
> >> /home/fs1/fs2/fs3 *(rw,nohide)
> >>
> >> and then a mount of the root 'mount /home /mnt'
> >> would end up mounting /home/fs1/fs2/fs3 not /home
> > 
> > That's strange.
> Yeah... I had this guy reporting from day one that the 
> latest nfs-utils broke his export world... 
>  
> > 
> >> Reverting the logic of commit 8e2fb3fc until
> >> a better solution can be found for the original
> >> problem.
> > 
> > I can't figure out why the patch below would help.  The only change I
> > can see is to the way that an export of "/" would be handled.
> The reason I didn't debug this further was I did not know how to 
> reproduce the original problem the patch was trying to fix... 

Could you remind me where the bug report for this is?

The patch reverted here seems obviously correct to me, so I'm concerned.

--b.
--
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
J. Bruce Fields March 25, 2013, 11:36 p.m. UTC | #5
On Mon, Mar 25, 2013 at 05:27:36PM -0400, J. Bruce Fields wrote:
> On Mon, Mar 25, 2013 at 04:22:21PM -0400, Steve Dickson wrote:
> > 
> > 
> > On 25/03/13 14:13, J. Bruce Fields wrote:
> > > On Sat, Mar 23, 2013 at 10:32:29AM -0400, Steve Dickson wrote:
> > >> commit 8e2fb3fc cause a regression in mount export
> > >> that are on different local file system.
> > >> Exports like (all on different filesystems)
> > >>
> > >> /home *(rw,fsid=0,crossmnt)
> > >> /home/fs1 *(rw,crossmnt)
> > >> /home/fs1/fs2/fs3 *(rw,nohide)
> > >>
> > >> and then a mount of the root 'mount /home /mnt'
> > >> would end up mounting /home/fs1/fs2/fs3 not /home
> > > 
> > > That's strange.
> > Yeah... I had this guy reporting from day one that the 
> > latest nfs-utils broke his export world... 
> >  
> > > 
> > >> Reverting the logic of commit 8e2fb3fc until
> > >> a better solution can be found for the original
> > >> problem.
> > > 
> > > I can't figure out why the patch below would help.  The only change I
> > > can see is to the way that an export of "/" would be handled.
> > The reason I didn't debug this further was I did not know how to 
> > reproduce the original problem the patch was trying to fix... 
> 
> Could you remind me where the bug report for this is?

Oh, right, it was https://bugzilla.redhat.com/show_bug.cgi?id=909882

I'm not seeing why you think this helps.

--b.
--
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
Steve Dickson March 27, 2013, 1:06 p.m. UTC | #6
On 25/03/13 19:36, J. Bruce Fields wrote:
> On Mon, Mar 25, 2013 at 05:27:36PM -0400, J. Bruce Fields wrote:
>> On Mon, Mar 25, 2013 at 04:22:21PM -0400, Steve Dickson wrote:
>>>
>>>
>>> On 25/03/13 14:13, J. Bruce Fields wrote:
>>>> On Sat, Mar 23, 2013 at 10:32:29AM -0400, Steve Dickson wrote:
>>>>> commit 8e2fb3fc cause a regression in mount export
>>>>> that are on different local file system.
>>>>> Exports like (all on different filesystems)
>>>>>
>>>>> /home *(rw,fsid=0,crossmnt)
>>>>> /home/fs1 *(rw,crossmnt)
>>>>> /home/fs1/fs2/fs3 *(rw,nohide)
>>>>>
>>>>> and then a mount of the root 'mount /home /mnt'
>>>>> would end up mounting /home/fs1/fs2/fs3 not /home
>>>>
>>>> That's strange.
>>> Yeah... I had this guy reporting from day one that the 
>>> latest nfs-utils broke his export world... 
>>>  
>>>>
>>>>> Reverting the logic of commit 8e2fb3fc until
>>>>> a better solution can be found for the original
>>>>> problem.
>>>>
>>>> I can't figure out why the patch below would help.  The only change I
>>>> can see is to the way that an export of "/" would be handled.
>>> The reason I didn't debug this further was I did not know how to 
>>> reproduce the original problem the patch was trying to fix... 
>>
>> Could you remind me where the bug report for this is?
> 
> Oh, right, it was https://bugzilla.redhat.com/show_bug.cgi?id=909882
> 
> I'm not seeing why you think this helps.
Because it does... ;-) 
     https://bugzilla.redhat.com/show_bug.cgi?id=909882#c10
     https://admin.fedoraproject.org/updates/FEDORA-2013-4396/nfs-utils-1.2.7-5.fc18

What I would like to is how to reproduce the original problem so
we can fix it without creating this regression.... 

steved.
 
--
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 mbox

Patch

diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index c8aa46f..978698d 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -371,8 +371,11 @@  export_matches(nfs_export *exp, char *dom, char *path, struct addrinfo *ai)
 static bool subexport(struct exportent *e1, struct exportent *e2)
 {
 	char *p1 = e1->e_path, *p2 = e2->e_path;
+	size_t l2 = strlen(p2);
+
 	return e2->e_flags & NFSEXP_CROSSMOUNT
-		&& is_subdirectory(p1, p2);
+		&& strncmp(p1, p2, l2) == 0
+		&& p1[l2] == '/';
 }
 
 struct parsed_fsid {