From: Andrew Deason <adeason@sinenomine.net>
Date: Thu, 3 Oct 2013 17:38:08 +0000 (-0500)
Subject: salvager: Ignore linktable-only RW volumes
X-Git-Tag: openafs-stable-1_8_0pre1~970
X-Git-Url: http://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=600712877ca0883c6ec609d51909336964b06cba

salvager: Ignore linktable-only RW volumes

In general, the salvager will try to salvage any volume if we find an
inode for that volume. However, for namei, we'll always have at least
one inode for the RW volume, even if we only have e.g. an RO volume at
a particular site, since the linktable special inode is always marked
as for the RW volume id. So, if we salvage a volume group that only
has an RO, normally we would also try to salvage the corresponding RW,
even if it doesn't exist. We would then recreate the "missing"
metadata files, so after salvaging, the RW appears to exist as a
normal volume.

The salvager currently tries to avoid this by skipping salvaging the
RW if we find more than one volume in the volume group, and if the RW
only has one special inode, and that one special inode is the
linktable. This solves the problem most of the time, but misses a few
corner cases:

 - If we found more than one linktable, we'll try to salvage the RW
   anyway. This shouldn't happen, but certain cases of corruption can
   cause incorrectly-named linktables, resulting in multiple
   linktables.

 - If we only find one volume (the RW), we'll still salvage the RW,
   even if the only inode for it is a single linktable. This can
   happen due to botched salvages in the past, or interrupted deletes
   and such. It's just cruft.

In any situation like those, we cause an RW volume to be created where
there previously was none. This can be a problem, since the RW volume
is unknown to the administrator, and does not appear in the VLDB. Such
"phantom" volumes can be very confusing and can cause problems in the
future. For example, if that same RW volume is moved to the server
with the "phantom" RW volume, we now have two of the same RW volume on
the same server on different partitions, which is a big problem.

So, to avoid these corner cases, check all of the special inodes to
see if all of them are linktables. Also perform this check if we don't
have any non-special inodes (even if we only see 1 volume), to catch
the "cruft" case above.

Change-Id: I00df021ebedce44f69302a48ed2716bd9bda124e
Reviewed-on: http://gerrit.openafs.org/10321
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
---

diff --git a/src/vol/vol-salvage.c b/src/vol/vol-salvage.c
index 4d3afb5..0861f02 100644
--- a/src/vol/vol-salvage.c
+++ b/src/vol/vol-salvage.c
@@ -2006,15 +2006,33 @@ DoSalvageVolumeGroup(struct SalvInfo *salvinfo, struct InodeSummary *isp, int nV
 	int rw = (i == 0);
 	struct InodeSummary *lisp = &isp[i];
 #ifdef AFS_NAMEI_ENV
-	/* If only the RO is present on this partition, the link table
-	 * shows up as a RW volume special file. Need to make sure the
-	 * salvager doesn't try to salvage the non-existent RW.
-	 */
-	if (rw && nVols > 1 && isp[i].nSpecialInodes == 1) {
-	    /* If this only special inode is the link table, continue */
-	    if (inodes->u.special.type == VI_LINKTABLE) {
-		haveRWvolume = 0;
-		continue;
+	if (rw && (nVols > 1 || isp[i].nSpecialInodes == isp[i].nInodes)) {
+	    /* If nVols > 1, we have more than one vol in this volgroup, so
+	     * the RW inodes we detected may just be for the linktable, and
+	     * there is no actual RW volume.
+	     *
+	     * Additionally, if we only have linktable inodes (no other
+	     * special inodes, no data inodes), there is also no actual RW
+	     * volume to salvage; this is just cruft left behind by something
+	     * else. In that case nVols will only be 1, though, so also
+	     * perform this linktables-only check if we don't have any
+	     * non-special inodes. */
+	    int inode_i;
+	    int all_linktables = 1;
+	    for (inode_i = 0; inode_i < isp[i].nSpecialInodes; inode_i++) {
+		if (inodes[inode_i].u.special.type != VI_LINKTABLE) {
+		    all_linktables = 0;
+		    break;
+		}
+	    }
+	    if (all_linktables) {
+		/* All we have are linktable special inodes, so skip salvaging
+		 * the RW; there was never an RW volume here. If we don't do
+		 * this, we risk creating a new "phantom" RW that the VLDB
+		 * doesn't know about, which is confusing and can cause
+		 * problems. */
+		 haveRWvolume = 0;
+		 continue;
 	    }
 	}
 #endif