Changeset 28162 in vbox for trunk/src/VBox/Main

Timestamp:

Apr 11, 2010 12:44:21 PM (15 years ago)

Author:

vboxsync

Message:

Main/OVF: isolate disk import code from other import code; clean up progress handling

Location:

trunk/src/VBox/Main

Files:

• ApplianceImpl.cpp (modified) (5 diffs)
• ApplianceImplExport.cpp (modified) (1 diff)
• ApplianceImplImport.cpp (modified) (19 diffs)
• include/ApplianceImpl.h (modified) (4 diffs)
• include/ApplianceImplPrivate.h (modified) (1 diff)

trunk/src/VBox/Main/ApplianceImpl.cpp

@@ -628 +628 @@
 }
 
-void Appliance::disksWeight(uint32_t &ulTotalMB, uint32_t &cDisks) const
-{
-    ulTotalMB = 0;
-    cDisks = 0;
-    /* Weigh the disk images according to their sizes */
+/**
+ * Refreshes the cDisks and ulTotalDisksMB members in the instance data.
+ * Requires that virtual system descriptions are present.
+ */
+void Appliance::disksWeight()
+{
+    m->ulTotalDisksMB = 0;
+    m->cDisks = 0;
+    // weigh the disk images according to their sizes
     list< ComObjPtr<VirtualSystemDescription> >::const_iterator it;
     for (it = m->virtualSystemDescriptions.begin();
@@ -647 +651 @@
         {
             const VirtualSystemDescriptionEntry *pHD = *itH;
-            ulTotalMB += pHD->ulSizeMB;
-            ++cDisks;
+            m->ulTotalDisksMB += pHD->ulSizeMB;
+            ++m->cDisks;
         }
     }
@@ -654 +658 @@
 }
 
-HRESULT Appliance::setUpProgressFS(ComObjPtr<Progress> &pProgress, const Bstr &bstrDescription)
+HRESULT Appliance::setUpProgress(ComObjPtr<Progress> &pProgress,
+                                 const Bstr &bstrDescription,
+                                 SetUpProgressMode mode)
 {
     HRESULT rc;
@@ -661 +667 @@
     pProgress.createObject();
 
-    /* Weigh the disk images according to their sizes */
-    uint32_t ulTotalMB;
-    uint32_t cDisks;
-    disksWeight(ulTotalMB, cDisks);
-
-    ULONG cOperations = 1 + cDisks;     // one op per disk plus 1 for the XML
-
+    // compute the disks weight (this sets ulTotalDisksMB and cDisks in the instance data)
+    disksWeight();
+
+    ULONG cOperations;
     ULONG ulTotalOperationsWeight;
-    if (ulTotalMB)
-    {
-        m->ulWeightPerOperation = (ULONG)((double)ulTotalMB * 1  / 100);    // use 1% of the progress for the XML
-        ulTotalOperationsWeight = ulTotalMB + m->ulWeightPerOperation;
+
+    cOperations =   1               // one for XML setup
+                  + m->cDisks;      // plus one per disk
+    if (m->ulTotalDisksMB)
+    {
+        m->ulWeightPerOperation = (ULONG)((double)m->ulTotalDisksMB * 1 / 100);    // use 1% of the progress for the XML
+        ulTotalOperationsWeight = m->ulTotalDisksMB + m->ulWeightPerOperation;
     }
     else
     {
         // no disks to export:
+        m->ulWeightPerOperation = 1;
         ulTotalOperationsWeight = 1;
-        m->ulWeightPerOperation = 1;
+    }
+
+    switch (mode)
+    {
+        case Regular:
+        break;
+
+        case ImportS3:
+        {
+            cOperations += 1 + 1;     // another one for the manifest file & another one for the import
+            ulTotalOperationsWeight = m->ulTotalDisksMB;
+            if (!m->ulTotalDisksMB)
+                // no disks to export:
+                ulTotalOperationsWeight = 1;
+
+            ULONG ulImportWeight = (ULONG)((double)ulTotalOperationsWeight * 50  / 100);  // use 50% for import
+            ulTotalOperationsWeight += ulImportWeight;
+
+            m->ulWeightPerOperation = ulImportWeight; /* save for using later */
+
+            ULONG ulInitWeight = (ULONG)((double)ulTotalOperationsWeight * 0.1  / 100);  // use 0.1% for init
+            ulTotalOperationsWeight += ulInitWeight;
+        }
+        break;
+
+        case WriteS3:
+        {
+            cOperations += 1 + 1;     // another one for the mf & another one for temporary creation
+
+            if (m->ulTotalDisksMB)
+            {
+                m->ulWeightPerOperation = (ULONG)((double)m->ulTotalDisksMB * 1  / 100);    // use 1% of the progress for OVF file upload (we didn't know the size at this point)
+                ulTotalOperationsWeight = m->ulTotalDisksMB + m->ulWeightPerOperation;
+            }
+            else
+            {
+                // no disks to export:
+                ulTotalOperationsWeight = 1;
+                m->ulWeightPerOperation = 1;
+            }
+            ULONG ulOVFCreationWeight = (ULONG)((double)ulTotalOperationsWeight * 50.0 / 100.0); /* Use 50% for the creation of the OVF & the disks */
+            ulTotalOperationsWeight += ulOVFCreationWeight;
+        }
+        break;
     }
 
     Log(("Setting up progress object: ulTotalMB = %d, cDisks = %d, => cOperations = %d, ulTotalOperationsWeight = %d, m->ulWeightPerOperation = %d\n",
-         ulTotalMB, cDisks, cOperations, ulTotalOperationsWeight, m->ulWeightPerOperation));
+         m->ulTotalDisksMB, m->cDisks, cOperations, ulTotalOperationsWeight, m->ulWeightPerOperation));
 
     rc = pProgress->init(mVirtualBox, static_cast<IAppliance*>(this),
@@ -691 +741 @@
                          bstrDescription, // CBSTR bstrFirstOperationDescription,
                          m->ulWeightPerOperation); // ULONG ulFirstOperationWeight,
-    return rc;
-}
-
-HRESULT Appliance::setUpProgressImportS3(ComObjPtr<Progress> &pProgress, const Bstr &bstrDescription)
-{
-    HRESULT rc;
-
-    /* Create the progress object */
-    pProgress.createObject();
-
-    /* Weigh the disk images according to their sizes */
-    uint32_t ulTotalMB;
-    uint32_t cDisks;
-    disksWeight(ulTotalMB, cDisks);
-
-    ULONG cOperations = 1 + 1 + 1 + cDisks;     // one op per disk plus 1 for init, plus 1 for the manifest file & 1 plus for the import */
-
-    ULONG ulTotalOperationsWeight = ulTotalMB;
-    if (!ulTotalOperationsWeight)
-        // no disks to export:
-        ulTotalOperationsWeight = 1;
-
-    ULONG ulImportWeight = (ULONG)((double)ulTotalOperationsWeight * 50  / 100);  // use 50% for import
-    ulTotalOperationsWeight += ulImportWeight;
-
-    m->ulWeightPerOperation = ulImportWeight; /* save for using later */
-
-    ULONG ulInitWeight = (ULONG)((double)ulTotalOperationsWeight * 0.1  / 100);  // use 0.1% for init
-    ulTotalOperationsWeight += ulInitWeight;
-
-    Log(("Setting up progress object: ulTotalMB = %d, cDisks = %d, => cOperations = %d, ulTotalOperationsWeight = %d, m->ulWeightPerOperation = %d\n",
-         ulTotalMB, cDisks, cOperations, ulTotalOperationsWeight, m->ulWeightPerOperation));
-
-    rc = pProgress->init(mVirtualBox, static_cast<IAppliance*>(this),
-                         bstrDescription,
-                         TRUE /* aCancelable */,
-                         cOperations, // ULONG cOperations,
-                         ulTotalOperationsWeight, // ULONG ulTotalOperationsWeight,
-                         Bstr(tr("Init")), // CBSTR bstrFirstOperationDescription,
-                         ulInitWeight); // ULONG ulFirstOperationWeight,
-    return rc;
-}
-
-HRESULT Appliance::setUpProgressWriteS3(ComObjPtr<Progress> &pProgress, const Bstr &bstrDescription)
-{
-    HRESULT rc;
-
-    /* Create the progress object */
-    pProgress.createObject();
-
-    /* Weigh the disk images according to their sizes */
-    uint32_t ulTotalMB;
-    uint32_t cDisks;
-    disksWeight(ulTotalMB, cDisks);
-
-    ULONG cOperations = 1 + 1 + 1 + cDisks;     // one op per disk plus 1 for the OVF, plus 1 for the mf & 1 plus to the temporary creation */
-
-    ULONG ulTotalOperationsWeight;
-    if (ulTotalMB)
-    {
-        m->ulWeightPerOperation = (ULONG)((double)ulTotalMB * 1  / 100);    // use 1% of the progress for OVF file upload (we didn't know the size at this point)
-        ulTotalOperationsWeight = ulTotalMB + m->ulWeightPerOperation;
-    }
-    else
-    {
-        // no disks to export:
-        ulTotalOperationsWeight = 1;
-        m->ulWeightPerOperation = 1;
-    }
-    ULONG ulOVFCreationWeight = (ULONG)((double)ulTotalOperationsWeight * 50.0 / 100.0); /* Use 50% for the creation of the OVF & the disks */
-    ulTotalOperationsWeight += ulOVFCreationWeight;
-
-    Log(("Setting up progress object: ulTotalMB = %d, cDisks = %d, => cOperations = %d, ulTotalOperationsWeight = %d, m->ulWeightPerOperation = %d\n",
-         ulTotalMB, cDisks, cOperations, ulTotalOperationsWeight, m->ulWeightPerOperation));
-
-    rc = pProgress->init(mVirtualBox, static_cast<IAppliance*>(this),
-                         bstrDescription,
-                         TRUE /* aCancelable */,
-                         cOperations, // ULONG cOperations,
-                         ulTotalOperationsWeight, // ULONG ulTotalOperationsWeight,
-                         bstrDescription, // CBSTR bstrFirstOperationDescription,
-                         ulOVFCreationWeight); // ULONG ulFirstOperationWeight,
     return rc;
 }

