Changeset 62004 in vbox for trunk

Timestamp:

Jul 4, 2016 1:06:44 PM (9 years ago)

Author:

vboxsync

Message:

Storage/iSCSI: Fix possible endless reconnect loop if logging in to the target succeeds but any other requests cause a disconnect. Limit the amount of logins between successful I/O requests to 3, more login attempts are denied and cause the VM to suspend with the apporpriate error about an unrespnsive iSCSI target

File:

• trunk/src/VBox/Storage/ISCSI.cpp (modified) (12 diffs)

trunk/src/VBox/Storage/ISCSI.cpp

@@ -612 +612 @@
     /** Table of commands waiting for a response from the target. */
     PISCSICMD           aCmdsWaiting[ISCSI_CMD_WAITING_ENTRIES];
+    /** Number of logins since last successful I/O.
+     * Used to catch the case where logging succeeds but
+     * processing read/write/flushes cause a disconnect.
+     */
+    volatile uint32_t   cLoginsSinceIo;
 
     /** Release log counter. */
@@ -1261 +1266 @@
 
     Assert(pImage->state == ISCSISTATE_FREE);
+
+    /*
+     * If there were too many logins without any successful I/O just fail
+     * and assume the target is not working properly.
+     */
+    if (ASMAtomicReadU32(&pImage->cLoginsSinceIo) == 3)
+        return VERR_BROKEN_PIPE;
 
     RTSemMutexRequest(pImage->Mutex, RT_INDEFINITE_WAIT);
@@ -1713 +1725 @@
         pImage->state = ISCSISTATE_NORMAL;
 
+    if (RT_SUCCESS(rc))
+        ASMAtomicIncU32(&pImage->cLoginsSinceIo);
+
     RTSemMutexRelease(pImage->Mutex);
 
@@ -2040 +2055 @@
 
 out:
+    if (RT_SUCCESS(rc))
+        ASMAtomicWriteU32(&pImage->cLoginsSinceIo, 0);
     LogFlowFunc(("returns %Rrc\n", rc));
     return rc;
@@ -3283 +3300 @@
 
 /**
+ * Clears all RX/TX PDU states and returns the command for the current
+ * pending TX PDU if existing.
+ *
+ * @returns Pointer to the iSCSI command for the current PDU transmitted or NULL
+ *          if none is waiting.
+ * @param   pImage    iSCSI connection state.
+ */
+static PISCSICMD iscsiPDURxTxClear(PISCSIIMAGE pImage)
+{
+    PISCSICMD pIScsiCmdHead = NULL;
+    PISCSIPDUTX pIScsiPDUTx = NULL;
+
+    /* Reset PDU we are receiving. */
+    iscsiRecvPDUReset(pImage);
+
+    /*
+     * Abort all PDUs we are about to transmit,
+     * the command need a new Itt if the relogin is successful.
+     */
+    while (pImage->pIScsiPDUTxHead)
+    {
+        pIScsiPDUTx = pImage->pIScsiPDUTxHead;
+        pImage->pIScsiPDUTxHead = pIScsiPDUTx->pNext;
+
+        PISCSICMD pIScsiCmd = pIScsiPDUTx->pIScsiCmd;
+        if (pIScsiCmd)
+        {
+            /* Place on command list. */
+            pIScsiCmd->pNext = pIScsiCmdHead;
+            pIScsiCmdHead = pIScsiCmd;
+        }
+        RTMemFree(pIScsiPDUTx);
+    }
+
+    /* Clear the tail pointer (safety precaution). */
+    pImage->pIScsiPDUTxTail = NULL;
+
+    /* Clear the current PDU too. */
+    if (pImage->pIScsiPDUTxCur)
+    {
+        pIScsiPDUTx = pImage->pIScsiPDUTxCur;
+
+        pImage->pIScsiPDUTxCur = NULL;
+        PISCSICMD pIScsiCmd = pIScsiPDUTx->pIScsiCmd;
+        if (pIScsiCmd)
+        {
+            pIScsiCmd->pNext = pIScsiCmdHead;
+            pIScsiCmdHead = pIScsiCmd;
+        }
+        RTMemFree(pIScsiPDUTx);
+    }
+
+    return pIScsiCmdHead;
+}
+
+/**
+ * Rests the iSCSI connection state and returns a list of iSCSI commands pending
+ * when this was called.
+ *
+ * @returns Pointer to the head of the pending iSCSI command list.
+ * @param   pImage    iSCSI connection state.
+ */
+static PISCSICMD iscsiReset(PISCSIIMAGE pImage)
+{
+    PISCSICMD pIScsiCmdHead = NULL;
+    PISCSICMD pIScsiCmdCur = NULL;
+
+    /* Clear all in flight PDUs. */
+    pIScsiCmdHead = iscsiPDURxTxClear(pImage);
+
+    /*
+     * Get all commands which are waiting for a response
+     * They need to be resend too after a successful reconnect.
+     */
+    PISCSICMD pIScsiCmd = iscsiCmdRemoveAll(pImage);
+    if (pIScsiCmd)
+    {
+        pIScsiCmdCur = pIScsiCmd;
+        while (pIScsiCmdCur->pNext)
+            pIScsiCmdCur = pIScsiCmdCur->pNext;
+
+        /*
+         * Place them in front of the list because they are the oldest requests
+         * and need to be processed first to minimize the risk to time out.
+         */
+        pIScsiCmdCur->pNext = pIScsiCmdHead;
+        pIScsiCmdHead = pIScsiCmd;
+    }
+
+    return pIScsiCmdHead;
+}
+
+/**
  * Reattaches the to the target after an error aborting
  * pending commands and resending them.
@@ -3290 +3400 @@
 static void iscsiReattach(PISCSIIMAGE pImage)
 {
-    int rc = VINF_SUCCESS;
-    PISCSICMD pIScsiCmdHead = NULL;
-    PISCSICMD pIScsiCmd = NULL;
-    PISCSICMD pIScsiCmdCur = NULL;
-    PISCSIPDUTX pIScsiPDUTx = NULL;
-
     /* Close connection. */
     iscsiTransportClose(pImage);
     pImage->state = ISCSISTATE_FREE;
 
