[PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Jeff Layton]

This will never work properly with CIFS, as the protocol has no ability
whatsoever for looking up files by filehandle. It *might* be possible to
eventually do this with SMB2, but that remains to be seen.

For now, it just plain doesn't work. Mark it BROKEN to discourage
distros from enabling it.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig
index 75c47cd..f0f960d 100644
--- a/fs/cifs/Kconfig
+++ b/fs/cifs/Kconfig
@@ -175,6 +175,6 @@ config CIFS_SMB2
 
 config CIFS_NFSD_EXPORT
 	  bool "Allow nfsd to export CIFS file system (EXPERIMENTAL)"
-	  depends on CIFS && EXPERIMENTAL
+	  depends on CIFS && EXPERIMENTAL && BROKEN
 	  help
 	   Allows NFS server to export a CIFS mounted share (nfsd over cifs)
-- 
1.7.4.4

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Christoph Hellwig]

On Sun, May 22, 2011 at 07:55:24AM -0400, Jeff Layton wrote:
> This will never work properly with CIFS, as the protocol has no ability
> whatsoever for looking up files by filehandle. It *might* be possible to
> eventually do this with SMB2, but that remains to be seen.
> 
> For now, it just plain doesn't work. Mark it BROKEN to discourage
> distros from enabling it.

It's actually dead code at this point - fs/nfsd/vfs.c:check_export()
refuses to export a filesystem that does not have a fh_to_dentry
operation, which cifs doesn't provide.

IMHO it's best to just remove the option and all surrounding code.

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Jeff Layton]

On Sun, 22 May 2011 07:59:38 -0400
Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:

> On Sun, May 22, 2011 at 07:55:24AM -0400, Jeff Layton wrote:
> > This will never work properly with CIFS, as the protocol has no ability
> > whatsoever for looking up files by filehandle. It *might* be possible to
> > eventually do this with SMB2, but that remains to be seen.
> > 
> > For now, it just plain doesn't work. Mark it BROKEN to discourage
> > distros from enabling it.
> 
> It's actually dead code at this point - fs/nfsd/vfs.c:check_export()
> refuses to export a filesystem that does not have a fh_to_dentry
> operation, which cifs doesn't provide.
> 
> IMHO it's best to just remove the option and all surrounding code.
> 

I agree and proposed a patch to do that a month or two ago. Steve
NAK'ed it for reasons that I don't quite understand. At this point,
I'm just trying to make sure distros don't ship with this enabled.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Steve French]

On Sun, May 22, 2011 at 7:17 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Sun, 22 May 2011 07:59:38 -0400
> Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>
>> On Sun, May 22, 2011 at 07:55:24AM -0400, Jeff Layton wrote:
>> > This will never work properly with CIFS, as the protocol has no ability
>> > whatsoever for looking up files by filehandle. It *might* be possible to
>> > eventually do this with SMB2, but that remains to be seen.
>> >
>> > For now, it just plain doesn't work. Mark it BROKEN to discourage
>> > distros from enabling it.
>>
>> It's actually dead code at this point - fs/nfsd/vfs.c:check_export()
>> refuses to export a filesystem that does not have a fh_to_dentry
>> operation, which cifs doesn't provide.
>>
>> IMHO it's best to just remove the option and all surrounding code.
>>
>
> I agree and proposed a patch to do that a month or two ago. Steve
> NAK'ed it for reasons that I don't quite understand.

We were waiting on clarification on NTCreateX open by inode number flag.

We have had multiple requests to be able to do limited
export of cifs data via nfs (for example for backup to/from
OS with only one of the protocols supported).   NFS clients
don't make this easy - because some don't handle ESTALE
by revalidating their dentries.

For nfs v3 clients that can't handle ESTALE, we are probably
stuck with having to wait for Samba to implement the flag.

I did want to see JRA's opinion and also query if other
NFS clients (other than Linux) can handle ESTALE
(by e.g. revalidating the dentry and repeating the operation).






-- 
Thanks,

Steve

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Jeff Layton]