trunk/src/VBox/Main/ApplianceImplExport.cpp

@@ -610 +610 @@
                                     aLocInfo.strPath.c_str());
 
-        /* todo: This progress init stuff should be done a little bit more generic */
-        if (aLocInfo.storageType == VFSType_File)
-            rc = setUpProgressFS(aProgress, progressDesc);
-        else
-            rc = setUpProgressWriteS3(aProgress, progressDesc);
+        rc = setUpProgress(aProgress, progressDesc, (aLocInfo.storageType == VFSType_File) ? Regular : WriteS3);
 
         /* Initialize our worker task */

trunk/src/VBox/Main/ApplianceImplImport.cpp

@@ -631 +631 @@
 HRESULT Appliance::readImpl(const LocationInfo &aLocInfo, ComObjPtr<Progress> &aProgress)
 {
-    BstrFmt bstrDesc = BstrFmt(tr("Read appliance '%s'"),
+    BstrFmt bstrDesc = BstrFmt(tr("Reading appliance '%s'"),
                                aLocInfo.strPath.c_str());
     HRESULT rc;
@@ -929 +929 @@
 HRESULT Appliance::importImpl(const LocationInfo &aLocInfo, ComObjPtr<Progress> &aProgress)
 {
-    Bstr progressDesc = BstrFmt(tr("Import appliance '%s'"),
+    Bstr progressDesc = BstrFmt(tr("Importing appliance '%s'"),
                                 aLocInfo.strPath.c_str());
     HRESULT rc = S_OK;
 
-    /* todo: This progress init stuff should be done a little bit more generic */
-    if (aLocInfo.storageType == VFSType_File)
-        rc = setUpProgressFS(aProgress, progressDesc);
-    else
-        rc = setUpProgressImportS3(aProgress, progressDesc);
+    rc = setUpProgress(aProgress,
+                       progressDesc,
+                       (aLocInfo.storageType == VFSType_File) ? Regular : ImportS3);
     if (FAILED(rc)) throw rc;
 
@@ -960 +958 @@
     // input pointers
     const LocationInfo          &locInfo;           // ptr to location info from Appliance::importFS()
+    Utf8Str                     strSourceDir;       // directory where source files reside
     const ovf::DiskImagesMap    &mapDisks;          // ptr to disks map in OVF
     ComObjPtr<Progress>         &pProgress;         // progress object passed into Appliance::importFS()
@@ -981 +980 @@
           fSessionOpen(false)
     {
+        // disk images have to be on the same place as the OVF file. So
+        // strip the filename out of the full file path
+        strSourceDir = aLocInfo.strPath;
+        strSourceDir.stripFilename();
     }
 };
 
 /**
- * Actual worker code for importing OVF data into VirtualBox. This is called from Appliance::taskThreadImportOrExport()
- * and therefore runs on the OVF import worker thread. This runs in two contexts:
- *
- * 1) in a first worker thread; in that case, Appliance::ImportMachines() called Appliance::importImpl();
- *
- * 2) in a second worker thread; in that case, Appliance::ImportMachines() called Appliance::importImpl(), which
- *    called Appliance::importS3(), which called Appliance::importImpl(), which then called this.
- *
- * @param pTask
+ * Checks if a manifest file exists in the given location and, if so, verifies
+ * that the relevant files (the OVF XML and the disks referenced by it, as
+ * represented by the VirtualSystemDescription instances contained in this appliance)
+ * match it. Requires a previous read() and interpret().
+ *
+ * @param locInfo
+ * @param reader
  * @return
  */
-HRESULT Appliance::importFS(const LocationInfo &locInfo,
-                            ComObjPtr<Progress> &pProgress)
+HRESULT Appliance::manifestVerify(const LocationInfo &locInfo,
+                                  const ovf::OVFReader &reader)
 {
-    LogFlowFuncEnter();
-    LogFlowFunc(("Appliance %p\n", this));
-
-    AutoCaller autoCaller(this);
-    if (FAILED(autoCaller.rc())) return autoCaller.rc();
-
-    AutoWriteLock appLock(this COMMA_LOCKVAL_SRC_POS);
-
-    if (!isApplianceIdle())
-        return E_ACCESSDENIED;
-
-    Assert(!pProgress.isNull());
-
-    // Change the appliance state so we can safely leave the lock while doing time-consuming
-    // disk imports; also the below method calls do all kinds of locking which conflicts with
-    // the appliance object lock
-    m->state = Data::ApplianceImporting;
-    appLock.release();
-
     HRESULT rc = S_OK;
 
-    const ovf::OVFReader &reader = *m->pReader;
-    // this is safe to access because this thread only gets started
-    // if pReader != NULL
-
-    // rollback for errors:
-    ImportStack stack(locInfo, reader.m_mapDisks, pProgress);
-    /* If a manifest file exists, verify the content. Therefore we need all
-     * files which are referenced by the OVF & the OVF itself */
     Utf8Str strMfFile = manifestFileName(locInfo.strPath);
-    list<Utf8Str> filesList;
     if (RTPathExists(strMfFile.c_str()))
     {
+        list<Utf8Str> filesList;
         Utf8Str strSrcDir(locInfo.strPath);
         strSrcDir.stripFilename();
-        /* Add every disks of every virtual system to an internal list */
+        // add every disks of every virtual system to an internal list
         list< ComObjPtr<VirtualSystemDescription> >::const_iterator it;
         for (it = m->virtualSystemDescriptions.begin();
@@ -1048 +1022 @@
             {
                 VirtualSystemDescriptionEntry *vsdeHD = *itH;
-                /* Find the disk from the OVF's disk list */
+                // find the disk from the OVF's disk list
                 ovf::DiskImagesMap::const_iterator itDiskImage = reader.m_mapDisks.find(vsdeHD->strRef);
                 const ovf::DiskImage &di = itDiskImage->second;
@@ -1055 +1029 @@
             }
         }
-        /* Create the test list */
-        PRTMANIFESTTEST pTestList = (PRTMANIFESTTEST)RTMemAllocZ(sizeof(RTMANIFESTTEST)*(filesList.size()+1));
+
+        // create the test list
+        PRTMANIFESTTEST pTestList = (PRTMANIFESTTEST)RTMemAllocZ(sizeof(RTMANIFESTTEST) * (filesList.size() + 1));
         pTestList[0].pszTestFile = (char*)locInfo.strPath.c_str();
         pTestList[0].pszTestDigest = (char*)m->strOVFSHA1Digest.c_str();
@@ -1071 +1046 @@
             pTestList[i].pszTestDigest = pszDigest;
         }
+
+        // this call can take a very long time
         size_t cIndexOnError;
-        vrc = RTManifestVerify(strMfFile.c_str(), pTestList, filesList.size() + 1, &cIndexOnError);
-        if (vrc == VERR_MANIFEST_DIGEST_MISMATCH)
-            rc = setError(VBOX_E_FILE_ERROR,
-                          tr("The SHA1 digest of '%s' doesn't match to the one in '%s'"),
-                          RTPathFilename(pTestList[cIndexOnError].pszTestFile),
-                          RTPathFilename(strMfFile.c_str()));
-        else if (RT_FAILURE(vrc))
-            rc = setError(VBOX_E_FILE_ERROR,
-                          tr("Couldn't verify the content of '%s' against the available files (%Rrc)"),
-                          RTPathFilename(strMfFile.c_str()),
-                          vrc);
-        /* Cleanup */
+        vrc = RTManifestVerify(strMfFile.c_str(),
+                               pTestList,
+                               filesList.size() + 1,
+                               &cIndexOnError);
+
+        // clean up
         for (size_t j = 1;
              j < filesList.size();
@@ -1089 +1060 @@
             RTStrFree(pTestList[j].pszTestDigest);
         RTMemFree(pTestList);
-        if (FAILED(rc)) return rc;
-    }
-
-    rc = stack.pSession.createInprocObject(CLSID_Session);
-    if (FAILED(rc)) return rc;
-
-    list<ovf::VirtualSystem>::const_iterator it;
-    list< ComObjPtr<VirtualSystemDescription> >::const_iterator it1;
-    /* Iterate through all virtual systems of that appliance */
-    size_t i = 0;
-    for (it = reader.m_llVirtualSystems.begin(),
-            it1 = m->virtualSystemDescriptions.begin();
-         it != reader.m_llVirtualSystems.end();
-         ++it, ++it1, ++i)
-    {
-        const ovf::VirtualSystem &vsysThis = *it;
-        ComObjPtr<VirtualSystemDescription> vsdescThis = (*it1);
-
-        ComPtr<IMachine> pNewMachine;
-
-        /* Catch possible errors */
-        try
+
+        if (vrc == VERR_MANIFEST_DIGEST_MISMATCH)
+            rc = setError(VBOX_E_FILE_ERROR,
+                          tr("The SHA1 digest of '%s' does not match the one in '%s'"),
+                          RTPathFilename(pTestList[cIndexOnError].pszTestFile),
+                          RTPathFilename(strMfFile.c_str()));
+        else if (RT_FAILURE(vrc))
+            rc = setError(VBOX_E_FILE_ERROR,
+                          tr("Could not verify the content of '%s' against the available files (%Rrc)"),
+                          RTPathFilename(strMfFile.c_str()),
+                          vrc);
+    }
+
+    return rc;
+}
+
+/**
+ * Actual worker code for importing OVF data into VirtualBox. This is called from Appliance::taskThreadImportOrExport()
+ * and therefore runs on the OVF import worker thread. This runs in two contexts:
+ *
+ * 1) in a first worker thread; in that case, Appliance::ImportMachines() called Appliance::importImpl();
+ *
+ * 2) in a second worker thread; in that case, Appliance::ImportMachines() called Appliance::importImpl(), which
+ *    called Appliance::importS3(), which called Appliance::importImpl(), which then called this.
+ *
+ * @param pTask
+ * @return
+ */
+HRESULT Appliance::importFS(const LocationInfo &locInfo,
+                            ComObjPtr<Progress> &pProgress)
+{
+    LogFlowFuncEnter();
+    LogFlowFunc(("Appliance %p\n", this));
+
+    AutoCaller autoCaller(this);
+    if (FAILED(autoCaller.rc())) return autoCaller.rc();
+
+    AutoWriteLock appLock(this COMMA_LOCKVAL_SRC_POS);
+
+    if (!isApplianceIdle())
+        return E_ACCESSDENIED;
+
+    Assert(!pProgress.isNull());
+
+    // Change the appliance state so we can safely leave the lock while doing time-consuming
+    // disk imports; also the below method calls do all kinds of locking which conflicts with
+    // the appliance object lock
+    m->state = Data::ApplianceImporting;
+    appLock.release();
+
+    HRESULT rc = S_OK;
+
+    const ovf::OVFReader &reader = *m->pReader;
+    // this is safe to access because this thread only gets started
+    // if pReader != NULL
+
+    // rollback for errors:
+    ImportStack stack(locInfo, reader.m_mapDisks, pProgress);
+
+    try
+    {
+        // if a manifest file exists, verify the content; we then need all files which are referenced by the OVF & the OVF itself
+        rc = manifestVerify(locInfo, reader);
+        if (FAILED(rc)) throw rc;
+
+        // create a session for the machine + disks we manipulate below
+        rc = stack.pSession.createInprocObject(CLSID_Session);
+        if (FAILED(rc)) throw rc;
+
+        list<ovf::VirtualSystem>::const_iterator it;
+        list< ComObjPtr<VirtualSystemDescription> >::const_iterator it1;
+        /* Iterate through all virtual systems of that appliance */
+        size_t i = 0;
+        for (it = reader.m_llVirtualSystems.begin(),
+                it1 = m->virtualSystemDescriptions.begin();
+             it != reader.m_llVirtualSystems.end();
+             ++it, ++it1, ++i)
         {
+            const ovf::VirtualSystem &vsysThis = *it;
+            ComObjPtr<VirtualSystemDescription> vsdescThis = (*it1);
+
+            ComPtr<IMachine> pNewMachine;
+
             // there are two ways in which we can create a vbox machine from OVF:
             // -- either this OVF was written by vbox 3.2 or later, in which case there is a <vbox:Machine> element
@@ -1122 +1153 @@
 
             if (vsdescThis->m->pConfig)
-                importVBoxMachine(vsdescThis, pNewMachine, stack);
+                importVBoxMachine(vsysThis, vsdescThis, pNewMachine, stack);
             else
                 importMachineGeneric(vsysThis, vsdescThis, pNewMachine, stack);
-        }
-        catch (HRESULT aRC)
-        {
-            rc = aRC;
-        }
-
-        if (FAILED(rc))
-            break;
-
-    } // for (it = pAppliance->m->llVirtualSystems.begin(),
+
+        } // for (it = pAppliance->m->llVirtualSystems.begin() ...
+    }
+    catch (HRESULT rc2)
+    {
+        rc = rc2;
+    }
 
     if (FAILED(rc))
@@ -1199 +1227 @@
 
     return rc;
+}
+
+/**
+ *
+ * @param vsysThis OVF virtual system containing the VirtualDisksMap. We don't pass that directly
+ *                 because it's a typedef which requires the whole ovfreader.h to be included otherwise.
+ * @param vsdeHD
+ * @param dstHdVBox
+ * @param stack
+ */
+void Appliance::importOneDiskImage(const ovf::DiskImage &di,
+                                   uint32_t ulSizeMB,
+                                   const Utf8Str &strTargetPath,
+                                   ComPtr<IMedium> &pTargetHD,
+                                   ImportStack &stack)
+{
+    ComPtr<IMedium> pSourceHD;
+    bool fSourceHdNeedsClosing = false;
+
+    try
+    {
+        // destination file must not exist
+        if (    strTargetPath.isEmpty()
+             || RTPathExists(strTargetPath.c_str())
+           )
+            /* This isn't allowed */
+            throw setError(VBOX_E_FILE_ERROR,
+                           tr("Destination file '%s' exists"),
+                           strTargetPath.c_str());
+
+        const Utf8Str &strSourceOVF = di.strHref;
+
+        // Make sure target directory exists
+        HRESULT rc = VirtualBox::ensureFilePathExists(strTargetPath.c_str());
+        if (FAILED(rc))
+            throw rc;
+
+        // subprogress object for hard disk
+        ComPtr<IProgress> pProgress2;
+
+        /* If strHref is empty we have to create a new file */
+        if (strSourceOVF.isEmpty())
+        {
+            // which format to use?
+            Bstr srcFormat = L"VDI";
+            if (   di.strFormat.compare("http://www.vmware.com/specifications/vmdk.html#sparse", Utf8Str::CaseInsensitive)
+                || di.strFormat.compare("http://www.vmware.com/specifications/vmdk.html#compressed", Utf8Str::CaseInsensitive)
+            )
+                srcFormat = L"VMDK";
+            // create an empty hard disk
+            rc = mVirtualBox->CreateHardDisk(srcFormat, Bstr(strTargetPath), pTargetHD.asOutParam());
+            if (FAILED(rc)) throw rc;
+
+            // create a dynamic growing disk image with the given capacity
+            rc = pTargetHD->CreateBaseStorage(di.iCapacity / _1M, MediumVariant_Standard, pProgress2.asOutParam());
+            if (FAILED(rc)) throw rc;
+
+            // advance to the next operation
+            stack.pProgress->SetNextOperation(BstrFmt(tr("Creating virtual disk image '%s'"), strTargetPath.c_str()),
+                                              ulSizeMB);     // operation's weight, as set up with the IProgress originally
+        }
+        else
+        {
+            // construct source file path
+            Utf8StrFmt strSrcFilePath("%s%c%s", stack.strSourceDir.c_str(), RTPATH_DELIMITER, strSourceOVF.c_str());
+            // source path must exist
+            if (!RTPathExists(strSrcFilePath.c_str()))
+                throw setError(VBOX_E_FILE_ERROR,
+                               tr("Source virtual disk image file '%s' doesn't exist"),
+                               strSrcFilePath.c_str());
+
+            // Clone the disk image (this is necessary cause the id has
+            // to be recreated for the case the same hard disk is
+            // attached already from a previous import)
+
+            // First open the existing disk image
+            rc = mVirtualBox->OpenHardDisk(Bstr(strSrcFilePath),
+                                        AccessMode_ReadOnly,
+                                        false,
+                                        NULL,
+                                        false,
+                                        NULL,
+                                        pSourceHD.asOutParam());
+            if (FAILED(rc)) throw rc;
+            fSourceHdNeedsClosing = true;
+
+            /* We need the format description of the source disk image */
+            Bstr srcFormat;
+            rc = pSourceHD->COMGETTER(Format)(srcFormat.asOutParam());
+            if (FAILED(rc)) throw rc;
+            /* Create a new hard disk interface for the destination disk image */
+            rc = mVirtualBox->CreateHardDisk(srcFormat, Bstr(strTargetPath), pTargetHD.asOutParam());
+            if (FAILED(rc)) throw rc;
+            /* Clone the source disk image */
+            rc = pSourceHD->CloneTo(pTargetHD, MediumVariant_Standard, NULL, pProgress2.asOutParam());
+            if (FAILED(rc)) throw rc;
+
+            /* Advance to the next operation */
+            stack.pProgress->SetNextOperation(BstrFmt(tr("Importing virtual disk image '%s'"), strSrcFilePath.c_str()),
+                                              ulSizeMB);     // operation's weight, as set up with the IProgress originally);
+        }
+
+        // now wait for the background disk operation to complete; this throws HRESULTs on error
+        waitForAsyncProgress(stack.pProgress, pProgress2);
+
+        if (fSourceHdNeedsClosing)
+        {
+            rc = pSourceHD->Close();
+            if (FAILED(rc)) throw rc;
+            fSourceHdNeedsClosing = false;
+        }
+
+        stack.llHardDisksCreated.push_back(pTargetHD);
+    }
+    catch (...)
+    {
+        if (fSourceHdNeedsClosing)
+            pSourceHD->Close();
+
+        throw;
+    }
 }
 
@@ -1610 +1759 @@
                     {
                         pController = &kt->second;
+                        break;
                     }
                 }
