Changeset 31333 in vbox

Timestamp:

Aug 3, 2010 1:00:54 PM (15 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

64371

Message:

Main: rework new implementation of Machine::Unregister() and Machine::Delete() to be more flexible and still easy to use

Location:

trunk/src/VBox

Files:

• Frontends/VBoxManage/VBoxManageMisc.cpp (modified) (1 diff)
• Frontends/VirtualBox/src/selector/VBoxSelectorWnd.cpp (modified) (1 diff)
• Frontends/VirtualBox/src/wizards/newvm/UINewVMWzd.cpp (modified) (1 diff)
• Main/ApplianceImplImport.cpp (modified) (2 diffs)
• Main/MachineImpl.cpp (modified) (13 diffs)
• Main/MediumImpl.cpp (modified) (3 diffs)
• Main/ProgressImpl.cpp (modified) (2 diffs)
• Main/SnapshotImpl.cpp (modified) (3 diffs)
• Main/idl/VirtualBox.xidl (modified) (9 diffs)
• Main/include/MachineImpl.h (modified) (4 diffs)
• Main/include/SnapshotImpl.h (modified) (1 diff)
• Main/testcase/tstOVF.cpp (modified) (1 diff)
• Main/webservice/websrv-cpp.xsl (modified) (1 diff)

trunk/src/VBox/Frontends/VBoxManage/VBoxManageMisc.cpp

@@ -159 +159 @@
     if (machine)
     {
-        SafeArray<BSTR> abstrFiles;
-        CHECK_ERROR(machine, Unregister(fDelete /* fAutoCleanup */,
-                                        ComSafeArrayAsOutParam(abstrFiles)));
+        SafeIfaceArray<IMedium> aMedia;
+        CleanupMode_T cleanupMode = CleanupMode_DetachAllReturnNone;
+        if (fDelete)
+            cleanupMode = CleanupMode_DetachAllReturnHardDisksOnly;
+        CHECK_ERROR(machine, Unregister(cleanupMode,
+                                        ComSafeArrayAsOutParam(aMedia)));
         if (SUCCEEDED(rc))
         {
-            for (size_t u = 0;
-                 u < abstrFiles.size();
-                 ++u)
-            {
-                Utf8Str strFile(abstrFiles[u]);
-                if (fDelete)
-                {
-                    RTPrintf("Deleting '%s'\n", strFile.c_str());
-                    RTFileDelete(strFile.c_str());
-                }
-                else
-                    RTPrintf("File '%s' is now obsolete and can be deleted\n", strFile.c_str());
-            }
-
             if (fDelete)
             {
-                CHECK_ERROR(machine, Delete());
+                ComPtr<IProgress> pProgress;
+                CHECK_ERROR(machine, Delete(ComSafeArrayAsInParam(aMedia), pProgress.asOutParam()));
+                CHECK_ERROR(pProgress, WaitForCompletion(-1));
             }
         }

trunk/src/VBox/Frontends/VirtualBox/src/selector/VBoxSelectorWnd.cpp

@@ -680 +680 @@
         {
             CMachine machine = item->machine();
-            QVector<QString> files = machine.Unregister(false /*fDetachMedia*/);
+            QVector<CMedium> aMedia = machine.Unregister(KCleanupMode_UnregisterOnly);          //  @todo replace with DetachAllReturnHardDisksOnly once a progress dialog is in place below
             if (machine.isOk() && item->accessible())
             {
                 /* delete machine settings */
-                machine.Delete();
+                CProgress progress = machine.Delete(aMedia);
+                progress.WaitForCompletion(-1);         // @todo do this nicely with a progress dialog, this can delete many files!
+
                 /* remove the item shortly: cmachine it refers to is no longer valid! */
                 int row = mVMModel->rowById (item->id());

trunk/src/VBox/Frontends/VirtualBox/src/wizards/newvm/UINewVMWzd.cpp

@@ -782 +782 @@
         {
             /* Unregister on failure */
-            QVector<QString> files = m_Machine.Unregister(false /*fDetachMedia*/);
+            QVector<CMedium> aMedia = m_Machine.Unregister(KCleanupMode_UnregisterOnly);   //  @todo replace with DetachAllReturnHardDisksOnly once a progress dialog is in place below
             if (vbox.isOk())
-                m_Machine.Delete();
+            {
+                CProgress progress = m_Machine.Delete(aMedia);
+                progress.WaitForCompletion(-1);         // @todo do this nicely with a progress dialog, this can delete lots of files
+            }
             return false;
         }

trunk/src/VBox/Main/ApplianceImplImport.cpp

@@ -1223 +1223 @@
     {
         // with _whatever_ error we've had, do a complete roll-back of
-        // machines and disks we've created; unfortunately this is
-        // not so trivially done...
-
-        HRESULT rc2;
-        // detach all hard disks from all machines we created
-        for (list<MyHardDiskAttachment>::iterator itM = stack.llHardDiskAttachments.begin();
-             itM != stack.llHardDiskAttachments.end();
-             ++itM)
-        {
-            const MyHardDiskAttachment &mhda = *itM;
-            rc2 = mhda.pMachine->LockMachine(stack.pSession, LockType_Write);
-            if (SUCCEEDED(rc2))
-            {
-                ComPtr<IMachine> sMachine;
-                rc2 = stack.pSession->COMGETTER(Machine)(sMachine.asOutParam());
-                if (SUCCEEDED(rc2))
-                {
-                    rc2 = sMachine->DetachDevice(Bstr(mhda.controllerType), mhda.lControllerPort, mhda.lDevice);
-                    rc2 = sMachine->SaveSettings();
-                }
-                stack.pSession->UnlockMachine();
-            }
-        }
-
-        // now clean up all hard disks we created
-        for (list< ComPtr<IMedium> >::iterator itHD = stack.llHardDisksCreated.begin();
-             itHD != stack.llHardDisksCreated.end();
-             ++itHD)
-        {
-            ComPtr<IMedium> pDisk = *itHD;
-            ComPtr<IProgress> pProgress2;
-            rc2 = pDisk->DeleteStorage(pProgress2.asOutParam());
-            rc2 = pProgress2->WaitForCompletion(-1);
-        }
-
-        // finally, deregister and remove all machines
+        // machines and disks we've created
+
         for (list<Guid>::iterator itID = m->llGuidsMachinesCreated.begin();
              itID != m->llGuidsMachinesCreated.end();
@@ -1266 +1232 @@
             Bstr bstrGuid = guid.toUtf16();
             ComPtr<IMachine> failedMachine;
-            SafeArray<BSTR> abstrPaths;
-            rc2 = mVirtualBox->GetMachine(bstrGuid, failedMachine.asOutParam());
+            HRESULT rc2 = mVirtualBox->GetMachine(bstrGuid, failedMachine.asOutParam());
             if (SUCCEEDED(rc2))
             {
-                rc2 = failedMachine->Unregister(false, ComSafeArrayAsOutParam(abstrPaths));
-                rc2 = failedMachine->Delete();
+                SafeIfaceArray<IMedium> aMedia;
+                rc2 = failedMachine->Unregister(CleanupMode_DetachAllReturnHardDisksOnly, ComSafeArrayAsOutParam(aMedia));
+                ComPtr<IProgress> pProgress2;
+                rc2 = failedMachine->Delete(ComSafeArrayAsInParam(aMedia), pProgress2.asOutParam());
+                pProgress2->WaitForCompletion(-1);
             }
         }

trunk/src/VBox/Main/MachineImpl.cpp

@@ -3974 +3974 @@
 
 /** @note Locks objects! */
-STDMETHODIMP Machine::Unregister(BOOL fAutoCleanup,
-                                 ComSafeArrayOut(BSTR, aFiles))
+STDMETHODIMP Machine::Unregister(CleanupMode_T cleanupMode,
+                                 ComSafeArrayOut(IMedium*, aMedia))
 {
     AutoCaller autoCaller(this);
@@ -3989 +3989 @@
     HRESULT rc = S_OK;
 
-    // this list collects the files that should be reported
-    // as to be deleted to the caller in aFiles (this includes the
-    // media files in llMedia below)
-    std::list<Utf8Str> llFilesForCaller;
-
     // discard saved state
     if (mData->mMachineState == MachineState_Saved)
@@ -3999 +3994 @@
         // add the saved state file to the list of files the caller should delete
         Assert(!mSSData->mStateFilePath.isEmpty());
-        llFilesForCaller.push_back(mSSData->mStateFilePath);
+        mData->llFilesToDelete.push_back(mSSData->mStateFilePath);
 
         mSSData->mStateFilePath.setNull();
@@ -4011 +4006 @@
     if (mData->mFirstSnapshot)
         cSnapshots = mData->mFirstSnapshot->getAllChildrenCount() + 1;
-    if (cSnapshots && !fAutoCleanup)
+    if (cSnapshots && cleanupMode == CleanupMode_UnregisterOnly)
         // fail now before we start detaching media
         return setError(VBOX_E_INVALID_OBJECT_STATE,
@@ -4034 +4029 @@
     {
         // we have media attachments: detach them all and add the Medium objects to our list
-        if (fAutoCleanup)
-            detachAllMedia(alock, NULL /* pSnapshot */, llMedia);
+        if (cleanupMode != CleanupMode_UnregisterOnly)
+            detachAllMedia(alock, NULL /* pSnapshot */, cleanupMode, llMedia);
         else
             return setError(VBOX_E_INVALID_OBJECT_STATE,
@@ -4061 +4056 @@
 
         // GO!
-        pFirstSnapshot->uninitRecursively(alock, llMedia, llFilesForCaller);
+        pFirstSnapshot->uninitRecursively(alock, cleanupMode, llMedia, mData->llFilesToDelete);
 
         mData->mMachineState = oldState;
     }
-
 
     if (FAILED(rc))
@@ -4081 +4075 @@
     alock.release();
 
-    if (fAutoCleanup)
-    {
-        // now go thru the list of attached media reported by prepareUnregister() and close them all
-        for (MediaList::const_iterator it = llMedia.begin();
-             it != llMedia.end();
-             ++it)
-        {
-            ComObjPtr<Medium> pMedium = *it;
-            Utf8Str strFile = pMedium->getLocationFull();
-
-            AutoCaller autoCaller2(pMedium);
-            if (FAILED(autoCaller2.rc())) return autoCaller2.rc();
-
-            ErrorInfoKeeper eik;
-            rc = pMedium->close(NULL /*fNeedsSaveSettings*/,     // we'll call saveSettings() in any case below
-                                autoCaller2);
-                // this uninitializes the medium
-
-            LogFlowThisFunc(("Medium::close() on %s yielded rc (%Rhra)\n", strFile.c_str(), rc));
-
-            if (rc == VBOX_E_OBJECT_IN_USE)
-                // can happen if the medium was still attached to another machine;
-                // do not report the file to the caller then, but don't report
-                // an error either
-                eik.setNull();
-            else if (SUCCEEDED(rc))
-                // report the path to the caller
-                llFilesForCaller.push_back(strFile);
-        }
-    }
-
-    // report all paths to the caller
-    SafeArray<BSTR> sfaFiles(llFilesForCaller.size());
-    size_t i = 0;
-    for (std::list<Utf8Str>::iterator it = llFilesForCaller.begin();
-         it != llFilesForCaller.end();
-         ++it)
-        Bstr(*it).detachTo(&sfaFiles[i++]);
-    sfaFiles.detachTo(ComSafeArrayOutArg(aFiles));
+    // return media to caller
+    SafeIfaceArray<IMedium> sfaMedia(llMedia);
+    sfaMedia.detachTo(ComSafeArrayOutArg(aMedia));
 
     mParent->unregisterMachine(this);
@@ -4127 +4085 @@
 }
 
-STDMETHODIMP Machine::Delete()
-{
+struct Machine::DeleteTask
+{
+    ComObjPtr<Machine>          pMachine;
+    std::list<Utf8Str>          llFilesToDelete;
+    ComObjPtr<Progress>         pProgress;
+};
+
+STDMETHODIMP Machine::Delete(ComSafeArrayIn(IMedium*, aMedia), IProgress **aProgress)
+{
+    LogFlowFuncEnter();
+
     AutoCaller autoCaller(this);
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+    bool fNeedsGlobalSaveSettings = false;
 
     HRESULT rc = checkStateDependency(MutableStateDep);
@@ -4140 +4109 @@
         return setError(VBOX_E_INVALID_VM_STATE,
                         tr("Cannot delete settings of a registered machine"));
+
+    DeleteTask *pTask = new DeleteTask;
+    pTask->pMachine = this;
+    com::SafeIfaceArray<IMedium> sfaMedia(ComSafeArrayInArg(aMedia));
+
+    // collect files to delete
+    pTask->llFilesToDelete = mData->llFilesToDelete;            // saved states pushed here by Unregister()
+    for (size_t i = 0; i < sfaMedia.size(); ++i)
+    {
+        IMedium *pIMedium(sfaMedia[i]);
+        Medium *pMedium = static_cast<Medium*>(pIMedium);
+        AutoCaller mediumAutoCaller(pMedium);
+        if (FAILED(mediumAutoCaller.rc())) return mediumAutoCaller.rc();
+
+        Utf8Str bstrLocation = pMedium->getLocationFull();
+        // close the medium now; if that succeeds, then that means the medium is no longer
+        // in use and we can add it to the list of files to delete
+        rc = pMedium->close(&fNeedsGlobalSaveSettings, mediumAutoCaller);
+        if (SUCCEEDED(rc))
+            pTask->llFilesToDelete.push_back(bstrLocation);
+    }
+    if (mData->pMachineConfigFile->fileExists())
+        pTask->llFilesToDelete.push_back(mData->m_strConfigFileFull);
+
+    pTask->pProgress.createObject();
+    pTask->pProgress->init(getVirtualBox(),
+                           this /* aInitiator */,
+                           Bstr(tr("Deleting files")),
+                           true /* fCancellable */,
+                           pTask->llFilesToDelete.size() + 1,   // cOperations
+                           BstrFmt(tr("Deleting '%s'"), pTask->llFilesToDelete.front().c_str()));
+
+    int vrc = RTThreadCreate(NULL,
+                             Machine::deleteThread,
+                             (void*)pTask,
+                             0,
+                             RTTHREADTYPE_MAIN_WORKER,
+                             0,
+                             "MachineDelete");
+
+    alock.release();
+
+    if (fNeedsGlobalSaveSettings)
+    {
+        AutoWriteLock vboxlock(mParent COMMA_LOCKVAL_SRC_POS);
+        mParent->saveSettings();
+    }
+
+    if (RT_FAILURE(vrc))
+    {
+        delete pTask;
+        return setError(E_FAIL, "Could not create MachineDelete thread (%Rrc)", vrc);
+    }
+
+    pTask->pProgress.queryInterfaceTo(aProgress);
+
+    LogFlowFuncLeave();
+
+    return S_OK;
+}
+
+/**
+ * Static task wrapper passed to RTThreadCreate() in Machine::Delete() which then
+ * calls Machine::deleteTaskWorker() on the actual machine object.
+ * @param Thread
+ * @param pvUser
+ * @return
+ */
+/*static*/
+DECLCALLBACK(int) Machine::deleteThread(RTTHREAD Thread, void *pvUser)
+{
+    LogFlowFuncEnter();
+
+    DeleteTask *pTask = (DeleteTask*)pvUser;
+    Assert(pTask);
+    Assert(pTask->pMachine);
+    Assert(pTask->pProgress);
+
+    HRESULT rc = pTask->pMachine->deleteTaskWorker(*pTask);
+    pTask->pProgress->notifyComplete(rc);
+
+    delete pTask;
+
+    LogFlowFuncLeave();
+
+    NOREF(Thread);
+
+    return VINF_SUCCESS;
+}
+
+/**
+ * Task thread implementation for Machine::Delete(), called from Machine::deleteThread().
+ * @param task
+ * @return
+ */
+HRESULT Machine::deleteTaskWorker(DeleteTask &task)
+{
+    AutoCaller autoCaller(this);
+    if (FAILED(autoCaller.rc())) return autoCaller.rc();
+
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
     ULONG uLogHistoryCount = 3;
@@ -4147 +4217 @@
         systemProperties->COMGETTER(LogHistoryCount)(&uLogHistoryCount);
 
+    // delete the files pushed on the task list by Machine::Delete()
+    // (this includes saved states of the machine and snapshots and
+    // medium storage files from the IMedium list passed in, and the
+    // machine XML file)
+    std::list<Utf8Str>::const_iterator it = task.llFilesToDelete.begin();
+    while (it != task.llFilesToDelete.end())
+    {
+        const Utf8Str &strFile = *it;
+        LogFunc(("Deleting file %s\n", strFile.c_str()));
+        RTFileDelete(strFile.c_str());
+
+        ++it;
+        if (it == task.llFilesToDelete.end())
+        {
+            task.pProgress->SetNextOperation(Bstr(tr("Cleaning up machine directory")), 1);
+            break;
+        }
+
+        task.pProgress->SetNextOperation(BstrFmt(tr("Deleting '%s'"), it->c_str()), 1);
+    }
+
     /* delete the settings only when the file actually exists */
     if (mData->pMachineConfigFile->fileExists())
     {
-        int vrc = RTFileDelete(mData->m_strConfigFileFull.c_str());
-        if (RT_FAILURE(vrc))
-            return setError(VBOX_E_IPRT_ERROR,
-                            tr("Could not delete the settings file '%s' (%Rrc)"),
-                            mData->m_strConfigFileFull.raw(),
-                            vrc);
-
         /* Delete any backup or uncommitted XML files. Ignore failures.
            See the fSafe parameter of xml::XmlFileWriter::write for details. */
@@ -8652 +8736 @@
 /**
  * Goes thru all medium attachments of the list and calls detachDevice() on each
- * of them and attaches all Medium objects found in the process to the given list.
+ * of them and attaches all Medium objects found in the process to the given list,
+ * depending on cleanupMode.
  *
  * This gets called from Machine::Unregister, both for the actual Machine and
@@ -8660 +8745 @@
  *
  * @param writeLock Machine lock from top-level caller; this gets passed to detachDevice.
- * @param llMedia Caller's list to receive Medium objects which got detached so caller can close() them.
  * @param pSnapshot Must be NULL when called for a "real" Machine or a snapshot object if called for a SnapshotMachine.
+ * @param cleanupMode If DetachAllReturnHardDisksOnly, only hard disk media get added to llMedia; if Full, then all media get added;
+ *          otherwise no media get added.
+ * @param llMedia Caller's list to receive Medium objects which got detached so caller can close() them, depending on cleanupMode.
  * @return
  */
 HRESULT Machine::detachAllMedia(AutoWriteLock &writeLock,
                                 Snapshot *pSnapshot,
+                                CleanupMode_T cleanupMode,
                                 MediaList &llMedia)
 {
@@ -8684 +8772 @@
 
         if (!pMedium.isNull())
-            llMedia.push_back(pMedium);
+        {
+            DeviceType_T devType = pMedium->getDeviceType();
+            if (    (    cleanupMode == CleanupMode_DetachAllReturnHardDisksOnly
+                      && devType == DeviceType_HardDisk)
+                 || (cleanupMode == CleanupMode_Full)
+               )
+                llMedia.push_back(pMedium);
+        }
 
         // real machine: then we need to use the proper method

trunk/src/VBox/Main/MediumImpl.cpp

@@ -3867 +3867 @@
     // we're accessing parent/child and backrefs, so lock the tree first, then ourselves
     AutoMultiWriteLock2 multilock(&m->pVirtualBox->getMediaTreeLockHandle(),
-                                   this->lockHandle()
-                                           COMMA_LOCKVAL_SRC_POS);
+                                  this->lockHandle()
+                                  COMMA_LOCKVAL_SRC_POS);
+
+    LogFlowFunc(("ENTER for %s\n", getLocationFull().c_str()));
 
     bool wasCreated = true;
@@ -3886 +3888 @@
     if (m->backRefs.size() != 0)
         return setError(VBOX_E_OBJECT_IN_USE,
-                        tr("Medium '%s' is attached to %d virtual machines"),
+                        tr("Medium '%s' cannot be closed because it is still attached to %d virtual machines"),
                            m->strLocationFull.raw(), m->backRefs.size());
 
@@ -3908 +3910 @@
     // of the medium tree cannot be guaranteed.
     uninit();
+
+    LogFlowFuncLeave();
 
     return rc;

trunk/src/VBox/Main/ProgressImpl.cpp

@@ -1084 +1084 @@
     AssertReturn(mCompleted == FALSE, E_FAIL);
 
+    LogFunc(("aResultCode=%d\n", aResultCode));
+
     if (mCanceled && SUCCEEDED(aResultCode))
         aResultCode = E_FAIL;
@@ -1105 +1107 @@
     /* wake up all waiting threads */
     if (mWaitersCount > 0)
-        RTSemEventMultiSignal (mCompletedSem);
+        RTSemEventMultiSignal(mCompletedSem);
 
     return rc;

trunk/src/VBox/Main/SnapshotImpl.cpp

@@ -828 +828 @@
  * Caller must hold the machine write lock (which protects the snapshots tree!)
  *
+ * @param writeLock Machine write lock, which can get released temporarily here.
+ * @param cleanupMode Cleanup mode; see Machine::detachAllMedia().
+ * @param llMedia List of media returned to caller, depending on cleanupMode.
  * @param llFilenames
  * @return
  */
 HRESULT Snapshot::uninitRecursively(AutoWriteLock &writeLock,
+                                    CleanupMode_T cleanupMode,
                                     MediaList &llMedia,
                                     std::list<Utf8Str> &llFilenames)
@@ -858 +862 @@
     {
         Snapshot *pChild = *it;
-        rc = pChild->uninitRecursively(writeLock, llMedia, llFilenames);
+        rc = pChild->uninitRecursively(writeLock, cleanupMode, llMedia, llFilenames);
         if (FAILED(rc))
             return rc;
@@ -866 +870 @@
     rc = m->pMachine->detachAllMedia(writeLock,
                                      this /* pSnapshot */,
+                                     cleanupMode,
                                      llMedia);
     if (FAILED(rc))

trunk/src/VBox/Main/idl/VirtualBox.xidl

@@ -2784 +2784 @@
       </param>
       <param name="aProgress" type="IProgress" dir="return">
-        <desc></desc>
+        <desc>Progress object to track the operation completion.</desc>
       </param>
     </method>
@@ -2824 +2824 @@
 
       <param name="aProgress" type="IProgress" dir="return">
-        <desc></desc>
+        <desc>Progress object to track the operation completion.</desc>
       </param>
     </method>
@@ -3222 +3222 @@
         to signal the completion of the progress object.
       </desc>
-      <param name="progress" type="IProgress" dir="in"/>
+      <param name="aProgress" type="IProgress" dir="in" />
     </method>
 
@@ -3636 +3636 @@
   </interface>
 
+  <enum name="CleanupMode"
+        uuid="67897c50-7cca-47a9-83f6-ce8fd8eb5441">
+    <desc>Cleanup mode, used with <link to="IMachine::unregister" />.
+    </desc>
+    <const name="UnregisterOnly"                  value="1">
+      <desc>Unregister only the machine, but neither delete snapshots nor detach media.</desc>
+    </const>
+    <const name="DetachAllReturnNone"             value="2">
+      <desc>Delete all snapshots and detach all media but return none; this will keep all media registered.</desc>
+    </const>
+    <const name="DetachAllReturnHardDisksOnly"    value="3">
+      <desc>Delete all snapshots, detach all media and return hard disks for closing, but not removeable media.</desc>
+    </const>
+    <const name="Full"                            value="4">
+      <desc>Delete all snapshots, detach all media and return all media for closing.</desc>
+    </const>
+  </enum>
+
   <interface
      name="IMachine" extends="$unknown"
-     uuid="e2d8679e-b144-41d9-9dfa-bbe2fd0ab2d1"
+     uuid="082c38ff-d9b3-4b12-b540-01516a931f17"
      wsmap="managed"
      >
@@ -3839 +3857 @@
          Priority of the virtual CPUs. Means to limit the number of CPU cycles
          a guest can use. The unit is percentage of host CPU cycles per second.
-         The valid range is 1 - 100. 100 (the default) implies no limit. 
+         The valid range is 1 - 100. 100 (the default) implies no limit.
        </desc>
     </attribute>
@@ -5186 +5204 @@
       <desc>
         Unregisters the machine, which must have been previously registered using
-        <link to="IVirtualBox::registerMachine"/>.
-
-        This method will succeed even if the machine currently has media attachments or
-        is in the <link to="MachineState::Saved">Saved</link> state. In those cases,
-        the caller receives the names of all files that have been made obsolete by the
-        call in the @a aFiles array parameter. This list will consist of the saved
-        state file and all storage files by media that were detached from the machine.
-
-        <note>
-          The @a fAutoCleanup parameter is temporary to avoid substantial changes to the
-          frontends in SVN trunk.It will go away before the 3.3 release and
-          always be true then. If fAutoCleanup is false, the API currently behaves like
-          the old VirtualBox::UnregisterMachine API. Cleaning up snapshots does not work
-          yet, so the method will still fail if the machine has snapshots.
-        </note>
-
-        The settings XML file of the machine object itself will not be included in that
-        list; call <link to="#delete" /> for that.
+        <link to="IVirtualBox::registerMachine"/>, and optionally do additional
+        cleanup before the machine is unregistered.
+
+        This method does not delete any files. It only changes the machine configuration and
+        the list of registered machines in the VirtualBox object. To delete the files which
+        belonged to the machine, including the XML file of the machine itself, call
+        <link to="#delete"/>, optionally with the array of IMedium objects that have been
+        returned from this method.
+
+        Depending on the @a cleanupMode argument, the thoroughness with with this
+        method cleans up the machine configuration before unregistering the machine
+        will vary.
+
+        <ul>
+          <li>With "UnregisterOnly", the machine will only be unregistered, but no additional
+            cleanup will be performed. The call will fail if the machine is in "Saved" state
+            or has any snapshots or any media attached (see <link to="IMediumAttachment" />.
+            It is the responsibility of the caller to delete all such configuration in this mode.
+            In this mode, the API behaves like the former @c IVirtualBox::unregisterMachine() API
+            which it replaces.</li>
+          <li>With "DetachAllReturnNone", the call will succeed even if the machine is in "Saved"
+            state or if it has snapshots or media attached. All media attached to the current machine
+            state or in snapshots will be detached. No medium objects will be closed or returned; all
+            of the machine's media will remain opened.</li>
+          <li>With "DetachAllReturnHardDisksOnly", the call will behave like with "DetachAllReturnNone",
+            except that all the hard disk medium objects which were detached from the machine will
+            be returned as an array. This allows for quickly passing them to the <link to="#delete" />
+            API for closing and deletion.</li>
+          <li>With "Full", the call will behave like with "DetachAllReturnHardDisksOnly", except
+            that all media will be returned in the array, including removeable media like DVDs and
+            floppies. This might be useful if the user wants to inspect in detail which media were
+            attached to the machine.</li>
+        </ul>
 
         The call will fail if the machine is currently locked (see <link to="ISession" />).
@@ -5209 +5242 @@
         before unregistering it.
 
-        After successful method invocation,
-        the <link to="IMachineRegisteredEvent"/> event is fired.
+        After successful method invocation, the <link to="IMachineRegisteredEvent"/> event
+        is fired.
 
         <note>
@@ -5220 +5253 @@
 
         <result name="VBOX_E_INVALID_OBJECT_STATE">
-          Machine has snapshot or is locked.
-        </result>
-      </desc>
-
-      <param name="fAutoCleanup" type="boolean" dir="in">
-        <desc>If true, the method will automatically detach all media from the
-          machine and its snapshots, call <link to="IMedium::close" /> on each
-          medium, and the paths of all media files involved will be reported to
-          the caller in the @a aFiles array so the caller can then delete the
-          image files.
-          If false, the method will fail if media attachments are present.
-        </desc>
-      </param>
-      <param name="aFiles" type="wstring" safearray="yes" dir="return">
-        <desc>
-          List of all files detached from medium attachments of the machine, if
-          @a fCloseMedia is true.
-        </desc>
+          Machine is currently locked for a session.
+        </result>
+      </desc>
+
+      <param name="cleanupMode" type="CleanupMode" dir="in">
+        <desc>How to clean up after the machine has been unregistered.</desc>
+      </param>
+      <param name="aMedia" type="IMedium" safearray="yes" dir="return">
+        <desc>List of media detached from the machine, depending on the @a cleanupMode parameter.</desc>
       </param>
     </method>
@@ -5243 +5267 @@
     <method name="delete">
       <desc>
-        Deletes the settings (machine XML) file of this machine from disk. The machine
-        must not be registered in order for this operation to succeed.
+        Deletes the files associated with this machine from disk. If medium objects are passed
+        in with the @a aMedia argument, they are closed and their storage files are deleted as well.
+        For convenience, this array of media files can be the same as the one returned from
+        a previous <a link="#unregister" /> call.
+
+        This method must only be called on machines which are either write-locked (i.e. on instances
+        returned by <link to="ISession::machine"/>) or on unregistered machines (i.e. not yet
+        registered machines created by <link to="IVirtualBox::createMachine"/> or opened by
+        <link to="IVirtualBox::openMachine"/>, or after having called <link to="#unregister"/>).
+
+        The following files will be deleted by this method:
+        <ul>
+          <li>If <link to="#unregister" /> had been previously called with a @a cleanupMode
+            argument other than "UnregisterOnly", this will delete all saved state files that
+            the machine had in use; possibly one if the machine was in "Saved" state and one
+            for each online snapshot that the machine had.</li>
+          <li>On each medium object passed in the @a aMedia array, this will call
+            <link to="IMedium::close" /> and then attempt to delete the medium's storage
+            on disk. The close() call will fail if the medium is still in use, e.g. because
+            it is still attached to a second machine.</li>
+          <li>Finally, the machine's own XML file will be deleted.</li>
+        </ul>
+
+        Since deleting large disk image files can be a time-consuming I/O operation, this
+        method operates asynchronously and returns an IProgress object to allow the caller
+        to monitor the progress. There will be one sub-operation for each file that is
+        being deleted (saved state or medium storage file).
+
         <note>
           <link to="#settingsModified"/> will return @c true after this
           method successfully returns.
         </note>
-        <note>
-          Calling this method is only valid on instances returned
-          by <link to="ISession::machine"/> and on new machines
-          created by <link to="IVirtualBox::createMachine"/> or
-          opened by <link to="IVirtualBox::openMachine"/> but not
-          yet registered, or on unregistered machines after calling
-          <link to="IMachine::unregister"/>.
-        </note>
-        <note>
-          The deleted machine settings file can be restored (saved again)
-          by calling <link to="#saveSettings"/>.
-        </note>
 
         <result name="VBOX_E_INVALID_VM_STATE">
-          Cannot delete settings of a registered machine or
-          machine not mutable.
+          Machine is registered but not write-locked.
         </result>
         <result name="VBOX_E_IPRT_ERROR">
           Could not delete the settings file.
         </result>
-
-      </desc>
+      </desc>
+      <param name="aMedia" type="IMedium" safearray="yes" dir="in">
+        <desc>List of media to be closed and whose storage files will be deleted.</desc>
+      </param>
+      <param name="aProgress" type="IProgress" dir="return">
+        <desc>Progress object to track the operation completion.</desc>
+      </param>
     </method>
 
     <method name="export">
-        <desc>Exports the machine to an OVF appliance. See <link to="IAppliance" /> for the
-              steps required to export VirtualBox machines to OVF.
-        </desc>
-
-        <param name="aAppliance" type="IAppliance" dir="in">
-            <desc>Appliance to export this machine to.</desc>
-        </param>
-        <param name="aDescription" type="IVirtualSystemDescription" dir="return">
-            <desc>VirtualSystemDescription object which is created for this machine.</desc>
-        </param>
+      <desc>Exports the machine to an OVF appliance. See <link to="IAppliance" /> for the
+            steps required to export VirtualBox machines to OVF.
+      </desc>
+
+      <param name="aAppliance" type="IAppliance" dir="in">
+        <desc>Appliance to export this machine to.</desc>
+      </param>
+      <param name="aDescription" type="IVirtualSystemDescription" dir="return">
+        <desc>VirtualSystemDescription object which is created for this machine.</desc>
+      </param>
     </method >
 

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

@@ -181 +181 @@
         ComObjPtr<Snapshot> mFirstSnapshot;
         ComObjPtr<Snapshot> mCurrentSnapshot;
+
+        // list of files to delete in Delete(); this list is filled by Unregister()
+        std::list<Utf8Str>  llFilesToDelete;
     };
 
@@ -484 +487 @@
     STDMETHOD(SaveSettings)();
     STDMETHOD(DiscardSettings)();
-    STDMETHOD(Unregister) (BOOL fAutoCleanup, ComSafeArrayOut(BSTR, aFiles));
-    STDMETHOD(Delete)();
+    STDMETHOD(Unregister)(CleanupMode_T cleanupMode, ComSafeArrayOut(IMedium*, aMedia));
+    STDMETHOD(Delete)(ComSafeArrayIn(IMedium*, aMedia), IProgress **aProgress);
     STDMETHOD(Export)(IAppliance *aAppliance, IVirtualSystemDescription **aDescription);
     STDMETHOD(GetSnapshot)(IN_BSTR aId, ISnapshot **aSnapshot);
@@ -782 +785 @@
     HRESULT detachAllMedia(AutoWriteLock &writeLock,
                            Snapshot *pSnapshot,
+                           CleanupMode_T cleanupMode,
                            MediaList &llMedia);
 
@@ -792 +796 @@
     void commit();
     void copyFrom(Machine *aThat);
+
+    struct DeleteTask;
+    static DECLCALLBACK(int) deleteThread(RTTHREAD Thread, void *pvUser);
+    HRESULT deleteTaskWorker(DeleteTask &task);
 
 #ifdef VBOX_WITH_GUEST_PROPS

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

@@ -125 +125 @@
 
     HRESULT uninitRecursively(AutoWriteLock &writeLock,
+                              CleanupMode_T cleanupMode,
                               MediaList &llMedia,
                               std::list<Utf8Str> &llFilenames);

trunk/src/VBox/Main/testcase/tstOVF.cpp

@@ -349 +349 @@
 
             RTPrintf("  Deleting machine %ls...\n", bstrUUID.raw());
-            SafeArray<BSTR> sfaFiles;
-            rc = pMachine->Unregister(true /* fDetachMedia */,
-                                      ComSafeArrayAsOutParam(sfaFiles));
+            SafeIfaceArray<IMedium> sfaMedia;
+            rc = pMachine->Unregister(CleanupMode_DetachAllReturnHardDisksOnly,
+                                      ComSafeArrayAsOutParam(sfaMedia));
             if (FAILED(rc)) throw MyError(rc, "Machine::Unregister() failed\n");
 
-            for (size_t u = 0;
-                 u < sfaFiles.size();
-                 ++u)
-            {
-                RTPrintf("    UnregisterMachine reported disk image %ls\n", sfaFiles[u]);
-                llFiles2Delete.push_back(sfaFiles[u]);
-            }
-
-            rc = pMachine->Delete();
+            ComPtr<IProgress> pProgress;
+            rc = pMachine->Delete(ComSafeArrayAsInParam(sfaMedia), pProgress.asOutParam());
             if (FAILED(rc)) throw MyError(rc, "Machine::DeleteSettings() failed\n");
+            rc = pProgress->WaitForCompletion(-1);
+            if (FAILED(rc)) throw MyError(rc, "Progress::WaitForCompletion() failed\n");
         }
     }

trunk/src/VBox/Main/webservice/websrv-cpp.xsl

@@ -568 +568 @@
           <xsl:call-template name="emitNewlineIndent8" />
           <xsl:value-of select="concat('    IUnknown *tmpObject2(tmpObject); tmpObject2->AddRef(); comcall_', $name, '[i] = tmpObject;')" />
+        </xsl:when>
+        <xsl:when test="//interface[@name=$type]">
+          <xsl:value-of select="concat('    ComPtr&lt;', $type, '&gt; tmpObject;')" />
+          <xsl:call-template name="emitNewlineIndent8" />
+          <xsl:value-of select="concat('    if ((rc = findComPtrFromId(soap, ', $structprefix, $name, '[i], tmpObject, true)))')" />
+          <xsl:call-template name="emitNewlineIndent8" />
+          <xsl:text>        break;</xsl:text>
+          <xsl:call-template name="emitNewlineIndent8" />
+          <xsl:value-of select="concat('    ', $type, ' *tmpObject2(tmpObject); tmpObject2->AddRef(); comcall_', $name, '[i] = tmpObject;')" />
         </xsl:when>
         <xsl:when test="$type='wstring'">