-    /* Reset PDU we are receiving. */
-    iscsiRecvPDUReset(pImage);
-
-    /*
-     * Abort all PDUs we are about to transmit,
-     * the command need a new Itt if the relogin is successful.
-     */
-    while (pImage->pIScsiPDUTxHead)
-    {
-        pIScsiPDUTx = pImage->pIScsiPDUTxHead;
-        pImage->pIScsiPDUTxHead = pIScsiPDUTx->pNext;
-
-        pIScsiCmd = pIScsiPDUTx->pIScsiCmd;
-
-        if (pIScsiCmd)
-        {
-            /* Place on command list. */
-            pIScsiCmd->pNext = pIScsiCmdHead;
-            pIScsiCmdHead = pIScsiCmd;
-        }
-        RTMemFree(pIScsiPDUTx);
-    }
-
-    /* Clear the tail pointer (safety precaution). */
-    pImage->pIScsiPDUTxTail = NULL;
-
-    /* Clear the current PDU too. */
-    if (pImage->pIScsiPDUTxCur)
-    {
-        pIScsiPDUTx = pImage->pIScsiPDUTxCur;
-
-        pImage->pIScsiPDUTxCur = NULL;
-        pIScsiCmd = pIScsiPDUTx->pIScsiCmd;
-
-        if (pIScsiCmd)
-        {
-            pIScsiCmd->pNext = pIScsiCmdHead;
-            pIScsiCmdHead = pIScsiCmd;
-        }
-        RTMemFree(pIScsiPDUTx);
-    }
-
-    /*
-     * Get all commands which are waiting for a response
-     * They need to be resend too after a successful reconnect.
-     */
-    pIScsiCmd = iscsiCmdRemoveAll(pImage);
-
-    if (pIScsiCmd)
-    {
-        pIScsiCmdCur = pIScsiCmd;
-        while (pIScsiCmdCur->pNext)
-            pIScsiCmdCur = pIScsiCmdCur->pNext;
-
-        /*
-         * Place them in front of the list because they are the oldest requests
-         * and need to be processed first to minimize the risk to time out.
-         */
-        pIScsiCmdCur->pNext = pIScsiCmdHead;
-        pIScsiCmdHead = pIScsiCmd;
-    }
+    /* Reset the state and get the currently pending commands. */
+    PISCSICMD pIScsiCmdHead = iscsiReset(pImage);
 
     /* Try to attach. */
-    rc = iscsiAttach(pImage);
+    int rc = iscsiAttach(pImage);
     if (RT_SUCCESS(rc))
     {
@@ -3371 +3416 @@
         while (pIScsiCmdHead)
         {
-            pIScsiCmd = pIScsiCmdHead;
+            PISCSICMD pIScsiCmd = pIScsiCmdHead;
             pIScsiCmdHead = pIScsiCmdHead->pNext;
 
@@ -3377 +3422 @@
 
             rc = iscsiPDUTxPrepare(pImage, pIScsiCmd);
-            AssertRC(rc);
-        }
-    }
-    else
+            if (RT_FAILURE(rc))
+                break;
+        }
+
+        if (RT_FAILURE(rc))
+        {
+            /* Another error, just give up and report an error. */
+            PISCSICMD pIScsiCmd = iscsiReset(pImage);
+
+            /* Concatenate both lists together so we can abort all requests below. */
+            if (pIScsiCmd)
+            {
+                PISCSICMD pIScsiCmdCur = pIScsiCmd;
+                while (pIScsiCmdCur->pNext)
+                    pIScsiCmdCur = pIScsiCmdCur->pNext;
+
+                pIScsiCmdCur->pNext = pIScsiCmdHead;
+                pIScsiCmdHead = pIScsiCmd;
+            }
+        }
+    }
+
+    if (RT_FAILURE(rc))
     {
         /*
@@ -3388 +3452 @@
         while (pIScsiCmdHead)
         {
-            pIScsiCmd = pIScsiCmdHead;
+            PISCSICMD pIScsiCmd = pIScsiCmdHead;
             pIScsiCmdHead = pIScsiCmdHead->pNext;
 
@@ -3451 +3515 @@
                         {
                             rc = iscsiPDUTxPrepare(pImage, pIScsiCmd);
-                            AssertRC(rc);
+                            if (RT_FAILURE(rc))
+                                iscsiReattach(pImage);
                         }
                         else
@@ -3698 +3763 @@
     size_t cbTransfered = 0;
     PSCSIREQ pScsiReq = (PSCSIREQ)pvUser;
+
+    if (RT_SUCCESS(rcReq))
+        ASMAtomicWriteU32(&pImage->cLoginsSinceIo, 0);
 
     if (   RT_SUCCESS(rcReq)
@@ -3894 +3962 @@
     pImage->cISCSIRetries   = 10;
     pImage->state           = ISCSISTATE_FREE;
+    pImage->cLoginsSinceIo  = 0;
     pImage->pvRecvPDUBuf    = RTMemAlloc(ISCSI_RECV_PDU_BUFFER_SIZE);
     pImage->cbRecvPDUBuf    = ISCSI_RECV_PDU_BUFFER_SIZE;