On Sun, 22 May 2011 20:12:04 -0500
Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Sun, May 22, 2011 at 7:17 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > On Sun, 22 May 2011 07:59:38 -0400
> > Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> >
> >> On Sun, May 22, 2011 at 07:55:24AM -0400, Jeff Layton wrote:
> >> > This will never work properly with CIFS, as the protocol has no ability
> >> > whatsoever for looking up files by filehandle. It *might* be possible to
> >> > eventually do this with SMB2, but that remains to be seen.
> >> >
> >> > For now, it just plain doesn't work. Mark it BROKEN to discourage
> >> > distros from enabling it.
> >>
> >> It's actually dead code at this point - fs/nfsd/vfs.c:check_export()
> >> refuses to export a filesystem that does not have a fh_to_dentry
> >> operation, which cifs doesn't provide.
> >>
> >> IMHO it's best to just remove the option and all surrounding code.
> >>
> >
> > I agree and proposed a patch to do that a month or two ago. Steve
> > NAK'ed it for reasons that I don't quite understand.
> 
> We were waiting on clarification on NTCreateX open by inode number flag.
> 

It's clarified -- it doesn't do anything for directories so it doesn't
help.

> We have had multiple requests to be able to do limited
> export of cifs data via nfs (for example for backup to/from
> OS with only one of the protocols supported).   NFS clients
> don't make this easy - because some don't handle ESTALE
> by revalidating their dentries.
> 
> For nfs v3 clients that can't handle ESTALE, we are probably
> stuck with having to wait for Samba to implement the flag.
> 

Flag?

> I did want to see JRA's opinion and also query if other
> NFS clients (other than Linux) can handle ESTALE
> (by e.g. revalidating the dentry and repeating the operation).
> 

The client can't always revalidate the dentry. For instance, suppose
you open a file or directory and then it gets renamed or removed on the
server. The client may have no idea what the new path actually *is* in
order to revalidate it.

The fact that CIFS has no lookup by filehandle makes this whole idea
dead in the water. SMB2 might be able to do something, but I'm not yet
convinced of that.

Either way, to do this will require more than the stubs in place now.
I think this code either needs to be removed until you come up with a
design that overcomes these problems, or marked broken (like this patch
does).

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Steve French]

On Mon, May 23, 2011 at 6:12 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Sun, 22 May 2011 20:12:04 -0500
> Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> On Sun, May 22, 2011 at 7:17 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> > On Sun, 22 May 2011 07:59:38 -0400
>> > Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>> >
>> >> On Sun, May 22, 2011 at 07:55:24AM -0400, Jeff Layton wrote:
>> >> > This will never work properly with CIFS, as the protocol has no ability
>> >> > whatsoever for looking up files by filehandle. It *might* be possible to
>> >> > eventually do this with SMB2, but that remains to be seen.

Shirish had done some experiments (and AFAIK has a small patch
that fixed NFS export over CIFS which works, with the usual restrictions
about having to return ESTALE if the NFS client tries to access
an inode which has been flushed from the cache on the server side).

>> For nfs v3 clients that can't handle ESTALE, we are probably
>> stuck with having to wait for Samba to implement the flag
> Flag?

NTCreateX:  FILE_OPEN_BY_FILE_ID

IIRC Shirish verified that the Windows client will emit this flag, but their
server had not gotten around to implementing it (although Samba could
implement it now that their is an open-by-handle syscall).



-- 
Thanks,

Steve

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Jeff Layton]

On Mon, 23 May 2011 08:42:39 -0500
Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Mon, May 23, 2011 at 6:12 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > On Sun, 22 May 2011 20:12:04 -0500
> > Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> >> On Sun, May 22, 2011 at 7:17 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >> > On Sun, 22 May 2011 07:59:38 -0400
> >> > Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> >> >
> >> >> On Sun, May 22, 2011 at 07:55:24AM -0400, Jeff Layton wrote:
> >> >> > This will never work properly with CIFS, as the protocol has no ability
> >> >> > whatsoever for looking up files by filehandle. It *might* be possible to
> >> >> > eventually do this with SMB2, but that remains to be seen.
> 
> Shirish had done some experiments (and AFAIK has a small patch
> that fixed NFS export over CIFS which works, with the usual restrictions
> about having to return ESTALE if the NFS client tries to access
> an inode which has been flushed from the cache on the server side).
> 