@@ -1615 +1765 @@
                 if (!pController)
                     throw setError(VBOX_E_FILE_ERROR,
-                                    tr("OVF wants a CD-ROM drive but cannot find IDE controller, which is required in this version of VirtualBox"));
+                                   tr("OVF wants a CD-ROM drive but cannot find IDE controller, which is required in this version of VirtualBox"));
 
                 // this is for rollback later
@@ -1663 +1813 @@
         // If there's an error here we need to close the session, so
         // we need another try/catch block.
-        ComPtr<IMedium> srcHdVBox;
-        bool fSourceHdNeedsClosing = false;
-
         try
         {
@@ -1673 +1820 @@
             if (FAILED(rc)) throw rc;
             stack.fSessionOpen = true;
-
-            /* The disk image has to be on the same place as the OVF file. So
-             * strip the filename out of the full file path. */
-            Utf8Str strSrcDir(stack.locInfo.strPath);
-            strSrcDir.stripFilename();
 
             /* Iterate over all given disk images */
@@ -1687 +1829 @@
                 VirtualSystemDescriptionEntry *vsdeHD = *itHD;
 
-                /* Check if the destination file exists already or the
-                    * destination path is empty. */
-                if (    vsdeHD->strVbox.isEmpty()
-                     || RTPathExists(vsdeHD->strVbox.c_str())
-                   )
-                    /* This isn't allowed */
-                    throw setError(VBOX_E_FILE_ERROR,
-                                   tr("Destination file '%s' exists"),
-                                   vsdeHD->strVbox.c_str());
-
-                /* Find the disk from the OVF's disk list */
-                ovf::DiskImagesMap::const_iterator itDiskImage = stack.mapDisks.find(vsdeHD->strRef);
                 // vsdeHD->strRef contains the disk identifier (e.g. "vmdisk1"), which should exist
                 // in the virtual system's disks map under that ID and also in the global images map
                 ovf::VirtualDisksMap::const_iterator itVirtualDisk = vsysThis.mapVirtualDisks.find(vsdeHD->strRef);
-
-                if (    itDiskImage == stack.mapDisks.end()
-                     || itVirtualDisk == vsysThis.mapVirtualDisks.end()
+                // and find the disk from the OVF's disk list
+                ovf::DiskImagesMap::const_iterator itDiskImage = stack.mapDisks.find(vsdeHD->strRef);
+                if (    (itVirtualDisk == vsysThis.mapVirtualDisks.end())
+                     || (itDiskImage == stack.mapDisks.end())
                    )
                     throw setError(E_FAIL,
@@ -1712 +1843 @@
                 const ovf::VirtualDisk &vd = itVirtualDisk->second;
 
-                /* Make sure all target directories exists */
-                rc = VirtualBox::ensureFilePathExists(vsdeHD->strVbox.c_str());
-                if (FAILED(rc))
-                    throw rc;
-
-                // subprogress object for hard disk
-                ComPtr<IProgress> pProgress2;
-
-                ComPtr<IMedium> dstHdVBox;
-                /* If strHref is empty we have to create a new file */
-                if (di.strHref.isEmpty())
-                {
-                    /* Which format to use? */
-                    Bstr srcFormat = L"VDI";
-                    if (   di.strFormat.compare("http://www.vmware.com/specifications/vmdk.html#sparse", Utf8Str::CaseInsensitive)
-                        || di.strFormat.compare("http://www.vmware.com/specifications/vmdk.html#compressed", Utf8Str::CaseInsensitive))
-                        srcFormat = L"VMDK";
-                    /* Create an empty hard disk */
-                    rc = mVirtualBox->CreateHardDisk(srcFormat, Bstr(vsdeHD->strVbox), dstHdVBox.asOutParam());
-                    if (FAILED(rc)) throw rc;
-
-                    /* Create a dynamic growing disk image with the given capacity */
-                    rc = dstHdVBox->CreateBaseStorage(di.iCapacity / _1M, MediumVariant_Standard, pProgress2.asOutParam());
-                    if (FAILED(rc)) throw rc;
-
-                    /* Advance to the next operation */
-                    stack.pProgress->SetNextOperation(BstrFmt(tr("Creating virtual disk image '%s'"), vsdeHD->strVbox.c_str()),
-                                                      vsdeHD->ulSizeMB);     // operation's weight, as set up with the IProgress originally
-                }
-                else
-                {
-                    /* Construct the source file path */
-                    Utf8StrFmt strSrcFilePath("%s%c%s", strSrcDir.c_str(), RTPATH_DELIMITER, di.strHref.c_str());
-                    /* Check if the source file exists */
-                    if (!RTPathExists(strSrcFilePath.c_str()))
-                        /* This isn't allowed */
-                        throw setError(VBOX_E_FILE_ERROR,
-                                        tr("Source virtual disk image file '%s' doesn't exist"),
-                                            strSrcFilePath.c_str());
-
-                    // Clone the disk image (this is necessary cause the id has
-                    // to be recreated for the case the same hard disk is
-                    // attached already from a previous import)
-
-                    // First open the existing disk image
-                    rc = mVirtualBox->OpenHardDisk(Bstr(strSrcFilePath),
-                                                    AccessMode_ReadOnly,
-                                                    false,
-                                                    NULL,
-                                                    false,
-                                                    NULL,
-                                                    srcHdVBox.asOutParam());
-                    if (FAILED(rc)) throw rc;
-                    fSourceHdNeedsClosing = true;
-
-                    /* We need the format description of the source disk image */
-                    Bstr srcFormat;
-                    rc = srcHdVBox->COMGETTER(Format)(srcFormat.asOutParam());
-                    if (FAILED(rc)) throw rc;
-                    /* Create a new hard disk interface for the destination disk image */
-                    rc = mVirtualBox->CreateHardDisk(srcFormat, Bstr(vsdeHD->strVbox), dstHdVBox.asOutParam());
-                    if (FAILED(rc)) throw rc;
-                    /* Clone the source disk image */
-                    rc = srcHdVBox->CloneTo(dstHdVBox, MediumVariant_Standard, NULL, pProgress2.asOutParam());
-                    if (FAILED(rc)) throw rc;
-
-                    /* Advance to the next operation */
-                    stack.pProgress->SetNextOperation(BstrFmt(tr("Importing virtual disk image '%s'"), strSrcFilePath.c_str()),
-                                                      vsdeHD->ulSizeMB);     // operation's weight, as set up with the IProgress originally);
-                }
-
-                // now wait for the background disk operation to complete; this throws HRESULTs on error
-                waitForAsyncProgress(stack.pProgress, pProgress2);
-
-                if (fSourceHdNeedsClosing)
-                {
-                    rc = srcHdVBox->Close();
-                    if (FAILED(rc)) throw rc;
-                    fSourceHdNeedsClosing = false;
-                }
-
-                stack.llHardDisksCreated.push_back(dstHdVBox);
+                ComPtr<IMedium> pTargetHD;
+                importOneDiskImage(di,
+                                   vsdeHD->ulSizeMB,
+                                   vsdeHD->strVbox,
+                                   pTargetHD,
+                                   stack);
+
                 /* Now use the new uuid to attach the disk image to our new machine */
                 ComPtr<IMachine> sMachine;
@@ -1799 +1855 @@
                 if (FAILED(rc)) throw rc;
                 Bstr hdId;
-                rc = dstHdVBox->COMGETTER(Id)(hdId.asOutParam());
+                rc = pTargetHD->COMGETTER(Id)(hdId.asOutParam());
                 if (FAILED(rc)) throw rc;
 
@@ -1838 +1894 @@
         catch(HRESULT /* aRC */)
         {
-            if (fSourceHdNeedsClosing)
-                srcHdVBox->Close();
-
             if (stack.fSessionOpen)
                 stack.pSession->Close();
@@ -1861 +1914 @@
  * @param stack
  */
-void Appliance::importVBoxMachine(ComObjPtr<VirtualSystemDescription> &vsdescThis,
+void Appliance::importVBoxMachine(const ovf::VirtualSystem &vsysThis,
+                                  ComObjPtr<VirtualSystemDescription> &vsdescThis,
                                   ComPtr<IMachine> &pReturnNewMachine,
                                   ImportStack &stack)

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

@@ -31 +31 @@
 class Progress;
 class VirtualSystemDescription;
+struct VirtualSystemDescriptionEntry;
 
 namespace ovf
@@ -37 +38 @@
     struct VirtualSystem;
     class OVFReader;
+    struct DiskImage;
 }
 
@@ -120 +122 @@
     void addWarning(const char* aWarning, ...);
 
-    void disksWeight(uint32_t &ulTotalMB, uint32_t &cDisks) const;
-    HRESULT setUpProgressFS(ComObjPtr<Progress> &pProgress, const Bstr &bstrDescription);
-    HRESULT setUpProgressImportS3(ComObjPtr<Progress> &pProgress, const Bstr &bstrDescription);
-    HRESULT setUpProgressWriteS3(ComObjPtr<Progress> &pProgress, const Bstr &bstrDescription);
+    void disksWeight();
+    enum SetUpProgressMode { Regular, ImportS3, WriteS3 };
+    HRESULT setUpProgress(ComObjPtr<Progress> &pProgress,
+                          const Bstr &bstrDescription,
+                          SetUpProgressMode mode);
 
     struct LocationInfo;
@@ -145 +148 @@
 
     HRESULT importImpl(const LocationInfo &aLocInfo, ComObjPtr<Progress> &aProgress);
+    HRESULT manifestVerify(const LocationInfo &locInfo, const ovf::OVFReader &reader);
 
     HRESULT importFS(const LocationInfo &locInfo, ComObjPtr<Progress> &aProgress);
+
     struct ImportStack;
-    void importVBoxMachine(ComObjPtr<VirtualSystemDescription> &vsdescThis,
-                           ComPtr<IMachine> &pNewMachine,
-                           ImportStack &stack);
+    void importOneDiskImage(const ovf::DiskImage &di,
+                            uint32_t ulSizeMB,
+                            const Utf8Str &strTargetPath,
+                            ComPtr<IMedium> &pTargetHD,
+                            ImportStack &stack);
     void importMachineGeneric(const ovf::VirtualSystem &vsysThis,
                               ComObjPtr<VirtualSystemDescription> &vsdescThis,
                               ComPtr<IMachine> &pNewMachine,
                               ImportStack &stack);
+    void importVBoxMachine(const ovf::VirtualSystem &vsysThis,
+                           ComObjPtr<VirtualSystemDescription> &vsdescThis,
+                           ComPtr<IMachine> &pNewMachine,
+                           ImportStack &stack);
+
     HRESULT importS3(TaskOVF *pTask);
 

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

@@ -80 +80 @@
 
     ULONG           ulWeightPerOperation;
+    ULONG           ulTotalDisksMB;
+    ULONG           cDisks;
     Utf8Str         strOVFSHA1Digest;
 };