Re: + knfsd-exportfs-add-exportfsh-header-fix.patch added to -mm tree

Re: + knfsd-exportfs-add-exportfsh-header-fix.patch added to -mm tree

[Christoph Hellwig]

On Tue, May 15, 2007 at 02:49:14PM -0700, akpm@linux-foundation.org wrote:
>  fs/cifs/export.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff -puN fs/cifs/export.c~knfsd-exportfs-add-exportfsh-header-fix fs/cifs/export.c
> --- a/fs/cifs/export.c~knfsd-exportfs-add-exportfsh-header-fix
> +++ a/fs/cifs/export.c
> @@ -29,7 +29,8 @@
>    */
>  
>  #include <linux/fs.h>
> - 
> +#include <linux/exportfs.h>
> +
>  #ifdef CONFIG_CIFS_EXPERIMENTAL

Looks like cifs has grown and export_operations table since I did the
patch.  But with only a get_parent method that returns and error it's
not useful at all, so we should rather remove the whole file:


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/fs/cifs/cifsfs.c
===================================================================
--- linux-2.6.orig/fs/cifs/cifsfs.c	2007-05-16 07:55:35.000000000 +0200
+++ linux-2.6/fs/cifs/cifsfs.c	2007-05-16 07:55:50.000000000 +0200
@@ -49,10 +49,6 @@
 static struct quotactl_ops cifs_quotactl_ops;
 #endif /* QUOTA */
 
-#ifdef CONFIG_CIFS_EXPERIMENTAL
-extern struct export_operations cifs_export_ops;
-#endif /* EXPERIMENTAL */
-
 int cifsFYI = 0;
 int cifsERROR = 1;
 int traceSMB = 0;