That restriction is not usual for servers, and makes the whole scheme
unreliable.

> >> For nfs v3 clients that can't handle ESTALE, we are probably
> >> stuck with having to wait for Samba to implement the flag
> > Flag?
> 
> NTCreateX:  FILE_OPEN_BY_FILE_ID
> 
> IIRC Shirish verified that the Windows client will emit this flag, but their
> server had not gotten around to implementing it (although Samba could
> implement it now that their is an open-by-handle syscall).
> 

Again, that does absolutely nothing for directories which also need to
be retrievable by filehandle.

I think it would be best to remove this code, but if that's
unacceptable for some reason it should at least be marked BROKEN.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Steve French]

On Mon, May 23, 2011 at 8:50 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Mon, 23 May 2011 08:42:39 -0500
> Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> On Mon, May 23, 2011 at 6:12 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> > On Sun, 22 May 2011 20:12:04 -0500
>> > Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> >
>> >> On Sun, May 22, 2011 at 7:17 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> >> > On Sun, 22 May 2011 07:59:38 -0400
>> >> > Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>> >> >
>> >> >> On Sun, May 22, 2011 at 07:55:24AM -0400, Jeff Layton wrote:
>> >> >> > This will never work properly with CIFS, as the protocol has no ability
>> >> >> > whatsoever for looking up files by filehandle. It *might* be possible to
>> >> >> > eventually do this with SMB2, but that remains to be seen.
>>
>> Shirish had done some experiments (and AFAIK has a small patch
>> that fixed NFS export over CIFS which works, with the usual restrictions
>> about having to return ESTALE if the NFS client tries to access
>> an inode which has been flushed from the cache on the server side).
>>
>
> That restriction is not usual for servers, and makes the whole scheme
> unreliable.

It must be reasonably common - as it was one of the motivations for
ESTALE return code, and was a motivation for the explicit "volatile
file handle" type that was introduced in nfs v4 (which makes it easier
for the client to tell when it has to be able to revalidate a file
handle).  I see mention of this in Solaris, Oracle so it is presumably
fairly common.

>> >> For nfs v3 clients that can't handle ESTALE, we are probably
>> >> stuck with having to wait for Samba to implement the flag
>> > Flag?
>>
>> NTCreateX:  FILE_OPEN_BY_FILE_ID
>>
>> IIRC Shirish verified that the Windows client will emit this flag, but their
>> server had not gotten around to implementing it (although Samba could
>> implement it now that their is an open-by-handle syscall).
>>
>
> Again, that does absolutely nothing for directories which also need to
> be retrievable by filehandle.

Why wouldn't that work?   Samba and Windows both support opening
directories - why wouldn't queryfileinfo work on those handles?


> I think it would be best to remove this code, but if that's
> unacceptable for some reason it should at least be marked BROKEN.

if Shirish's patch can't (or doesn't) make it work I agree that it
should be removed or marked broken - but I have not tested his patch.


-- 
Thanks,

Steve

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Jeff Layton]

On Mon, 23 May 2011 09:02:55 -0500
Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Mon, May 23, 2011 at 8:50 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > On Mon, 23 May 2011 08:42:39 -0500
> > Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> >> On Mon, May 23, 2011 at 6:12 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >> > On Sun, 22 May 2011 20:12:04 -0500
> >> > Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> >
> >> >> On Sun, May 22, 2011 at 7:17 AM, Jeff Layton <jlayton-H+wXaHxf7aJhl2p70BpVqQ@public.gmane.orgm> wrote:
> >> >> > On Sun, 22 May 2011 07:59:38 -0400
> >> >> > Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> >> >> >
> >> >> >> On Sun, May 22, 2011 at 07:55:24AM -0400, Jeff Layton wrote:
> >> >> >> > This will never work properly with CIFS, as the protocol has no ability
> >> >> >> > whatsoever for looking up files by filehandle. It *might* be possible to
> >> >> >> > eventually do this with SMB2, but that remains to be seen.
> >>
> >> Shirish had done some experiments (and AFAIK has a small patch
> >> that fixed NFS export over CIFS which works, with the usual restrictions
> >> about having to return ESTALE if the NFS client tries to access
> >> an inode which has been flushed from the cache on the server side).
> >>
> >
> > That restriction is not usual for servers, and makes the whole scheme
> > unreliable.
> 
> It must be reasonably common - as it was one of the motivations for
> ESTALE return code, and was a motivation for the explicit "volatile
> file handle" type that was introduced in nfs v4 (which makes it easier
> for the client to tell when it has to be able to revalidate a file
> handle).  I see mention of this in Solaris, Oracle so it is presumably
> fairly common.
> 

