Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 29 Dec 2023 23:20:11 GMT
From:      Rick Macklem <rmacklem@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 925d9b3abac2 - stable/14 - nfscl: Make NFSv4.2 Copy set atime on infd
Message-ID:  <202312292320.3BTNKBuB097682@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/14 has been updated by rmacklem:

URL: https://cgit.FreeBSD.org/src/commit/?id=925d9b3abac22404d671d40be7b2166a534cc8f8

commit 925d9b3abac22404d671d40be7b2166a534cc8f8
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2023-10-18 20:07:39 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2023-12-29 23:18:14 +0000

    nfscl: Make NFSv4.2 Copy set atime on infd
    
    RFC7862 does not specify infile atime behaviour when a NFSv4.2 Copy
    operation is performed.  Since the collective opinion of a mailing
    list discussion (on freebsd-hackers@) seemed to indicate that
    copy_file_range(2) should update atime on the infd,
    even if there is no data copied, this
    patch attempts to ensure that behaviour.
    
    For Copy, it preceeds the Copy operation with a Setattr of
    TimeAccess_Set(NFSv4. speak for atime) for the invp.  For the case
    where no data will be copied, it does a Setattr RPC to set
    TimeAccess_Set for the invp.
    
    A __FreeBSD_version bump will be done as a separate commit, since
    this patch changes the internal interface between the nfscommon and
    nfscl modules.
    
    (cherry picked from commit 57ce37f9dcd0ec749e5e4512e1e44eb963565c68)
---
 sys/fs/nfs/nfs_commonsubs.c     |  2 +-
 sys/fs/nfsclient/nfs_clrpcops.c | 35 +++++++++++++++++++++++++++++++++++
 sys/fs/nfsclient/nfs_clvnops.c  | 21 ++++++++++++++++++---
 3 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/sys/fs/nfs/nfs_commonsubs.c b/sys/fs/nfs/nfs_commonsubs.c
