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 <adeason@sinenomine.net>
Reviewed-by: Derrick Brashear <shadow@dementia.org>

diff --git a/src/vol/fssync-server.c b/src/vol/fssync-server.c
index 65e3537..c88b9f0 100644 (file)
--- 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;
 }
 
 /**