It's not common on a properly working or implemented server. There are
corner cases that can cause it to occur -- basically ESTALE is the
server's way of saying "I don't recognize this filehandle". For cifs
though, it's a real problem since you'll have no way to look up the
filehandle if it doesn't happen to be in the cache.

> >> >> For nfs v3 clients that can't handle ESTALE, we are probably
> >> >> stuck with having to wait for Samba to implement the flag
> >> > Flag?
> >>
> >> NTCreateX:  FILE_OPEN_BY_FILE_ID
> >>
> >> IIRC Shirish verified that the Windows client will emit this flag, but their
> >> server had not gotten around to implementing it (although Samba could
> >> implement it now that their is an open-by-handle syscall).
> >>
> >
> > Again, that does absolutely nothing for directories which also need to
> > be retrievable by filehandle.
> 
> Why wouldn't that work?   Samba and Windows both support opening
> directories - why wouldn't queryfileinfo work on those handles?
> 

QueryFileInfo might work. FIND_FIRST/NEXT won't.

> 
> > I think it would be best to remove this code, but if that's
> > unacceptable for some reason it should at least be marked BROKEN.
> 
> if Shirish's patch can't (or doesn't) make it work I agree that it
> should be removed or marked broken - but I have not tested his patch.
> 
> 

I've not seen this patch so I can't comment. I remain very, very
skeptical though without a solution that works for directories.

A server that works only under specific, difficult to predict
conditions is worse than useless.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Shirish Pargaonkar]

On Mon, May 23, 2011 at 8:42 AM, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, May 23, 2011 at 6:12 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> On Sun, 22 May 2011 20:12:04 -0500
>> Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>>> On Sun, May 22, 2011 at 7:17 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>> > On Sun, 22 May 2011 07:59:38 -0400
>>> > Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>>> >
>>> >> On Sun, May 22, 2011 at 07:55:24AM -0400, Jeff Layton wrote:
>>> >> > This will never work properly with CIFS, as the protocol has no ability
>>> >> > whatsoever for looking up files by filehandle. It *might* be possible to
>>> >> > eventually do this with SMB2, but that remains to be seen.
>
> Shirish had done some experiments (and AFAIK has a small patch
> that fixed NFS export over CIFS which works, with the usual restrictions
> about having to return ESTALE if the NFS client tries to access
> an inode which has been flushed from the cache on the server side).
>
>>> For nfs v3 clients that can't handle ESTALE, we are probably
>>> stuck with having to wait for Samba to implement the flag
>> Flag?
>
> NTCreateX:  FILE_OPEN_BY_FILE_ID
>
> IIRC Shirish verified that the Windows client will emit this flag, but their
> server had not gotten around to implementing it (although Samba could
> implement it now that their is an open-by-handle syscall).

Steve, I am not sure whether Windows client would emit this flag or not.
This is the failure on the Windows client with a test program.

The very same program works fine on the Windows server i.e. the API
is OK with a local program opening a local file by fileid but I think Windows
client does not even transmit such a request to the server.

 ULONG status = NtCreatefile(&f, GENERIC_ALL, &oa, iosb, NULL,
FILE_ATTRIBUTE_NORMAL, FILE_SHARE_READ | FILE_SHARE_WRITE, FILE_OPEN,
FILE_OPEN_BY_FILE_ID | FILE_NON_DIRECTORY_FILE, NULL, 0);

returns error status: C0000008, handle: 0

if the file is remote but no error if the file is local.