index f2305795e53e..832713e6c1de 100644
--- a/sys/fs/nfs/nfs_commonsubs.c
+++ b/sys/fs/nfs/nfs_commonsubs.c
@@ -296,7 +296,7 @@ static struct {
 	{ NFSV4OP_OPEN, 8, "CreateLayGet", 12, },
 	{ NFSV4OP_IOADVISE, 1, "Advise", 6, },
 	{ NFSV4OP_ALLOCATE, 2, "Allocate", 8, },
-	{ NFSV4OP_SAVEFH, 5, "Copy", 4, },
+	{ NFSV4OP_SAVEFH, 6, "Copy", 4, },
 	{ NFSV4OP_SEEK, 2, "Seek", 4, },
 	{ NFSV4OP_SEEK, 1, "SeekDS", 6, },
 	{ NFSV4OP_GETXATTR, 2, "Getxattr", 8, },
diff --git a/sys/fs/nfsclient/nfs_clrpcops.c b/sys/fs/nfsclient/nfs_clrpcops.c
index 9d56d06c1c84..e9acedfb6473 100644
--- a/sys/fs/nfsclient/nfs_clrpcops.c
+++ b/sys/fs/nfsclient/nfs_clrpcops.c
@@ -8734,6 +8734,7 @@ nfsrpc_copyrpc(vnode_t invp, off_t inoff, vnode_t outvp, off_t outoff,
 	struct nfsrv_descript *nd = &nfsd;
 	struct nfsmount *nmp;
 	nfsattrbit_t attrbits;
+	struct vattr va;
 	uint64_t len;
 
 	nmp = VFSTONFS(outvp->v_mount);
@@ -8744,14 +8745,35 @@ nfsrpc_copyrpc(vnode_t invp, off_t inoff, vnode_t outvp, off_t outoff,
 	if (len > nfs_maxcopyrange)
 		len = nfs_maxcopyrange;
 	NFSCL_REQSTART(nd, NFSPROC_COPY, invp, cred);
+	/*
+	 * First do a Setattr of atime to the server's clock
+	 * time.  The FreeBSD "collective" was of the opinion
+	 * that setting atime was necessary for this syscall.
+	 * Do the Setattr before the Copy, so that it can be
+	 * handled well if the server replies NFSERR_DELAY to
+	 * the Setattr operation.
+	 */
+	NFSM_BUILD(tl, uint32_t *, NFSX_UNSIGNED);
+	*tl = txdr_unsigned(NFSV4OP_SETATTR);
+	nfsm_stateidtom(nd, instateidp, NFSSTATEID_PUTSTATEID);
+	VATTR_NULL(&va);
+	va.va_atime.tv_sec = va.va_atime.tv_nsec = 0;
+	va.va_vaflags = VA_UTIMES_NULL;
+	nfscl_fillsattr(nd, &va, invp, 0, 0);
+
+	/* Now Getattr the invp attributes. */
 	NFSM_BUILD(tl, uint32_t *, NFSX_UNSIGNED);
 	*tl = txdr_unsigned(NFSV4OP_GETATTR);
 	NFSGETATTR_ATTRBIT(&attrbits);
 	nfsrv_putattrbit(nd, &attrbits);
+
+	/* Set outvp. */
 	NFSM_BUILD(tl, uint32_t *, NFSX_UNSIGNED);
 	*tl = txdr_unsigned(NFSV4OP_PUTFH);
 	(void)nfsm_fhtom(nmp, nd, VTONFS(outvp)->n_fhp->nfh_fh,
 	    VTONFS(outvp)->n_fhp->nfh_len, 0);
+
+	/* Do the Copy. */
 	NFSM_BUILD(tl, uint32_t *, NFSX_UNSIGNED);
 	*tl = txdr_unsigned(NFSV4OP_COPY);
 	nfsm_stateidtom(nd, instateidp, NFSSTATEID_PUTSTATEID);
@@ -8766,12 +8788,25 @@ nfsrpc_copyrpc(vnode_t invp, off_t inoff, vnode_t outvp, off_t outoff,
 		*tl++ = newnfs_false;
 	*tl++ = newnfs_true;
 	*tl++ = 0;
+
+	/* Get the outvp attributes. */
 	*tl = txdr_unsigned(NFSV4OP_GETATTR);
 	NFSWRITEGETATTR_ATTRBIT(&attrbits);
 	nfsrv_putattrbit(nd, &attrbits);
+
 	error = nfscl_request(nd, invp, p, cred);
 	if (error != 0)
 		return (error);
+	/* Skip over the Setattr reply. */
+	if ((nd->nd_flag & ND_NOMOREDATA) == 0) {
+		NFSM_DISSECT(tl, uint32_t *, 2 * NFSX_UNSIGNED);
+		if (*(tl + 1) == 0) {
+			error = nfsrv_getattrbits(nd, &attrbits, NULL, NULL);
+			if (error != 0)
+				goto nfsmout;
+		} else
+			nd->nd_flag |= ND_NOMOREDATA;
+	}
 	if ((nd->nd_flag & ND_NOMOREDATA) == 0) {
 		/* Get the input file's attributes. */
 		NFSM_DISSECT(tl, uint32_t *, 2 * NFSX_UNSIGNED);
diff --git a/sys/fs/nfsclient/nfs_clvnops.c b/sys/fs/nfsclient/nfs_clvnops.c
index e93f8e83daa4..a690e988b4b3 100644
--- a/sys/fs/nfsclient/nfs_clvnops.c
+++ b/sys/fs/nfsclient/nfs_clvnops.c
@@ -3873,7 +3873,7 @@ nfs_copy_file_range(struct vop_copy_file_range_args *ap)
 	struct vnode *outvp = ap->a_outvp;
 	struct mount *mp;
 	struct nfsvattr innfsva, outnfsva;
-	struct vattr *vap;
+	struct vattr va, *vap;
 	struct uio io;
 	struct nfsmount *nmp;
 	size_t len, len2;
@@ -3980,10 +3980,25 @@ generic_copy:
 			 * will not reply NFSERR_INVAL.
 			 * Setting "len == 0" for the RPC would be preferred,
 			 * but some Linux servers do not support that.
+			 * If the len is being set to 0, do a Setattr RPC to
+			 * set the server's atime.  This behaviour was the
+			 * preferred one for the FreeBSD "collective".
 			 */
-			if (inoff >= vap->va_size)
+			if (inoff >= vap->va_size) {
 				*ap->a_lenp = len = 0;
-			else if (inoff + len > vap->va_size)
+				VATTR_NULL(&va);
+				va.va_atime.tv_sec = va.va_atime.tv_nsec = 0;
+				va.va_vaflags = VA_UTIMES_NULL;
+				inattrflag = 0;
+				error = nfsrpc_setattr(invp, &va, NULL,
+				    ap->a_incred, curthread, &innfsva,
+				    &inattrflag);
+				if (inattrflag != 0)
+					ret = nfscl_loadattrcache(&invp,
+					    &innfsva, NULL, 0, 1);
+				if (error == 0 && ret != 0)
+					error = ret;
+			} else if (inoff + len > vap->va_size)
 				*ap->a_lenp = len = vap->va_size - inoff;
 		} else
 			error = 0;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202312292320.3BTNKBuB097682>