All of lore.kernel.org
 help / color / mirror / Atom feed
* open by handle support for NFS V2
@ 2017-06-29 13:34 Christoph Hellwig
  2017-06-29 13:34 ` [PATCH 1/4] nfs: replace d_add with d_splice_alias in atomic_open Christoph Hellwig
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Christoph Hellwig @ 2017-06-29 13:34 UTC (permalink / raw)
  To: trond.myklebust; +Cc: jlayton, schumaker.anna, bfields, linux-nfs

Hi all,

this resurrects parts of an old series to add open by handle support to
NFS.  The prime intent here is to support the actual open by handle
ioctls, although it will also allow very crude re-exporting.  Without
the other patches from Jeff's series that re-exporting will suck badly
though.

Changes since V1:
 - rebased on top of the nfs op constifycation (as merged in the nfsd tree)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/4] nfs: replace d_add with d_splice_alias in atomic_open
  2017-06-29 13:34 open by handle support for NFS V2 Christoph Hellwig
@ 2017-06-29 13:34 ` Christoph Hellwig
  2017-06-29 13:34 ` [PATCH 2/4] nfs: add a nfs_ilookup helper Christoph Hellwig
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2017-06-29 13:34 UTC (permalink / raw)
  To: trond.myklebust; +Cc: jlayton, schumaker.anna, bfields, linux-nfs, Peng Tao

From: Peng Tao <tao.peng@primarydata.com>

It's a trival change but follows knfsd export document that asks
for d_splice_alias during lookup.

Signed-off-by: Peng Tao <tao.peng@primarydata.com>
---
 fs/nfs/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 32ccd7754f8a..0296c06dcdc5 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1512,7 +1512,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
 		d_drop(dentry);
 		switch (err) {
 		case -ENOENT:
-			d_add(dentry, NULL);
+			d_splice_alias(NULL, dentry);
 			nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
 			break;
 		case -EISDIR:
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/4] nfs: add a nfs_ilookup helper
  2017-06-29 13:34 open by handle support for NFS V2 Christoph Hellwig
  2017-06-29 13:34 ` [PATCH 1/4] nfs: replace d_add with d_splice_alias in atomic_open Christoph Hellwig
@ 2017-06-29 13:34 ` Christoph Hellwig
  2017-06-29 13:34 ` [PATCH 3/4] nfs4: add NFSv4 LOOKUPP handlers Christoph Hellwig
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2017-06-29 13:34 UTC (permalink / raw)
  To: trond.myklebust; +Cc: jlayton, schumaker.anna, bfields, linux-nfs, Peng Tao

From: Peng Tao <tao.peng@primarydata.com>

This helper will allow to find an existing NFS inode by the file handle
and fattr.

