NFSv4 fixes from Connectathon 2004:

remove unused pid field of file context struct
map nfs4 error codes to errnos
eliminate redundant code from nfs4_request
use zero stateid on setattr that doesn't set file size
use same clientid on all mounts until reboot
invalidate dirty bufs in nfs4_close, to play it safe
open file for writing if truncating and it's not already open

Approved by:	alfred

5 files changed, 94 insertions, 113 deletions

diff --git a/sys/nfs4client/nfs4_socket.c b/sys/nfs4client/nfs4_socket.c
index f997d52..0fb1237 100644
--- a/sys/nfs4client/nfs4_socket.c
+++ b/sys/nfs4client/nfs4_socket.c
@@ -113,6 +113,62 @@ static struct rpc_program nfs_program = {
 #endif
 
 
+static struct {
+	short	nfserr;
+	short	syserr;
+} nfs_errtbl[] = {
+	{ NFS_OK,		0 },
+	{ NFSERR_PERM,		EPERM },
+	{ NFSERR_NOENT,		ENOENT },
+	{ NFSERR_IO,		EIO },
+	{ NFSERR_NXIO,		ENXIO },
+	{ NFSERR_ACCES,		EACCES },
+	{ NFSERR_EXIST,		EEXIST },
+	{ NFSERR_XDEV,		EXDEV },
+	{ NFSERR_MLINK,		EMLINK },
+	{ NFSERR_NODEV,		ENODEV },
+	{ NFSERR_NOTDIR,	ENOTDIR },
+	{ NFSERR_ISDIR,		EISDIR },
+	{ NFSERR_INVAL,		EINVAL },
+	{ NFSERR_FBIG,		EFBIG },
+	{ NFSERR_NOSPC,		ENOSPC },
+	{ NFSERR_ROFS,		EROFS },
+	{ NFSERR_MLINK,		EMLINK },
+	{ NFSERR_NAMETOL,	ENAMETOOLONG },
+	{ NFSERR_NOTEMPTY,	ENOTEMPTY },
+	{ NFSERR_NOTSUPP,	EOPNOTSUPP },
+#ifdef EDQUOT
+	{ NFSERR_DQUOT,		EDQUOT },
+#endif
+	{ NFSERR_STALE,		ESTALE },
+	{ NFSERR_DENIED,	EAGAIN },
+	{ NFSERR_SYMLINK,	ELOOP },
+	{ NFSERR_BADXDR,	EBADRPC },
+	{ NFSERR_WRONGSEC,	EPERM },
+	{ -1,			EIO }
+};
+
+static int
+nfs4_nfserr_to_syserr(int nfserr)
+{
+	int i, syserr;
+	
+	/* XXX : not the optimal algorithm, but will do for now! */
+	for (i = 0; nfs_errtbl[i].nfserr != -1; i++) {
+		if (nfs_errtbl[i].nfserr == nfserr)
+			break;
+	}
+#ifdef NFS4_MAP_UNKNOWN_ERR
+	syserr = nfs_errtbl[i].syserr;
+#else
+	if (nfs_errtbl[i].nfserr != -1)
+		syserr = nfs_errtbl[i].syserr;
+	else
+		syserr = nfserr;
+#endif
+	return syserr;
+}
+
 int
 nfs4_connect(struct nfsmount *nmp)
 {
@@ -198,61 +254,17 @@ nfs4_request(struct vnode *vp, struct mbuf *mrest, int procnum,
     struct mbuf **mdp, caddr_t *dposp)
 {
 	int error;
-	u_int32_t *tl;
-	struct nfsmount * nmp = VFSTONFS(vp->v_mount);
-	struct rpcclnt * clnt = &nmp->nm_rpcclnt;
-	struct mbuf *md, *mrep;
-	caddr_t dpos;
-	struct rpc_reply reply;
 
-	if ((error = rpcclnt_request(clnt, mrest, procnum, td, cred,
-	    &reply)) != 0) {
-		goto out;
-	}
-
-	/* XXX: don't free mrest if an error occured, to allow caller to retry*/
-	m_freem(mrest);
-	mrep = reply.mrep;
-	md = reply.result_md;
-	dpos = reply.result_dpos;
-
-	tl = nfsm_dissect(u_int32_t *, NFSX_UNSIGNED);
-	if (*tl != 0) {
-		error = fxdr_unsigned(int, *tl);
+	error = nfs4_request_mnt(VFSTONFS(vp->v_mount), mrest, procnum,
+				 td, cred, mrp, mdp, dposp);
 
-		#if 0
-		if ((nmp->nm_flag & NFSMNT_NFSV3) &&
-		    error == NFSERR_TRYLATER) {
-			m_freem(mrep);
-			error = 0;
-			waituntil = time_second + trylater_delay;
-			while (time_second < waituntil)
-				(void) tsleep(&lbolt, PSOCK, "nqnfstry", 0);
-			trylater_delay *= nfs_backoff[trylater_cnt];
-			if (trylater_cnt < NFS_NBACKOFF - 1)
-				trylater_cnt++;
-			goto tryagain;
-		}
-		#endif
-
-		/*
- 		** If the File Handle was stale, invalidate the
- 		** lookup cache, just in case.
- 		**/
-		if (error == ESTALE)
-			cache_purge(vp);
-		goto out;
-	}
+	/*
+	 ** If the File Handle was stale, invalidate the
+	 ** lookup cache, just in case.
+	 **/
+	if (error == ESTALE)
+		cache_purge(vp);
 
-	*mrp = mrep;
-	*mdp = md;
-	*dposp = dpos;
-	return (0);
-nfsmout:
-out:
-	m_freem(reply.mrep);
-	*mrp = NULL;
-	*mdp = NULL;
 	return (error);
 }
 
@@ -309,7 +321,7 @@ out:
 	m_freem(reply.mrep);
 	*mrp = NULL;
 	*mdp = NULL;
-	return (error);
+	return (nfs4_nfserr_to_syserr(error));
 }
 
 
diff --git a/sys/nfs4client/nfs4_subs.c b/sys/nfs4client/nfs4_subs.c
index c28c73b..9cf7b0e 100644
--- a/sys/nfs4client/nfs4_subs.c
+++ b/sys/nfs4client/nfs4_subs.c
@@ -421,10 +421,11 @@ nfsm_v4build_setattr_xx(struct nfs4_compound *cp, struct vattr *vap,
     struct nfs4_fctx *fcp, struct mbuf **mb, caddr_t *bpos)
 {
 	int error;
+	static char zero_stateid[NFSX_V4STATEID];
 
 	nfsm_buildf_xx(mb, bpos, "uo",
 	    NFSV4OP_SETATTR,
-	    NFSX_V4STATEID, fcp->stateid);
+	    NFSX_V4STATEID, fcp ? fcp->stateid : zero_stateid);
 	error = nfsm_v4build_attrs_xx(vap, mb, bpos);
 	if (error == 0)
 		cp->req_nops++;
diff --git a/sys/nfs4client/nfs4_vfsops.c b/sys/nfs4client/nfs4_vfsops.c
index 550a31a..ba03cc8 100644
--- a/sys/nfs4client/nfs4_vfsops.c
+++ b/sys/nfs4client/nfs4_vfsops.c
@@ -770,7 +770,7 @@ nfs4_do_setclientid(struct nfsmount *nmp, struct ucred *cred)
 	struct route ro;
 	char *ipsrc = NULL, uaddr[24], name[24];
 	int try = 0;
-	static int seq;
+	static unsigned long seq;
 	int error;
 
 #ifndef NFS4_USE_RPCCLNT
@@ -784,7 +784,7 @@ nfs4_do_setclientid(struct nfsmount *nmp, struct ucred *cred)
 
 	/* Try not to re-use clientids */
 	if (seq == 0)
-		seq = time_second & 0xffffff;
+		seq = time_second;
 
 #ifdef NFS4_USE_RPCCLNT
 	scid.cb_netid = (nmp->nm_rpcclnt.rc_sotype == SOCK_STREAM) ? "tcp" : "udp";
@@ -811,7 +811,7 @@ nfs4_do_setclientid(struct nfsmount *nmp, struct ucred *cred)
 	RTFREE(ro.ro_rt);
 
  try_again:
-	sprintf(name, "%s-%d", ipsrc, seq++);
+	sprintf(name, "%s-%d", ipsrc, (int) ((seq + try) % 1000000L));
 	scid.namelen = strlen(name);
 	scid.name = name;
 	nfs_v4initcompound(&cp);
diff --git a/sys/nfs4client/nfs4_vnops.c b/sys/nfs4client/nfs4_vnops.c
index 48e4549..15889d5 100644
--- a/sys/nfs4client/nfs4_vnops.c
+++ b/sys/nfs4client/nfs4_vnops.c
@@ -90,6 +90,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/lockf.h>
 #include <sys/stat.h>
 #include <sys/sysctl.h>
+#include <sys/lockmgr.h>
 
 #include <vm/vm.h>
 #include <vm/vm_extern.h>
@@ -498,7 +499,7 @@ nfs4_openrpc(struct vnode *dvp, struct vnode **vpp, struct componentname *cnp,
 
 	/*
 	 * Since we are currently only one lockowner; we only open the
-	 * file one each for reading and writing.
+	 * file once each for reading and writing.
 	 */
 	if (fcp->refcnt++ != 0) {
 		*vpp = vp;
@@ -507,7 +508,6 @@ nfs4_openrpc(struct vnode *dvp, struct vnode **vpp, struct componentname *cnp,
 	}
 
 	fcp->lop = &nfs4_masterlowner;
-	fcp->pid = cnp->cn_thread->td_proc->p_pid;
 	fcp->np = np;
 
 	nfs_v4initcompound(&cp);
@@ -723,30 +723,7 @@ nfs4_closerpc(struct vnode *vp, struct ucred *cred, struct thread *td, int flags
 
 /*
  * nfs close vnode op
- * What an NFS client should do upon close after writing is a debatable issue.
- * Most NFS clients push delayed writes to the server upon close, basically for
- * two reasons:
- * 1 - So that any write errors may be reported back to the client process
- *     doing the close system call. By far the two most likely errors are
- *     NFSERR_NOSPC and NFSERR_DQUOT to indicate space allocation failure.
- * 2 - To put a worst case upper bound on cache inconsistency between
- *     multiple clients for the file.
- * There is also a consistency problem for Version 2 of the protocol w.r.t.
- * not being able to tell if other clients are writing a file concurrently,
- * since there is no way of knowing if the changed modify time in the reply
- * is only due to the write for this client.
- * (NFS Version 3 provides weak cache consistency data in the reply that
- *  should be sufficient to detect and handle this case.)
- *
- * The current code does the following:
- * for NFS Version 2 - play it safe and flush/invalidate all dirty buffers
- * for NFS Version 3 - flush dirty buffers to the server but don't invalidate
- *                     or commit them (this satisfies 1 and 2 except for the
- *                     case where the server crashes after this close but
- *                     before the commit RPC, which is felt to be "good
- *                     enough". Changing the last argument to nfs_flush() to
- *                     a 1 would force a commit operation, if it is felt a
- *                     commit is necessary now.
+ * play it safe for now (see comments in v2/v3 nfs_close regarding dirty buffers)
  */
 /* ARGSUSED */
 static int
@@ -760,32 +737,9 @@ nfs4_close(struct vop_close_args *ap)
 		return (0);
 
 	if (np->n_flag & NMODIFIED) {
-		if (NFS_ISV3(vp)) {
-			/*
-			 * Under NFSv3 we have dirty buffers to
-			 * dispose of.  We must flush them to the NFS
-			 * server.  We have the option of waiting all
-			 * the way through the commit rpc or just
-			 * waiting for the initial write.  The default
-			 * is to only wait through the initial write
-			 * so the data is in the server's cache, which
-			 * is roughly similar to the state a standard
-			 * disk subsystem leaves the file in on
-			 * close().
-			 *
-			 * We cannot clear the NMODIFIED bit in
-			 * np->n_flag due to potential races with
-			 * other processes, and certainly cannot clear
-			 * it if we don't commit.
-			 */
-			int cm = nfsv3_commit_on_close ? 1 : 0;
-			error = nfs4_flush(vp, ap->a_cred, MNT_WAIT, ap->a_td, cm);
-			/* np->n_flag &= ~NMODIFIED; */
-		} else {
-			vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, ap->a_td);
-			error = nfs_vinvalbuf(vp, V_SAVE, ap->a_cred, ap->a_td, 1);
-			VOP_UNLOCK(vp, 0, ap->a_td);
-		}
+		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, ap->a_td);
+		error = nfs_vinvalbuf(vp, V_SAVE, ap->a_cred, ap->a_td, 1);
+		VOP_UNLOCK(vp, 0, ap->a_td);
 		np->n_attrstamp = 0;
 	}
 
@@ -946,6 +900,21 @@ nfs4_setattr(struct vop_setattr_args *ap)
   		(error = nfs_vinvalbuf(vp, V_SAVE, ap->a_cred,
 		 ap->a_td, 1)) == EINTR)
 		return (error);
+
+	if (vap->va_size != VNOVAL && np->n_wfc.refcnt == 0) {
+		/* Have to open the file before we can truncate it */
+		struct componentname cn;
+
+		cn.cn_nameptr = np->n_name;
+		cn.cn_namelen = np->n_namelen;
+		cn.cn_cred = ap->a_cred;
+		cn.cn_thread = ap->a_td;
+		error = nfs4_openrpc(np->n_dvp, &vp, &cn, FWRITE, NULL);
+		if (error)
+			return error;
+		np->n_flag |= NTRUNCATE;
+	}
+
 	error = nfs4_setattrrpc(vp, vap, ap->a_cred, ap->a_td);
 	if (error && vap->va_size != VNOVAL) {
 		np->n_size = np->n_vattr.va_size = tsize;
@@ -967,7 +936,7 @@ nfs4_setattrrpc(struct vnode *vp, struct vattr *vap, struct ucred *cred,
 	struct nfs4_compound cp;
 	struct nfs4_oparg_getattr ga;
 	struct nfsnode *np = VTONFS(vp);
-	struct nfs4_fctx *fcp = &np->n_wfc;
+	struct nfs4_fctx *fcp;
 
 	nfsstats.rpccnt[NFSPROC_SETATTR]++;
 	mreq = nfsm_reqhead(vp, NFSV4PROC_COMPOUND, 0);
@@ -975,6 +944,7 @@ nfs4_setattrrpc(struct vnode *vp, struct vattr *vap, struct ucred *cred,
 	bpos = mtod(mb, caddr_t);
 
 	ga.bm = &nfsv4_getattrbm;
+	fcp = (vap->va_size != VNOVAL) ? &np->n_wfc : NULL;
 	nfs_v4initcompound(&cp);
 
 	nfsm_v4build_compound(&cp, "nfs4_setattrrpc");
@@ -994,7 +964,7 @@ nfs4_setattrrpc(struct vnode *vp, struct vattr *vap, struct ucred *cred,
 
 	nfs4_vnop_loadattrcache(vp, &ga.fa, NULL);
 
-	/* XXX -- need to implement this in nfs4_setattr*/
+	/* TODO: do the settatr and close in a single compound rpc */
 	if (np->n_flag & NTRUNCATE) {
 		error = nfs4_closerpc(vp, cred, td, FWRITE);
 		np->n_flag &= ~NTRUNCATE;
diff --git a/sys/nfsclient/nfs.h b/sys/nfsclient/nfs.h
index e10a548..bae407f 100644
--- a/sys/nfsclient/nfs.h
+++ b/sys/nfsclient/nfs.h
@@ -119,8 +119,6 @@
  */
 struct nfs4_fctx {
 	TAILQ_ENTRY(nfs4_fstate) next;
-
-	pid_t		pid;
 	uint32_t	refcnt;
 	struct nfs4_lowner *lop;
 	struct nfsnode *np;