git: 57ce37f9dcd0 - main - nfscl: Make NFSv4.2 Copy set atime on infd
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 18 Oct 2023 20:08:55 UTC
The branch main has been updated by rmacklem:
URL: https://cgit.FreeBSD.org/src/commit/?id=57ce37f9dcd0ec749e5e4512e1e44eb963565c68
commit 57ce37f9dcd0ec749e5e4512e1e44eb963565c68
Author: Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2023-10-18 20:07:39 +0000
Commit: Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2023-10-18 20:07:39 +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.
MFC after: 1 month
---
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 8fe9158384a0..14351d915ba2 100644
--- a/sys/fs/nfsclient/nfs_clrpcops.c
+++ b/sys/fs/nfsclient/nfs_clrpcops.c
@@ -8617,6 +8617,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);
@@ -8627,14 +8628,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);
@@ -8649,12 +8671,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 383d45ba260b..fd88531789d9 100644
--- a/sys/fs/nfsclient/nfs_clvnops.c
+++ b/sys/fs/nfsclient/nfs_clvnops.c
@@ -3889,7 +3889,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;
@@ -3998,10 +3998,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;