>
>
>
> --
> Thanks,
>
> Steve
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Gerald Carter]

[-- Attachment #1: Type: text/plain, Size: 865 bytes --]

On 05/23/2011 08:42 AM, Steve French wrote:

> Shirish had done some experiments (and AFAIK has a small patch
> that fixed NFS export over CIFS which works, with the usual restrictions
> about having to return ESTALE if the NFS client tries to access
> an inode which has been flushed from the cache on the server side).
> 
>>> For nfs v3 clients that can't handle ESTALE, we are probably
>>> stuck with having to wait for Samba to implement the flag
>> Flag?
> 
> NTCreateX:  FILE_OPEN_BY_FILE_ID
> 
> IIRC Shirish verified that the Windows client will emit this flag, but their
> server had not gotten around to implementing it (although Samba could
> implement it now that their is an open-by-handle syscall).

Hey Jeff/Steve,

I found in [MS-CIFS] that says a server MUST return STATUS_NOT_SUPPORTED
for this option.



Cheers, Jerry


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Steve French]

On Mon, May 23, 2011 at 11:31 AM, Gerald Carter <jerry-I3iY/6WR96Zg9hUCZPvPmw@public.gmane.org> wrote:
> On 05/23/2011 08:42 AM, Steve French wrote:
>
>> Shirish had done some experiments (and AFAIK has a small patch
>> that fixed NFS export over CIFS which works, with the usual restrictions
>> about having to return ESTALE if the NFS client tries to access
>> an inode which has been flushed from the cache on the server side).
>>
>>>> For nfs v3 clients that can't handle ESTALE, we are probably
>>>> stuck with having to wait for Samba to implement the flag
>>> Flag?
>>
>> NTCreateX:  FILE_OPEN_BY_FILE_ID
>>
>> IIRC Shirish verified that the Windows client will emit this flag, but their
>> server had not gotten around to implementing it (although Samba could
>> implement it now that their is an open-by-handle syscall).
>
> Hey Jeff/Steve,
>
> I found in [MS-CIFS] that says a server MUST return STATUS_NOT_SUPPORTED
> for this option.
Yes - Microsoft has said that their servers do return
STATUS_NOT_SUPPORTED but this is not "MUST" in exactly the same sense
as in an IETF RFC, and the discussion I had with JRA was about servers
which support the Unix/POSIX extensions allowing this.



-- 
Thanks,

Steve

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Jeff Layton]

On Mon, 23 May 2011 11:58:50 -0500
Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Mon, May 23, 2011 at 11:31 AM, Gerald Carter <jerry-I3iY/6WR96Zg9hUCZPvPmw@public.gmane.org> wrote:
> > On 05/23/2011 08:42 AM, Steve French wrote:
> >
> >> Shirish had done some experiments (and AFAIK has a small patch
> >> that fixed NFS export over CIFS which works, with the usual restrictions
> >> about having to return ESTALE if the NFS client tries to access
> >> an inode which has been flushed from the cache on the server side).
> >>
> >>>> For nfs v3 clients that can't handle ESTALE, we are probably
> >>>> stuck with having to wait for Samba to implement the flag
> >>> Flag?
> >>
> >> NTCreateX:  FILE_OPEN_BY_FILE_ID
> >>
> >> IIRC Shirish verified that the Windows client will emit this flag, but their
> >> server had not gotten around to implementing it (although Samba could
> >> implement it now that their is an open-by-handle syscall).
> >
> > Hey Jeff/Steve,
> >
> > I found in [MS-CIFS] that says a server MUST return STATUS_NOT_SUPPORTED
> > for this option.
> Yes - Microsoft has said that their servers do return
> STATUS_NOT_SUPPORTED but this is not "MUST" in exactly the same sense
> as in an IETF RFC, and the discussion I had with JRA was about servers
> which support the Unix/POSIX extensions allowing this.
> 

Reality check -- this is only going to be useful iff:

1) such a thing were to materialize on the server side (and that will
probably be in samba only)

...and...

2) someone writes the code to take advantage of it

...and that still won't fix the aforementioned problem with
directories.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Steve French]

