Changeset 51623 in vbox for trunk/src

Timestamp:

Jun 16, 2014 7:09:13 PM (11 years ago)

Author:

vboxsync

Message:

Storage: Fix possible data corruption or read errors if a request accesses the same region while another write is active and allocating a new data block in the image

File:

• trunk/src/VBox/Storage/VD.cpp (modified) (9 diffs)

trunk/src/VBox/Storage/VD.cpp

@@ -278 +278 @@
      * the current one completes. - NIL_VDIOCTX if unlocked. */
     volatile PVDIOCTX      pIoCtxLockOwner;
+    /** If the disk was locked by a growing write, flush or discard request this
+     * contains the start offset to check for interfering I/O while it is in progress. */
+    uint64_t               uOffsetStartLocked;
+    /** If the disk was locked by a growing write, flush or discard request this contains
+     * the first non affected offset to check for interfering I/O while it is in progress. */
+    uint64_t               uOffsetEndLocked;
 
     /** Pointer to the L2 disk cache if any. */
@@ -1550 +1556 @@
 DECLINLINE(void) vdIoCtxDefer(PVBOXHDD pDisk, PVDIOCTX pIoCtx)
 {
-    LogFlowFunc(("Deferring write pIoCtx=%#p\n", pIoCtx));
+    LogFlowFunc(("Deferring I/O context pIoCtx=%#p\n", pIoCtx));
 
     Assert(!pIoCtx->pIoCtxParent && !(pIoCtx->fFlags & VDIOCTX_FLAGS_BLOCKED));
@@ -1646 +1652 @@
     if (   RT_SUCCESS(rc)
         && !pIoCtx->cMetaTransfersPending
-        && !pIoCtx->cDataTransfersPending)
+        && !pIoCtx->cDataTransfersPending
+        && !(pIoCtx->fFlags & VDIOCTX_FLAGS_BLOCKED))
         rc = VINF_VD_ASYNC_IO_FINISHED;
     else if (   RT_SUCCESS(rc)
@@ -1973 +1980 @@
     size_t cbThisRead;
 
+    /*
+     * Check whether there is a full block write in progress which was not allocated.
+     * Defer I/O if the range interferes but only if it does not belong to the
+     * write doing the allocation.
+     */
+    if (   pDisk->pIoCtxLockOwner != NIL_VDIOCTX
+        && uOffset >= pDisk->uOffsetStartLocked
+        && uOffset < pDisk->uOffsetEndLocked
+        && (   !pIoCtx->pIoCtxParent
+            || pIoCtx->pIoCtxParent != pDisk->pIoCtxLockOwner))
+    {
+        Log(("Interferring read while allocating a new block => deferring read\n"));
+        vdIoCtxDefer(pDisk, pIoCtx);
+        return VINF_SUCCESS;
+    }
+
     /* Loop until all reads started or we have a backend which needs to read metadata. */
     do
@@ -2002 +2025 @@
         else
         {
-
             /*
              * Try to read from the given image.
@@ -2921 +2943 @@
          * unwanted expanding of images. VMDK is an example. */
         cbThisWrite = cbWrite;
+
+        /*
+         * Check whether there is a full block write in progress which was not allocated.
+         * Defer I/O if the range interferes.
+         */
+        if (   pDisk->pIoCtxLockOwner != NIL_VDIOCTX
+            && uOffset >= pDisk->uOffsetStartLocked
+            && uOffset < pDisk->uOffsetEndLocked)
+        {
+            Log(("Interferring write while allocating a new block => deferring write\n"));
+            vdIoCtxDefer(pDisk, pIoCtx);
+            rc = VERR_VD_ASYNC_IO_IN_PROGRESS;
+            break;
+        }
+
         fWrite =   (pImage->uOpenFlags & VD_OPEN_FLAGS_HONOR_SAME)
                  ? 0 : VD_WRITE_NO_ALLOC;
-        rc = pImage->Backend->pfnWrite(pImage->pBackendData, uOffset,
-                                            cbThisWrite, pIoCtx,
-                                            &cbThisWrite, &cbPreRead,
-                                            &cbPostRead, fWrite);
+        rc = pImage->Backend->pfnWrite(pImage->pBackendData, uOffset, cbThisWrite,
+                                       pIoCtx, &cbThisWrite, &cbPreRead, &cbPostRead,
+                                       fWrite);
         if (rc == VERR_VD_BLOCK_FREE)
         {
@@ -2963 +2999 @@
                 LogFlowFunc(("Disk is growing because of pIoCtx=%#p pIoCtxWrite=%#p\n",
                              pIoCtx, pIoCtxWrite));
+
+                /* Save the current range for the growing operation to check for intersecting requests later. */
+                pDisk->uOffsetStartLocked = uOffset - cbPreRead;
+                pDisk->uOffsetEndLocked = uOffset + cbThisWrite + cbPostRead;
 
                 pIoCtxWrite->Type.Child.cbPreRead  = cbPreRead;
@@ -3050 +3090 @@
     if (RT_SUCCESS(rc))
     {
+        /* Mark the whole disk as locked. */
+        pDisk->uOffsetStartLocked = 0;
+        pDisk->uOffsetEndLocked = UINT64_C(0xffffffffffffffff);
+
         vdResetModifiedFlag(pDisk);
         rc = pImage->Backend->pfnFlush(pImage->pBackendData, pIoCtx);
@@ -3319 +3363 @@
         size_t   cbDiscardLeft = pIoCtx->Req.Discard.cbDiscardLeft;
         size_t   cbThisDiscard;
+
+        pDisk->uOffsetStartLocked = offStart;
+        pDisk->uOffsetEndLocked = offStart + cbDiscardLeft;
 
         if (RT_UNLIKELY(!pDiscard))