salvager: Remove VolumeSummary->fileName

The 'fileName' field in VolumeSummary serves two apparent purposes:

 - Storing the filename of the volume header file (V0XXX.vol).

 - Indicating whether or not a given VolumeSummary object is
   referenced by any inodes on disk. fileName is set by
   AskVolumeSummary/GetVolumeSummary, and is cleared in
   SalvageFileSys1 when a matching inodeSummary entry is found.

This is very confusing. The first purpose is completely unnecessary;
we can always calculate the filename from the volume id for the
volume, and we already enforce the filename to be of that specific
format. The second purpose is very unclear in the current code, and
overloads the meaning of the field.

So instead, remove fileName entirely. Code that was using it to locate
the header file are changed to use VolumeExternalName_r. Code that was
using the field to determine if the volume is "unused" is changed to
use a field just called "unused", set to 0 or 1.

Reviewed-on: http://gerrit.openafs.org/6786
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit 2a1719faa810663d6d9cf774339e286c18ef2717)

Change-Id: I295ef6c982991bdeeb533c55e6ba6d3175789ce6
Reviewed-on: http://gerrit.openafs.org/9461
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>

diff --git a/src/vol/vol-salvage.c b/src/vol/vol-salvage.c
index 0a712943e8f00bbd6038a47200929e1f69f8da9f..91e5cb12386262b0bf67eeb5b484bce551e0aaf5 100644 (file)
--- a/src/vol/vol-salvage.c
+++ b/src/vol/vol-salvage.c
@@ -919,7 +919,7 @@ SalvageFileSys1(struct DiskPartition64 *partP, VolumeId singleVolumeNumber)
             * If there is one here that is not in the inode volume list,
             * delete it now. */
            for (; vsp < esp && (vsp->header.parent < rwvid); vsp++) {
-               if (vsp->fileName)
+               if (vsp->unused)
                    DeleteExtraVolumeHeaderFile(salvinfo, vsp);
            }
            /* Now match up the volume summary info from the root directory with the
@@ -928,7 +928,7 @@ SalvageFileSys1(struct DiskPartition64 *partP, VolumeId singleVolumeNumber)
            for (tsp = vsp; tsp < esp && (tsp->header.parent == rwvid); tsp++) {
                if (tsp->header.id == vid) {
                    salvinfo->inodeSummary[j].volSummary = tsp;
-                   tsp->fileName = 0;
+                   tsp->unused = 0;
                    break;
                }
            }
@@ -941,7 +941,7 @@ SalvageFileSys1(struct DiskPartition64 *partP, VolumeId singleVolumeNumber)
 
     /* Delete any additional volumes that were listed in the partition but which didn't have any corresponding inodes */
     for (; vsp < esp; vsp++) {
-       if (vsp->fileName)
+       if (vsp->unused)
            DeleteExtraVolumeHeaderFile(salvinfo, vsp);
     }
 
@@ -1000,7 +1000,14 @@ void
 DeleteExtraVolumeHeaderFile(struct SalvInfo *salvinfo, struct VolumeSummary *vsp)
 {
     char path[64];
-    sprintf(path, "%s" OS_DIRSEP "%s", salvinfo->fileSysPath, vsp->fileName);
+    char filename[VMAXPATHLEN];
+
+    if (vsp->deleted) {
+       return;
+    }
+
+    VolumeExternalName_r(vsp->header.id, filename, sizeof(filename));
+    sprintf(path, "%s" OS_DIRSEP "%s", salvinfo->fileSysPath, filename);
 
     if (!Showmode)
        Log("The volume header file %s is not associated with any actual data (%sdeleted)\n", path, (Testing ? "would have been " : ""));
@@ -1013,7 +1020,7 @@ DeleteExtraVolumeHeaderFile(struct SalvInfo *salvinfo, struct VolumeSummary *vsp
                afs_printable_uint32_lu(vsp->header.id));
        }
 
-       /* make sure we actually delete the fileName file; ENOENT
+       /* make sure we actually delete the header file; ENOENT
         * is fine, since VDestroyVolumeDiskHeader probably already
         * unlinked it */
        if (unlink(path) && errno != ENOENT) {
@@ -1024,7 +1031,6 @@ DeleteExtraVolumeHeaderFile(struct SalvInfo *salvinfo, struct VolumeSummary *vsp
        }
        vsp->deleted = 1;
     }
-    vsp->fileName = 0;
 }
 
 int