On Mon, May 23, 2011 at 12:45 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Mon, 23 May 2011 11:58:50 -0500
> Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> On Mon, May 23, 2011 at 11:31 AM, Gerald Carter <jerry-I3iY/6WR96Zg9hUCZPvPmw@public.gmane.org> wrote:
>> > On 05/23/2011 08:42 AM, Steve French wrote:
>> >
>> >> Shirish had done some experiments (and AFAIK has a small patch
>> >> that fixed NFS export over CIFS which works, with the usual restrictions
>> >> about having to return ESTALE if the NFS client tries to access
>> >> an inode which has been flushed from the cache on the server side).
>> >>
>> >>>> For nfs v3 clients that can't handle ESTALE, we are probably
>> >>>> stuck with having to wait for Samba to implement the flag
>> >>> Flag?
>> >>
>> >> NTCreateX:  FILE_OPEN_BY_FILE_ID
>> >>
>> >> IIRC Shirish verified that the Windows client will emit this flag, but their
>> >> server had not gotten around to implementing it (although Samba could
>> >> implement it now that their is an open-by-handle syscall).
>> >
>> > Hey Jeff/Steve,
>> >
>> > I found in [MS-CIFS] that says a server MUST return STATUS_NOT_SUPPORTED
>> > for this option.
>> Yes - Microsoft has said that their servers do return
>> STATUS_NOT_SUPPORTED but this is not "MUST" in exactly the same sense
>> as in an IETF RFC, and the discussion I had with JRA was about servers
>> which support the Unix/POSIX extensions allowing this.
>>
>
> Reality check -- this is only going to be useful iff:
>
> 1) such a thing were to materialize on the server side (and that will
> probably be in samba only)
>
> ...and...
>
> 2) someone writes the code to take advantage of it
>
> ...and that still won't fix the aforementioned problem with
> directories.

Hang on - it is useful already.   With Shirish's patch he was able to
back up a Windows or Samba server via NFS.    For running more
complicated applications for long periods of time - it depends on the
NFS client whether ESTALE is handled or not - but at least ESTALE is a
documented return code and that the Linux NFS implementation did not
yet merge Peter Staubach's fix for ESTALE is a bug in Linux NFS (some
OS do handle ESTALE properly).



-- 
Thanks,

Steve

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Jeff Layton]

On Mon, 23 May 2011 12:52:36 -0500
Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Mon, May 23, 2011 at 12:45 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > On Mon, 23 May 2011 11:58:50 -0500
> > Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> >> On Mon, May 23, 2011 at 11:31 AM, Gerald Carter <jerry-I3iY/6WR96Ypug/h7KTFAQ@public.gmane.orgg> wrote:
> >> > On 05/23/2011 08:42 AM, Steve French wrote:
> >> >
> >> >> Shirish had done some experiments (and AFAIK has a small patch
> >> >> that fixed NFS export over CIFS which works, with the usual restrictions
> >> >> about having to return ESTALE if the NFS client tries to access
> >> >> an inode which has been flushed from the cache on the server side).
> >> >>
> >> >>>> For nfs v3 clients that can't handle ESTALE, we are probably
> >> >>>> stuck with having to wait for Samba to implement the flag
> >> >>> Flag?
> >> >>
> >> >> NTCreateX:  FILE_OPEN_BY_FILE_ID
> >> >>
> >> >> IIRC Shirish verified that the Windows client will emit this flag, but their
> >> >> server had not gotten around to implementing it (although Samba could
> >> >> implement it now that their is an open-by-handle syscall).
> >> >
> >> > Hey Jeff/Steve,
> >> >
> >> > I found in [MS-CIFS] that says a server MUST return STATUS_NOT_SUPPORTED
> >> > for this option.
> >> Yes - Microsoft has said that their servers do return
> >> STATUS_NOT_SUPPORTED but this is not "MUST" in exactly the same sense
> >> as in an IETF RFC, and the discussion I had with JRA was about servers
> >> which support the Unix/POSIX extensions allowing this.
> >>
> >
> > Reality check -- this is only going to be useful iff:
> >
> > 1) such a thing were to materialize on the server side (and that will
> > probably be in samba only)
> >
> > ...and...
> >
> > 2) someone writes the code to take advantage of it
> >
> > ...and that still won't fix the aforementioned problem with
> > directories.
> 
> Hang on - it is useful already.   With Shirish's patch he was able to
> back up a Windows or Samba server via NFS.    For running more
> complicated applications for long periods of time - it depends on the
> NFS client whether ESTALE is handled or not - but at least ESTALE is a
> documented return code and that the Linux NFS implementation did not
> yet merge Peter Staubach's fix for ESTALE is a bug in Linux NFS (some
> OS do handle ESTALE properly).
> 

