From: Andrew Deason Date: Mon, 2 Nov 2009 23:18:19 +0000 (-0600) Subject: DAFS: Wait for exclusive ops in FSYNC_VOL_OFF X-Git-Tag: openafs-devel-1_5_67~89 X-Git-Url: https://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=5e6842283f5c2fdf0fe3306993a6e18c2e590716 DAFS: Wait for exclusive ops in FSYNC_VOL_OFF In the FSYNC_VOL_OFF handler, fssync-server.c errors out if the call to VGetVolumeByVp_r fails. However, this can fail if the volume is in an error state such as SALVAGING. Normally we don't even call GetVolume when the volume is salvaging, but the volume state can change to SALVAGING inside GetVolume. This is particularly likely to happen on a demand salvage, since we switch to the SALVSYNC_REQ state when scheduling the salvage, and if we are still in that state when the salvaged child requests a VOL_OFF, we will fail to get the heavyweight ref. Fix this in two ways. First, we VWaitExclusiveState_r before examining states for the short-circuit logic so our view of the volume state is more accurate. Second, re-examine the volume state after the call to GetVolume, and perform the same short-circuit logic, since the volume state may have changed during GetVolume. Change-Id: I4ebb87691c28170b42e0056b342477a12d0f6888 Reviewed-on: http://gerrit.openafs.org/769 Tested-by: Andrew Deason Reviewed-by: Derrick Brashear --- diff --git a/src/vol/fssync-server.c b/src/vol/fssync-server.c index 65e3537..c88b9f0 100644 --- a/src/vol/fssync-server.c +++ b/src/vol/fssync-server.c @@ -642,7 +642,7 @@ FSYNC_com_VolOff(FSSYNC_VolOp_command * vcom, SYNC_response * res) Volume * vp; Error error; #ifdef AFS_DEMAND_ATTACH_FS - Volume *nvp; + Volume *nvp, *rvp = NULL; #endif if (SYNC_verifyProtocolString(vcom->vop->partName, sizeof(vcom->vop->partName))) { @@ -727,6 +727,12 @@ FSYNC_com_VolOff(FSSYNC_VolOp_command * vcom, SYNC_response * res) } } + /* wait for exclusive ops, so we have an accurate picture of the + * vol attach state */ + VCreateReservation_r(vp); + VWaitExclusiveState_r(vp); + rvp = vp; + /* filter based upon requestor * * volume utilities are not allowed to check out volumes @@ -780,8 +786,34 @@ FSYNC_com_VolOff(FSSYNC_VolOp_command * vcom, SYNC_response * res) /* convert to heavyweight ref */ nvp = VGetVolumeByVp_r(&error, vp); + VCancelReservation_r(rvp); + rvp = NULL; if (!nvp) { + /* + * It's possible for VGetVolumeByVp_r to have dropped and + * re-acquired VOL_LOCK, so volume state may have changed + * back to one of the states we tested for above. Since + * GetVolume can return NULL in some of those states, just + * test for the states again here. + */ + switch (V_attachState(vp)) { + case VOL_STATE_UNATTACHED: + case VOL_STATE_PREATTACHED: + case VOL_STATE_SALVAGING: + case VOL_STATE_ERROR: + /* register the volume operation metadata with the volume + * + * if the volume is currently pre-attached, attach2() + * will evaluate the vol op metadata to determine whether + * attaching the volume would be safe */ + VRegisterVolOp_r(vp, &info); + vp->pending_vol_op->vol_op_state = FSSYNC_VolOpRunningUnknown; + goto done; + default: + break; + } + Log("FSYNC_com_VolOff: failed to get heavyweight reference to volume %u\n", vcom->vop->volume); res->hdr.reason = FSYNC_VOL_PKG_ERROR; @@ -843,12 +875,18 @@ FSYNC_com_VolOff(FSSYNC_VolOp_command * vcom, SYNC_response * res) vp = NULL; } } + goto done; + + deny: + code = SYNC_DENIED; done: +#ifdef AFS_DEMAND_ATTACH_FS + if (rvp) { + VCancelReservation_r(rvp); + } +#endif return code; - - deny: - return SYNC_DENIED; } /**