Skip to contentSkip to conversation

NFSD: Process time setting separately only for nfsv2

PATCH In review Kinglong Mee 2 participants 3 messages May 27, 2015 – May 28, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 NFSD: Process time setting separately only for nfsv2

NFSv2 does not different between "set-[ac]time-to-now" which only requires access, and "set-[ac]time-to-X" which requires ownership.

But, NFSv3/v4 are support the difference.
Just let those codes valid for NFSv2.

---
 fs/nfsd/vfs.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
fs/nfsd/vfs.c +4 -3
index 84d770b..bf4a15b 100644
@@ -300,7 +300,7 @@ commit_metadata(struct svc_fh *fhp)
  * NFS semantics and what Linux expects.
  */
 static void
-nfsd_sanitize_attrs(struct inode *inode, struct iattr *iap)
+nfsd_sanitize_attrs(struct svc_rqst *rqstp, struct inode *inode, struct iattr *iap)
 {
 	/*
 	 * NFSv2 does not differentiate between "set-[ac]time-to-now"
@@ -315,7 +315,8 @@ nfsd_sanitize_attrs(struct inode *inode, struct iattr *iap)
 	 */
 #define BOTH_TIME_SET (ATTR_ATIME_SET | ATTR_MTIME_SET)
 #define	MAX_TOUCH_TIME_ERROR (30*60)
-	if ((iap->ia_valid & BOTH_TIME_SET) == BOTH_TIME_SET &&
+	if (rqstp->rq_vers == 2 &&
+	    (iap->ia_valid & BOTH_TIME_SET) == BOTH_TIME_SET &&
 	    iap->ia_mtime.tv_sec == iap->ia_atime.tv_sec) {
 		/*
 		 * Looks probable.
@@ -435,7 +436,7 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
 	if (!iap->ia_valid)
 		goto out;
 
-	nfsd_sanitize_attrs(inode, iap);
+	nfsd_sanitize_attrs(rqstp, inode, iap);
 
 	/*
 	 * The size case is special, it changes the file in addition to the
Signature · 1 line
2.4.1
#1 lore raw
Quoted 5 lines · Kinglong Mee

NFSv2 does not different between "set-[ac]time-to-now" which only requires access, and "set-[ac]time-to-X" which requires ownership.

But, NFSv3/v4 are support the difference.
Just let those codes valid for NFSv2.

Looks like this is already done by Andreas Gruenbacher's "nfsd: Disable NFSv2 timestamp workaround for NFSv3+" in my for-4.2-incoming branch. Do you see any problem with that patch?

--b.

Quoted 40 lines
---
 fs/nfsd/vfs.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
fs/nfsd/vfs.c +4 -3
index 84d770b..bf4a15b 100644
@@ -300,7 +300,7 @@ commit_metadata(struct svc_fh *fhp)
  * NFS semantics and what Linux expects.
  */
 static void
-nfsd_sanitize_attrs(struct inode *inode, struct iattr *iap)
+nfsd_sanitize_attrs(struct svc_rqst *rqstp, struct inode *inode, struct iattr *iap)
 {
 	/*
 	 * NFSv2 does not differentiate between "set-[ac]time-to-now"
@@ -315,7 +315,8 @@ nfsd_sanitize_attrs(struct inode *inode, struct iattr *iap)
 	 */
 #define BOTH_TIME_SET (ATTR_ATIME_SET | ATTR_MTIME_SET)
 #define	MAX_TOUCH_TIME_ERROR (30*60)
-	if ((iap->ia_valid & BOTH_TIME_SET) == BOTH_TIME_SET &&
+	if (rqstp->rq_vers == 2 &&
+	    (iap->ia_valid & BOTH_TIME_SET) == BOTH_TIME_SET &&
 	    iap->ia_mtime.tv_sec == iap->ia_atime.tv_sec) {
 		/*
 		 * Looks probable.
@@ -435,7 +436,7 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
 	if (!iap->ia_valid)
 		goto out;
 
-	nfsd_sanitize_attrs(inode, iap);
+	nfsd_sanitize_attrs(rqstp, inode, iap);
 
 	/*
 	 * The size case is special, it changes the file in addition to the
Signature · 1 line
2.4.1
#2 lore raw
Quoted 9 lines · J. Bruce Fields
Quoted 5 lines · Kinglong Mee

NFSv2 does not different between "set-[ac]time-to-now" which only requires access, and "set-[ac]time-to-X" which requires ownership.

But, NFSv3/v4 are support the difference.
Just let those codes valid for NFSv2.

Looks like this is already done by Andreas Gruenbacher's "nfsd: Disable NFSv2 timestamp workaround for NFSv3+" in my for-4.2-incoming branch.

Sorry, I find that patch now.
Please ignore this.

Quoted 1 line

Do you see any problem with that patch?

No,
That patch is great!

thanks,
Kinglong Mee

Quoted 44 lines

--b.

Quoted 40 lines
---
 fs/nfsd/vfs.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
fs/nfsd/vfs.c +4 -3
index 84d770b..bf4a15b 100644
@@ -300,7 +300,7 @@ commit_metadata(struct svc_fh *fhp)
  * NFS semantics and what Linux expects.
  */
 static void
-nfsd_sanitize_attrs(struct inode *inode, struct iattr *iap)
+nfsd_sanitize_attrs(struct svc_rqst *rqstp, struct inode *inode, struct iattr *iap)
 {
 	/*
 	 * NFSv2 does not differentiate between "set-[ac]time-to-now"
@@ -315,7 +315,8 @@ nfsd_sanitize_attrs(struct inode *inode, struct iattr *iap)
 	 */
 #define BOTH_TIME_SET (ATTR_ATIME_SET | ATTR_MTIME_SET)
 #define	MAX_TOUCH_TIME_ERROR (30*60)
-	if ((iap->ia_valid & BOTH_TIME_SET) == BOTH_TIME_SET &&
+	if (rqstp->rq_vers == 2 &&
+	    (iap->ia_valid & BOTH_TIME_SET) == BOTH_TIME_SET &&
 	    iap->ia_mtime.tv_sec == iap->ia_atime.tv_sec) {
 		/*
 		 * Looks probable.
@@ -435,7 +436,7 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
 	if (!iap->ia_valid)
 		goto out;
 
-	nfsd_sanitize_attrs(inode, iap);
+	nfsd_sanitize_attrs(rqstp, inode, iap);
 
 	/*
 	 * The size case is special, it changes the file in addition to the
Signature · 1 line
2.4.1