Again, this is not reliable. Your backup in this case might work if
you're lucky, or it might not if the server happens to crash and reboot
or an inode gets pushed out of the cache. A NFS server that doesn't
work after a reboot is worse than useless -- it's risky.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Steve French]

On Mon, May 23, 2011 at 1:07 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Mon, 23 May 2011 12:52:36 -0500
> Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> On Mon, May 23, 2011 at 12:45 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> > On Mon, 23 May 2011 11:58:50 -0500
>> > Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> >
>> >> On Mon, May 23, 2011 at 11:31 AM, Gerald Carter <jerry@plainjoe.org> wrote:
>> >> > On 05/23/2011 08:42 AM, Steve French wrote:
>> >> >
>> >> >> Shirish had done some experiments (and AFAIK has a small patch
>> >> >> that fixed NFS export over CIFS which works, with the usual restrictions
>> >> >> about having to return ESTALE if the NFS client tries to access
>> >> >> an inode which has been flushed from the cache on the server side).
>> >> >>
>> >> >>>> For nfs v3 clients that can't handle ESTALE, we are probably
>> >> >>>> stuck with having to wait for Samba to implement the flag
>> >> >>> Flag?
>> >> >>
>> >> >> NTCreateX:  FILE_OPEN_BY_FILE_ID
>> >> >>
>> >> >> IIRC Shirish verified that the Windows client will emit this flag, but their
>> >> >> server had not gotten around to implementing it (although Samba could
>> >> >> implement it now that their is an open-by-handle syscall).
>> >> >
>> >> > Hey Jeff/Steve,
>> >> >
>> >> > I found in [MS-CIFS] that says a server MUST return STATUS_NOT_SUPPORTED
>> >> > for this option.
>> >> Yes - Microsoft has said that their servers do return
>> >> STATUS_NOT_SUPPORTED but this is not "MUST" in exactly the same sense
>> >> as in an IETF RFC, and the discussion I had with JRA was about servers
>> >> which support the Unix/POSIX extensions allowing this.
>> >>
>> >
>> > Reality check -- this is only going to be useful iff:
>> >
>> > 1) such a thing were to materialize on the server side (and that will
>> > probably be in samba only)
>> >
>> > ...and...
>> >
>> > 2) someone writes the code to take advantage of it
>> >
>> > ...and that still won't fix the aforementioned problem with
>> > directories.
>>
>> Hang on - it is useful already.   With Shirish's patch he was able to
>> back up a Windows or Samba server via NFS.    For running more
>> complicated applications for long periods of time - it depends on the
>> NFS client whether ESTALE is handled or not - but at least ESTALE is a
>> documented return code and that the Linux NFS implementation did not
>> yet merge Peter Staubach's fix for ESTALE is a bug in Linux NFS (some
>> OS do handle ESTALE properly).
>>
>
> Again, this is not reliable. Your backup in this case might work if
> you're lucky, or it might not if the server happens to crash and reboot
> or an inode gets pushed out of the cache. A NFS server that doesn't
> work after a reboot is worse than useless -- it's risky.

Until Peter's Linux NFS fix is in - aren't we in that situation
already with other fs.



-- 
Thanks,

Steve

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Christoph Hellwig]

On Mon, May 23, 2011 at 01:21:35PM -0500, Steve French wrote:
> Until Peter's Linux NFS fix is in - aren't we in that situation
> already with other fs.

That patch is not going to help with the fundamental problem that
you won't be able to ever find an inode that went out of cache.

