Changeset 7900 in vbox for trunk/src/VBox/Devices

Timestamp:

Apr 11, 2008 8:13:26 AM (17 years ago)

Author:

vboxsync

Message:

Fix generic image merging code (make the appropriate images writable during the merge). Prepare for more aggressive redundant write elimination. Fix determining backends. Fix code which was broken by the VINF_VDI_BLOCK_FREE->VERR_VDI_BLOCK_FREE change. Also finally get the modification UUID handling right. Missing code and a really silly typo. Print the backend and the filename in the right place when doing a image dump.

File:

• trunk/src/VBox/Devices/Storage/VBoxHDD-new.cpp (modified) (22 diffs)

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

@@ -343 +343 @@
     {
         /* generate new last-modified uuid */
-        if (!(pDisk->uModified | VD_IMAGE_MODIFIED_DISABLE_UUID_UPDATE))
+        if (!(pDisk->uModified & VD_IMAGE_MODIFIED_DISABLE_UUID_UPDATE))
         {
             RTUUID Uuid;
@@ -453 +453 @@
 /**
  * internal: write a complete block (only used for diff images), taking the
- * remaining data from parent images. This implementation optimized out writes
+ * remaining data from parent images. This implementation optimizes out writes
  * that do not change the data relative to the state as of the parent images.
  * All backends which support differential/growing images support this.
@@ -538 +538 @@
 {
     int rc;
+    unsigned fWrite;
     size_t cbThisWrite;
     size_t cbPreRead, cbPostRead;
@@ -546 +547 @@
         /* Try to write the possibly partial block to the last opened image.
          * This works when the block is already allocated in this image or
-         * if it is a full-block write, which automatically allocates a new
-         * block if needed. */
+         * if it is a full-block write (and allocation isn't suppressed below).
+         * For image formats which don't support zero blocks, it's beneficial
+         * to avoid unnecessarily allocating unchanged blocks. This prevents
+         * unwanted expanding of images. VMDK is an example. */
         cbThisWrite = cbWrite;
+        fWrite =   (pImage->uOpenFlags & VD_OPEN_FLAGS_HONOR_SAME)
+                 ? 0 : VD_WRITE_NO_ALLOC;
         rc = pImage->Backend->pfnWrite(pImage->pvBackendData, uOffset, pvBuf,
                                        cbThisWrite, &cbThisWrite, &cbPreRead,
-                                       &cbPostRead, 0);
+                                       &cbPostRead, fWrite);
         if (rc == VERR_VDI_BLOCK_FREE)
         {
@@ -830 +835 @@
  *          VINF_SUCCESS if a plugin was found. 
  *                       ppszFormat contains the string which can be used as backend name.
- *          VERR_NOT_SUPPORTED if no plugin was found.
+ *          VERR_NOT_SUPPORTED if no backend was found.
  * @param   pszFilename     Name of the image file for which the backend is queried.
  * @param   ppszFormat      Receives pointer of the UTF-8 string which contains the format name.
@@ -873 +878 @@
             }
         }
+        if (fPluginFound)
+            break;
 
         /* Then check if plugin backends support this file format. */
@@ -978 +985 @@
                 break;
         }
+        if (rc == VERR_NO_MORE_FILES)
+            rc = VERR_NOT_SUPPORTED;
+
         RTStrFree(pszPluginFilter);
         if (pPluginDirEntry)
@@ -1103 +1113 @@
         }
 
+        /* Force sane optimization settings. It's not worth avoiding writes
+         * to fixed size images. The overhead would have almost no payback. */
+        if (enmImageType == VD_IMAGE_TYPE_FIXED)
+            pImage->uOpenFlags |= VD_OPEN_FLAGS_HONOR_SAME;
+
         /** @todo optionally check UUIDs */
 
@@ -1160 +1175 @@
             /* Image successfully opened, make it the last image. */
             vdAddImageToList(pDisk, pImage);
