Changeset 3317 in vbox for trunk/src

Timestamp:

Jun 27, 2007 2:45:20 PM (18 years ago)

Author:

vboxsync

Message:

Main: Improved error handling when opening hard disks using the generic IVirtualBox::openHardDisk() call (#2017).

Location:

trunk/src/VBox/Main

Files:

• HardDiskImpl.cpp (modified) (18 diffs)
• include/HardDiskImpl.h (modified) (4 diffs)

trunk/src/VBox/Main/HardDiskImpl.cpp

@@ -33 +33 @@
 #include <iprt/cpputils.h>
 #include <VBox/VBoxHDD.h>
-#include <VBox/VBoxHDD-new.h>
 #include <VBox/err.h>
 
@@ -485 +484 @@
 
     /* create the hard disk creation thread, pass operation data */
-    int vrc = RTThreadCreate (NULL, HVirtualDiskImage::vdiTaskThread,
+    int vrc = RTThreadCreate (NULL, HVirtualDiskImage::VDITaskThread,
                               (void *) task, 0, RTTHREADTYPE_MAIN_HEAVY_WORKER,
                               0, "VDITask");
@@ -1062 +1061 @@
     AssertReturn (*aLocation, E_INVALIDARG);
 
-    HRESULT rc = S_OK;
+    static const struct
+    {
+        HardDiskStorageType_T type;
+        const char *ext;
+    }
+    storageTypes[] =
+    {
+        { HardDiskStorageType_VMDKImage, ".vmdk" },
+        { HardDiskStorageType_VirtualDiskImage, ".vdi" },
+    };
 
     /* try to guess the probe order by extension */
+    size_t first = -1;
     Utf8Str loc = aLocation;
     char *ext = RTPathExt (loc);
 
-    HardDiskStorageType_T order [2];
-    size_t cOrder = ELEMENTS (order);
-
-    if (RTPathCompare (ext, ".vmdk") == 0)
-    {
-        /// @todo This is a hack. The proper solution would be to save the
-        /// error info from the first try and restore that if everything fails.
-        /// That way a non-working VMDK will get a proper VMDK diagnostics
-        /// message and not some incomprehensible VDI error.
-        order [0] = HardDiskStorageType_VMDKImage;
-        cOrder = 1;
-    }
-    else
-    {
-        order [0] = HardDiskStorageType_VirtualDiskImage;
-        order [1] = HardDiskStorageType_VMDKImage;
-    }
-
-    for (size_t i = 0; i < cOrder; ++ i)
-    {
-        switch (order [i])
+    for (size_t i = 0; i < ELEMENTS (storageTypes); ++ i)
+    {
+        if (RTPathCompare (ext, storageTypes [i].ext) == 0)
+        {
+            first = i;
+            break;
+        }
+    }
+
+    HRESULT rc = S_OK;
+
+    HRESULT firstRC = S_OK;
+    com::ErrorInfoKeeper firstErr (true /* aIsNull */);
+
+    for (size_t i = 0; i < ELEMENTS (storageTypes); ++ i)
+    {
+        size_t j = first == -1 ? i : i == 0 ? first : i == first ? 0 : i;
+        switch (storageTypes [j].type)
         {
             case HardDiskStorageType_VirtualDiskImage:
@@ -1118 +1124 @@
             default:
             {
-                ComAssertComRCRetRC (E_FAIL);
+                AssertComRCReturnRC (E_FAIL);
             }
         }
-    }
-
-    return rc;
+
+        Assert (FAILED (rc));
+
+        /* remember the first encountered error */
+        if (j == first)
+        {
+            firstRC = rc;
+            firstErr.fetch();
+        }
+    }
+
+    if (first != -1)
+    {
+        Assert (FAILED (firstRC));
+        /* firstErr will restore the error info upon destruction */
+        return firstRC;
+    }
+
+    /* There was no exact extension match; chances are high that an error we
+     * got after probing is useless. Use a generic error message instead. */
+
+    firstErr.forget();
+
+    return setError (E_FAIL,
+        tr ("Could not recognize the format of the hard disk '%ls'. "
+            "Either the given format is not supported or hard disk data "
+            "is corrupt"),
+        aLocation);
 }
 
@@ -2599 +2630 @@
 
     /* create the hard disk creation thread, pass operation data */
-    vrc = RTThreadCreate (NULL, vdiTaskThread, (void *) task, 0,
+    vrc = RTThreadCreate (NULL, VDITaskThread, (void *) task, 0,
                           RTTHREADTYPE_MAIN_HEAVY_WORKER, 0, "VDITask");
     ComAssertMsgRC (vrc, ("Could not create a thread (%Vrc)", vrc));
@@ -2618 +2649 @@
 
 /* static */
-DECLCALLBACK(int) HVirtualDiskImage::vdiTaskThread (RTTHREAD thread, void *pvUser)
+DECLCALLBACK(int) HVirtualDiskImage::VDITaskThread (RTTHREAD thread, void *pvUser)
 {
     VDITask *task = static_cast <VDITask *> (pvUser);
     AssertReturn (task, VERR_GENERAL_FAILURE);
 
-    LogFlow (("vdiTaskThread(): operation=%d, size=%llu\n",
-              task->operation, task->size));
+    LogFlowFunc (("operation=%d, size=%llu\n", task->operation, task->size));
 
     VDIIMAGETYPE type = (VDIIMAGETYPE) 0;
@@ -2675 +2705 @@
     }
 
-    LogFlow (("vdiTaskThread(): rc=%08X\n", rc));
+    LogFlowFunc (("rc=%08X\n", rc));
 
     AutoLock alock (task->vdi);
@@ -3313 +3343 @@
     mActualSize = 0;
 
+    /* initialize the container */
+    int vrc = VDCreate ("VMDK", VDError, this, &mContainer);
+    ComAssertRCRet (vrc, E_FAIL);
+
     return S_OK;
 }
@@ -3318 +3352 @@
 void HVMDKImage::FinalRelease()
 {
+    if (mContainer != NULL)
+        VDDestroy (mContainer);
+
     HardDisk::FinalRelease();
 }
@@ -3918 +3955 @@
 }
 
