From: Andrew Deason Date: Wed, 2 Dec 2009 19:37:27 +0000 (-0600) Subject: salvager: avoid needing temp files to stay around X-Git-Tag: openafs-devel-1_5_73~163 X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=f96ca93b733ef27cd8a58fcc7462ce4a2f4b2f4a;p=packages%2Fo%2Fopenafs.git salvager: avoid needing temp files to stay around The salvager makes use of a couple of temporary files to store some information while doing a salvage. Instead of referring to these files by path name everywhere, pass around file handles instead. That way we can unlink the files immediately, and they will be deleted on close. This removes one of the roadblocks to allowing multiple salvages on the same partition to occur at once (since otherwise other salvagers would remove the temporary files on startup), and also makes it much less likely that old temporary files will be left lying around in the first place. Change-Id: Idfc696c2c75e21db717c720bd950af6e2766b9aa Reviewed-on: http://gerrit.openafs.org/1263 Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear --- diff --git a/src/vol/listinodes.c b/src/vol/listinodes.c index b16cb6bbe..e1bc3e3d2 100644 --- a/src/vol/listinodes.c +++ b/src/vol/listinodes.c @@ -33,7 +33,7 @@ * -2 - Unable to completely write temp file. Produces warning message in log. */ int -ListViceInodes(char *devname, char *mountedOn, char *resultFile, +ListViceInodes(char *devname, char *mountedOn, FILE *inodeFile, afs_uint32 (*judgeInode) (), afs_uint32 judgeParam, int *forcep, int forceR, char *wpath, void *rock) { @@ -190,11 +190,10 @@ struct dinode *ginode(); int -ListViceInodes(char *devname, char *mountedOn, char *resultFile, +ListViceInodes(char *devname, char *mountedOn, FILE *inodeFile, int (*judgeInode) (), afs_uint32 judgeParam, int *forcep, int forceR, char *wpath, void *rock) { - FILE *inodeFile = NULL; char dev[50], rdev[51]; struct stat status; struct dinode *p; @@ -282,14 +281,6 @@ ListViceInodes(char *devname, char *mountedOn, char *resultFile, return -1; } - if (resultFile) { - inodeFile = fopen(resultFile, "w"); - if (inodeFile == NULL) { - Log("Unable to create inode description file %s\n", resultFile); - goto out; - } - } - /* * calculate the maximum number of inodes possible */ @@ -351,16 +342,11 @@ ListViceInodes(char *devname, char *mountedOn, char *resultFile, err = -2; goto out1; } - if (fclose(inodeFile) == EOF) { - Log("Unable to successfully close inode file for %s\n", partition); - err = -2; - goto out1; - } /* * Paranoia: check that the file is really the right size */ - if (stat(resultFile, &status) == -1) { + if (fstat(fileno(inodeFile), &status) == -1) { Log("Unable to successfully stat inode file for %s\n", partition); err = -2; goto out1; @@ -381,8 +367,6 @@ ListViceInodes(char *devname, char *mountedOn, char *resultFile, out1: if (pfd >= 0) close(pfd); - if (inodeFile) - fclose(inodeFile); return err; } @@ -657,11 +641,10 @@ xfs_RenameFiles(char *dir, xfs_Rename_t * renames, int n_renames) int -xfs_ListViceInodes(char *devname, char *mountedOn, char *resultFile, +xfs_ListViceInodes(char *devname, char *mountedOn, FILE *inodeFile, int (*judgeInode) (), afs_uint32 judgeParam, int *forcep, int forceR, char *wpath, void *rock) { - FILE *inodeFile = NULL; i_list_inode_t info; int info_size = sizeof(i_list_inode_t); int fd; @@ -693,14 +676,6 @@ xfs_ListViceInodes(char *devname, char *mountedOn, char *resultFile, return -1; } - if (resultFile) { - inodeFile = fopen(resultFile, "w"); - if (inodeFile == NULL) { - Log("Unable to create inode description file %s\n", resultFile); - return -1; - } - } - if ((top_dirp = opendir(mountedOn)) == NULL) { Log("Can't open directory %s to read inodes.\n", mountedOn); return -1; @@ -830,22 +805,16 @@ xfs_ListViceInodes(char *devname, char *mountedOn, char *resultFile, if (inodeFile) { if (fflush(inodeFile) == EOF) { ("Unable to successfully flush inode file for %s\n", mountedOn); - fclose(inodeFile); return errors ? -1 : -2; } if (fsync(fileno(inodeFile)) == -1) { Log("Unable to successfully fsync inode file for %s\n", mountedOn); - fclose(inodeFile); - return errors ? -1 : -2; - } - if (fclose(inodeFile) == EOF) { - Log("Unable to successfully close inode file for %s\n", mountedOn); return errors ? -1 : -2; } /* * Paranoia: check that the file is really the right size */ - if (stat(resultFile, &status) == -1) { + if (fstat(fileno(inodeFile), &status) == -1) { Log("Unable to successfully stat inode file for %s\n", partition); return errors ? -1 : -2; } @@ -871,19 +840,16 @@ xfs_ListViceInodes(char *devname, char *mountedOn, char *resultFile, closedir(top_dirp); if (renames) free((char *)renames); - if (inodeFile) - fclose(inodeFile); return -1; } #endif int -ListViceInodes(char *devname, char *mountedOn, char *resultFile, +ListViceInodes(char *devname, char *mountedOn, FILE *inodeFile, int (*judgeInode) (), afs_uint32 judgeParam, int *forcep, int forceR, char *wpath, void *rock) { - FILE *inodeFile = NULL; char dev[50], rdev[51]; struct stat status; struct efs_dinode *p; @@ -908,7 +874,7 @@ ListViceInodes(char *devname, char *mountedOn, char *resultFile, } #ifdef AFS_SGI_XFS_IOPS_ENV if (!strcmp("xfs", root_inode.st_fstype)) { - return xfs_ListViceInodes(devname, mountedOn, resultFile, judgeInode, + return xfs_ListViceInodes(devname, mountedOn, inodeFile, judgeInode, judgeParam, forcep, forceR, wpath, rock); } else #endif @@ -945,7 +911,7 @@ BUFAREA sblk; extern char *afs_rawname(); int -ListViceInodes(char *devname, char *mountedOn, char *resultFile, +ListViceInodes(char *devname, char *mountedOn, FILE *inodeFile, int (*judgeInode) (), afs_uint32 judgeParam, int *forcep, int forceR, char *wpath, void *rock) { @@ -959,7 +925,6 @@ ListViceInodes(char *devname, char *mountedOn, char *resultFile, #endif } super; int i, c, e, bufsize, code, err = 0; - FILE *inodeFile = NULL; char dev[50], rdev[100], err1[512], *ptr1; struct dinode *inodes = NULL, *einodes, *dptr; struct stat status; @@ -1000,13 +965,6 @@ ListViceInodes(char *devname, char *mountedOn, char *resultFile, goto out; } - if (resultFile) { - inodeFile = fopen(resultFile, "w"); - if (inodeFile == NULL) { - Log("Unable to create inode description file %s\n", resultFile); - goto out; - } - } #ifdef AFS_AIX_ENV /* * char *FSlabel(), *fslabel=0; @@ -1286,16 +1244,11 @@ ListViceInodes(char *devname, char *mountedOn, char *resultFile, err = -2; goto out1; } - if (fclose(inodeFile) == EOF) { - Log("Unable to successfully close inode file for %s\n", partition); - err = -2; - goto out1; - } /* * Paranoia: check that the file is really the right size */ - if (stat(resultFile, &status) == -1) { + if (fstat(fileno(inodeFile), &status) == -1) { Log("Unable to successfully stat inode file for %s\n", partition); err = -2; goto out1; @@ -1315,8 +1268,6 @@ ListViceInodes(char *devname, char *mountedOn, char *resultFile, err = -1; out1: close(pfd); - if (inodeFile) - fclose(inodeFile); if (inodes) free(inodes); return err; diff --git a/src/vol/namei_ops.c b/src/vol/namei_ops.c index 25edb7e96..955ca7d95 100644 --- a/src/vol/namei_ops.c +++ b/src/vol/namei_ops.c @@ -1218,60 +1218,44 @@ VerifyDirPerms(char *path) * If the resultFile is NULL, then don't call the write routine. */ int -ListViceInodes(char *devname, char *mountedOn, char *resultFile, +ListViceInodes(char *devname, char *mountedOn, FILE *inodeFile, int (*judgeInode) (struct ViceInodeInfo * info, afs_uint32 vid, void *rock), afs_uint32 singleVolumeNumber, int *forcep, int forceR, char *wpath, void *rock) { - FILE *fp = (FILE *) - 1; int ninodes; struct afs_stat status; *forcep = 0; /* no need to salvage until further notice */ - if (resultFile) { - fp = afs_fopen(resultFile, "w"); - if (!fp) { - Log("Unable to create inode description file %s\n", resultFile); - return -1; - } - } - /* Verify protections on directories. */ mode_errors = 0; VerifyDirPerms(mountedOn); ninodes = - namei_ListAFSFiles(mountedOn, WriteInodeInfo, fp, judgeInode, + namei_ListAFSFiles(mountedOn, WriteInodeInfo, inodeFile, judgeInode, singleVolumeNumber, rock); - if (!resultFile) + if (!inodeFile) return ninodes; if (ninodes < 0) { - fclose(fp); return ninodes; } - if (fflush(fp) == EOF) { + if (fflush(inodeFile) == EOF) { Log("Unable to successfully flush inode file for %s\n", mountedOn); - fclose(fp); return -2; } - if (fsync(fileno(fp)) == -1) { + if (fsync(fileno(inodeFile)) == -1) { Log("Unable to successfully fsync inode file for %s\n", mountedOn); - fclose(fp); - return -2; - } - if (fclose(fp) == EOF) { - Log("Unable to successfully close inode file for %s\n", mountedOn); return -2; } /* * Paranoia: check that the file is really the right size */ - if (afs_stat(resultFile, &status) == -1) { + if (afs_fstat(fileno(inodeFile), &status) == -1) { Log("Unable to successfully stat inode file for %s\n", mountedOn); return -2; } diff --git a/src/vol/namei_ops.h b/src/vol/namei_ops.h index fff38a28e..be2222cd4 100644 --- a/src/vol/namei_ops.h +++ b/src/vol/namei_ops.h @@ -53,7 +53,7 @@ int namei_ListAFSFiles(char *dev, int (*judge_fun) (struct ViceInodeInfo * info, afs_uint32 vid, void *rock), afs_uint32 singleVolumeNumber, void *rock); -int ListViceInodes(char *devname, char *mountedOn, char *resultFile, +int ListViceInodes(char *devname, char *mountedOn, FILE *inodeFile, int (*judgeInode) (struct ViceInodeInfo * info, afs_uint32 vid, void *rock), afs_uint32 singleVolumeNumber, int *forcep, int forceR, diff --git a/src/vol/ntops.c b/src/vol/ntops.c index 7c5b1e8f5..2f81be502 100644 --- a/src/vol/ntops.c +++ b/src/vol/ntops.c @@ -997,56 +997,41 @@ WriteInodeInfo(FILE * fp, struct ViceInodeInfo *info, char *dir, char *name) * This code optimizes single volume salvages by just looking at that one * volume's directory. * - * If the resultFile is NULL, then don't call the write routine. + * If the inodeFile is NULL, then don't call the write routine. */ int -ListViceInodes(char *devname, char *mountedOn, char *resultFile, +ListViceInodes(char *devname, char *mountedOn, FILE *inodeFile, int (*judgeInode) (struct ViceInodeInfo * info, afs_uint32 vid, void *rock), afs_uint32 singleVolumeNumber, int *forcep, int forceR, char *wpath, void *rock) { - FILE *fp = (FILE *) - 1; int ninodes; struct stat status; - if (resultFile) { - fp = fopen(resultFile, "w"); - if (!fp) { - Log("Unable to create inode description file %s\n", resultFile); - return -1; - } - } ninodes = - nt_ListAFSFiles(wpath, WriteInodeInfo, fp, judgeInode, + nt_ListAFSFiles(wpath, WriteInodeInfo, inodeFile, judgeInode, singleVolumeNumber, rock); - if (!resultFile) + if (!inodeFile) return ninodes; if (ninodes < 0) { - fclose(fp); return ninodes; } - if (fflush(fp) == EOF) { + if (fflush(inodeFile) == EOF) { Log("Unable to successfully flush inode file for %s\n", mountedOn); - fclose(fp); return -2; } - if (fsync(fileno(fp)) == -1) { + if (fsync(fileno(inodeFile)) == -1) { Log("Unable to successfully fsync inode file for %s\n", mountedOn); - fclose(fp); - return -2; - } - if (fclose(fp) == EOF) { - Log("Unable to successfully close inode file for %s\n", mountedOn); return -2; } /* * Paranoia: check that the file is really the right size */ - if (stat(resultFile, &status) == -1) { + if (fstat(fileno(inodeFile), &status) == -1) { Log("Unable to successfully stat inode file for %s\n", mountedOn); return -2; } diff --git a/src/vol/ntops.h b/src/vol/ntops.h index 9db6f8ab6..9ca1b56f9 100644 --- a/src/vol/ntops.h +++ b/src/vol/ntops.h @@ -51,7 +51,7 @@ int nt_ListAFSFiles(char *dev, char *dir, char *file), FILE * fp, int (*judge_fun) (struct ViceInodeInfo *, afs_uint32 vid, void *rock), afs_uint32 singleVolumeNumber, void *rock); -int ListViceInodes(char *devname, char *mountedOn, char *resultFile, +int ListViceInodes(char *devname, char *mountedOn, FILE *inodeFile, int (*judgeInode) (struct ViceInodeInfo * info, afs_uint32 vid, void *rock), afs_uint32 singleVolumeNumber, int *forcep, int forceR, char *wpath, void *rock); diff --git a/src/vol/nuke.c b/src/vol/nuke.c index a79d64d8f..ef082c834 100644 --- a/src/vol/nuke.c +++ b/src/vol/nuke.c @@ -177,16 +177,9 @@ nuke(char *aname, afs_int32 avolid) * all we need to do to call ListViceInodes is find the inodes for the * volume we're nuking. */ -#ifdef AFS_NAMEI_ENV code = ListViceInodes(lastDevComp, aname, NULL, NukeProc, avolid, &forceSal, 0, wpath, &allInodes); -#else - code = - ListViceInodes(lastDevComp, aname, "/tmp/vNukeXX", NukeProc, avolid, - &forceSal, 0, wpath, &allInodes); - unlink("/tmp/vNukeXX"); /* clean it up now */ -#endif if (code == 0) { /* actually do the idecs now */ for (ti = allInodes; ti; ti = ti->next) { diff --git a/src/vol/vol-salvage.c b/src/vol/vol-salvage.c index 2b6730e6f..5c90fd16a 100644 --- a/src/vol/vol-salvage.c +++ b/src/vol/vol-salvage.c @@ -713,10 +713,12 @@ SalvageFileSys1(struct DiskPartition64 *partP, VolumeId singleVolumeNumber) { char *name, *tdir; char inodeListPath[256]; + FILE *inodeFile; static char tmpDevName[100]; static char wpath[100]; struct VolumeSummary *vsp, *esp; int i, j; + int code; fileSysPartition = partP; fileSysDevice = fileSysPartition->device; @@ -785,9 +787,10 @@ SalvageFileSys1(struct DiskPartition64 *partP, VolumeId singleVolumeNumber) snprintf(inodeListPath, 255, "%s/salvage.inodes.%s.%d", tdir, name, getpid()); #endif - if (GetInodeSummary(inodeListPath, singleVolumeNumber) < 0) { - unlink(inodeListPath); - return; + + inodeFile = fopen(inodeListPath, "w+b"); + if (!inodeFile) { + Abort("Error %d when creating inode description file %s; not salvaged\n", errno, inodeListPath); } #ifdef AFS_NT40_ENV /* Using nt_unlink here since we're really using the delete on close @@ -795,15 +798,22 @@ SalvageFileSys1(struct DiskPartition64 *partP, VolumeId singleVolumeNumber) * mean to unlink the file at that point. Those places have been * modified to actually do that so that the NT crt can be used there. */ - inodeFd = - _open_osfhandle((intptr_t)nt_open(inodeListPath, O_RDWR, 0), O_RDWR); - nt_unlink(inodeListPath); /* NT's crt unlink won't if file is open. */ + code = nt_unlink(inodeListPath); #else - inodeFd = afs_open(inodeListPath, O_RDONLY); - unlink(inodeListPath); + code = unlink(inodeListPath); #endif + if (code < 0) { + Log("Error %d when trying to unlink %s\n", errno, inodeListPath); + } + + if (GetInodeSummary(inodeFile, singleVolumeNumber) < 0) { + fclose(inodeFile); + return; + } + inodeFd = fileno(inodeFile); if (inodeFd == -1) Abort("Temporary file %s is missing...\n", inodeListPath); + afs_lseek(inodeFd, 0L, SEEK_SET); if (ListInodeOption) { PrintInodeList(); return; @@ -873,7 +883,7 @@ SalvageFileSys1(struct DiskPartition64 *partP, VolumeId singleVolumeNumber) fileSysPartition->name, (Testing ? " (READONLY mode)" : "")); } - close(inodeFd); /* SalvageVolumeGroup was the last which needed it. */ + fclose(inodeFile); /* SalvageVolumeGroup was the last which needed it. */ } void @@ -1045,10 +1055,11 @@ OnlyOneVolume(struct ViceInodeInfo *inodeinfo, afs_uint32 singleVolumeNumber, vo * be unlinked by the caller. */ int -GetInodeSummary(char *path, VolumeId singleVolumeNumber) +GetInodeSummary(FILE *inodeFile, VolumeId singleVolumeNumber) { struct afs_stat status; int forceSal, err; + int code; struct ViceInodeInfo *ip; struct InodeSummary summary; char summaryFileName[50]; @@ -1065,23 +1076,22 @@ GetInodeSummary(char *path, VolumeId singleVolumeNumber) /* This file used to come from vfsck; cobble it up ourselves now... */ if ((err = - ListViceInodes(dev, fileSysPath, path, + ListViceInodes(dev, fileSysPath, inodeFile, singleVolumeNumber ? OnlyOneVolume : 0, singleVolumeNumber, &forceSal, forceR, wpath, NULL)) < 0) { if (err == -2) { - Log("*** I/O error %d when writing a tmp inode file %s; Not salvaged %s ***\nIncrease space on partition or use '-tmpdir'\n", errno, path, dev); + Log("*** I/O error %d when writing a tmp inode file; Not salvaged %s ***\nIncrease space on partition or use '-tmpdir'\n", errno, dev); return -1; } - unlink(path); Abort("Unable to get inodes for \"%s\"; not salvaged\n", dev); } if (forceSal && !ForceSalvage) { Log("***Forced salvage of all volumes on this partition***\n"); ForceSalvage = 1; } - inodeFd = afs_open(path, O_RDWR); + fseek(inodeFile, 0L, SEEK_SET); + inodeFd = fileno(inodeFile); if (inodeFd == -1 || afs_fstat(inodeFd, &status) == -1) { - unlink(path); Abort("No inode description file for \"%s\"; not salvaged\n", dev); } tdir = (tmpdir ? tmpdir : part); @@ -1094,18 +1104,29 @@ GetInodeSummary(char *path, VolumeId singleVolumeNumber) #endif summaryFile = afs_fopen(summaryFileName, "a+"); if (summaryFile == NULL) { - close(inodeFd); - unlink(path); Abort("Unable to create inode summary file\n"); } + +#ifdef AFS_NT40_ENV + /* Using nt_unlink here since we're really using the delete on close + * semantics of unlink. In most places in the salvager, we really do + * mean to unlink the file at that point. Those places have been + * modified to actually do that so that the NT crt can be used there. + */ + code = nt_unlink(summaryFileName); +#else + code = unlink(summaryFileName); +#endif + if (code < 0) { + Log("Error %d when trying to unlink %s\n", errno, summaryFileName); + } + if (!canfork || debug || Fork() == 0) { int nInodes; unsigned long st_size=(unsigned long) status.st_size; nInodes = st_size / sizeof(struct ViceInodeInfo); if (nInodes == 0) { fclose(summaryFile); - close(inodeFd); - unlink(summaryFileName); if (!singleVolumeNumber) /* Remove the FORCESALVAGE file */ RemoveTheForce(fileSysPath); else { @@ -1126,27 +1147,18 @@ GetInodeSummary(char *path, VolumeId singleVolumeNumber) ip = (struct ViceInodeInfo *)malloc(nInodes*sizeof(struct ViceInodeInfo)); if (ip == NULL) { fclose(summaryFile); - close(inodeFd); - unlink(path); - unlink(summaryFileName); Abort ("Unable to allocate enough space to read inode table; %s not salvaged\n", dev); } if (read(inodeFd, ip, st_size) != st_size) { fclose(summaryFile); - close(inodeFd); - unlink(path); - unlink(summaryFileName); Abort("Unable to read inode table; %s not salvaged\n", dev); } qsort(ip, nInodes, sizeof(struct ViceInodeInfo), CompareInodes); if (afs_lseek(inodeFd, 0, SEEK_SET) == -1 || write(inodeFd, ip, st_size) != st_size) { fclose(summaryFile); - close(inodeFd); - unlink(path); - unlink(summaryFileName); Abort("Unable to rewrite inode table; %s not salvaged\n", dev); } summary.index = 0; @@ -1155,7 +1167,6 @@ GetInodeSummary(char *path, VolumeId singleVolumeNumber) if (fwrite(&summary, sizeof(summary), 1, summaryFile) != 1) { Log("Difficulty writing summary file (errno = %d); %s not salvaged\n", errno, dev); fclose(summaryFile); - close(inodeFd); return -1; } summary.index += (summary.nInodes); @@ -1166,7 +1177,6 @@ GetInodeSummary(char *path, VolumeId singleVolumeNumber) if (fflush(summaryFile) == EOF || fsync(fileno(summaryFile)) == -1) { Log("Unable to write summary file (errno = %d); %s not salvaged\n", errno, dev); fclose(summaryFile); - close(inodeFd); return -1; } if (canfork && !debug) { @@ -1176,9 +1186,6 @@ GetInodeSummary(char *path, VolumeId singleVolumeNumber) } else { if (Wait("Inode summary") == -1) { fclose(summaryFile); - close(inodeFd); - unlink(path); - unlink(summaryFileName); Exit(1); /* salvage of this partition aborted */ } } @@ -1196,8 +1203,6 @@ GetInodeSummary(char *path, VolumeId singleVolumeNumber) nVolumesInInodeFile =(unsigned long)(status.st_size) / sizeof(struct InodeSummary); Log("%d nVolumesInInodeFile %d \n",nVolumesInInodeFile,(unsigned long)(status.st_size)); fclose(summaryFile); - close(inodeFd); - unlink(summaryFileName); return 0; } diff --git a/src/vol/vol-salvage.h b/src/vol/vol-salvage.h index e1fd52405..b0900d356 100644 --- a/src/vol/vol-salvage.h +++ b/src/vol/vol-salvage.h @@ -236,7 +236,7 @@ extern void CountVolumeInodes(register struct ViceInodeInfo *ip, int maxInodes, extern void DeleteExtraVolumeHeaderFile(register struct VolumeSummary *vsp); extern void DistilVnodeEssence(VolumeId vid, VnodeClass class, Inode ino, Unique * maxu); -extern int GetInodeSummary(char *path, VolumeId singleVolumeNumber); +extern int GetInodeSummary(FILE *inodeFile, VolumeId singleVolumeNumber); extern void GetVolumeSummary(VolumeId singleVolumeNumber); extern int JudgeEntry(void *dirVal, char *name, afs_int32 vnodeNumber, afs_int32 unique);