+            if (!(uOpenFlags & VD_OPEN_FLAGS_READONLY))
+                pDisk->uModified = VD_IMAGE_MODIFIED_FIRST;
         }
         else
@@ -1296 +1313 @@
         }
 
+        pImage->uOpenFlags = uOpenFlags & VD_OPEN_FLAGS_HONOR_SAME;
         rc = pImage->Backend->pfnCreate(pImage->pszFilename, enmType, cbSize,
                                         uImageFlags, pszComment, pPCHSGeometry,
-                                        pLCHSGeometry, uOpenFlags, pfnProgress,
-                                        pvUser, 0, 99, pDisk->pfnError,
-                                        pDisk->pvErrorUser,
+                                        pLCHSGeometry,
+                                        uOpenFlags & ~VD_OPEN_FLAGS_HONOR_SAME,
+                                        pfnProgress, pvUser, 0, 99,
+                                        pDisk->pfnError, pDisk->pvErrorUser,
                                         &pImage->pvBackendData);
 
         if (VBOX_SUCCESS(rc))
         {
+            /* Force sane optimization settings. It's not worth avoiding writes
+             * to fixed size images. The overhead would have almost no payback. */
+            if (enmType == VD_IMAGE_TYPE_FIXED)
+                pImage->uOpenFlags |= VD_OPEN_FLAGS_HONOR_SAME;
+
             /** @todo optionally check UUIDs */
 
@@ -1351 +1375 @@
             /* Image successfully opened, make it the last image. */
             vdAddImageToList(pDisk, pImage);
+            if (!(uOpenFlags & VD_OPEN_FLAGS_READONLY))
+                pDisk->uModified = VD_IMAGE_MODIFIED_FIRST;
         }
         else
@@ -1456 +1482 @@
         }
 
+        pImage->uOpenFlags = uOpenFlags & VD_OPEN_FLAGS_HONOR_SAME;
         rc = pImage->Backend->pfnCreate(pImage->pszFilename,
                                         VD_IMAGE_TYPE_NORMAL, pDisk->cbSize,
                                         uImageFlags, pszComment,
                                         &pDisk->PCHSGeometry,
-                                        &pDisk->LCHSGeometry, uOpenFlags,
+                                        &pDisk->LCHSGeometry,
+                                        uOpenFlags & ~VD_OPEN_FLAGS_HONOR_SAME,
                                         pfnProgress, pvUser, 0, 99,
                                         pDisk->pfnError, pDisk->pvErrorUser,
                                         &pImage->pvBackendData);
 
+        if (VBOX_SUCCESS(rc) && pDisk->cImages != 0)
+        {
+            /* Switch previous image to read-only mode. */
+            unsigned uOpenFlagsPrevImg;
+            uOpenFlagsPrevImg = pDisk->pLast->Backend->pfnGetOpenFlags(pDisk->pLast->pvBackendData);
+            if (!(uOpenFlagsPrevImg & VD_OPEN_FLAGS_READONLY))
+            {
+                uOpenFlagsPrevImg |= VD_OPEN_FLAGS_READONLY;
+                rc = pDisk->pLast->Backend->pfnSetOpenFlags(pDisk->pLast->pvBackendData, uOpenFlagsPrevImg);
+            }
+        }
+
+        if (VBOX_SUCCESS(rc))
+        {
+            RTUUID Uuid;
+            int rc2;
+
+            rc2 = pDisk->pLast->Backend->pfnGetUuid(pDisk->pLast->pvBackendData,
+                                                    &Uuid);
+            if (VBOX_SUCCESS(rc2))
+                pImage->Backend->pfnSetParentUuid(pImage->pvBackendData, &Uuid);
+            rc2 = pDisk->pLast->Backend->pfnGetModificationUuid(pDisk->pLast->pvBackendData,
+                                                                &Uuid);
+            if (VBOX_SUCCESS(rc2))
+                pImage->Backend->pfnSetParentModificationUuid(pImage->pvBackendData,
+                                                              &Uuid);
+        }
+
         if (VBOX_SUCCESS(rc))
         {
@@ -1474 +1530 @@
             /* Image successfully opened, make it the last image. */
             vdAddImageToList(pDisk, pImage);
+            if (!(uOpenFlags & VD_OPEN_FLAGS_READONLY))
+                pDisk->uModified = VD_IMAGE_MODIFIED_FIRST;
         }
         else
