[PATCH] mountd: regression in crossmounts

[PATCH] mountd: regression in crossmounts

[Steve Dickson]

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(-)

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

Re: [PATCH] mountd: regression in crossmounts

[Steve Dickson]

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 {
> 

Re: [PATCH] mountd: regression in crossmounts

[J. Bruce Fields]

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

Re: [PATCH] mountd: regression in crossmounts

[Steve Dickson]

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.

Re: [PATCH] mountd: regression in crossmounts

[J. Bruce Fields]

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.

Re: [PATCH] mountd: regression in crossmounts

[J. Bruce Fields]

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.

Re: [PATCH] mountd: regression in crossmounts

[Steve Dickson]

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.