@@ -114,10 +110,6 @@ cifs_read_super(struct super_block *sb, 
 
 	sb->s_magic = CIFS_MAGIC_NUMBER;
 	sb->s_op = &cifs_super_ops;
-#ifdef CONFIG_CIFS_EXPERIMENTAL
-	if (experimEnabled != 0)
-		sb->s_export_op = &cifs_export_ops;
-#endif /* EXPERIMENTAL */	
 /*	if (cifs_sb->tcon->ses->server->maxBuf > MAX_CIFS_HDR_SIZE + 512)
 	    sb->s_blocksize = cifs_sb->tcon->ses->server->maxBuf - MAX_CIFS_HDR_SIZE; */
 #ifdef CONFIG_CIFS_QUOTA
Index: linux-2.6/fs/cifs/export.c
===================================================================
--- linux-2.6.orig/fs/cifs/export.c	2007-05-16 07:55:59.000000000 +0200
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,52 +0,0 @@
-/*
- *   fs/cifs/export.c
- *
- *   Copyright (C) International Business Machines  Corp., 2007
- *   Author(s): Steve French (sfrench@us.ibm.com)
- *
- *   Common Internet FileSystem (CIFS) client
- * 
- *   Operations related to support for exporting files via NFSD
- *
- *   This library is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU Lesser General Public License as published
- *   by the Free Software Foundation; either version 2.1 of the License, or
- *   (at your option) any later version.
- *
- *   This library is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY; without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU Lesser General Public License for more details.
- *
- *   You should have received a copy of the GNU Lesser General Public License
- *   along with this library; if not, write to the Free Software
- *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- */
- 
- /* 
-  * See Documentation/filesystems/Exporting
-  * and examples in fs/exportfs
-  */
-
-#include <linux/fs.h>
- 
-#ifdef CONFIG_CIFS_EXPERIMENTAL
- 
-static struct dentry *cifs_get_parent(struct dentry *dentry)
-{
- 	/* BB need to add code here eventually to enable export via NFSD */
- 	return ERR_PTR(-EACCES);
-}
- 
-struct export_operations cifs_export_ops = {
- 	.get_parent = cifs_get_parent,
-/*	Following five export operations are unneeded so far and can default */ 	
-/* 	.get_dentry =
- 	.get_name =
- 	.find_exported_dentry =
- 	.decode_fh = 
- 	.encode_fs =  */
- };
- 
-#endif /* EXPERIMENTAL */
- 

Re: + knfsd-exportfs-add-exportfsh-header-fix.patch added to -mm tree

[Christoph Hellwig]

On Wed, May 16, 2007 at 08:57:21AM +0200, Christoph Hellwig wrote:
> On Tue, May 15, 2007 at 02:49:14PM -0700, akpm@linux-foundation.org wrote:
> >  fs/cifs/export.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff -puN fs/cifs/export.c~knfsd-exportfs-add-exportfsh-header-fix fs/cifs/export.c
> > --- a/fs/cifs/export.c~knfsd-exportfs-add-exportfsh-header-fix
> > +++ a/fs/cifs/export.c
> > @@ -29,7 +29,8 @@
> >    */
> >  
> >  #include <linux/fs.h>
> > - 
> > +#include <linux/exportfs.h>
> > +
> >  #ifdef CONFIG_CIFS_EXPERIMENTAL
> 
> Looks like cifs has grown and export_operations table since I did the
> patch.  But with only a get_parent method that returns and error it's
> not useful at all, so we should rather remove the whole file:

That patch was missing the Makefile hunk, here's the proper one:
(I wish there was a way to avoid having to do quilt add everytime)


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/fs/cifs/cifsfs.c
===================================================================
--- linux-2.6.orig/fs/cifs/cifsfs.c	2007-05-16 07:55:35.000000000 +0200
+++ linux-2.6/fs/cifs/cifsfs.c	2007-05-16 07:55:50.000000000 +0200
@@ -49,10 +49,6 @@
 static struct quotactl_ops cifs_quotactl_ops;
 #endif /* QUOTA */
 
-#ifdef CONFIG_CIFS_EXPERIMENTAL
-extern struct export_operations cifs_export_ops;
-#endif /* EXPERIMENTAL */
-
 int cifsFYI = 0;
 int cifsERROR = 1;
 int traceSMB = 0;
@@ -114,10 +110,6 @@ cifs_read_super(struct super_block *sb, 
 
 	sb->s_magic = CIFS_MAGIC_NUMBER;
 	sb->s_op = &cifs_super_ops;
-#ifdef CONFIG_CIFS_EXPERIMENTAL
-	if (experimEnabled != 0)
-		sb->s_export_op = &cifs_export_ops;
-#endif /* EXPERIMENTAL */	
 /*	if (cifs_sb->tcon->ses->server->maxBuf > MAX_CIFS_HDR_SIZE + 512)
 	    sb->s_blocksize = cifs_sb->tcon->ses->server->maxBuf - MAX_CIFS_HDR_SIZE; */
 #ifdef CONFIG_CIFS_QUOTA
Index: linux-2.6/fs/cifs/export.c
===================================================================
--- linux-2.6.orig/fs/cifs/export.c	2007-05-16 07:55:59.000000000 +0200
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,52 +0,0 @@
-/*
- *   fs/cifs/export.c
- *
- *   Copyright (C) International Business Machines  Corp., 2007
- *   Author(s): Steve French (sfrench@us.ibm.com)
- *
- *   Common Internet FileSystem (CIFS) client
- * 
- *   Operations related to support for exporting files via NFSD
- *
- *   This library is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU Lesser General Public License as published
- *   by the Free Software Foundation; either version 2.1 of the License, or
- *   (at your option) any later version.
- *
- *   This library is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY; without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU Lesser General Public License for more details.
- *
- *   You should have received a copy of the GNU Lesser General Public License
- *   along with this library; if not, write to the Free Software
- *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- */
- 
- /* 
-  * See Documentation/filesystems/Exporting
-  * and examples in fs/exportfs
-  */
-
-#include <linux/fs.h>
- 
-#ifdef CONFIG_CIFS_EXPERIMENTAL
- 
-static struct dentry *cifs_get_parent(struct dentry *dentry)
-{
- 	/* BB need to add code here eventually to enable export via NFSD */
- 	return ERR_PTR(-EACCES);
-}
- 
-struct export_operations cifs_export_ops = {
- 	.get_parent = cifs_get_parent,
-/*	Following five export operations are unneeded so far and can default */ 	
-/* 	.get_dentry =
- 	.get_name =
- 	.find_exported_dentry =
- 	.decode_fh = 
- 	.encode_fs =  */
- };
- 
-#endif /* EXPERIMENTAL */
- 

Re: + knfsd-exportfs-add-exportfsh-header-fix.patch added to -mm tree

[Steven French]

Any ideas what are the minimum export operation(s) that cifs would need to 
add to export under nfsd?  It was not clear to me after reading the 
Exporting document in Documentation directory.

(some users had wanted to export files from Windows servers to nfs clients 
files by putting an nfs server mounted over cifs in between - I realize 
that this can corrupt data due to nfs client caching etc., as even in some 
cases could happen if you try to export a cluster file system under nfsd).


Steve French
Senior Software Engineer
Linux Technology Center - IBM Austin
phone: 512-838-2294
email: sfrench at-sign us dot ibm dot com

Re: + knfsd-exportfs-add-exportfsh-header-fix.patch added to -mm tree

[J. Bruce Fields]

On Wed, May 16, 2007 at 09:55:41AM -0500, Steven French wrote:
> Any ideas what are the minimum export operation(s) that cifs would need to 
> add to export under nfsd?  It was not clear to me after reading the 
> Exporting document in Documentation directory.
> 
> (some users had wanted to export files from Windows servers to nfs clients 
> files by putting an nfs server mounted over cifs in between - I realize 
> that this can corrupt data due to nfs client caching etc., as even in some 
> cases could happen if you try to export a cluster file system under nfsd).

What cases are you thinking of?

--b.

Re: + knfsd-exportfs-add-exportfsh-header-fix.patch added to -mm tree

[Steven French]

I thought that until a few days ago, a sequence like the following (two 
nfs servers exporting the same clustered data)

on client 1 lock range A through B of file1 (exported from nfs server 1)
on client 2 lock range A through C of file 1 (exported from nfs server 2)
on client 1 write  A through B
on client 2 write A through C
on client 1 unlock A through B
on client 2 unlock A through C

would corrupt data (theoretically could be fixed as nfsd calls lock 
methods 
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=fd85b8170dabbf021987875ef7f903791f4f181e) 
but the more obvious point is that with two nfsd servers exporting the 
same file data via the same cluster fs (under nfsd), the latencies can be 
longer and the opportunity for stale metadata (file sizes) and also writes 
getting reordered is higher.


Steve French
Senior Software Engineer
Linux Technology Center - IBM Austin
phone: 512-838-2294
email: sfrench at-sign us dot ibm dot com



"J. Bruce Fields" <bfields@fieldses.org> 
05/16/2007 11:02 AM

To
Steven French/Austin/IBM@IBMUS
cc
Christoph Hellwig <hch@lst.de>, akpm@linux-foundation.org, 
hch@infradead.org, linux-fsdevel@vger.kernel.org, neilb@suse.de
Subject
Re: + knfsd-exportfs-add-exportfsh-header-fix.patch added to -mm tree






On Wed, May 16, 2007 at 09:55:41AM -0500, Steven French wrote:
> Any ideas what are the minimum export operation(s) that cifs would need 
to 
> add to export under nfsd?  It was not clear to me after reading the 
> Exporting document in Documentation directory.
> 
> (some users had wanted to export files from Windows servers to nfs 
clients 
> files by putting an nfs server mounted over cifs in between - I realize 
> that this can corrupt data due to nfs client caching etc., as even in 
some 
> cases could happen if you try to export a cluster file system under 
nfsd).

What cases are you thinking of?

--b.

Re: + knfsd-exportfs-add-exportfsh-header-fix.patch added to -mm tree

[J. Bruce Fields]

On Wed, May 16, 2007 at 12:03:57PM -0500, Steven French wrote:
> I thought that until a few days ago, a sequence like the following (two 
> nfs servers exporting the same clustered data)
> 
> on client 1 lock range A through B of file1 (exported from nfs server 1)
> on client 2 lock range A through C of file 1 (exported from nfs server 2)
> on client 1 write  A through B
> on client 2 write A through C
> on client 1 unlock A through B
> on client 2 unlock A through C
> 
> would corrupt data (theoretically could be fixed as nfsd calls lock 
> methods 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=fd85b8170dabbf021987875ef7f903791f4f181e) 

Right.

> but the more obvious point is that with two nfsd servers exporting the 
> same file data via the same cluster fs (under nfsd), the latencies can be 
> longer and the opportunity for stale metadata (file sizes)

Hm.  How could nfsd get stale metadata?

I'm just (probably naively) assuming that a "cluster" filesystem
attempts to provide much higher cache consistency than actually
necessary to keep nfs clients happy.  But, if not, it would be nice to
understand the problem.

--b.

Re: + knfsd-exportfs-add-exportfsh-header-fix.patch added to -mm tree

[Neil Brown]

On Wednesday May 16, sfrench@us.ibm.com wrote:
> Any ideas what are the minimum export operation(s) that cifs would need to 
> add to export under nfsd?  It was not clear to me after reading the 
> Exporting document in Documentation directory.

You need to be able to map a dentry to a filehandle (you get about 20
bytes) and back again.
If CIFS provides some fix-length identifier for files, then you might
be able to do it.  If not, cannot really do it at all.  And I suspect
the later.  (There are other requirements, like get_parent, but we
could probably work around those if we really needed to).

Theoretically, you could make it work with NFSv4 and volatile file
handles, but I doubt it would really work in practice.  I don't think
the "volatile" concept quite stretch as far as you would need.

Probably the best way to nfs-export a CIFS filesystem is to use the
user-space nfs server.  It caches recently used filenames and uses a
filehandle which is a hash of the name.  It works on a best-effort
basis, and if, for example, the server restarts, you will lose
connections to open files.  While it is not perfect, it can be very
useful in some situations.

NeilBrown

Re: + knfsd-exportfs-add-exportfsh-header-fix.patch added to -mm tree

[Steven French]

> If CIFS provides some fix-length identifier for files, then 
> you might be able to do it

Most CIFS servers (Windows on NTFS, Samba etc.) can return a "unique 
identifier" (a 64 bit inode number), in conjunction with the volume id, 
that is probably good enough ... right?  This can be returned on various 
calls (level 0x03EE "file_internal_info" - returns only this number).  If 
reverse lookup is required - ie given a "unique identifier" what is its 
path name - there are probably a few different ways to handle this but 
presumably local filesystems run into the same issue.


Steve French
Senior Software Engineer
Linux Technology Center - IBM Austin
phone: 512-838-2294
email: sfrench at-sign us dot ibm dot com

Re: + knfsd-exportfs-add-exportfsh-header-fix.patch added to -mm tree

[Neil Brown]

>On Wednesday May 16, sfrench@us.ibm.com wrote:
> > If CIFS provides some fix-length identifier for files, then 
> > you might be able to do it
> 
> Most CIFS servers (Windows on NTFS, Samba etc.) can return a "unique 
> identifier" (a 64 bit inode number), in conjunction with the volume id, 
> that is probably good enough ... right?  This can be returned on various 
> calls (level 0x03EE "file_internal_info" - returns only this number).  If 
> reverse lookup is required - ie given a "unique identifier" what is its 
> path name - there are probably a few different ways to handle this but 
> presumably local filesystems run into the same issue.

Yes, that "unique identifier" sounds like it would be suitable to put
in the filehandle.
But reverse lookup is definitely required.
Providing you can turn this into a 'struct inode *' in the filesystem,
the code in exportfs/ can help turn that into a fully connected
dentry.

NeilBrown

Re: + knfsd-exportfs-add-exportfsh-header-fix.patch added to -mm tree

[Christoph Hellwig]

On Wed, May 16, 2007 at 09:55:41AM -0500, Steven French wrote:
> Any ideas what are the minimum export operation(s) that cifs would need to 
> add to export under nfsd?  It was not clear to me after reading the 
> Exporting document in Documentation directory.
> 
> (some users had wanted to export files from Windows servers to nfs clients 
> files by putting an nfs server mounted over cifs in between - I realize 
> that this can corrupt data due to nfs client caching etc., as even in some 
> cases could happen if you try to export a cluster file system under nfsd).

In current mainline knfsd will let you export a filesystem with no method
present at all, but you really need a ->get_parent to not corrupt data
once the dcache gets flushed or you reboot.  After this patchset you
will also need a ->get_dentry instead of the iget fallback before which
was the wrong thing for almost every filesystem.  Unless you actually
have traditional unix filesystem stable 32bit inode numbers and a generation
number you will also need a ->decode_fh and ->encode_fh method, but the
names and semantics of all these will change with some more patches I
have pending.  These patches will also document the requirements a little
better and enforce stricter checks of the required methods.