From: Mike Meffie Date: Tue, 27 Jan 2009 14:24:23 +0000 (+0000) Subject: dafs-vol-offline-race-20090127 X-Git-Tag: openafs-devel-1_5_61~566 X-Git-Url: https://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=72d502be69ea3208634bb0665fd37e990d1fa38e dafs-vol-offline-race-20090127 LICENSE IPL10 FIXES 124215 avoid race when taking volumes offline in dafs --- diff --git a/src/vol/fssync-server.c b/src/vol/fssync-server.c index eae04be..d5256fd 100644 --- a/src/vol/fssync-server.c +++ b/src/vol/fssync-server.c @@ -741,6 +741,9 @@ FSYNC_com_VolOff(FSSYNC_VolOp_command * vcom, SYNC_response * res) if (VIsErrorState(V_attachState(vp))) { goto deny; } + if (vp->salvage.requested) { + goto deny; + } break; default: @@ -761,6 +764,7 @@ FSYNC_com_VolOff(FSSYNC_VolOp_command * vcom, SYNC_response * res) * 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; @@ -797,6 +801,9 @@ FSYNC_com_VolOff(FSSYNC_VolOp_command * vcom, SYNC_response * res) vcom->hdr->reason == V_DUMP ? "dump" : "UNKNOWN"); } +#ifdef AFS_DEMAND_ATTACH_FS + vp->pending_vol_op->vol_op_state = FSSYNC_VolOpRunningOnline; +#endif VPutVolume_r(vp); } else { if (VVolOpSetVBusy_r(vp, &info)) { @@ -812,7 +819,19 @@ FSYNC_com_VolOff(FSSYNC_VolOp_command * vcom, SYNC_response * res) strlcpy(vcom->v->partName, vp->partition->name, sizeof(vcom->v->partName)); } +#ifdef AFS_DEMAND_ATTACH_FS + VOfflineForVolOp_r(&error, vp, "A volume utility is running."); + if (error==0) { + assert(vp->nUsers==0); + vp->pending_vol_op->vol_op_state = FSSYNC_VolOpRunningOffline; + } + else { + VDeregisterVolOp_r(vp); + code = SYNC_DENIED; + } +#else VOffline_r(vp, "A volume utility is running."); +#endif vp = NULL; } } @@ -1430,6 +1449,7 @@ FSYNC_com_to_info(FSSYNC_VolOp_command * vcom, FSSYNC_VolOp_info * info) { memcpy(&info->com, vcom->hdr, sizeof(SYNC_command_hdr)); memcpy(&info->vop, vcom->vop, sizeof(FSSYNC_VolOp_hdr)); + info->vol_op_state = FSSYNC_VolOpPending; } /** diff --git a/src/vol/fssync.h b/src/vol/fssync.h index 2596cbe..16fa9da 100644 --- a/src/vol/fssync.h +++ b/src/vol/fssync.h @@ -97,6 +97,18 @@ typedef struct FSSYNC_VolOp_command { struct offlineInfo * volumes; } FSSYNC_VolOp_command; + +/** + * volume operation processing state. + */ +enum FSSYNC_VolOpState { + FSSYNC_VolOpPending = 0, /**< volume operation request received */ + FSSYNC_VolOpDenied = 1, /**< volume operation request denied */ + FSSYNC_VolOpRunningOnline = 2, /**< volume operation is running while volume is online */ + FSSYNC_VolOpRunningOffline = 3, /**< volume operation is running while volume is offline */ + FSSYNC_VolOpRunningUnknown = 4, /**< volume operation is running; VVolOpLeaveOnline_r resolution unknown */ +}; + /** * volume operation information node. * @@ -110,6 +122,7 @@ typedef struct FSSYNC_VolOp_command { typedef struct FSSYNC_VolOp_info { SYNC_command_hdr com; FSSYNC_VolOp_hdr vop; + enum FSSYNC_VolOpState vol_op_state; } FSSYNC_VolOp_info; diff --git a/src/vol/volume.c b/src/vol/volume.c index 043cf09..b69de93 100644 --- a/src/vol/volume.c +++ b/src/vol/volume.c @@ -2322,7 +2322,19 @@ attach2(Error * ec, VolId volumeId, char *path, register struct VolumeHeader * h /* check for pending volume operations */ if (vp->pending_vol_op) { /* see if the pending volume op requires exclusive access */ - if (!VVolOpLeaveOnline_r(vp, vp->pending_vol_op)) { + switch (vp->pending_vol_op->vol_op_state) { + case FSSYNC_VolOpPending: + /* this should never happen */ + assert(vp->pending_vol_op->vol_op_state != FSSYNC_VolOpPending); + break; + + case FSSYNC_VolOpRunningUnknown: + vp->pending_vol_op->vol_op_state = + (VVolOpLeaveOnline_r(vp, vp->pending_vol_op) ? + FSSYNC_VolOpRunningOnline : FSSYNC_VolOpRunningOffline); + /* fall through */ + + case FSSYNC_VolOpRunningOffline: /* mark the volume down */ *ec = VOFFLINE; VChangeState_r(vp, VOL_STATE_UNATTACHED); @@ -2777,7 +2789,8 @@ GetVolume(Error * ec, Error * client_ec, VolId volumeId, Volume * hint, int flag * - VOL_STATE_SHUTTING_DOWN */ if ((V_attachState(vp) == VOL_STATE_ERROR) || - (V_attachState(vp) == VOL_STATE_SHUTTING_DOWN)) { + (V_attachState(vp) == VOL_STATE_SHUTTING_DOWN) || + (V_attachState(vp) == VOL_STATE_GOING_OFFLINE)) { *ec = VNOVOL; vp = NULL; break; @@ -2801,7 +2814,6 @@ GetVolume(Error * ec, Error * client_ec, VolId volumeId, Volume * hint, int flag /* allowable states: * - PREATTACHED * - ATTACHED - * - GOING_OFFLINE * - SALVAGING */ @@ -2889,7 +2901,12 @@ GetVolume(Error * ec, Error * client_ec, VolId volumeId, Volume * hint, int flag /* * this test MUST happen after the volume header is loaded */ - if (vp->pending_vol_op && !VVolOpLeaveOnline_r(vp, vp->pending_vol_op)) { + + /* only valid before/during demand attachment */ + assert(!vp->pending_vol_op || vp->pending_vol_op != FSSYNC_VolOpRunningUnknown); + + /* deny getvolume due to running mutually exclusive vol op */ + if (vp->pending_vol_op && vp->pending_vol_op->vol_op_state==FSSYNC_VolOpRunningOffline) { /* * volume cannot remain online during this volume operation. * notify client. @@ -3153,6 +3170,67 @@ VOffline_r(Volume * vp, char *message) #endif /* AFS_DEMAND_ATTACH_FS */ } +#ifdef AFS_DEMAND_ATTACH_FS +/** + * Take a volume offline in order to perform a volume operation. + * + * @param[inout] ec address in which to store error code + * @param[in] vp volume object pointer + * @param[in] message volume offline status message + * + * @pre + * - VOL_LOCK is held + * - caller MUST hold a heavyweight ref on vp + * + * @post + * - volume is taken offline + * - if possible, volume operation is promoted to running state + * - on failure, *ec is set to nonzero + * + * @note Although this function does not return any value, it may + * still fail to promote our pending volume operation to + * a running state. Any caller MUST check the value of *ec, + * and MUST NOT blindly assume success. + * + * @warning if the caller does not hold a lightweight ref on vp, + * then it MUST NOT reference vp after this function + * returns to the caller. + * + * @internal volume package internal use only + */ +void +VOfflineForVolOp_r(Error *ec, Volume *vp, char *message) +{ + assert(vp->pending_vol_op); + if (!V_inUse(vp)) { + VPutVolume_r(vp); + *ec = 1; + return; + } + if (V_offlineMessage(vp)[0] == '\0') + strncpy(V_offlineMessage(vp), message, sizeof(V_offlineMessage(vp))); + V_offlineMessage(vp)[sizeof(V_offlineMessage(vp)) - 1] = '\0'; + + vp->goingOffline = 1; + VChangeState_r(vp, VOL_STATE_GOING_OFFLINE); + VCreateReservation_r(vp); + VPutVolume_r(vp); + + /* Wait for the volume to go offline */ + while (!VIsOfflineState(V_attachState(vp))) { + /* do not give corrupted volumes to the volserver */ + if (vp->salvage.requested && vp->pending_vol_op->com.programType != salvageServer) { + *ec = 1; + goto error; + } + VWaitStateChange_r(vp); + } + *ec = 0; + error: + VCancelReservation_r(vp); +} +#endif /* AFS_DEMAND_ATTACH_FS */ + void VOffline(Volume * vp, char *message) { @@ -3796,11 +3874,12 @@ VDeregisterVolOp_r(Volume * vp) int VVolOpLeaveOnline_r(Volume * vp, FSSYNC_VolOp_info * vopinfo) { - return (vopinfo->com.command == FSYNC_VOL_NEEDVOLUME && + return (vopinfo->vol_op_state == FSSYNC_VolOpRunningOnline || + (vopinfo->com.command == FSYNC_VOL_NEEDVOLUME && (vopinfo->com.reason == V_READONLY || (!VolumeWriteable(vp) && (vopinfo->com.reason == V_CLONE || - vopinfo->com.reason == V_DUMP)))); + vopinfo->com.reason == V_DUMP))))); } /** diff --git a/src/vol/volume_inline.h b/src/vol/volume_inline.h index 4883b43..ef54b45 100644 --- a/src/vol/volume_inline.h +++ b/src/vol/volume_inline.h @@ -73,6 +73,29 @@ VIsErrorState(VolState state) } /** + * tell caller whether V_attachState is an offline condition. + * + * @param state volume state enumeration + * + * @return whether volume state is in offline state + * @retval 0 state is not an offline state + * @retval 1 state is an offline state + * + * @note DEMAND_ATTACH_FS only + */ +static_inline int +VIsOfflineState(VolState state) +{ + switch (state) { + case VOL_STATE_UNATTACHED: + case VOL_STATE_ERROR: + case VOL_STATE_SALVAGING: + return 1; + } + return 0; +} + +/** * tell caller whether V_attachState is valid. * * @param state volume state enumeration