From 6e29c74fbd304e29b9d6309b4a79db17416fb081 Mon Sep 17 00:00:00 2001 From: Mark Vitale Date: Fri, 25 Jan 2013 18:47:49 -0500 Subject: [PATCH] salvager: prevent assertion during -orphans attach Improve JudgeEntry() detection of orphaned directories to prevent unintentional deletion of their '.' and '..' entries. This in turn prevents a later assert (opr_Verify) when we try to delete and re-add '..' in order to attach the orphan. In JudgeEntry(), 2 sources of information about a directory entry are compared for consistency: - vnodeEssence (unique) from its vnode index entry - name, vnodeNumber and unique from its dir blob entry A directory entry may be ignored, deleted, or repaired/replaced, based upon the results of these and other tests (e.g. dirOprhaned). The '.' and '..' entries are treated as special cases because we do not want to delete them at this point if this directory is orphaned. However, the current test for orphanhood (vnodeEssence->unique == 0) is not sufficient; it could be zero for other reasons. This commit now uses the dirOrphaned flag to test for this. However, we are still interested in doing the right thing for '.' and '..' entries with vnodeEssence->unique == 0. This may indicate that the dir blob entry is pointing at the wrong vnode, and that vnode has unique==0. The current code incorrectly ignores (returns 0) this case. This commit now now falls through to the repair/replace code so that we can find the correct vnode for this entry. The current code assumes that the 'vnodeEssence == 0 && !dirOrphaned' case doesn't exist. Thanks to Andrew Deason for his assistance. Reviewed-on: http://gerrit.openafs.org/9104 Tested-by: BuildBot Reviewed-by: Derrick Brashear (cherry picked from commit e8faeae6dcae0e566de2b21d53d3f78f3cc44e3f) Change-Id: Ibc9e6ddf1f281e3a3a560ed1eefcdb4776e12caa Reviewed-on: http://gerrit.openafs.org/10165 Reviewed-by: Andrew Deason Reviewed-by: Mark Vitale Tested-by: BuildBot Reviewed-by: Derrick Brashear Reviewed-by: Stephan Wiesand --- src/vol/vol-salvage.c | 55 ++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/src/vol/vol-salvage.c b/src/vol/vol-salvage.c index e4a590254..ceca9b52a 100644 --- a/src/vol/vol-salvage.c +++ b/src/vol/vol-salvage.c @@ -3121,31 +3121,42 @@ JudgeEntry(void *arock, char *name, afs_int32 vnodeNumber, * or if the directory is orphaned. */ if (!vnodeEssence->unique || (vnodeEssence->unique) != unique) { - if (!vnodeEssence->unique - && ((strcmp(name, "..") == 0) || (strcmp(name, ".") == 0))) { - /* This is an orphaned directory. Don't delete the . or .. - * entry. Otherwise, it will get created in the next - * salvage and deleted again here. So Just skip it. - */ - return 0; - } - todelete = ((!vnodeEssence->unique || dirOrphaned) ? 1 : 0); - if (!Showmode) { - Log("dir vnode %u: %s" OS_DIRSEP "%s (vnode %u): unique changed from %u to %u %s\n", dir->vnodeNumber, (dir->name ? dir->name : "??"), name, vnodeNumber, unique, vnodeEssence->unique, (!todelete ? "" : (Testing ? "-- would have deleted" : "-- deleted"))); - } - if (!Testing) { - AFSFid fid; - fid.Vnode = vnodeNumber; - fid.Unique = vnodeEssence->unique; - CopyOnWrite(salvinfo, dir); - osi_Assert(Delete(&dir->dirHandle, name) == 0); - if (!todelete) - osi_Assert(Create(&dir->dirHandle, name, &fid) == 0); + if (todelete + && ((strcmp(name, "..") == 0) || (strcmp(name, ".") == 0))) { + if (dirOrphaned) { + /* This is an orphaned directory. Don't delete the . or .. + * entry. Otherwise, it will get created in the next + * salvage and deleted again here. So Just skip it. + * */ + return 0; + } + /* (vnodeEssence->unique == 0 && ('.' || '..')); + * Entries arriving here should be deleted, but the directory + * is not orphaned. Therefore, the entry must be pointing at + * the wrong vnode. Skip the 'else' clause and fall through; + * the code below will repair the entry so it correctly points + * at the vnode of the current directory (if '.') or the parent + * directory (if '..'). */ + } else { + if (!Showmode) { + Log("dir vnode %u: %s" OS_DIRSEP "%s (vnode %u): unique changed from %u to %u %s\n", + dir->vnodeNumber, (dir->name ? dir->name : "??"), name, vnodeNumber, unique, + vnodeEssence->unique, (!todelete ? "" : (Testing ? "-- would have deleted" : "-- deleted"))); + } + if (!Testing) { + AFSFid fid; + fid.Vnode = vnodeNumber; + fid.Unique = vnodeEssence->unique; + CopyOnWrite(salvinfo, dir); + osi_Assert(Delete(&dir->dirHandle, name) == 0); + if (!todelete) + osi_Assert(Create(&dir->dirHandle, name, &fid) == 0); + } + if (todelete) + return 0; /* no need to continue */ } - if (todelete) - return 0; /* no need to continue */ } if (strcmp(name, ".") == 0) { -- 2.39.5