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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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