salvager: Handle multiple/inconsistent linktables
authorAndrew Deason <adeason@sinenomine.net>
Thu, 3 Oct 2013 17:51:41 +0000 (12:51 -0500)
committerD Brashear <shadow@your-file-system.com>
Wed, 5 Feb 2014 15:06:36 +0000 (07:06 -0800)
The ListAFSSubDirs code in namei_ops.c currently detects
incorrectly-named linktable files, and whines about them and says the
salvager will handle them. However, the salvager doesn't really handle
them, since we just use the first linktable we find (FindLinkHandle)
without checking any of the information about it.

So, check for these. Fix FindLinkHandle to only consider a linktable
the "real" linktable to use if it actually matches the volume group id
we're salvaging. Also delete any inconsistent linktables via the new
function CheckDupLinktable later on.

Note that inconsistently-named linktables have been known to have been
created in the past due to a bug in the salvager (fixed by ae227049),
and possibly due to other unknown issues.

Change-Id: Iac461e1254e1f73406a2bc74eaa5a5f53d697304
Reviewed-on: http://gerrit.openafs.org/10322
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: D Brashear <shadow@your-file-system.com>

src/vol/vol-salvage.c

index 0861f02..b1931ba 100644 (file)
@@ -1801,8 +1801,8 @@ GetVolumeSummary(struct SalvInfo *salvinfo, VolumeId singleVolumeNumber)
     return 0;
 }
 
-/* Find the link table. This should be associated with the RW volume or, if
- * a RO only site, then the RO volume. For now, be cautious and hunt carefully.
+/* Find the link table. This should be associated with the RW volume, even
+ * if there is only an RO volume at this site.
  */
 Inode
 FindLinkHandle(struct InodeSummary *isp, int nVols,
@@ -1814,13 +1814,66 @@ FindLinkHandle(struct InodeSummary *isp, int nVols,
     for (i = 0; i < nVols; i++) {
        ip = allInodes + isp[i].index;
        for (j = 0; j < isp[i].nSpecialInodes; j++) {
-           if (ip[j].u.special.type == VI_LINKTABLE)
+           if (ip[j].u.special.volumeId == isp->RWvolumeId &&
+               ip[j].u.special.parentId == isp->RWvolumeId &&
+               ip[j].u.special.type == VI_LINKTABLE) {
                return ip[j].inodeNumber;
+           }
        }
     }
     return (Inode) - 1;
 }
 
+#ifdef AFS_NAMEI_ENV
+static int
+CheckDupLinktable(struct SalvInfo *salvinfo, struct InodeSummary *isp, struct ViceInodeInfo *ip)
+{
+    afs_ino_str_t stmp;
+    if (ip->u.vnode.vnodeNumber != INODESPECIAL) {
+       /* not a linktable; process as a normal file */
+       return 0;
+    }
+    if (ip->u.special.type != VI_LINKTABLE) {
+       /* not a linktable; process as a normal file */
+       return 0;
+    }
+
+    /* make sure nothing inc/decs it */
+    ip->linkCount = 0;
+
+    if (ip->u.special.volumeId == ip->u.special.parentId) {
+       /* This is a little weird, but shouldn't break anything, and there is
+        * no known way that this can happen; just do nothing, in case deleting
+        * it would screw something up. */
+       Log("Inode %s appears to be a valid linktable for id (%u), but it's not\n",
+           PrintInode(stmp, ip->inodeNumber), ip->u.special.parentId);
+       Log("the linktable for our volume group (%u). This is unusual, since\n",
+           isp->RWvolumeId);
+       Log("there should only be one linktable per volume group. I'm leaving\n");
+       Log("it alone, just to be safe.\n");
+       return -1;
+    }
+
+    Log("Linktable %s appears to be invalid (parentid/volumeid mismatch: %u != %u)\n",
+        PrintInode(stmp, ip->inodeNumber), ip->u.special.parentId, ip->u.special.volumeId);
+    if (Testing) {
+       Log("Would have deleted linktable inode %s\n", PrintInode(stmp, ip->inodeNumber));
+    } else {
+       IHandle_t *tmpH;
+       namei_t ufs_name;
+
+       Log("Deleting linktable inode %s\n", PrintInode(stmp, ip->inodeNumber));
+       IH_INIT(tmpH, salvinfo->fileSysDevice, isp->RWvolumeId, ip->inodeNumber);
+       namei_HandleToName(&ufs_name, tmpH);
+       if (unlink(ufs_name.n_path) < 0) {
+           Log("Error %d unlinking path %s\n", errno, ufs_name.n_path);
+       }
+    }
+
+    return -1;
+}
+#endif
+
 int
 CreateLinkTable(struct SalvInfo *salvinfo, struct InodeSummary *isp, Inode ino)
 {
@@ -2074,6 +2127,9 @@ DoSalvageVolumeGroup(struct SalvInfo *salvinfo, struct InodeSummary *isp, int nV
                dec_VGLinkH = ip->linkCount - salvinfo->VGLinkH_cnt;
                VGLinkH_p1 = ip->u.param[0];
                continue;       /* Deal with this last. */
+           } else if (CheckDupLinktable(salvinfo, isp, ip)) {
+               /* Don't touch this inode; CheckDupLinktable has handled it */
+               continue;
            }
 #endif
            if (ip->linkCount != 0 && TraceBadLinkCounts) {