Changeset 27211 in vbox

Timestamp:

Mar 9, 2010 12:41:26 PM (15 years ago)

Author:

vboxsync

Message:

VD: Fix data corruption during snapshot merging if the merge direction is parent -> child. Fix updating of the parent uuid for child -> parent merges if there are still childs after the images to be merged

File:

• trunk/src/VBox/Devices/Storage/VBoxHDD.cpp (modified) (20 diffs)

trunk/src/VBox/Devices/Storage/VBoxHDD.cpp

@@ -300 +300 @@
  * will give us.
  */
-static int vdReadHelper(PVBOXHDD pDisk, PVDIMAGE pImage, uint64_t uOffset,
-                        void *pvBuf, size_t cbRead)
+static int vdReadHelper(PVBOXHDD pDisk, PVDIMAGE pImage, PVDIMAGE pImageParentOverride,
+                        uint64_t uOffset, void *pvBuf, size_t cbRead)
 {
     int rc;
@@ -313 +313 @@
          * stale data when different block sizes are used for the images. */
         cbThisRead = cbRead;
-        rc = VERR_VD_BLOCK_FREE;
-        for (PVDIMAGE pCurrImage = pImage;
-             pCurrImage != NULL && rc == VERR_VD_BLOCK_FREE;
-             pCurrImage = pCurrImage->pPrev)
-        {
-            rc = pCurrImage->Backend->pfnRead(pCurrImage->pvBackendData,
-                                              uOffset, pvBuf, cbThisRead,
-                                              &cbThisRead);
+
+        /*
+         * Try to read from the given image.
+         * If the block is not allocated read from override chain if present.
+         */
+        rc = pImage->Backend->pfnRead(pImage->pvBackendData,
+                                      uOffset, pvBuf, cbThisRead,
+                                      &cbThisRead);
+
+        if (rc == VERR_VD_BLOCK_FREE)
+        {
+            for (PVDIMAGE pCurrImage = pImageParentOverride ? pImageParentOverride : pImage->pPrev;
+                 pCurrImage != NULL && rc == VERR_VD_BLOCK_FREE;
+                 pCurrImage = pCurrImage->pPrev)
+            {
+                rc = pCurrImage->Backend->pfnRead(pCurrImage->pvBackendData,
+                                                  uOffset, pvBuf, cbThisRead,
+                                                  &cbThisRead);
+            }
         }
 
@@ -345 +356 @@
 {
     PVDPARENTSTATEDESC pParentState = (PVDPARENTSTATEDESC)pvUser;
-    return vdReadHelper(pParentState->pDisk, pParentState->pImage, uOffset,
+    return vdReadHelper(pParentState->pDisk, pParentState->pImage, NULL, uOffset,
                         pvBuf, cbRead);
 }
@@ -395 +406 @@
  */
 static int vdWriteHelperStandard(PVBOXHDD pDisk, PVDIMAGE pImage,
+                                 PVDIMAGE pImageParentOverride,
                                  uint64_t uOffset, size_t cbWrite,
                                  size_t cbThisWrite, size_t cbPreRead,
@@ -405 +417 @@
     if (cbPreRead)
     {
-        rc = vdReadHelper(pDisk, pImage, uOffset - cbPreRead, pvTmp,
-                          cbPreRead);
+        rc = vdReadHelper(pDisk, pImage, pImageParentOverride,
+                          uOffset - cbPreRead, pvTmp, cbPreRead);
         if (RT_FAILURE(rc))
             return rc;
@@ -440 +452 @@
                    (char *)pvBuf + cbThisWrite, cbWriteCopy);
         if (cbReadImage)
-            rc = vdReadHelper(pDisk, pImage,
+            rc = vdReadHelper(pDisk, pImage, pImageParentOverride,
                               uOffset + cbThisWrite + cbWriteCopy,
                               (char *)pvTmp + cbPreRead + cbThisWrite + cbWriteCopy,
@@ -472 +484 @@
  */
 static int vdWriteHelperOptimized(PVBOXHDD pDisk, PVDIMAGE pImage,
+                                  PVDIMAGE pImageParentOverride,
                                   uint64_t uOffset, size_t cbWrite,
                                   size_t cbThisWrite, size_t cbPreRead,
@@ -501 +514 @@
     /* Read the entire data of the block so that we can compare whether it will
      * be modified by the write or not. */
-    rc = vdReadHelper(pDisk, pImage, uOffset - cbPreRead, pvTmp,
+    rc = vdReadHelper(pDisk, pImage, pImageParentOverride, uOffset - cbPreRead, pvTmp,
                       cbPreRead + cbThisWrite + cbPostRead - cbFill);
     if (RT_FAILURE(rc))
@@ -548 +561 @@
  * write optimizations.
  */
-static int vdWriteHelper(PVBOXHDD pDisk, PVDIMAGE pImage, uint64_t uOffset,
-                         const void *pvBuf, size_t cbWrite)
+static int vdWriteHelper(PVBOXHDD pDisk, PVDIMAGE pImage, PVDIMAGE pImageParentOverride,
+                         uint64_t uOffset, const void *pvBuf, size_t cbWrite)
 {
     int rc;
@@ -580 +593 @@
                 /* Optimized write, suppress writing to a so far unallocated
                  * block if the data is in fact not changed. */
-                rc = vdWriteHelperOptimized(pDisk, pImage, uOffset, cbWrite,
+                rc = vdWriteHelperOptimized(pDisk, pImage, pImageParentOverride,
+                                            uOffset, cbWrite,
                                             cbThisWrite, cbPreRead, cbPostRead,
                                             pvBuf, pvTmp);
@@ -590 +604 @@
                  * backend has some further optimization enabled) cause the
                  * block to be allocated. */
-                rc = vdWriteHelperStandard(pDisk, pImage, uOffset, cbWrite,
+                rc = vdWriteHelperStandard(pDisk, pImage, pImageParentOverride,
+                                           uOffset, cbWrite,
                                            cbThisWrite, cbPreRead, cbPostRead,
                                            pvBuf, pvTmp);
@@ -1839 +1854 @@
     return rc;
 }
+
 
 /**
@@ -1944 +1960 @@
                         if (RT_FAILURE(rc))
                             break;
-                        rc = vdWriteHelper(pDisk, pImageTo, uOffset, pvBuf,
+                        rc = vdWriteHelper(pDisk, pImageTo, pImageFrom->pPrev,
+                                           uOffset, pvBuf,
                                            cbThisRead);
                         if (RT_FAILURE(rc))
@@ -1970 +1987 @@
         else
         {
+            /*
+             * We may need to update the parent uuid of the child coming after the
+             * last image to be merged. We have to reopen it read/write.
+             *
+             * This is done before we do the actual merge to prevent an incosistent
+             * chain if the mode change fails for some reason.
+             */
+            if (pImageFrom->pNext)
+            {
+                PVDIMAGE pImageChild = pImageFrom->pNext;
+
+                /* We need to open the image in read/write mode. */
+                uOpenFlags = pImageChild->Backend->pfnGetOpenFlags(pImageChild->pvBackendData);
+
+                if (uOpenFlags  & VD_OPEN_FLAGS_READONLY)
+                {
+                    uOpenFlags  &= ~VD_OPEN_FLAGS_READONLY;
+                    rc = pImageChild->Backend->pfnSetOpenFlags(pImageChild->pvBackendData,
+                                                               uOpenFlags);
+                    if (RT_FAILURE(rc))
+                        break;
+                }
+            }
+
             /* Merge child state into parent. This means writing all blocks
              * which are allocated in the image up to the source image to the
@@ -1996 +2037 @@
                     if (RT_FAILURE(rc))
                         break;
-                    rc = vdWriteHelper(pDisk, pImageTo, uOffset, pvBuf,
+                    rc = vdWriteHelper(pDisk, pImageTo, NULL, uOffset, pvBuf,
                                        cbThisRead);
                     if (RT_FAILURE(rc))
@@ -2036 +2077 @@
         else
         {
+            /* Update the parent uuid of the child of the last merged image. */
             if (pImageFrom->pNext)
             {
@@ -2041 +2083 @@
                                                    &Uuid);
                 AssertRC(rc);
+
                 rc = pImageFrom->Backend->pfnSetParentUuid(pImageFrom->pNext->pvBackendData,
                                                            &Uuid);
                 AssertRC(rc);
+
+                /* Reopen readonly if neccessary */
+                if (pImageFrom->pNext != pDisk->pLast)
+                {
+                    PVDIMAGE pImageChild = pImageFrom->pNext;
+
+                    uOpenFlags = pImageChild->Backend->pfnGetOpenFlags(pImageChild->pvBackendData);
+                    uOpenFlags |= VD_OPEN_FLAGS_READONLY;
+                    rc = pImageChild->Backend->pfnSetOpenFlags(pImageChild->pvBackendData,
+                                                               uOpenFlags);
+                    /** @todo: What to do if this fails?. Breaking would
+                     * prevent the merged images from being removed 
+                     * from the chain possibly causing inconsistent behavior.
+                     */
+                    if (RT_FAILURE(rc))
+                        break;
+                }
             }
         }
@@ -2308 +2368 @@
             size_t cbThisRead = RT_MIN(VD_MERGE_BUFFER_SIZE, cbRemaining);
 
-            rc = vdReadHelper(pDiskFrom, pImageFrom, uOffset, pvBuf,
+            rc = vdReadHelper(pDiskFrom, pImageFrom, NULL, uOffset, pvBuf,
                               cbThisRead);
             if (RT_FAILURE(rc))
                 break;
 
-            rc = vdWriteHelper(pDiskTo, pImageTo, uOffset, pvBuf,
+            rc = vdWriteHelper(pDiskTo, pImageTo, NULL, uOffset, pvBuf,
                                cbThisRead);
             if (RT_FAILURE(rc))
@@ -2650 +2710 @@
         AssertPtrBreakStmt(pImage, rc = VERR_VD_NOT_OPENED);
 
-        rc = vdReadHelper(pDisk, pImage, uOffset, pvBuf, cbRead);
+        rc = vdReadHelper(pDisk, pImage, NULL, uOffset, pvBuf, cbRead);
     } while (0);
 
@@ -2697 +2757 @@
 
         vdSetModifiedFlag(pDisk);
-        rc = vdWriteHelper(pDisk, pImage, uOffset, pvBuf, cbWrite);
+        rc = vdWriteHelper(pDisk, pImage, NULL, uOffset, pvBuf, cbWrite);
     } while (0);