@@ -1543 +1601 @@
         AssertBreak(pImageFrom != pImageTo, rc = VERR_INVALID_PARAMETER);
 
-        /* Check if destination image is writable. */
+        /* Make sure destination image is writable. */
         unsigned uOpenFlags = pImageTo->Backend->pfnGetOpenFlags(pImageTo->pvBackendData);
         if (uOpenFlags & VD_OPEN_FLAGS_READONLY)
         {
-            rc = VERR_VDI_IMAGE_READ_ONLY;
-            break;
+            uOpenFlags &= ~VD_OPEN_FLAGS_READONLY;
+            rc = pImageTo->Backend->pfnSetOpenFlags(pImageTo->pvBackendData,
+                                                    uOpenFlags);
+            if (VBOX_FAILURE(rc))
+                break;
         }
 
@@ -1581 +1642 @@
                                                 uOffset, pvBuf, cbThisRead,
                                                 &cbThisRead);
-                if (VBOX_FAILURE(rc))
-                    break;
                 if (rc == VERR_VDI_BLOCK_FREE)
                 {
@@ -1598 +1657 @@
                                                           &cbThisRead);
                     }
-                    if (VBOX_FAILURE(rc))
-                        break;
 
                     if (rc != VERR_VDI_BLOCK_FREE)
                     {
+                        if (VBOX_FAILURE(rc))
+                            break;
                         rc = vdWriteHelper(pDisk, pImageTo, uOffset, pvBuf,
                                            cbThisRead);
@@ -1608 +1667 @@
                             break;
                     }
+                    else
+                        rc = VINF_SUCCESS;
                 }
+                else if (VBOX_FAILURE(rc))
+                    break;
 
                 uOffset += cbThisRead;
@@ -1624 +1687 @@
             {
                 size_t cbThisRead = RT_MIN(VD_MERGE_BUFFER_SIZE, cbRemaining);
+                rc = VERR_VDI_BLOCK_FREE;
                 /* Search for image with allocated block. Do not attempt to
                  * read more than the previous reads marked as valid. Otherwise
@@ -1636 +1700 @@
                                                       cbThisRead, &cbThisRead);
                 }
-                if (VBOX_FAILURE(rc))
-                    break;
 
                 if (rc != VERR_VDI_BLOCK_FREE)
                 {
+                    if (VBOX_FAILURE(rc))
+                        break;
                     rc = vdWriteHelper(pDisk, pImageTo, uOffset, pvBuf,
                                        cbThisRead);
@@ -1646 +1710 @@
                         break;
                 }
+                else
+                    rc = VINF_SUCCESS;
 
                 uOffset += cbThisRead;
@@ -1679 +1745 @@
                 AssertRC(rc);
             }
+        }
+
+        /* Make sure destination image is back to read only if necessary. */
+        if (pImageTo != pDisk->pLast && pImageFrom != pDisk->pLast)
+        {
+            uOpenFlags = pImageTo->Backend->pfnGetOpenFlags(pImageTo->pvBackendData);
+            uOpenFlags |= VD_OPEN_FLAGS_READONLY;
+            rc = pImageTo->Backend->pfnSetOpenFlags(pImageTo->pvBackendData,
+                                                    uOpenFlags);
+            if (VBOX_FAILURE(rc))
+                break;
         }
 
@@ -3185 +3262 @@
         {
             RTLogPrintf("Dumping VD image \"%s\" (Backend=%s)\n",
-                        pImage->Backend->pszBackendName, pImage->pszFilename);
+                        pImage->pszFilename, pImage->Backend->pszBackendName);
             pImage->Backend->pfnDump(pImage->pvBackendData);
         }