Signed-off-by: Peng Tao <tao.peng@primarydata.com>
[hch: split from a larger patch]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/inode.c         | 22 ++++++++++++++++++++++
 include/linux/nfs_fs.h |  1 +
 2 files changed, 23 insertions(+)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 1de93ba78dc9..a84eab1a18a7 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -386,6 +386,28 @@ void nfs_setsecurity(struct inode *inode, struct nfs_fattr *fattr,
 #endif
 EXPORT_SYMBOL_GPL(nfs_setsecurity);
 
+/* Search for inode identified by fh, fileid and i_mode in inode cache. */
+struct inode *
+nfs_ilookup(struct super_block *sb, struct nfs_fattr *fattr, struct nfs_fh *fh)
+{
+	struct nfs_find_desc desc = {
+		.fh	= fh,
+		.fattr	= fattr,
+	};
+	struct inode *inode;
+	unsigned long hash;
+
+	if (!(fattr->valid & NFS_ATTR_FATTR_FILEID) ||
+	    !(fattr->valid & NFS_ATTR_FATTR_TYPE))
+		return NULL;
+
+	hash = nfs_fattr_to_ino_t(fattr);
+	inode = ilookup5(sb, hash, nfs_find_actor, &desc);
+
+	dprintk("%s: returning %p\n", __func__, inode);
+	return inode;
+}
+
 /*
  * This is our front-end to iget that looks up inodes by file handle
  * instead of inode number.
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index bb0eb2c9acca..e52cc55ac300 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -332,6 +332,7 @@ extern void nfs_zap_caches(struct inode *);
 extern void nfs_invalidate_atime(struct inode *);
 extern struct inode *nfs_fhget(struct super_block *, struct nfs_fh *,
 				struct nfs_fattr *, struct nfs4_label *);
+struct inode *nfs_ilookup(struct super_block *sb, struct nfs_fattr *, struct nfs_fh *);
 extern int nfs_refresh_inode(struct inode *, struct nfs_fattr *);
 extern int nfs_post_op_update_inode(struct inode *inode, struct nfs_fattr *fattr);
 extern int nfs_post_op_update_inode_force_wcc(struct inode *inode, struct nfs_fattr *fattr);
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/4] nfs4: add NFSv4 LOOKUPP handlers
  2017-06-29 13:34 open by handle support for NFS V2 Christoph Hellwig
  2017-06-29 13:34 ` [PATCH 1/4] nfs: replace d_add with d_splice_alias in atomic_open Christoph Hellwig
  2017-06-29 13:34 ` [PATCH 2/4] nfs: add a nfs_ilookup helper Christoph Hellwig
@ 2017-06-29 13:34 ` Christoph Hellwig
  2017-06-29 13:34 ` [PATCH 4/4] nfs: add export operations Christoph Hellwig
  2017-06-29 15:46 ` open by handle support for NFS V2 J. Bruce Fields
  4 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2017-06-29 13:34 UTC (permalink / raw)
  To: trond.myklebust; +Cc: jlayton, schumaker.anna, bfields, linux-nfs, Jeff Layton

From: Jeff Layton <jeff.layton@primarydata.com>

This will be needed in order to implement the get_parent export op
for nfsd.

Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
---
 fs/nfs/nfs4proc.c       | 49 ++++++++++++++++++++++++++++++++
 fs/nfs/nfs4trace.h      | 29 +++++++++++++++++++
 fs/nfs/nfs4xdr.c        | 75 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/nfs4.h    |  1 +
 include/linux/nfs_xdr.h | 17 ++++++++++-
 5 files changed, 170 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index c08c46a3b8cd..6fd9eee8e4ee 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3802,6 +3802,54 @@ nfs4_proc_lookup_mountpoint(struct inode *dir, const struct qstr *name,
 	return (client == NFS_CLIENT(dir)) ? rpc_clone_client(client) : client;
 }
 
+static int _nfs4_proc_lookupp(struct inode *inode,
+		struct nfs_fh *fhandle, struct nfs_fattr *fattr,
+		struct nfs4_label *label)
+{
+	struct rpc_clnt *clnt = NFS_CLIENT(inode);
+	struct nfs_server *server = NFS_SERVER(inode);
+	int		       status;
+	struct nfs4_lookupp_arg args = {
+		.bitmask = server->attr_bitmask,
+		.fh = NFS_FH(inode),
+	};
+	struct nfs4_lookupp_res res = {
+		.server = server,
+		.fattr = fattr,
+		.label = label,
+		.fh = fhandle,
+	};
+	struct rpc_message msg = {
+		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_LOOKUPP],
+		.rpc_argp = &args,
+		.rpc_resp = &res,
+	};
+
+	args.bitmask = nfs4_bitmask(server, label);
+
+	nfs_fattr_init(fattr);
+
+	dprintk("NFS call  lookupp ino=0x%lx\n", inode->i_ino);
+	status = nfs4_call_sync(clnt, server, &msg, &args.seq_args,
+				&res.seq_res, 0);
+	dprintk("NFS reply lookupp: %d\n", status);
+	return status;
+}
+
+static int nfs4_proc_lookupp(struct inode *inode, struct nfs_fh *fhandle,
+			     struct nfs_fattr *fattr, struct nfs4_label *label)
+{
+	struct nfs4_exception exception = { };
+	int err;
+	do {
+		err = _nfs4_proc_lookupp(inode, fhandle, fattr, label);
+		trace_nfs4_lookupp(inode, err);
+		err = nfs4_handle_exception(NFS_SERVER(inode), err,
+				&exception);
+	} while (exception.retry);
+	return err;
+}
+
 static int _nfs4_proc_access(struct inode *inode, struct nfs_access_entry *entry)
 {
 	struct nfs_server *server = NFS_SERVER(inode);
@@ -9312,6 +9360,7 @@ const struct nfs_rpc_ops nfs_v4_clientops = {
 	.getattr	= nfs4_proc_getattr,
 	.setattr	= nfs4_proc_setattr,
 	.lookup		= nfs4_proc_lookup,
+	.lookupp	= nfs4_proc_lookupp,
 	.access		= nfs4_proc_access,
 	.readlink	= nfs4_proc_readlink,
 	.create		= nfs4_proc_create,
diff --git a/fs/nfs/nfs4trace.h b/fs/nfs/nfs4trace.h
index 845d0eadefc9..be1da19c65d6 100644
--- a/fs/nfs/nfs4trace.h
+++ b/fs/nfs/nfs4trace.h
@@ -891,6 +891,35 @@ DEFINE_NFS4_LOOKUP_EVENT(nfs4_remove);
 DEFINE_NFS4_LOOKUP_EVENT(nfs4_get_fs_locations);
 DEFINE_NFS4_LOOKUP_EVENT(nfs4_secinfo);
 
+TRACE_EVENT(nfs4_lookupp,
+		TP_PROTO(
+			const struct inode *inode,
+			int error
+		),
+
+		TP_ARGS(inode, error),
+
+		TP_STRUCT__entry(
+			__field(dev_t, dev)
+			__field(u64, ino)
+			__field(int, error)
+		),
+
+		TP_fast_assign(
+			__entry->dev = inode->i_sb->s_dev;
+			__entry->ino = NFS_FILEID(inode);
+			__entry->error = error;
+		),
+
+		TP_printk(
+			"error=%d (%s) inode=%02x:%02x:%llu",
+			__entry->error,
+			show_nfsv4_errors(__entry->error),
+			MAJOR(__entry->dev), MINOR(__entry->dev),
+			(unsigned long long)__entry->ino
+		)
+);
+
 TRACE_EVENT(nfs4_rename,
 		TP_PROTO(
 			const struct inode *olddir,
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 0f1f290c97cd..ad50ae499b6b 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -159,6 +159,8 @@ static int nfs4_stat_to_errno(int);
 				(op_decode_hdr_maxsz)
 #define encode_lookup_maxsz	(op_encode_hdr_maxsz + nfs4_name_maxsz)
 #define decode_lookup_maxsz	(op_decode_hdr_maxsz)
+#define encode_lookupp_maxsz	(op_encode_hdr_maxsz)
+#define decode_lookupp_maxsz	(op_decode_hdr_maxsz)
 #define encode_share_access_maxsz \
 				(2)
 #define encode_createmode_maxsz	(1 + encode_attrs_maxsz + encode_verifier_maxsz)
@@ -618,6 +620,18 @@ static int nfs4_stat_to_errno(int);
 				decode_lookup_maxsz + \
 				decode_getattr_maxsz + \
 				decode_getfh_maxsz)
+#define NFS4_enc_lookupp_sz	(compound_encode_hdr_maxsz + \
+				encode_sequence_maxsz + \
+				encode_putfh_maxsz + \
+				encode_lookupp_maxsz + \
+				encode_getattr_maxsz + \
+				encode_getfh_maxsz)
+#define NFS4_dec_lookupp_sz	(compound_decode_hdr_maxsz + \
+				decode_sequence_maxsz + \
+				decode_putfh_maxsz + \
+				decode_lookupp_maxsz + \
+				decode_getattr_maxsz + \
+				decode_getfh_maxsz)
 #define NFS4_enc_lookup_root_sz (compound_encode_hdr_maxsz + \
 				encode_sequence_maxsz + \
 				encode_putrootfh_maxsz + \
@@ -1368,6 +1382,11 @@ static void encode_lookup(struct xdr_stream *xdr, const struct qstr *name, struc
 	encode_string(xdr, name->len, name->name);
 }
 
+static void encode_lookupp(struct xdr_stream *xdr, struct compound_hdr *hdr)
+{
+	encode_op_hdr(xdr, OP_LOOKUPP, decode_lookupp_maxsz, hdr);
+}
+
 static void encode_share_access(struct xdr_stream *xdr, u32 share_access)
 {
 	__be32 *p;
@@ -2123,6 +2142,26 @@ static void nfs4_xdr_enc_lookup(struct rpc_rqst *req, struct xdr_stream *xdr,
 }
 
 /*
+ * Encode LOOKUPP request
+ */
+static void nfs4_xdr_enc_lookupp(struct rpc_rqst *req, struct xdr_stream *xdr,
+		const void *data)
+{
+	const struct nfs4_lookupp_arg *args = data;
+	struct compound_hdr hdr = {
+		.minorversion = nfs4_xdr_minorversion(&args->seq_args),
+	};
+
+	encode_compound_hdr(xdr, req, &hdr);
+	encode_sequence(xdr, &args->seq_args, &hdr);
+	encode_putfh(xdr, args->fh, &hdr);
+	encode_lookupp(xdr, &hdr);
+	encode_getfh(xdr, &hdr);
+	encode_getfattr(xdr, args->bitmask, &hdr);
+	encode_nops(&hdr);
+}
+
+/*
  * Encode LOOKUP_ROOT request
  */
 static void nfs4_xdr_enc_lookup_root(struct rpc_rqst *req,
@@ -5058,6 +5097,11 @@ static int decode_lookup(struct xdr_stream *xdr)
 	return decode_op_hdr(xdr, OP_LOOKUP);
 }
 
+static int decode_lookupp(struct xdr_stream *xdr)
+{
+	return decode_op_hdr(xdr, OP_LOOKUPP);
+}
+
 /* This is too sick! */
 static int decode_space_limit(struct xdr_stream *xdr,
 		unsigned long *pagemod_limit)
@@ -6238,6 +6282,36 @@ static int nfs4_xdr_dec_lookup(struct rpc_rqst *rqstp, struct xdr_stream *xdr,
 }
 
 /*
+ * Decode LOOKUPP response
+ */
+static int nfs4_xdr_dec_lookupp(struct rpc_rqst *rqstp, struct xdr_stream *xdr,
+		void *data)
+{
+	struct nfs4_lookupp_res *res = data;
+	struct compound_hdr hdr;
+	int status;
+
+	status = decode_compound_hdr(xdr, &hdr);
+	if (status)
+		goto out;
+	status = decode_sequence(xdr, &res->seq_res, rqstp);
+	if (status)
+		goto out;
+	status = decode_putfh(xdr);
+	if (status)
+		goto out;
+	status = decode_lookupp(xdr);
+	if (status)
+		goto out;
+	status = decode_getfh(xdr, res->fh);
+	if (status)
+		goto out;
+	status = decode_getfattr_label(xdr, res->fattr, res->label, res->server);
+out:
+	return status;
+}
+
+/*
  * Decode LOOKUP_ROOT response
  */
 static int nfs4_xdr_dec_lookup_root(struct rpc_rqst *rqstp,
@@ -7614,6 +7688,7 @@ const struct rpc_procinfo nfs4_procedures[] = {
 	PROC(ACCESS,		enc_access,		dec_access),
 	PROC(GETATTR,		enc_getattr,		dec_getattr),
 	PROC(LOOKUP,		enc_lookup,		dec_lookup),
+	PROC(LOOKUPP,		enc_lookupp,		dec_lookupp),
 	PROC(LOOKUP_ROOT,	enc_lookup_root,	dec_lookup_root),
 	PROC(REMOVE,		enc_remove,		dec_remove),
 	PROC(RENAME,		enc_rename,		dec_rename),
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 1b1ca04820a3..47239c336688 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -479,6 +479,7 @@ enum {
 	NFSPROC4_CLNT_ACCESS,
 	NFSPROC4_CLNT_GETATTR,
 	NFSPROC4_CLNT_LOOKUP,
+	NFSPROC4_CLNT_LOOKUPP,
 	NFSPROC4_CLNT_LOOKUP_ROOT,
 	NFSPROC4_CLNT_REMOVE,
 	NFSPROC4_CLNT_RENAME,
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index b28c83475ee8..7a664a5fcc25 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1012,7 +1012,6 @@ struct nfs4_link_res {
 	struct nfs_fattr *		dir_attr;
 };
 
-
 struct nfs4_lookup_arg {
 	struct nfs4_sequence_args	seq_args;
 	const struct nfs_fh *		dir_fh;
@@ -1028,6 +1027,20 @@ struct nfs4_lookup_res {
 	struct nfs4_label		*label;
 };
 
+struct nfs4_lookupp_arg {
+	struct nfs4_sequence_args	seq_args;
+	const struct nfs_fh		*fh;
+	const u32			*bitmask;
+};
+
+struct nfs4_lookupp_res {
+	struct nfs4_sequence_res	seq_res;
+	const struct nfs_server		*server;
+	struct nfs_fattr		*fattr;
+	struct nfs_fh			*fh;
+	struct nfs4_label		*label;
+};
+
 struct nfs4_lookup_root_arg {
 	struct nfs4_sequence_args	seq_args;
 	const u32 *			bitmask;
@@ -1567,6 +1580,8 @@ struct nfs_rpc_ops {
 	int	(*lookup)  (struct inode *, const struct qstr *,
 			    struct nfs_fh *, struct nfs_fattr *,
 			    struct nfs4_label *);
+	int	(*lookupp) (struct inode *, struct nfs_fh *,
+			    struct nfs_fattr *, struct nfs4_label *);
 	int	(*access)  (struct inode *, struct nfs_access_entry *);
 	int	(*readlink)(struct inode *, struct page *, unsigned int,
 			    unsigned int);
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/4] nfs: add export operations
  2017-06-29 13:34 open by handle support for NFS V2 Christoph Hellwig
                   ` (2 preceding siblings ...)
  2017-06-29 13:34 ` [PATCH 3/4] nfs4: add NFSv4 LOOKUPP handlers Christoph Hellwig
@ 2017-06-29 13:34 ` Christoph Hellwig
  2017-06-29 15:46 ` open by handle support for NFS V2 J. Bruce Fields
  4 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2017-06-29 13:34 UTC (permalink / raw)
  To: trond.myklebust; +Cc: jlayton, schumaker.anna, bfields, linux-nfs, Peng Tao

From: Peng Tao <tao.peng@primarydata.com>

This support for opening files on NFS by file handle, both through the
open_by_handle syscall, and for re-exporting NFS (for example using a
different version).  The support is very basic for now, as each open by
handle will have to do an NFSv4 open operation on the wire.  In the
future this will hopefully be mitigated by an open file cache, as well
as various optimizations in NFS for this specific case.

Signed-off-by: Peng Tao <tao.peng@primarydata.com>
[hch: incorporated various changes, resplit the patches, new changelog]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/Makefile   |   2 +-
 fs/nfs/export.c   | 177 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfs/internal.h |   2 +
 fs/nfs/super.c    |   2 +
 4 files changed, 182 insertions(+), 1 deletion(-)
 create mode 100644 fs/nfs/export.c

diff --git a/fs/nfs/Makefile b/fs/nfs/Makefile
index 98f4e5728a67..1fb118902d57 100644
--- a/fs/nfs/Makefile
+++ b/fs/nfs/Makefile
@@ -7,7 +7,7 @@ obj-$(CONFIG_NFS_FS) += nfs.o
 CFLAGS_nfstrace.o += -I$(src)
 nfs-y 			:= client.o dir.o file.o getroot.o inode.o super.o \
 			   io.o direct.o pagelist.o read.o symlink.o unlink.o \
-			   write.o namespace.o mount_clnt.o nfstrace.o
+			   write.o namespace.o mount_clnt.o nfstrace.o export.o
 nfs-$(CONFIG_ROOT_NFS)	+= nfsroot.o
 nfs-$(CONFIG_SYSCTL)	+= sysctl.o
 nfs-$(CONFIG_NFS_FSCACHE) += fscache.o fscache-index.o
diff --git a/fs/nfs/export.c b/fs/nfs/export.c
new file mode 100644
index 000000000000..249cb96cc5b5
--- /dev/null
+++ b/fs/nfs/export.c
@@ -0,0 +1,177 @@
+/*
+ * Copyright (c) 2015, Primary Data, Inc. All rights reserved.
+ *
+ * Tao Peng <bergwolf@primarydata.com>
+ */
+#include <linux/dcache.h>
+#include <linux/exportfs.h>
+#include <linux/nfs.h>
+#include <linux/nfs_fs.h>
+
+#include "internal.h"
+#include "nfstrace.h"
+
+#define NFSDBG_FACILITY		NFSDBG_VFS
+
+enum {
+	FILEID_HIGH_OFF = 0,	/* inode fileid high */
+	FILEID_LOW_OFF,		/* inode fileid low */
+	FILE_I_TYPE_OFF,	/* inode type */
+	EMBED_FH_OFF		/* embeded server fh */
+};
+
+
+static struct nfs_fh *nfs_exp_embedfh(__u32 *p)
+{
+	return (struct nfs_fh *)(p + EMBED_FH_OFF);
+}
+
+/*
+ * Let's break subtree checking for now... otherwise we'll have to embed parent fh
+ * but there might not be enough space.
+ */
+static int
+nfs_encode_fh(struct inode *inode, __u32 *p, int *max_len, struct inode *parent)
+{
+	struct nfs_fh *server_fh = NFS_FH(inode);
+	struct nfs_fh *clnt_fh = nfs_exp_embedfh(p);
+	size_t fh_size = offsetof(struct nfs_fh, data) + server_fh->size;
+	int len = EMBED_FH_OFF + XDR_QUADLEN(fh_size);
+
+	dprintk("%s: max fh len %d inode %p parent %p",
+		__func__, *max_len, inode, parent);
+
+	if (*max_len < len || IS_AUTOMOUNT(inode)) {
+		dprintk("%s: fh len %d too small, required %d\n",
+			__func__, *max_len, len);
+		*max_len = len;
+		return FILEID_INVALID;
+	}
+	if (IS_AUTOMOUNT(inode)) {
+		*max_len = FILEID_INVALID;
+		goto out;
+	}
+
+	p[FILEID_HIGH_OFF] = NFS_FILEID(inode) >> 32;
+	p[FILEID_LOW_OFF] = NFS_FILEID(inode);
+	p[FILE_I_TYPE_OFF] = inode->i_mode & S_IFMT;
+	p[len - 1] = 0; /* Padding */
+	nfs_copy_fh(clnt_fh, server_fh);
+	*max_len = len;
+out:
+	dprintk("%s: result fh fileid %llu mode %u size %d\n",
+		__func__, NFS_FILEID(inode), inode->i_mode, *max_len);
+	return *max_len;
+}
+
+static struct dentry *
+nfs_fh_to_dentry(struct super_block *sb, struct fid *fid,
+		 int fh_len, int fh_type)
+{
+	struct nfs4_label *label = NULL;
+	struct nfs_fattr *fattr = NULL;
+	struct nfs_fh *server_fh = nfs_exp_embedfh(fid->raw);
+	size_t fh_size = offsetof(struct nfs_fh, data) + server_fh->size;
+	const struct nfs_rpc_ops *rpc_ops;
+	struct dentry *dentry;
+	struct inode *inode;
+	int len = EMBED_FH_OFF + XDR_QUADLEN(fh_size);
+	u32 *p = fid->raw;
+	int ret;
+
+	/* NULL translates to ESTALE */
+	if (fh_len < len || fh_type != len)
+		return NULL;
+
+	fattr = nfs_alloc_fattr();
+	if (fattr == NULL) {
+		dentry = ERR_PTR(-ENOMEM);
+		goto out;
+	}
+
+	fattr->fileid = ((u64)p[FILEID_HIGH_OFF] << 32) + p[FILEID_LOW_OFF];
+	fattr->mode = p[FILE_I_TYPE_OFF];
+	fattr->valid |= NFS_ATTR_FATTR_FILEID | NFS_ATTR_FATTR_TYPE;
+
+	dprintk("%s: fileid %llu mode %d\n", __func__, fattr->fileid, fattr->mode);
+
+	inode = nfs_ilookup(sb, fattr, server_fh);
+	if (inode)
+		goto out_found;
+
+	label = nfs4_label_alloc(NFS_SB(sb), GFP_KERNEL);
+	if (IS_ERR(label)) {
+		dentry = ERR_CAST(label);
+		goto out_free_fattr;
+	}
+
+	rpc_ops = NFS_SB(sb)->nfs_client->rpc_ops;
+	ret = rpc_ops->getattr(NFS_SB(sb), server_fh, fattr, label);
+	if (ret) {
+		dprintk("%s: getattr failed %d\n", __func__, ret);
+		dentry = ERR_PTR(ret);
+		goto out_free_label;
+	}
+
+	inode = nfs_fhget(sb, server_fh, fattr, label);
+
+out_found:
+	dentry = d_obtain_alias(inode);
+
+out_free_label:
+	nfs4_label_free(label);
+out_free_fattr:
+	nfs_free_fattr(fattr);
+out:
+	return dentry;
+}
+
+static struct dentry *
+nfs_get_parent(struct dentry *dentry)
+{
+	int ret;
+	struct inode *inode = d_inode(dentry), *pinode;
+	struct super_block *sb = inode->i_sb;
+	struct nfs_server *server = NFS_SB(sb);
+	struct nfs_fattr *fattr = NULL;
+	struct nfs4_label *label = NULL;
+	struct dentry *parent;
+	struct nfs_rpc_ops const *ops = server->nfs_client->rpc_ops;
+	struct nfs_fh fh;
+
+	if (!ops->lookupp)
+		return ERR_PTR(-EACCES);
+
+	fattr = nfs_alloc_fattr();
+	if (fattr == NULL) {
+		parent = ERR_PTR(-ENOMEM);
+		goto out;
+	}
+
+	label = nfs4_label_alloc(server, GFP_KERNEL);
+	if (IS_ERR(label)) {
+		parent = ERR_CAST(label);
+		goto out_free_fattr;
+	}
+
+	ret = ops->lookupp(inode, &fh, fattr, label);
+	if (ret) {
+		parent = ERR_PTR(ret);
+		goto out_free_label;
+	}
+
+	pinode = nfs_fhget(sb, &fh, fattr, label);
+	parent = d_obtain_alias(pinode);
+out_free_label:
+	nfs4_label_free(label);
+out_free_fattr:
+	nfs_free_fattr(fattr);
+out:
+	return parent;
+}
+
+const struct export_operations nfs_export_ops = {
+	.encode_fh = nfs_encode_fh,
+	.fh_to_dentry = nfs_fh_to_dentry,
+	.get_parent = nfs_get_parent,
+};
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 9976d8498cf3..35230b1b7a74 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -10,6 +10,8 @@
 
 #define NFS_MS_MASK (MS_RDONLY|MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_SYNCHRONOUS)
 
+extern const struct export_operations nfs_export_ops;
+
 struct nfs_string;
 
 /* Maximum number of readahead requests
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 2f3822a4a7d5..508ad3a1cf4e 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2339,6 +2339,7 @@ void nfs_fill_super(struct super_block *sb, struct nfs_mount_info *mount_info)
 		 */
 		sb->s_flags |= MS_POSIXACL;
 		sb->s_time_gran = 1;
+		sb->s_export_op = &nfs_export_ops;
 	}
 
  	nfs_initialise_sb(sb);
@@ -2359,6 +2360,7 @@ void nfs_clone_super(struct super_block *sb, struct nfs_mount_info *mount_info)
 	sb->s_xattr = old_sb->s_xattr;
 	sb->s_op = old_sb->s_op;
 	sb->s_time_gran = 1;
+	sb->s_export_op = old_sb->s_export_op;
 
 	if (server->nfs_client->rpc_ops->version != 2) {
 		/* The VFS shouldn't apply the umask to mode bits. We will do
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: open by handle support for NFS V2
  2017-06-29 13:34 open by handle support for NFS V2 Christoph Hellwig
                   ` (3 preceding siblings ...)
  2017-06-29 13:34 ` [PATCH 4/4] nfs: add export operations Christoph Hellwig
@ 2017-06-29 15:46 ` J. Bruce Fields
  2017-06-30 17:00   ` Trond Myklebust
  4 siblings, 1 reply; 16+ messages in thread
From: J. Bruce Fields @ 2017-06-29 15:46 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: trond.myklebust, jlayton, schumaker.anna, linux-nfs

On Thu, Jun 29, 2017 at 06:34:49AM -0700, Christoph Hellwig wrote:
> this resurrects parts of an old series to add open by handle support to
> NFS.  The prime intent here is to support the actual open by handle
> ioctls, although it will also allow very crude re-exporting.  Without
> the other patches from Jeff's series that re-exporting will suck badly
> though.

Why do we want this?

Any re-export support is going to have some major limitations.  (No file
locking, and re-export of NFSv4 probably not possible?)

Last I heard the only motivation was extremely specific to Primary
Data's setup.  I'm happy to help them, but I think we need *some*
evidence this will be useful to upstream users.

--b.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: open by handle support for NFS V2
  2017-06-29 15:46 ` open by handle support for NFS V2 J. Bruce Fields
@ 2017-06-30 17:00   ` Trond Myklebust
  2017-06-30 17:38     ` bfields
  2017-07-07  2:41     ` NeilBrown
  0 siblings, 2 replies; 16+ messages in thread
From: Trond Myklebust @ 2017-06-30 17:00 UTC (permalink / raw)
  To: bfields, hch; +Cc: jlayton, linux-nfs, schumaker.anna

T24gVGh1LCAyMDE3LTA2LTI5IGF0IDExOjQ2IC0wNDAwLCBKLiBCcnVjZSBGaWVsZHMgd3JvdGU6
DQo+IE9uIFRodSwgSnVuIDI5LCAyMDE3IGF0IDA2OjM0OjQ5QU0gLTA3MDAsIENocmlzdG9waCBI
ZWxsd2lnIHdyb3RlOg0KPiA+IHRoaXMgcmVzdXJyZWN0cyBwYXJ0cyBvZiBhbiBvbGQgc2VyaWVz
IHRvIGFkZCBvcGVuIGJ5IGhhbmRsZQ0KPiA+IHN1cHBvcnQgdG8NCj4gPiBORlMuwqDCoFRoZSBw
cmltZSBpbnRlbnQgaGVyZSBpcyB0byBzdXBwb3J0IHRoZSBhY3R1YWwgb3BlbiBieSBoYW5kbGUN
Cj4gPiBpb2N0bHMsIGFsdGhvdWdoIGl0IHdpbGwgYWxzbyBhbGxvdyB2ZXJ5IGNydWRlIHJlLQ0K
PiA+IGV4cG9ydGluZy7CoMKgV2l0aG91dA0KPiA+IHRoZSBvdGhlciBwYXRjaGVzIGZyb20gSmVm
ZidzIHNlcmllcyB0aGF0IHJlLWV4cG9ydGluZyB3aWxsIHN1Y2sNCj4gPiBiYWRseQ0KPiA+IHRo
b3VnaC4NCj4gDQo+IFdoeSBkbyB3ZSB3YW50IHRoaXM/DQo+IA0KPiBBbnkgcmUtZXhwb3J0IHN1
cHBvcnQgaXMgZ29pbmcgdG8gaGF2ZSBzb21lIG1ham9yIGxpbWl0YXRpb25zLsKgwqAoTm8NCj4g
ZmlsZQ0KPiBsb2NraW5nLCBhbmQgcmUtZXhwb3J0IG9mIE5GU3Y0IHByb2JhYmx5IG5vdCBwb3Nz
aWJsZT8pDQo+IA0KPiBMYXN0IEkgaGVhcmQgdGhlIG9ubHkgbW90aXZhdGlvbiB3YXMgZXh0cmVt
ZWx5IHNwZWNpZmljIHRvIFByaW1hcnkNCj4gRGF0YSdzIHNldHVwLsKgwqBJJ20gaGFwcHkgdG8g
aGVscCB0aGVtLCBidXQgSSB0aGluayB3ZSBuZWVkICpzb21lKg0KPiBldmlkZW5jZSB0aGlzIHdp
bGwgYmUgdXNlZnVsIHRvIHVwc3RyZWFtIHVzZXJzLg0KPiANCg0KVGhlIG1haW4gdXNlIGNhc2Ug
Zm9yIG9wZW4gYnkgZmlsZWhhbmRsZSB3YXMgKGFuZCBzdGlsbCBzaG91bGQgYmUpIHRoZQ0KcHJv
bWlzZSBvZiBiZWluZyBhYmxlIHRvIGRvIHRoZSBzb3J0IG9mIHRyaWNrcyB5b3Ugbm9ybWFsbHkg
YXNzb2NpYXRlDQp3aXRoIG9iamVjdCBzdG9yYWdlIG9uIGEgc3RhbmRhcmQgZmlsZXN5c3RlbS4N
Cg0KSW1hZ2luZSB0aGF0IHlvdSBhcmUgdHJ5aW5nIHRvIGJ1aWxkIGFuIGFwcGxpY2F0aW9uIGZv
ciBpbmRleGluZyBhbmQNCnNlYXJjaGluZyB0aGUgZGF0YSBvbiB5b3VyIHN0b3JhZ2UuIFlvdSBi
YXNpY2FsbHkgd2FudCB0byB0cmF3bCB0aHJvdWdoDQp0aGUgZmlsZXN5c3RlbSBvbiBhIHJlZ3Vs
YXIgYmFzaXMgYW5kIGJ1aWxkIHVwIGEgZGF0YWJhc2Ugb2Yga2V5IHdvcmRzDQphbmQgb3RoZXIg
bWV0YWRhdGEgdG8gdGVsbCB5b3Ugd2hhdCBpcyBpbiB0aGUgZmlsZXMuIEZvciB0aGF0IGtpbmQg
b2YNCmFwcGxpY2F0aW9uLCB0aGUgbmFtZXNwYWNlIGlzIGEgcmVhbCBQSVRBIHRvIGRlYWwgd2l0
aCwgYmVjYXVzZSBmaWxlcw0KZ2V0IHJlbmFtZWQsIG1vdmVkIGFuZCBkZWxldGVkIGFsbCB0aGUg
dGltZTsgc28gaWYgeW91IGNhbiBzdG9yZQ0Kc29tZXRoaW5nIHRoYXQgaXMgaW5kZXBlbmRlbnQg
b2YgdGhlIG5hbWVzcGFjZSBhbmQgdGhhdCB3aWxsIGdpdmUgeW91DQphY2Nlc3MgdG8gdGhlIGZp
bGUgY29udGVudHMsIHRoZW4gd2h5IHdvdWxkbid0IHlvdSBkbyBzbz8gTm9ybWFsbHksDQphcHBs
aWNhdGlvbnMgbGlrZSB0aGF0IHVzZSB0aGUgaW5vZGUgbnVtYmVyLCBidXQgeW91IGNhbid0IG9w
ZW4gYSBmaWxlDQpieSBpbm9kZSBudW1iZXIsIGFuZCB5b3UgaGF2ZSB0aGUgc2FtZSBwcm9ibGVt
cyB3aXRoIGlub2RlIG51bWJlciByZXVzZQ0KdGhhdCBhIE5GUyBzZXJ2ZXIgaGFzLg0KDQpUaGF0
J3MgdGhlIHNvcnQgb2YgdGhpbmcgSSdkIHRoaW5rIHdlIHdhbnQgdG8gYWxsb3cgdGhyb3VnaCBv
cGVuIGJ5DQpmaWxlaGFuZGxlLCBhbmQgSSBzZWUgbm8gcmVhc29uIHdoeSBORlMgc2hvdWxkIGJl
IGV4Y2x1ZGVkIGZyb20gdGhhdA0KdHlwZSBvZiBhcHBsaWNhdGlvbi4NCg0KLS0gDQpUcm9uZCBN
eWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lciwgUHJpbWFyeURhdGENCnRyb25k
Lm15a2xlYnVzdEBwcmltYXJ5ZGF0YS5jb20NCg==


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: open by handle support for NFS V2
  2017-06-30 17:00   ` Trond Myklebust
@ 2017-06-30 17:38     ` bfields
  2017-07-01  9:34       ` Mkrtchyan, Tigran
  2017-07-07  2:41     ` NeilBrown
  1 sibling, 1 reply; 16+ messages in thread
From: bfields @ 2017-06-30 17:38 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: hch, jlayton, linux-nfs, schumaker.anna

On Fri, Jun 30, 2017 at 05:00:59PM +0000, Trond Myklebust wrote:
> The main use case for open by filehandle was (and still should be) the
> promise of being able to do the sort of tricks you normally associate
> with object storage on a standard filesystem.
> 
> Imagine that you are trying to build an application for indexing and
> searching the data on your storage. You basically want to trawl through
> the filesystem on a regular basis and build up a database of key words
> and other metadata to tell you what is in the files. For that kind of
> application, the namespace is a real PITA to deal with, because files
> get renamed, moved and deleted all the time; so if you can store
> something that is independent of the namespace and that will give you
> access to the file contents, then why wouldn't you do so? Normally,
> applications like that use the inode number, but you can't open a file
> by inode number, and you have the same problems with inode number reuse
> that a NFS server has.
> 
> That's the sort of thing I'd think we want to allow through open by
> filehandle, and I see no reason why NFS should be excluded from that
> type of application.

Thanks, that makes sense.

We've had open_by_handle support for most filesystems since 2011, is
there evidence of anyone doing this?

--b.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: open by handle support for NFS V2
  2017-06-30 17:38     ` bfields
@ 2017-07-01  9:34       ` Mkrtchyan, Tigran
  2017-07-01 12:17         ` J. Bruce Fields
  2017-07-03  6:39         ` DENIEL Philippe
  0 siblings, 2 replies; 16+ messages in thread
From: Mkrtchyan, Tigran @ 2017-07-01  9:34 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: Trond Myklebust, Christoph Hellwig, Jeff Layton, linux-nfs,
	schumaker anna



----- Original Message -----
> From: "J. Bruce Fields" <bfields@fieldses.org>
> To: "Trond Myklebust" <trondmy@primarydata.com>
> Cc: "Christoph Hellwig" <hch@lst.de>, "Jeff Layton" <jlayton@poochiereds.net>, "linux-nfs" <linux-nfs@vger.kernel.org>,
> "schumaker anna" <schumaker.anna@gmail.com>
> Sent: Friday, June 30, 2017 7:38:18 PM
> Subject: Re: open by handle support for NFS V2

> On Fri, Jun 30, 2017 at 05:00:59PM +0000, Trond Myklebust wrote:
>> The main use case for open by filehandle was (and still should be) the
>> promise of being able to do the sort of tricks you normally associate
>> with object storage on a standard filesystem.
>> 
>> Imagine that you are trying to build an application for indexing and
>> searching the data on your storage. You basically want to trawl through
>> the filesystem on a regular basis and build up a database of key words
>> and other metadata to tell you what is in the files. For that kind of
>> application, the namespace is a real PITA to deal with, because files
>> get renamed, moved and deleted all the time; so if you can store
>> something that is independent of the namespace and that will give you
>> access to the file contents, then why wouldn't you do so? Normally,
>> applications like that use the inode number, but you can't open a file
>> by inode number, and you have the same problems with inode number reuse
>> that a NFS server has.
>> 
>> That's the sort of thing I'd think we want to allow through open by
>> filehandle, and I see no reason why NFS should be excluded from that
>> type of application.
> 
> Thanks, that makes sense.
> 
> We've had open_by_handle support for most filesystems since 2011, is
> there evidence of anyone doing this?


I am pretty sure that nfs-ganesha is using this in one the backend
implementations.

Tigran.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: open by handle support for NFS V2
  2017-07-01  9:34       ` Mkrtchyan, Tigran
@ 2017-07-01 12:17         ` J. Bruce Fields
  2017-07-03  6:39         ` DENIEL Philippe
  1 sibling, 0 replies; 16+ messages in thread
From: J. Bruce Fields @ 2017-07-01 12:17 UTC (permalink / raw)
  To: Mkrtchyan, Tigran
  Cc: Trond Myklebust, Christoph Hellwig, Jeff Layton, linux-nfs,
	schumaker anna

On Sat, Jul 01, 2017 at 11:34:36AM +0200, Mkrtchyan, Tigran wrote:
> 
> 
> ----- Original Message -----
> > From: "J. Bruce Fields" <bfields@fieldses.org>
> > To: "Trond Myklebust" <trondmy@primarydata.com>
> > Cc: "Christoph Hellwig" <hch@lst.de>, "Jeff Layton" <jlayton@poochiereds.net>, "linux-nfs" <linux-nfs@vger.kernel.org>,
> > "schumaker anna" <schumaker.anna@gmail.com>
> > Sent: Friday, June 30, 2017 7:38:18 PM
> > Subject: Re: open by handle support for NFS V2
> 
> > On Fri, Jun 30, 2017 at 05:00:59PM +0000, Trond Myklebust wrote:
> >> The main use case for open by filehandle was (and still should be) the
> >> promise of being able to do the sort of tricks you normally associate
> >> with object storage on a standard filesystem.
> >> 
> >> Imagine that you are trying to build an application for indexing and
> >> searching the data on your storage. You basically want to trawl through
> >> the filesystem on a regular basis and build up a database of key words
> >> and other metadata to tell you what is in the files. For that kind of
> >> application, the namespace is a real PITA to deal with, because files
> >> get renamed, moved and deleted all the time; so if you can store
> >> something that is independent of the namespace and that will give you
> >> access to the file contents, then why wouldn't you do so? Normally,
> >> applications like that use the inode number, but you can't open a file
> >> by inode number, and you have the same problems with inode number reuse
> >> that a NFS server has.
> >> 
> >> That's the sort of thing I'd think we want to allow through open by
> >> filehandle, and I see no reason why NFS should be excluded from that
> >> type of application.
> > 
> > Thanks, that makes sense.
> > 
> > We've had open_by_handle support for most filesystems since 2011, is
> > there evidence of anyone doing this?
> 
> 
> I am pretty sure that nfs-ganesha is using this in one the backend
> implementations.

Yes, but I was curious about the kind of application Trond describes.

--b.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: open by handle support for NFS V2
  2017-07-01  9:34       ` Mkrtchyan, Tigran
  2017-07-01 12:17         ` J. Bruce Fields
@ 2017-07-03  6:39         ` DENIEL Philippe
  1 sibling, 0 replies; 16+ messages in thread
From: DENIEL Philippe @ 2017-07-03  6:39 UTC (permalink / raw)
  To: Mkrtchyan, Tigran, J. Bruce Fields
  Cc: Trond Myklebust, Christoph Hellwig, Jeff Layton, linux-nfs,
	schumaker anna

On 07/01/17 11:34, Mkrtchyan, Tigran wrote:
>
> ----- Original Message -----
>> From: "J. Bruce Fields" <bfields@fieldses.org>
>> To: "Trond Myklebust" <trondmy@primarydata.com>
>> Cc: "Christoph Hellwig" <hch@lst.de>, "Jeff Layton" <jlayton@poochiereds.net>, "linux-nfs" <linux-nfs@vger.kernel.org>,
>> "schumaker anna" <schumaker.anna@gmail.com>
>> Sent: Friday, June 30, 2017 7:38:18 PM
>> Subject: Re: open by handle support for NFS V2
>> On Fri, Jun 30, 2017 at 05:00:59PM +0000, Trond Myklebust wrote:
>>> The main use case for open by filehandle was (and still should be) the
>>> promise of being able to do the sort of tricks you normally associate
>>> with object storage on a standard filesystem.
>>>
>>> Imagine that you are trying to build an application for indexing and
>>> searching the data on your storage. You basically want to trawl through
>>> the filesystem on a regular basis and build up a database of key words
>>> and other metadata to tell you what is in the files. For that kind of
>>> application, the namespace is a real PITA to deal with, because files
>>> get renamed, moved and deleted all the time; so if you can store
>>> something that is independent of the namespace and that will give you
>>> access to the file contents, then why wouldn't you do so? Normally,
>>> applications like that use the inode number, but you can't open a file
>>> by inode number, and you have the same problems with inode number reuse
>>> that a NFS server has.
>>>
>>> That's the sort of thing I'd think we want to allow through open by
>>> filehandle, and I see no reason why NFS should be excluded from that
>>> type of application.
>> Thanks, that makes sense.
>>
>> We've had open_by_handle support for most filesystems since 2011, is
>> there evidence of anyone doing this?
>
> I am pretty sure that nfs-ganesha is using this in one the backend
> implementations.
>
> Tigran.
Tigran is right: nfs-ganesha does something a bit similar to this in its 
"FSAL_VFS" module (the backed module based the usage of on 
open_by_handle_at() and name_to_handle_at() ).

     regards

         Philippe

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



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: open by handle support for NFS V2
  2017-06-30 17:00   ` Trond Myklebust
  2017-06-30 17:38     ` bfields
@ 2017-07-07  2:41     ` NeilBrown
  2017-07-07  3:19       ` Trond Myklebust
  1 sibling, 1 reply; 16+ messages in thread
From: NeilBrown @ 2017-07-07  2:41 UTC (permalink / raw)
  To: Trond Myklebust, bfields, hch; +Cc: jlayton, linux-nfs, schumaker.anna

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

On Fri, Jun 30 2017, Trond Myklebust wrote:

> On Thu, 2017-06-29 at 11:46 -0400, J. Bruce Fields wrote:
>> On Thu, Jun 29, 2017 at 06:34:49AM -0700, Christoph Hellwig wrote:
>> > this resurrects parts of an old series to add open by handle
>> > support to
>> > NFS.  The prime intent here is to support the actual open by handle
>> > ioctls, although it will also allow very crude re-
>> > exporting.  Without
>> > the other patches from Jeff's series that re-exporting will suck
>> > badly
>> > though.
>> 
>> Why do we want this?
>> 
>> Any re-export support is going to have some major limitations.  (No
>> file
>> locking, and re-export of NFSv4 probably not possible?)
>> 
>> Last I heard the only motivation was extremely specific to Primary
>> Data's setup.  I'm happy to help them, but I think we need *some*
>> evidence this will be useful to upstream users.
>> 
>
> The main use case for open by filehandle was (and still should be) the
> promise of being able to do the sort of tricks you normally associate
> with object storage on a standard filesystem.
>
> Imagine that you are trying to build an application for indexing and
> searching the data on your storage. You basically want to trawl through
> the filesystem on a regular basis and build up a database of key words
> and other metadata to tell you what is in the files. For that kind of
> application, the namespace is a real PITA to deal with, because files
> get renamed, moved and deleted all the time; so if you can store
> something that is independent of the namespace and that will give you
> access to the file contents, then why wouldn't you do so? Normally,
> applications like that use the inode number, but you can't open a file
> by inode number, and you have the same problems with inode number reuse
> that a NFS server has.
>
> That's the sort of thing I'd think we want to allow through open by
> filehandle, and I see no reason why NFS should be excluded from that
> type of application.

Given that the goal, and presumably the testing, is focused on this
use-case, I wonder if we should take steps to disable the NFS-re-export
use case.
As the patch stands, I suspect that NFS re-export would appear to work,
but - as Bruce suggests - would likely hit some problems.  This might
not be a user-friendly thing to do.

Probably the ideal would be to keep re-export disabled by default, but
to allow it to be enabled using a module parameter.
I'm not sure the best way for NFS to tell nfsd that export shouldn't be
trusted.
Maybe add a "flags" field to struct export_operations, which can contain
a "No NFS export" flag ??

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: open by handle support for NFS V2
  2017-07-07  2:41     ` NeilBrown
@ 2017-07-07  3:19       ` Trond Myklebust
  2017-07-07  4:27         ` NeilBrown
  0 siblings, 1 reply; 16+ messages in thread
From: Trond Myklebust @ 2017-07-07  3:19 UTC (permalink / raw)
  To: bfields, hch, neilb; +Cc: jlayton, linux-nfs, schumaker.anna

T24gRnJpLCAyMDE3LTA3LTA3IGF0IDEyOjQxICsxMDAwLCBOZWlsQnJvd24gd3JvdGU6DQo+IE9u
IEZyaSwgSnVuIDMwIDIwMTcsIFRyb25kIE15a2xlYnVzdCB3cm90ZToNCj4gDQo+ID4gT24gVGh1
LCAyMDE3LTA2LTI5IGF0IDExOjQ2IC0wNDAwLCBKLiBCcnVjZSBGaWVsZHMgd3JvdGU6DQo+ID4g
PiBPbiBUaHUsIEp1biAyOSwgMjAxNyBhdCAwNjozNDo0OUFNIC0wNzAwLCBDaHJpc3RvcGggSGVs
bHdpZw0KPiA+ID4gd3JvdGU6DQo+ID4gPiA+IHRoaXMgcmVzdXJyZWN0cyBwYXJ0cyBvZiBhbiBv
bGQgc2VyaWVzIHRvIGFkZCBvcGVuIGJ5IGhhbmRsZQ0KPiA+ID4gPiBzdXBwb3J0IHRvDQo+ID4g
PiA+IE5GUy7CoMKgVGhlIHByaW1lIGludGVudCBoZXJlIGlzIHRvIHN1cHBvcnQgdGhlIGFjdHVh
bCBvcGVuIGJ5DQo+ID4gPiA+IGhhbmRsZQ0KPiA+ID4gPiBpb2N0bHMsIGFsdGhvdWdoIGl0IHdp
bGwgYWxzbyBhbGxvdyB2ZXJ5IGNydWRlIHJlLQ0KPiA+ID4gPiBleHBvcnRpbmcuwqDCoFdpdGhv
dXQNCj4gPiA+ID4gdGhlIG90aGVyIHBhdGNoZXMgZnJvbSBKZWZmJ3Mgc2VyaWVzIHRoYXQgcmUt
ZXhwb3J0aW5nIHdpbGwNCj4gPiA+ID4gc3Vjaw0KPiA+ID4gPiBiYWRseQ0KPiA+ID4gPiB0aG91
Z2guDQo+ID4gPiANCj4gPiA+IFdoeSBkbyB3ZSB3YW50IHRoaXM/DQo+ID4gPiANCj4gPiA+IEFu
eSByZS1leHBvcnQgc3VwcG9ydCBpcyBnb2luZyB0byBoYXZlIHNvbWUgbWFqb3INCj4gPiA+IGxp
bWl0YXRpb25zLsKgwqAoTm8NCj4gPiA+IGZpbGUNCj4gPiA+IGxvY2tpbmcsIGFuZCByZS1leHBv
cnQgb2YgTkZTdjQgcHJvYmFibHkgbm90IHBvc3NpYmxlPykNCj4gPiA+IA0KPiA+ID4gTGFzdCBJ
IGhlYXJkIHRoZSBvbmx5IG1vdGl2YXRpb24gd2FzIGV4dHJlbWVseSBzcGVjaWZpYyB0bw0KPiA+
ID4gUHJpbWFyeQ0KPiA+ID4gRGF0YSdzIHNldHVwLsKgwqBJJ20gaGFwcHkgdG8gaGVscCB0aGVt
LCBidXQgSSB0aGluayB3ZSBuZWVkICpzb21lKg0KPiA+ID4gZXZpZGVuY2UgdGhpcyB3aWxsIGJl
IHVzZWZ1bCB0byB1cHN0cmVhbSB1c2Vycy4NCj4gPiA+IA0KPiA+IA0KPiA+IFRoZSBtYWluIHVz
ZSBjYXNlIGZvciBvcGVuIGJ5IGZpbGVoYW5kbGUgd2FzIChhbmQgc3RpbGwgc2hvdWxkIGJlKQ0K
PiA+IHRoZQ0KPiA+IHByb21pc2Ugb2YgYmVpbmcgYWJsZSB0byBkbyB0aGUgc29ydCBvZiB0cmlj
a3MgeW91IG5vcm1hbGx5DQo+ID4gYXNzb2NpYXRlDQo+ID4gd2l0aCBvYmplY3Qgc3RvcmFnZSBv
biBhIHN0YW5kYXJkIGZpbGVzeXN0ZW0uDQo+ID4gDQo+ID4gSW1hZ2luZSB0aGF0IHlvdSBhcmUg
dHJ5aW5nIHRvIGJ1aWxkIGFuIGFwcGxpY2F0aW9uIGZvciBpbmRleGluZw0KPiA+IGFuZA0KPiA+
IHNlYXJjaGluZyB0aGUgZGF0YSBvbiB5b3VyIHN0b3JhZ2UuIFlvdSBiYXNpY2FsbHkgd2FudCB0
byB0cmF3bA0KPiA+IHRocm91Z2gNCj4gPiB0aGUgZmlsZXN5c3RlbSBvbiBhIHJlZ3VsYXIgYmFz
aXMgYW5kIGJ1aWxkIHVwIGEgZGF0YWJhc2Ugb2Yga2V5DQo+ID4gd29yZHMNCj4gPiBhbmQgb3Ro
ZXIgbWV0YWRhdGEgdG8gdGVsbCB5b3Ugd2hhdCBpcyBpbiB0aGUgZmlsZXMuIEZvciB0aGF0IGtp
bmQNCj4gPiBvZg0KPiA+IGFwcGxpY2F0aW9uLCB0aGUgbmFtZXNwYWNlIGlzIGEgcmVhbCBQSVRB
IHRvIGRlYWwgd2l0aCwgYmVjYXVzZQ0KPiA+IGZpbGVzDQo+ID4gZ2V0IHJlbmFtZWQsIG1vdmVk
IGFuZCBkZWxldGVkIGFsbCB0aGUgdGltZTsgc28gaWYgeW91IGNhbiBzdG9yZQ0KPiA+IHNvbWV0
aGluZyB0aGF0IGlzIGluZGVwZW5kZW50IG9mIHRoZSBuYW1lc3BhY2UgYW5kIHRoYXQgd2lsbCBn
aXZlDQo+ID4geW91DQo+ID4gYWNjZXNzIHRvIHRoZSBmaWxlIGNvbnRlbnRzLCB0aGVuIHdoeSB3
b3VsZG4ndCB5b3UgZG8gc28/IE5vcm1hbGx5LA0KPiA+IGFwcGxpY2F0aW9ucyBsaWtlIHRoYXQg
dXNlIHRoZSBpbm9kZSBudW1iZXIsIGJ1dCB5b3UgY2FuJ3Qgb3BlbiBhDQo+ID4gZmlsZQ0KPiA+
IGJ5IGlub2RlIG51bWJlciwgYW5kIHlvdSBoYXZlIHRoZSBzYW1lIHByb2JsZW1zIHdpdGggaW5v
ZGUgbnVtYmVyDQo+ID4gcmV1c2UNCj4gPiB0aGF0IGEgTkZTIHNlcnZlciBoYXMuDQo+ID4gDQo+
ID4gVGhhdCdzIHRoZSBzb3J0IG9mIHRoaW5nIEknZCB0aGluayB3ZSB3YW50IHRvIGFsbG93IHRo
cm91Z2ggb3BlbiBieQ0KPiA+IGZpbGVoYW5kbGUsIGFuZCBJIHNlZSBubyByZWFzb24gd2h5IE5G
UyBzaG91bGQgYmUgZXhjbHVkZWQgZnJvbQ0KPiA+IHRoYXQNCj4gPiB0eXBlIG9mIGFwcGxpY2F0
aW9uLg0KPiANCj4gR2l2ZW4gdGhhdCB0aGUgZ29hbCwgYW5kIHByZXN1bWFibHkgdGhlIHRlc3Rp
bmcsIGlzIGZvY3VzZWQgb24gdGhpcw0KPiB1c2UtY2FzZSwgSSB3b25kZXIgaWYgd2Ugc2hvdWxk
IHRha2Ugc3RlcHMgdG8gZGlzYWJsZSB0aGUgTkZTLXJlLQ0KPiBleHBvcnQNCj4gdXNlIGNhc2Uu
DQo+IEFzIHRoZSBwYXRjaCBzdGFuZHMsIEkgc3VzcGVjdCB0aGF0IE5GUyByZS1leHBvcnQgd291
bGQgYXBwZWFyIHRvDQo+IHdvcmssDQo+IGJ1dCAtIGFzIEJydWNlIHN1Z2dlc3RzIC0gd291bGQg
bGlrZWx5IGhpdCBzb21lIHByb2JsZW1zLsKgwqBUaGlzIG1pZ2h0DQo+IG5vdCBiZSBhIHVzZXIt
ZnJpZW5kbHkgdGhpbmcgdG8gZG8uDQo+IA0KPiBQcm9iYWJseSB0aGUgaWRlYWwgd291bGQgYmUg
dG8ga2VlcCByZS1leHBvcnQgZGlzYWJsZWQgYnkgZGVmYXVsdCwNCj4gYnV0DQo+IHRvIGFsbG93
IGl0IHRvIGJlIGVuYWJsZWQgdXNpbmcgYSBtb2R1bGUgcGFyYW1ldGVyLg0KPiBJJ20gbm90IHN1
cmUgdGhlIGJlc3Qgd2F5IGZvciBORlMgdG8gdGVsbCBuZnNkIHRoYXQgZXhwb3J0IHNob3VsZG4n
dA0KPiBiZQ0KPiB0cnVzdGVkLg0KPiBNYXliZSBhZGQgYSAiZmxhZ3MiIGZpZWxkIHRvIHN0cnVj
dCBleHBvcnRfb3BlcmF0aW9ucywgd2hpY2ggY2FuDQo+IGNvbnRhaW4NCj4gYSAiTm8gTkZTIGV4
cG9ydCIgZmxhZyA/Pw0KPiANCg0KWW91IGNvdWxkLCBidXQgdGhlIHJlYXNvbiB3aHkgd2UgZGV2
ZWxvcGVkIHRoZSBjb2RlIGluIHRoZSBmaXJzdCBwbGFjZSwNCndhcyBiZWNhdXNlIHdlIGhhdmUg
YW4gaW50ZXJuYWwgdXNlIGNhc2Ugd2hpY2ggZG9lcyBpbnZvbHZlIHJlLWV4cG9ydA0Kb2YgTkZT
LCBzbyB3ZSdyZSBmYW1pbGlhciB3aXRoIHRoZSBsaW1pdGF0aW9ucy4NCg0KRllJLCB0aGUgbGlt
aXRhdGlvbnMgYXJlOg0KMSkgUmVjb3Zlcnkgb2YgbG9ja3MgaXMgbm90IHBvc3NpYmxlIHdoZW4g
dGhlIHJlLWV4cG9ydGluZyBzZXJ2ZXIgaXMNCnRoZSBvbmUgYmVpbmcgcmVib290ZWQuIEl0IHdv
cmtzIGp1c3QgZmluZSBmb3IgYWxsIG90aGVyIGNhc2VzLg0KMikgUmUtZXhwb3J0aW5nIGFueXRo
aW5nIHRvIE5GU3YyIGlzIG5vdCBwb3NzaWJsZS4NCjMpIFJlLWV4cG9ydCBvZiBmaWxlc3lzdGVt
cyB3aXRoIHZlcnkgbGFyZ2UgZmlsZW5hbmRsZXMgY291bGQgYmUNCnByb2JsZW1hdGljLiBUaGlz
IHdvdWxkIGFsc28gYWZmZWN0IG9wZW4tYnktZmlsZWhhbmRsZS4gSW4gcHJhY3RpY2UgaXQNCnR1
cm5zIG91dCB0byBiZSBhIG5vbi1pc3N1ZSBiZWNhdXNlIG5vYm9keSB1c2VzIGZpbGVoYW5kbGVz
ID4gNjQgYnl0ZXMuDQo0KSBTdGF0ZWxlc3MgTkZTdjMgcmVhZHMgYW5kIHdyaXRlcyBjYW4gYmUg
YSBwcm9ibGVtIHdpdGggTkZTdjQgd2hlbg0KdGhlIGFwcGxpY2F0aW9uIHVzZWVzIG9kZCBtb2Rl
Yml0IHNldHRpbmdzIHN1Y2ggYXMgMDAwLg0KDQpJT1c6IGluIHByYWN0aWNlIHRoaXMgaXMgbm8g
d29yc2UgdGhhbiBhbGwgdGhlIG90aGVyIHJlLWV4cG9ydGVycyBzdWNoDQphcyBhbHJlYWR5IGV4
aXN0IGZvciBnbHVzdGVyIC0+IE5GU3YzLCBjZXBoIC0+IE5GU3YzLCBORlMgLT4gU01CLC4uLg0K
YW5kIHdoaWNoIGV2ZXJ5b25lIHNlZW1zIGhhcHB5IHRvIHVzZS4NCg0KLS0gDQpUcm9uZCBNeWts
ZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lciwgUHJpbWFyeURhdGENCnRyb25kLm15
a2xlYnVzdEBwcmltYXJ5ZGF0YS5jb20NCg==


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: open by handle support for NFS V2
  2017-07-07  3:19       ` Trond Myklebust
@ 2017-07-07  4:27         ` NeilBrown
  2017-07-17 12:02           ` Jeff Layton
  0 siblings, 1 reply; 16+ messages in thread
From: NeilBrown @ 2017-07-07  4:27 UTC (permalink / raw)
  To: Trond Myklebust, bfields, hch; +Cc: jlayton, linux-nfs, schumaker.anna

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

On Fri, Jul 07 2017, Trond Myklebust wrote:

> On Fri, 2017-07-07 at 12:41 +1000, NeilBrown wrote:
>> On Fri, Jun 30 2017, Trond Myklebust wrote:
>> 
>> > On Thu, 2017-06-29 at 11:46 -0400, J. Bruce Fields wrote:
>> > > On Thu, Jun 29, 2017 at 06:34:49AM -0700, Christoph Hellwig
>> > > wrote:
>> > > > this resurrects parts of an old series to add open by handle
>> > > > support to
>> > > > NFS.  The prime intent here is to support the actual open by
>> > > > handle
>> > > > ioctls, although it will also allow very crude re-
>> > > > exporting.  Without
>> > > > the other patches from Jeff's series that re-exporting will
>> > > > suck
>> > > > badly
>> > > > though.
>> > > 
>> > > Why do we want this?
>> > > 
>> > > Any re-export support is going to have some major
>> > > limitations.  (No
>> > > file
>> > > locking, and re-export of NFSv4 probably not possible?)
>> > > 
>> > > Last I heard the only motivation was extremely specific to
>> > > Primary
>> > > Data's setup.  I'm happy to help them, but I think we need *some*
>> > > evidence this will be useful to upstream users.
>> > > 
>> > 
>> > The main use case for open by filehandle was (and still should be)
>> > the
>> > promise of being able to do the sort of tricks you normally
>> > associate
>> > with object storage on a standard filesystem.
>> > 
>> > Imagine that you are trying to build an application for indexing
>> > and
>> > searching the data on your storage. You basically want to trawl
>> > through
>> > the filesystem on a regular basis and build up a database of key
>> > words
>> > and other metadata to tell you what is in the files. For that kind
>> > of
>> > application, the namespace is a real PITA to deal with, because
>> > files
>> > get renamed, moved and deleted all the time; so if you can store
>> > something that is independent of the namespace and that will give
>> > you
>> > access to the file contents, then why wouldn't you do so? Normally,
>> > applications like that use the inode number, but you can't open a
>> > file
>> > by inode number, and you have the same problems with inode number
>> > reuse
>> > that a NFS server has.
>> > 
>> > That's the sort of thing I'd think we want to allow through open by
>> > filehandle, and I see no reason why NFS should be excluded from
>> > that
>> > type of application.
>> 
>> Given that the goal, and presumably the testing, is focused on this
>> use-case, I wonder if we should take steps to disable the NFS-re-
>> export
>> use case.
>> As the patch stands, I suspect that NFS re-export would appear to
>> work,
>> but - as Bruce suggests - would likely hit some problems.  This might
>> not be a user-friendly thing to do.
>> 
>> Probably the ideal would be to keep re-export disabled by default,
>> but
>> to allow it to be enabled using a module parameter.
>> I'm not sure the best way for NFS to tell nfsd that export shouldn't
>> be
>> trusted.
>> Maybe add a "flags" field to struct export_operations, which can
>> contain
>> a "No NFS export" flag ??
>> 
>
> You could, but the reason why we developed the code in the first place,
> was because we have an internal use case which does involve re-export
> of NFS, so we're familiar with the limitations.
>
> FYI, the limitations are:
> 1) Recovery of locks is not possible when the re-exporting server is
> the one being rebooted. It works just fine for all other cases.
> 2) Re-exporting anything to NFSv2 is not possible.
> 3) Re-export of filesystems with very large filenandles could be
> problematic. This would also affect open-by-filehandle. In practice it
> turns out to be a non-issue because nobody uses filehandles > 64 bytes.
> 4) Stateless NFSv3 reads and writes can be a problem with NFSv4 when
> the application usees odd modebit settings such as 000.
>
> IOW: in practice this is no worse than all the other re-exporters such
> as already exist for gluster -> NFSv3, ceph -> NFSv3, NFS -> SMB,...
> and which everyone seems happy to use.
>

Thanks.  That isn't as bad as I feared.  Maybe it would be useful to put
notes like in this in the nfs(5) man page.
For the NFSv2 export I suspect that in some cases you might be
able to successfully mount, but then not access anything.
The linux NFS server uses a very small file handles for an export-point.
It might be good to have nfs_encode_fh always fail if *max_len <= 32/4
just to ensure that NFSv2 doesn't even get off the ground.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: open by handle support for NFS V2
  2017-07-07  4:27         ` NeilBrown
@ 2017-07-17 12:02           ` Jeff Layton
  0 siblings, 0 replies; 16+ messages in thread
From: Jeff Layton @ 2017-07-17 12:02 UTC (permalink / raw)
  To: NeilBrown, Trond Myklebust, bfields, hch; +Cc: linux-nfs, schumaker.anna

On Fri, 2017-07-07 at 14:27 +1000, NeilBrown wrote:
> On Fri, Jul 07 2017, Trond Myklebust wrote:
> 
> > On Fri, 2017-07-07 at 12:41 +1000, NeilBrown wrote:
> > > On Fri, Jun 30 2017, Trond Myklebust wrote:
> > > 
> > > > On Thu, 2017-06-29 at 11:46 -0400, J. Bruce Fields wrote:
> > > > > On Thu, Jun 29, 2017 at 06:34:49AM -0700, Christoph Hellwig
> > > > > wrote:
> > > > > > this resurrects parts of an old series to add open by handle
> > > > > > support to
> > > > > > NFS.  The prime intent here is to support the actual open by
> > > > > > handle
> > > > > > ioctls, although it will also allow very crude re-
> > > > > > exporting.  Without
> > > > > > the other patches from Jeff's series that re-exporting will
> > > > > > suck
> > > > > > badly
> > > > > > though.
> > > > > 
> > > > > Why do we want this?
> > > > > 
> > > > > Any re-export support is going to have some major
> > > > > limitations.  (No
> > > > > file
> > > > > locking, and re-export of NFSv4 probably not possible?)
> > > > > 
> > > > > Last I heard the only motivation was extremely specific to
> > > > > Primary
> > > > > Data's setup.  I'm happy to help them, but I think we need *some*
> > > > > evidence this will be useful to upstream users.
> > > > > 
> > > > 
> > > > The main use case for open by filehandle was (and still should be)
> > > > the
> > > > promise of being able to do the sort of tricks you normally
> > > > associate
> > > > with object storage on a standard filesystem.
> > > > 
> > > > Imagine that you are trying to build an application for indexing
> > > > and
> > > > searching the data on your storage. You basically want to trawl
> > > > through
> > > > the filesystem on a regular basis and build up a database of key
> > > > words
> > > > and other metadata to tell you what is in the files. For that kind
> > > > of
> > > > application, the namespace is a real PITA to deal with, because
> > > > files
> > > > get renamed, moved and deleted all the time; so if you can store
> > > > something that is independent of the namespace and that will give
> > > > you
> > > > access to the file contents, then why wouldn't you do so? Normally,
> > > > applications like that use the inode number, but you can't open a
> > > > file
> > > > by inode number, and you have the same problems with inode number
> > > > reuse
> > > > that a NFS server has.
> > > > 
> > > > That's the sort of thing I'd think we want to allow through open by
> > > > filehandle, and I see no reason why NFS should be excluded from
> > > > that
> > > > type of application.
> > > 
> > > Given that the goal, and presumably the testing, is focused on this
> > > use-case, I wonder if we should take steps to disable the NFS-re-
> > > export
> > > use case.
> > > As the patch stands, I suspect that NFS re-export would appear to
> > > work,
> > > but - as Bruce suggests - would likely hit some problems.  This might
> > > not be a user-friendly thing to do.
> > > 
> > > Probably the ideal would be to keep re-export disabled by default,
> > > but
> > > to allow it to be enabled using a module parameter.
> > > I'm not sure the best way for NFS to tell nfsd that export shouldn't
> > > be
> > > trusted.
> > > Maybe add a "flags" field to struct export_operations, which can
> > > contain
> > > a "No NFS export" flag ??
> > > 
> > 
> > You could, but the reason why we developed the code in the first place,
> > was because we have an internal use case which does involve re-export
> > of NFS, so we're familiar with the limitations.
> > 
> > FYI, the limitations are:
> > 1) Recovery of locks is not possible when the re-exporting server is
> > the one being rebooted. It works just fine for all other cases.
> > 2) Re-exporting anything to NFSv2 is not possible.
> > 3) Re-export of filesystems with very large filenandles could be
> > problematic. This would also affect open-by-filehandle. In practice it
> > turns out to be a non-issue because nobody uses filehandles > 64 bytes.
> > 4) Stateless NFSv3 reads and writes can be a problem with NFSv4 when
> > the application usees odd modebit settings such as 000.
> > 
> > IOW: in practice this is no worse than all the other re-exporters such
> > as already exist for gluster -> NFSv3, ceph -> NFSv3, NFS -> SMB,...
> > and which everyone seems happy to use.
> > 
> 
> Thanks.  That isn't as bad as I feared.  Maybe it would be useful to put
> notes like in this in the nfs(5) man page.
> For the NFSv2 export I suspect that in some cases you might be
> able to successfully mount, but then not access anything.
> The linux NFS server uses a very small file handles for an export-point.
> It might be good to have nfs_encode_fh always fail if *max_len <= 32/4
> just to ensure that NFSv2 doesn't even get off the ground.
> 

I just saw this on the LWN article entitled "4.13 Merge window, part 2":

   - NFS filesystems can now be re-exported over NFS.

I'm posting a comment there to try and clarify this, but it's a little
worrisome that this misconception is already getting out. I think we
need to do something to clear up this confusion, whether via
documentation or other means.
-- 
Jeff Layton <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/4] nfs: replace d_add with d_splice_alias in atomic_open
  2017-06-27 15:43 open by handle support for NFS Christoph Hellwig
@ 2017-06-27 15:44 ` Christoph Hellwig
  0 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2017-06-27 15:44 UTC (permalink / raw)
  To: trond.myklebust; +Cc: jlayton, linux-nfs

From: Peng Tao <tao.peng@primarydata.com>

It's a trival change but follows knfsd export document that asks
for d_splice_alias during lookup.

Signed-off-by: Peng Tao <tao.peng@primarydata.com>
---
 fs/nfs/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 32ccd7754f8a..0296c06dcdc5 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1512,7 +1512,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
 		d_drop(dentry);
 		switch (err) {
 		case -ENOENT:
-			d_add(dentry, NULL);
+			d_splice_alias(NULL, dentry);
 			nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
 			break;
 		case -EISDIR:
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-07-17 12:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-29 13:34 open by handle support for NFS V2 Christoph Hellwig
2017-06-29 13:34 ` [PATCH 1/4] nfs: replace d_add with d_splice_alias in atomic_open Christoph Hellwig
2017-06-29 13:34 ` [PATCH 2/4] nfs: add a nfs_ilookup helper Christoph Hellwig
2017-06-29 13:34 ` [PATCH 3/4] nfs4: add NFSv4 LOOKUPP handlers Christoph Hellwig
2017-06-29 13:34 ` [PATCH 4/4] nfs: add export operations Christoph Hellwig
2017-06-29 15:46 ` open by handle support for NFS V2 J. Bruce Fields
2017-06-30 17:00   ` Trond Myklebust
2017-06-30 17:38     ` bfields
2017-07-01  9:34       ` Mkrtchyan, Tigran
2017-07-01 12:17         ` J. Bruce Fields
2017-07-03  6:39         ` DENIEL Philippe
2017-07-07  2:41     ` NeilBrown
2017-07-07  3:19       ` Trond Myklebust
2017-07-07  4:27         ` NeilBrown
2017-07-17 12:02           ` Jeff Layton
  -- strict thread matches above, loose matches on Subject: below --
2017-06-27 15:43 open by handle support for NFS Christoph Hellwig
2017-06-27 15:44 ` [PATCH 1/4] nfs: replace d_add with d_splice_alias in atomic_open Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.