From da2d64ce2f85e6b807a01f4a696a25fd989181b1 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Wed, 2 Jan 2013 14:09:06 -0500 Subject: [PATCH] afs: Check dv against localhero aincr For operations that modify directories, we call afs_LocalHero to determine if we can perform the directory modification in our local cache, and avoid fetching the dir blob from the fileserver. Currently, afs_LocalHero assumes that the DV received from the fileserver is correct, and will update the cache DV as long as we have a valid callback on the file. If for any reason the client cache falls out of sync with what's on the fileserver, this can cause the client to incorrectly believe its cache is up to date. Since, the cached data will be marked with the newest DV, even if the DV on the server has jumped to be larger than we expected. While the client cache should never fall out of sync with the fileserver, in the past this has been possible due to other bugs (fileserver idle dead processing and client VNOSERVICE handling). Assuming that the given DV is correct is also just unnecesarily fragile, since we can always check if it is correct, so just check it, and add some comments helping explain what's going on here. Note that regular file writes effectively already check this. Note that this change makes use of the 'aincr' argument to afs_LocalHero, which was previously unused. aincr appears to have been used for a purpose similar to this before OpenAFS 1.0, but was removed, possibly accidentally. It is possible this change negatively affects, or even breaks (unlikely), functionality with the AFS<->DFS translator. Although nothing of the sort has been seen, it is difficult to know one way or the other, due to the lack of available DFS translators. Reviewed-on: http://gerrit.openafs.org/8864 Tested-by: BuildBot Reviewed-by: Derrick Brashear (cherry picked from commit 004e9f25634d76be58b5a60dfc3c9a058d8bcd26) Change-Id: Ie0a3ab5defc5dde3284ad7527fdd05835a85da7f Reviewed-on: http://gerrit.openafs.org/8898 Tested-by: BuildBot Reviewed-by: Andrew Deason Reviewed-by: Chas Williams - CONTRACTOR Reviewed-by: Mark Vitale Reviewed-by: Stephan Wiesand --- src/afs/VNOPS/afs_vnop_create.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/afs/VNOPS/afs_vnop_create.c b/src/afs/VNOPS/afs_vnop_create.c index f8371e20c..b0a562ce8 100644 --- a/src/afs/VNOPS/afs_vnop_create.c +++ b/src/afs/VNOPS/afs_vnop_create.c @@ -521,13 +521,25 @@ afs_LocalHero(struct vcache *avc, struct dcache *adc, AFS_STATCNT(afs_LocalHero); hset64(avers, astat->dataVersionHigh, astat->DataVersion); - /* this *is* the version number, no matter what */ + /* avers *is* the version number now, no matter what */ + if (adc) { + /* does what's in the dcache *now* match what's in the vcache *now*, + * and do we have a valid callback? if not, our local copy is not "ok" */ ok = (hsame(avc->f.m.DataVersion, adc->f.versionNo) && avc->callback && (avc->f.states & CStatd) && avc->cbExpires >= osi_Time()); } else { ok = 0; } + if (ok) { + /* check that the DV on the server is what we expect it to be */ + afs_hyper_t newDV; + hset(newDV, adc->f.versionNo); + hadd32(newDV, aincr); + if (!hsame(avers, newDV)) { + ok = 0; + } + } #if defined(AFS_SGI_ENV) osi_Assert(avc->v.v_type == VDIR); #endif -- 2.39.5