@@ -1258,7 +1264,7 @@ GetInodeSummary(struct SalvInfo *salvinfo, FILE *inodeFile, VolumeId singleVolum
                GetVolumeSummary(salvinfo, singleVolumeNumber);
 
                for (i = 0, vsp = salvinfo->volumeSummaryp; i < salvinfo->nVolumes; i++) {
-                   if (vsp->fileName) {
+                   if (vsp->unused) {
                        if (vsp->header.id == singleVolumeNumber) {
                            foundSVN = 1;
                        }
@@ -1507,7 +1513,7 @@ AskVolumeSummary(struct SalvInfo *salvinfo, VolumeId singleVolumeNumber)
 
                DiskToVolumeHeader(&vsp->header, &diskHdr);
                VolumeExternalName_r(q_res.children[i], name, sizeof(name));
-               vsp->fileName = ToString(name);
+               vsp->unused = 1;
                salvinfo->nVolumes++;
                vsp++;
            }
@@ -1678,7 +1684,7 @@ RecordHeader(struct DiskPartition64 *dp, const char *name,
            return 1;
        }
 
-       summary.fileName = ToString(base);
+       summary.unused = 1;
        params->nVolumes++;
 
        if (params->nVolumes > params->totalVolumes) {
@@ -2392,12 +2398,7 @@ SalvageVolumeHeaderFile(struct SalvInfo *salvinfo, struct InodeSummary *isp,
        if (memcmp
            (&isp->volSummary->header, &tempHeader,
             sizeof(struct VolumeHeader))) {
-           /* We often remove the name before calling us, so we make a fake one up */
-           if (isp->volSummary->fileName) {
-               strcpy(headerName, isp->volSummary->fileName);
-           } else {
-               (void)afs_snprintf(headerName, sizeof headerName, VFORMAT, afs_printable_uint32_lu(isp->volumeId));
-           }
+           VolumeExternalName_r(isp->volumeId, headerName, sizeof(headerName));
            (void)afs_snprintf(path, sizeof path, "%s" OS_DIRSEP "%s", salvinfo->fileSysPath, headerName);
 
            Log("Header file %s is damaged or no longer valid%s\n", path,
@@ -4298,7 +4299,9 @@ MaybeZapVolume(struct SalvInfo *salvinfo, struct InodeSummary *isp,
            if (!Testing) {
                afs_int32 code;
                char path[64];
-               sprintf(path, "%s" OS_DIRSEP "%s", salvinfo->fileSysPath, isp->volSummary->fileName);
+               char filename[VMAXPATHLEN];
+               VolumeExternalName_r(isp->volumeId, filename, sizeof(filename));
+               sprintf(path, "%s" OS_DIRSEP "%s", salvinfo->fileSysPath, filename);
 
                code = VDestroyVolumeDiskHeader(salvinfo->fileSysPartition, isp->volumeId, isp->RWvolumeId);
                if (code) {
@@ -4307,7 +4310,7 @@ MaybeZapVolume(struct SalvInfo *salvinfo, struct InodeSummary *isp,
                        afs_printable_uint32_lu(isp->volumeId));
                }
 
-               /* make sure we actually delete the fileName file; ENOENT
+               /* make sure we actually delete the header file; ENOENT
                 * is fine, since VDestroyVolumeDiskHeader probably already
                 * unlinked it */
                if (unlink(path) && errno != ENOENT) {

diff --git a/src/vol/vol-salvage.h b/src/vol/vol-salvage.h
index f0efef42f7165553bd7b82c9915912905d68d13f..44d2ce27a81f8959875d815656f55a3f74962959 100644 (file)
--- a/src/vol/vol-salvage.h
+++ b/src/vol/vol-salvage.h
@@ -52,8 +52,6 @@ struct VolumeSummary {                /* Volume summary an entry for each
                                 * volume in a volume directory.
                                 * Assumption: one volume directory per
                                 * partition */
-    char *fileName;            /* File name on the partition for the volume
-                                * header */
     struct VolumeHeader header;
     /* volume number, rw volume number, inode
      * numbers of each major component of
@@ -63,6 +61,8 @@ struct VolumeSummary {                /* Volume summary an entry for each
     byte wouldNeedCallback;    /* set if the file server should issue
                                 * call backs for all the files in this volume when
                                 * the volume goes back on line */
+    byte unused;                /* is this volume 'extra'? i.e. not referenced
+                                 * by anything? */
 };
 
 struct VnodeInfo {