In short cifs in it's current form is fundamentally unsuitable for
NFS exporting, and no papering over is going to fix that.  Offering
it to users is highly dangerous.

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Steve French]

On Mon, May 23, 2011 at 1:23 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> On Mon, May 23, 2011 at 01:21:35PM -0500, Steve French wrote:
>> Until Peter's Linux NFS fix is in - aren't we in that situation
>> already with other fs.
>
> That patch is not going to help with the fundamental problem that
> you won't be able to ever find an inode that went out of cache.

Isn't that what Peter's fix (and Solaris and other clients do) - they
revalidate the inode via another nfs lookup when it has gone stale.



-- 
Thanks,

Steve

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Steve French]

On Mon, May 23, 2011 at 1:24 PM, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, May 23, 2011 at 1:23 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>> On Mon, May 23, 2011 at 01:21:35PM -0500, Steve French wrote:
>>> Until Peter's Linux NFS fix is in - aren't we in that situation
>>> already with other fs.
>>
>> That patch is not going to help with the fundamental problem that
>> you won't be able to ever find an inode that went out of cache.
>
> Isn't that what Peter's fix (and Solaris and other clients do) - they
> revalidate the inode via another nfs lookup when it has gone stale.

With NFSv4, handles from network file systems, 9p, and FAT32 etc. can
be treated explicitly as "volatile" - an alternative is nfsd
restricting export to NFSv4 for those file systems.



-- 
Thanks,

Steve

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Jeff Layton]

On Mon, 23 May 2011 13:24:41 -0500
Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Mon, May 23, 2011 at 1:23 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> > On Mon, May 23, 2011 at 01:21:35PM -0500, Steve French wrote:
> >> Until Peter's Linux NFS fix is in - aren't we in that situation
> >> already with other fs.
> >
> > That patch is not going to help with the fundamental problem that
> > you won't be able to ever find an inode that went out of cache.
> 
> Isn't that what Peter's fix (and Solaris and other clients do) - they
> revalidate the inode via another nfs lookup when it has gone stale.
> 

Only if the client actually has a valid path to work with. That's not
guaranteed. Even if it were, pushing responsibility for this out to the
client totally violates the protocol.

It's the server's job to be able to identify filehandles that the
client presents. It should not have to rely on the client to look up
the right path to ensure that it's in its cache.

Even if the client were to do that, it's not 100% certain that it will
still be in the cache after the LOOKUP call and before the call that
wants to use a filehandle. If the server is under serious memory
pressure it could get pushed out of the cache again within that window.


-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH] cifs: mark CONFIG_CIFS_NFSD_EXPORT as BROKEN

[Steve French]

On Mon, May 23, 2011 at 1:47 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Mon, 23 May 2011 13:24:41 -0500
> Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> On Mon, May 23, 2011 at 1:23 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>> > On Mon, May 23, 2011 at 01:21:35PM -0500, Steve French wrote:
>> >> Until Peter's Linux NFS fix is in - aren't we in that situation
>> >> already with other fs.
>> >
>> > That patch is not going to help with the fundamental problem that
>> > you won't be able to ever find an inode that went out of cache.
>>
>> Isn't that what Peter's fix (and Solaris and other clients do) - they
>> revalidate the inode via another nfs lookup when it has gone stale.
>>
>
> Only if the client actually has a valid path to work with. That's not
> guaranteed. Even if it were, pushing responsibility for this out to the
> client totally violates the protocol.
>
> It's the server's job to be able to identify filehandles that the
> client presents. It should not have to rely on the client to look up
> the right path to ensure that it's in its cache.
>
> Even if the client were to do that, it's not 100% certain that it will
> still be in the cache after the LOOKUP call and before the call that
> wants to use a filehandle. If the server is under serious memory
> pressure it could get pushed out of the cache again within that window.

If the file is open the dentry will be in cache... right?

In any case, Linux has been doing this for many years with FAT.   See
fat_fh_to_dentry (in fs/fat/inode.c)

it calls ilookup on the fh - if it isn't in cache it calls
d_obtain_alias with null which causes ESTALE to go back from nfsd to
the nfs client.



-- 
Thanks,

Steve