2013-03-23 14:34:59

by Steve Dickson

[permalink] [raw]
Subject: [PATCH] mountd: regression in crossmounts

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 <[email protected]>
---
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



2013-03-25 21:27:37

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH] mountd: regression in crossmounts

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.

2013-03-25 23:36:51

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH] mountd: regression in crossmounts

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.

2013-03-25 18:13:15

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH] mountd: regression in crossmounts

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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2013-03-25 20:22:23

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] mountd: regression in crossmounts



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.

2013-03-27 13:06:21

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] mountd: regression in crossmounts



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.


2013-03-25 14:18:16

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] mountd: regression in crossmounts



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 <[email protected]>
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 {
>