From 465e613b5d73380374eb69a226f4227e976e1f5c Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Mon, 29 Aug 2011 17:41:31 -0500 Subject: [PATCH] DAFS: Remove VOL_SALVAGE_INVALIDATE_HEADER Currently VRequestSalvage_r takes a flag, VOL_SALVAGE_INVALIDATE_HEADER, which causes the header for the specified volume to be freed (via FreeVolumeHeader). This is almost never safe to do, since there may be other users of the specified volume that can be accessing the volume header at the same time. There is also no reason to invalidate the header at the time of the VRequestSalvage_r call, since the header must be invalidated when we detach the volume (other utilities may change header information). So, if there are any problems in the future because we do not invalidate the header at the time of VRequestSalvage_r, it is the fault of the detachment/offlining logic. So, remove VOL_SALVAGE_INVALIDATE_HEADER and all of its users. Take this opportunity to correctly document the VRequestSalvage_r headers in the VRequestSalvage_r comment, as it was previously missing the VOL_SALVAGE_NO_OFFLINE flag. Reviewed-on: http://gerrit.openafs.org/5319 Reviewed-by: Derrick Brashear Tested-by: BuildBot (cherry picked from commit 4552dc552687267fce3c7a6a9c7f4a1e9395c8e5) Change-Id: Ib991ecefea2115a3c4308ed3c261af7433c9b858 Reviewed-on: http://gerrit.openafs.org/5722 Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear --- src/vol/volume.c | 39 ++++++++++++--------------------------- src/vol/volume.h | 3 +-- 2 files changed, 13 insertions(+), 29 deletions(-) diff --git a/src/vol/volume.c b/src/vol/volume.c index a308bd56d..c4841dd60 100644 --- a/src/vol/volume.c +++ b/src/vol/volume.c @@ -3198,8 +3198,7 @@ attach2(Error * ec, VolId volumeId, char *path, struct DiskPartition64 *partp, if (!VCanScheduleSalvage()) { Log("VAttachVolume: Error attaching volume %s; volume needs salvage; error=%u\n", path, *ec); } - VRequestSalvage_r(ec, vp, SALVSYNC_ERROR, VOL_SALVAGE_INVALIDATE_HEADER | - VOL_SALVAGE_NO_OFFLINE); + VRequestSalvage_r(ec, vp, SALVSYNC_ERROR, VOL_SALVAGE_NO_OFFLINE); vp->nUsers = 0; goto locked_error; @@ -3222,8 +3221,7 @@ attach2(Error * ec, VolId volumeId, char *path, struct DiskPartition64 *partp, if (!VCanScheduleSalvage()) { Log("VAttachVolume: volume salvage flag is ON for %s; volume needs salvage\n", path); } - VRequestSalvage_r(ec, vp, SALVSYNC_NEEDED, VOL_SALVAGE_INVALIDATE_HEADER | - VOL_SALVAGE_NO_OFFLINE); + VRequestSalvage_r(ec, vp, SALVSYNC_NEEDED, VOL_SALVAGE_NO_OFFLINE); vp->nUsers = 0; #else /* AFS_DEMAND_ATTACH_FS */ @@ -3245,8 +3243,7 @@ attach2(Error * ec, VolId volumeId, char *path, struct DiskPartition64 *partp, if (!VCanScheduleSalvage()) { Log("VAttachVolume: volume %s needs to be salvaged; not attached.\n", path); } - VRequestSalvage_r(ec, vp, SALVSYNC_NEEDED, VOL_SALVAGE_INVALIDATE_HEADER | - VOL_SALVAGE_NO_OFFLINE); + VRequestSalvage_r(ec, vp, SALVSYNC_NEEDED, VOL_SALVAGE_NO_OFFLINE); vp->nUsers = 0; #else /* AFS_DEMAND_ATTACH_FS */ @@ -3268,8 +3265,7 @@ attach2(Error * ec, VolId volumeId, char *path, struct DiskPartition64 *partp, #if defined(AFS_DEMAND_ATTACH_FS) /* schedule a salvage so the volume goes away on disk */ - VRequestSalvage_r(ec, vp, SALVSYNC_ERROR, VOL_SALVAGE_INVALIDATE_HEADER | - VOL_SALVAGE_NO_OFFLINE); + VRequestSalvage_r(ec, vp, SALVSYNC_ERROR, VOL_SALVAGE_NO_OFFLINE); VChangeState_r(vp, VOL_STATE_ERROR); vp->nUsers = 0; #endif /* AFS_DEMAND_ATTACH_FS */ @@ -3287,8 +3283,7 @@ attach2(Error * ec, VolId volumeId, char *path, struct DiskPartition64 *partp, VGetBitmap_r(ec, vp, i); if (*ec) { #ifdef AFS_DEMAND_ATTACH_FS - VRequestSalvage_r(ec, vp, SALVSYNC_ERROR, VOL_SALVAGE_INVALIDATE_HEADER | - VOL_SALVAGE_NO_OFFLINE); + VRequestSalvage_r(ec, vp, SALVSYNC_ERROR, VOL_SALVAGE_NO_OFFLINE); vp->nUsers = 0; #endif /* AFS_DEMAND_ATTACH_FS */ Log("VAttachVolume: error getting bitmap for volume (%s)\n", @@ -3336,8 +3331,7 @@ attach2(Error * ec, VolId volumeId, char *path, struct DiskPartition64 *partp, "%lu; needs salvage\n", (int)*ec, afs_printable_uint32_lu(V_id(vp))); #ifdef AFS_DEMAND_ATTACH_FS - VRequestSalvage_r(ec, vp, SALVSYNC_ERROR, VOL_SALVAGE_INVALIDATE_HEADER | - VOL_SALVAGE_NO_OFFLINE); + VRequestSalvage_r(ec, vp, SALVSYNC_ERROR, VOL_SALVAGE_NO_OFFLINE); vp->nUsers = 0; #else /* !AFS_DEMAND_ATTACH_FS */ *ec = VSALVAGE; @@ -3376,7 +3370,7 @@ attach2(Error * ec, VolId volumeId, char *path, struct DiskPartition64 *partp, #ifdef AFS_DEMAND_ATTACH_FS error_state = VOL_STATE_ERROR; /* see if we can recover */ - VRequestSalvage_r(ec, vp, SALVSYNC_NEEDED, VOL_SALVAGE_INVALIDATE_HEADER); + VRequestSalvage_r(ec, vp, SALVSYNC_NEEDED, 0 /*flags*/); #endif } #ifdef AFS_DEMAND_ATTACH_FS @@ -3861,7 +3855,7 @@ GetVolume(Error * ec, Error * client_ec, VolId volumeId, Volume * hint, int nowa vp->hashid); #ifdef AFS_DEMAND_ATTACH_FS if (VCanScheduleSalvage()) { - VRequestSalvage_r(ec, vp, SALVSYNC_ERROR, VOL_SALVAGE_INVALIDATE_HEADER); + VRequestSalvage_r(ec, vp, SALVSYNC_ERROR, 0 /*flags*/); } else { FreeVolume(vp); vp = NULL; @@ -4039,7 +4033,7 @@ VForceOffline_r(Volume * vp, int flags) } #ifdef AFS_DEMAND_ATTACH_FS - VRequestSalvage_r(&error, vp, SALVSYNC_ERROR, VOL_SALVAGE_INVALIDATE_HEADER); + VRequestSalvage_r(&error, vp, SALVSYNC_ERROR, 0 /*flags*/); #endif /* AFS_DEMAND_ATTACH_FS */ #ifdef AFS_PTHREAD_ENV @@ -5030,8 +5024,8 @@ VCheckSalvage(Volume * vp) * @param[in] flags see flags note below * * @note flags: - * VOL_SALVAGE_INVALIDATE_HEADER causes volume header cache entry - * to be invalidated. + * VOL_SALVAGE_NO_OFFLINE do not need to wait to offline the volume; it has + * not been fully attached * * @pre VOL_LOCK is held. * @@ -5116,15 +5110,6 @@ VRequestSalvage_r(Error * ec, Volume * vp, int reason, int flags) *ec = VSALVAGE; code = 1; } - if (flags & VOL_SALVAGE_INVALIDATE_HEADER) { - /* Instead of ReleaseVolumeHeader, we do FreeVolumeHeader() - so that the the next VAttachVolumeByVp_r() invocation - of attach2() will pull in a cached header - entry and fail, then load a fresh one from disk and attach - it to the volume. - */ - FreeVolumeHeader(vp); - } } return code; } @@ -5772,7 +5757,7 @@ VAllocBitmapEntry_r(Error * ec, Volume * vp, VGetBitmap_r(ec, vp, i); if (*ec) { #ifdef AFS_DEMAND_ATTACH_FS - VRequestSalvage_r(ec, vp, SALVSYNC_ERROR, VOL_SALVAGE_INVALIDATE_HEADER); + VRequestSalvage_r(ec, vp, SALVSYNC_ERROR, 0 /*flags*/); #else /* AFS_DEMAND_ATTACH_FS */ DeleteVolumeFromHashTable(vp); vp->shuttingDown = 1; /* Let who has it free it. */ diff --git a/src/vol/volume.h b/src/vol/volume.h index 329c76fe2..89ff9eea3 100644 --- a/src/vol/volume.h +++ b/src/vol/volume.h @@ -994,8 +994,7 @@ extern int VWalkVolumeHeaders(struct DiskPartition64 *dp, const char *partpath, #define VOL_FREE_BITMAP_WAIT 0x1 /* for demand attach, wait for other exclusive ops to end */ /* VRequestSalvage_r flags */ -#define VOL_SALVAGE_INVALIDATE_HEADER 0x1 /* for demand attach fs, invalidate volume header cache */ -#define VOL_SALVAGE_NO_OFFLINE 0x2 /* we do not need to wait to offline the volume; it has +#define VOL_SALVAGE_NO_OFFLINE 0x1 /* we do not need to wait to offline the volume; it has * not been fully attached */ -- 2.39.5