Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 22 Oct 2018 15:33:05 +0000 (UTC)
From:      Andriy Gapon <avg@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r339595 - head/sys/fs/nfsserver
Message-ID:  <201810221533.w9MFX5E9043598@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: avg
Date: Mon Oct 22 15:33:05 2018
New Revision: 339595
URL: https://svnweb.freebsd.org/changeset/base/339595

Log:
  nfsrvd_readdirplus: for some errors, do not fail the entire request
  
  Instead, a failing entry is skipped.
  This change consist of two logical changes.
  
  A failure to vget or lookup an entry is considered to be a result of a
  concurrent removal, which is the only reasonable explanation given that
  the filesystem is busied.  So, the entry would be silently skipped.
  
  In the case of a failure to get attributes of an entry for an NFSv3
  request, the entry would be silently skipped.  There can be legitimate
  reasons for the failure, but NFSv3 does not provide any means to report
  the error, so we have two options: either fail the whole request or
  ignore the failed entry.  Traditionally, the old NFS server used the
  latter option, so the code is reverted to it.  Making the whole
  directory unreadable because of a single entry seems to be unpractical.
  
  Additionally, some bits of code are slightly re-arranged to account for
  the new control flow and to honor style(9).
  
  Reviewed by:	rmacklem
  Sponsored by:	Panzura
  Differential Revision: https://reviews.freebsd.org/D15424

Modified:
  head/sys/fs/nfsserver/nfs_nfsdport.c

Modified: head/sys/fs/nfsserver/nfs_nfsdport.c
==============================================================================
--- head/sys/fs/nfsserver/nfs_nfsdport.c	Mon Oct 22 15:18:49 2018	(r339594)
+++ head/sys/fs/nfsserver/nfs_nfsdport.c	Mon Oct 22 15:33:05 2018	(r339595)
@@ -2416,10 +2416,22 @@ again:
 						}
 					}
 				}
-				if (!r) {
-				    if (refp == NULL &&
-					((nd->nd_flag & ND_NFSV3) ||
-					 NFSNONZERO_ATTRBIT(&attrbits))) {
+
+				/*
+				 * If we failed to look up the entry, then it
+				 * has become invalid, most likely removed.
+				 */
+				if (r != 0) {
+					if (needs_unbusy)
+						vfs_unbusy(new_mp);
+					goto invalid;
+				}
+				KASSERT(refp != NULL || nvp != NULL,
+				    ("%s: undetected lookup error", __func__));
+
+				if (refp == NULL &&
+				    ((nd->nd_flag & ND_NFSV3) ||
+				     NFSNONZERO_ATTRBIT(&attrbits))) {
 					r = nfsvno_getfh(nvp, &nfh, p);
 					if (!r)
 					    r = nfsvno_getattr(nvp, nvap, nd, p,
@@ -2440,17 +2452,25 @@ again:
 					    if (new_mp == mp)
 						new_mp = nvp->v_mount;
 					}
-				    }
-				} else {
-				    nvp = NULL;
 				}
-				if (r) {
+
+				/*
+				 * If we failed to get attributes of the entry,
+				 * then just skip it for NFSv3 (the traditional
+				 * behavior in the old NFS server).
+				 * For NFSv4 the behavior is controlled by
+				 * RDATTRERROR: we either ignore the error or
+				 * fail the request.
+				 * Note that RDATTRERROR is never set for NFSv3.
+				 */
+				if (r != 0) {
 					if (!NFSISSET_ATTRBIT(&attrbits,
 					    NFSATTRBIT_RDATTRERROR)) {
-						if (nvp != NULL)
-							vput(nvp);
+						vput(nvp);
 						if (needs_unbusy != 0)
 							vfs_unbusy(new_mp);
+						if ((nd->nd_flag & ND_NFSV3))
+							goto invalid;
 						nd->nd_repstat = r;
 						break;
 					}
@@ -2519,6 +2539,7 @@ again:
 			if (dirlen <= cnt)
 				entrycnt++;
 		}
+invalid:
 		cpos += dp->d_reclen;
 		dp = (struct dirent *)cpos;
 		cookiep++;

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