-DECLCALLBACK(void) VDError (void *pvUser, int rc, RT_SRC_POS_DECL,
-                            const char *pszFormat, va_list va)
-{
-    /// @todo pass the error message to the operation initiator
-}
-
 /**
  *  Helper to query information about the VDI hard disk.
@@ -3963 +3994 @@
     Bstr errMsg;
 
-    PVBOXHDD hdd = NULL;
+    /* reset any previous error report from VDError() */
+    mLastVDError.setNull();
 
     do
     {
         Guid id, parentId;
-
-        /// @todo make PVBOXHDD a member variable and init/destroy it upon
-        /// image creation/deletion instead of doing that here every time.
-
-        vrc = VDCreate ("VMDK", VDError, NULL, &hdd);
-        if (VBOX_FAILURE (vrc))
-            break;
 
         /// @todo changed from VD_OPEN_FLAGS_READONLY to VD_OPEN_FLAGS_NORMAL,
@@ -3981 +4006 @@
         /// obviously. This of course changes locking behavior, but for now
         /// this is acceptable. A better solution needs to be found later.
-        vrc = VDOpen (hdd, filePath, VD_OPEN_FLAGS_NORMAL);
+        vrc = VDOpen (mContainer, filePath, VD_OPEN_FLAGS_NORMAL);
         if (VBOX_FAILURE (vrc))
             break;
 
-        vrc = VDGetUuid (hdd, 0, id.ptr());
+        vrc = VDGetUuid (mContainer, 0, id.ptr());
         if (VBOX_FAILURE (vrc))
             break;
-        vrc = VDGetParentUuid (hdd, 0, parentId.ptr());
+        vrc = VDGetParentUuid (mContainer, 0, parentId.ptr());
         if (VBOX_FAILURE (vrc))
             break;
@@ -4055 +4080 @@
         if (!mParent)
         {
-            uint64_t size = VDGetSize (hdd);
+            uint64_t size = VDGetSize (mContainer);
             /* convert to MBytes */
             mSize = size / 1024 / 1024;
@@ -4062 +4087 @@
     while (0);
 
-    if (hdd)
-        VDDestroy (hdd);
+    VDCloseAll (mContainer);
 
     /* enter the lock again */
@@ -4081 +4105 @@
             if (!errMsg.isNull())
                 *aAccessError = errMsg;
+            else if (!mLastVDError.isNull())
+                *aAccessError = mLastVDError;
             else if (VBOX_FAILURE (vrc))
                 *aAccessError = Utf8StrFmt (
@@ -4110 +4136 @@
     }
 
+    /* cleanup the last error report from VDError() */
+    mLastVDError.setNull();
+
     return rc;
 }
@@ -4131 +4160 @@
 
 /* static */
-DECLCALLBACK(int) HVMDKImage::vdiTaskThread (RTTHREAD thread, void *pvUser)
+DECLCALLBACK(int) HVMDKImage::VDITaskThread (RTTHREAD thread, void *pvUser)
 {
     AssertMsgFailed (("Not implemented"));
@@ -4137 +4166 @@
 
 /// @todo (r=dmik) later
-//  Use code from HVirtualDiskImage::vdiTaskThread as an example.
-}
+//  Use code from HVirtualDiskImage::VDITaskThread as an example.
+}
+
+/* static */
+DECLCALLBACK(void) HVMDKImage::VDError (void *pvUser, int rc, RT_SRC_POS_DECL,
+                                        const char *pszFormat, va_list va)
+{
+    HVMDKImage *that = static_cast <HVMDKImage *> (pvUser);
+    AssertReturnVoid (that != NULL);
+
+    /// @todo pass the error message to the operation initiator
+    Utf8Str err = Utf8StrFmt (pszFormat, va);
+    if (VBOX_FAILURE (rc))
+        err = Utf8StrFmt ("%s (%Vrc)", err.raw(), rc);
+
+    if (that->mLastVDError.isNull())
+        that->mLastVDError = err;
+    else
+        that->mLastVDError = Utf8StrFmt
+            ("%s.\n%s", that->mLastVDError.raw(), err.raw());
+}
+

trunk/src/VBox/Main/include/HardDiskImpl.h

@@ -27 +27 @@
 
 #include <VBox/cfgldr.h>
+#include <VBox/VBoxHDD-new.h>
 
 #include <iprt/semaphore.h>
@@ -296 +297 @@
 
     /** VDI asynchronous operation thread function */
-    static DECLCALLBACK(int) vdiTaskThread (RTTHREAD thread, void *pvUser);
+    static DECLCALLBACK(int) VDITaskThread (RTTHREAD thread, void *pvUser);
 
     enum State
@@ -508 +509 @@
 
     /** VDI asynchronous operation thread function */
-    static DECLCALLBACK(int) vdiTaskThread (RTTHREAD thread, void *pvUser);
+    static DECLCALLBACK(int) VDITaskThread (RTTHREAD thread, void *pvUser);
+
+    static DECLCALLBACK(void) VDError (void *pvUser, int rc, RT_SRC_POS_DECL,
+                                       const char *pszFormat, va_list va);
 
     enum State
@@ -531 +535 @@
     Bstr mFilePathFull;
 
+    PVBOXHDD mContainer;
+
+    Utf8Str mLastVDError;
+
     friend class HardDisk;
 };