Changeset 31228 in vbox

Timestamp:

Jul 29, 2010 7:44:50 PM (14 years ago)

Author:

vboxsync

Message:

Main: beginnings of deleting all snapshots in Machine::Unregister() -- works partially, more to be done

Location:

trunk/src/VBox/Main

Files:

• MachineImpl.cpp (modified) (12 diffs)
• SnapshotImpl.cpp (modified) (2 diffs)
• include/MachineImpl.h (modified) (1 diff)
• include/SnapshotImpl.h (modified) (2 diffs)

trunk/src/VBox/Main/MachineImpl.cpp

@@ -3589 +3589 @@
                         aDevice, aControllerPort, aControllerName);
 
-    rc = detachDevice(pAttach, alock, &fNeedsSaveSettings);
+    rc = detachDevice(pAttach, alock, NULL /* pSnapshot */, &fNeedsSaveSettings);
 
     if (fNeedsSaveSettings)
@@ -4080 +4080 @@
     // this list collects the files that should be reported
     // as to be deleted to the caller in aFiles
-    std::list<Bstr> llFilesForCaller;
+    std::list<Utf8Str> llFilesForCaller;
 
     // discard saved state
@@ -4087 +4087 @@
         // add the saved state file to the list of files the caller should delete
         Assert(!mSSData->mStateFilePath.isEmpty());
-        llFilesForCaller.push_back(Bstr(mSSData->mStateFilePath));
+        llFilesForCaller.push_back(mSSData->mStateFilePath);
 
         mSSData->mStateFilePath.setNull();
@@ -4096 +4096 @@
     }
 
-    // @todo optionally nuke snapshots
     size_t snapshotCount = 0;
     if (mData->mFirstSnapshot)
         snapshotCount = mData->mFirstSnapshot->getAllChildrenCount() + 1;
     if (snapshotCount)
-        return setError(VBOX_E_INVALID_OBJECT_STATE,
-                        tr("Cannot unregister the machine '%ls' because it has %d snapshots"),
-                        mUserData->mName.raw(), snapshotCount);
+    {
+        if (fAutoCleanup)
+        {
+            // caller wants automatic detachment: then do that and report all media to the array
+
+            // Snapshot::beginDeletingSnapshot() needs the machine state to be this
+            MachineState_T oldState = mData->mMachineState;
+            mData->mMachineState = MachineState_DeletingSnapshot;
+
+            // make a copy of the first snapshot so the refcount does not drop to 0
+            // in beginDeletingSnapshot, which sets pFirstSnapshot to 0 (that hangs
+            // because of the AutoCaller voodoo)
+            ComObjPtr<Snapshot> pFirstSnapshot = mData->mFirstSnapshot;
+
+            // go!
+            pFirstSnapshot->uninitRecursively(alock, llMedia, llFilesForCaller);
+
+            mData->mMachineState = oldState;
+        }
+        else
+            return setError(VBOX_E_INVALID_OBJECT_STATE,
+                            tr("Cannot unregister the machine '%ls' because it has %d snapshots"),
+                            mUserData->mName.raw(), snapshotCount);
+    }
 
     if (    !mMediaData.isNull()      // can be NULL if machine is inaccessible
@@ -4109 +4129 @@
        )
     {
-        // we have media attachments:
+        // we have media attachments: detach them all and add the Medium objects to our list
         if (fAutoCleanup)
-        {
-            // caller wants automatic detachment: then do that and report all media to the array
-
-            // make a temporary list because detachDevice invalidates iterators into
-            // mMediaData->mAttachments
-            MediaData::AttachmentList llAttachments2 = mMediaData->mAttachments;
-
-            for (MediaData::AttachmentList::iterator it = llAttachments2.begin();
-                 it != llAttachments2.end();
-                 ++it)
-            {
-                ComObjPtr<MediumAttachment> pAttach = *it;
-                ComObjPtr<Medium> pMedium = pAttach->getMedium();
-
-                if (!pMedium.isNull())
-                    llMedia.push_back(pMedium);
-
-                rc = detachDevice(pAttach,
-                                  alock,
-                                  NULL /* pfNeedsSaveSettings */);
-                if (FAILED(rc))
-                    break;
-            };
-        }
+            detachAllMedia(alock, NULL /* pSnapshot */, llMedia);
         else
             return setError(VBOX_E_INVALID_OBJECT_STATE,
@@ -4163 +4160 @@
         {
             ComObjPtr<Medium> pMedium = *it;
-            Bstr bstrFile = pMedium->getLocationFull();
+            Utf8Str strFile = pMedium->getLocationFull();
 
             AutoCaller autoCaller2(pMedium);
@@ -4172 +4169 @@
                                 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)
@@ -4180 +4179 @@
             else if (SUCCEEDED(rc))
                 // report the path to the caller
-                llFilesForCaller.push_back(bstrFile);
+                llFilesForCaller.push_back(strFile);
         }
     }
@@ -4187 +4186 @@
     SafeArray<BSTR> sfaFiles(llFilesForCaller.size());
     size_t i = 0;
-    for (std::list<Bstr>::iterator it = llFilesForCaller.begin();
+    for (std::list<Utf8Str>::iterator it = llFilesForCaller.begin();
          it != llFilesForCaller.end();
          ++it)
-        it->detachTo(&sfaFiles[i++]);
+        Bstr(*it).detachTo(&sfaFiles[i++]);
     sfaFiles.detachTo(ComSafeArrayOutArg(aFiles));
 
@@ -8710 +8709 @@
  * @param pAttach Medium attachment to detach.
  * @param writeLock Machine write lock which the caller must have locked once. This may be released temporarily in here.
+ * @param pSnapshot If NULL, then the detachment is for the current machine. Otherwise this is for a SnapshotMachine, and this must be its snapshot.
  * @param pfNeedsSaveSettings Optional pointer to a bool that must have been initialized to false and that will be set to true
  *                by this function if the caller should invoke VirtualBox::saveSettings() because the global settings have changed.
@@ -8716 +8716 @@
 HRESULT Machine::detachDevice(MediumAttachment *pAttach,
                               AutoWriteLock &writeLock,
+                              Snapshot *pSnapshot,
                               bool *pfNeedsSaveSettings)
 {
@@ -8751 +8752 @@
     mMediaData.backup();
 
-    /* we cannot use erase (it) below because backup() above will create
-    * a copy of the list and make this copy active, but the iterator
-    * still refers to the original and is not valid for the copy */
+    // we cannot use erase (it) below because backup() above will create
+    // a copy of the list and make this copy active, but the iterator
+    // still refers to the original and is not valid for the copy
     mMediaData->mAttachments.remove(pAttach);
 
-    /* For non-hard disk media, detach straight away. */
-    if (mediumType != DeviceType_HardDisk && !oldmedium.isNull())
-        oldmedium->detachFrom(mData->mUuid);
+    if (!oldmedium.isNull())
+    {
+        // if this is from a snapshot, do not defer detachment to commitMedia()
+        if (pSnapshot)
+            oldmedium->detachFrom(mData->mUuid, pSnapshot->getId());
+        // else if non-hard disk media, do not defer detachment to commitMedia() either
+        else if (mediumType != DeviceType_HardDisk)
+            oldmedium->detachFrom(mData->mUuid);
+    }
+
+    return S_OK;
+}
+
+/**
+ * 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.
+ *
+ * This gets called from Machine::Unregister, both for the actual Machine and
+ * the SnapshotMachine objects that might be found in the snapshots.
+ *
+ * Requires caller and locking.
+ *
+ * @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.
+ * @return
+ */
+HRESULT Machine::detachAllMedia(AutoWriteLock &writeLock,
+                                Snapshot *pSnapshot,
+                                MediaList &llMedia)
+{
+    Assert(isWriteLockOnCurrentThread());
+
+    HRESULT rc;
+
+    // make a temporary list because detachDevice invalidates iterators into
+    // mMediaData->mAttachments
+    MediaData::AttachmentList llAttachments2 = mMediaData->mAttachments;
+
+    for (MediaData::AttachmentList::iterator it = llAttachments2.begin();
+         it != llAttachments2.end();
+         ++it)
+    {
+        ComObjPtr<MediumAttachment> pAttach = *it;
+        ComObjPtr<Medium> pMedium = pAttach->getMedium();
+
+        if (!pMedium.isNull())
+            llMedia.push_back(pMedium);
+
+        // real machine: then we need to use the proper method
+        rc = detachDevice(pAttach,
+                          writeLock,
+                          pSnapshot,
+                          NULL /* pfNeedsSaveSettings */);
+
+        if (FAILED(rc))
+            return rc;
+    }
 
     return S_OK;

trunk/src/VBox/Main/SnapshotImpl.cpp

@@ -251 +251 @@
 
         /* we've changed the base of the current state so mark it as
-            * modified as it no longer guaranteed to be its copy */
+         * modified as it no longer guaranteed to be its copy */
         m->pMachine->mData->mCurrentStateModified = TRUE;
     }
@@ -809 +809 @@
 
     return saveSnapshotImpl(data, aAttrsOnly);
+}
+
+/**
+ * Part of the cleanup engine of Machine::Unregister().
+ *
+ * This recursively removes all medium attachments from the snapshot's machine
+ * and returns the snapshot's saved state file name, if any, and then calls
+ * uninit() on "this" itself.
+ *
+ * This recurses into children first, so the given MediaList receives child
+ * media first before their parents. If the caller wants to close all media,
+ * they should go thru the list from the beginning to the end because media
+ * cannot be closed if they have children.
+ *
+ * This calls uninit() on itself, so the snapshots tree becomes invalid after this.
+ * It does not alter the main machine's snapshot pointers (pFirstSnapshot, pCurrentSnapshot).
+ *
+ * Caller must hold the machine write lock (which protects the snapshots tree!)
+ *
+ * @param llFilenames
+ * @return
+ */
+HRESULT Snapshot::uninitRecursively(AutoWriteLock &writeLock,
+                                    MediaList &llMedia,
+                                    std::list<Utf8Str> &llFilenames)
+{
+    Assert(m->pMachine->isWriteLockOnCurrentThread());
+
+    HRESULT rc = S_OK;
+
+    // recurse into children first so that the child media appear on
+    // the list first; this way caller can close the media from the
+    // beginning to the end because parent media can't be closed if
+    // they have children
+
+    // make a copy of the children list since uninit() modifies it
+    SnapshotsList llChildrenCopy(m->llChildren);
+    for (SnapshotsList::iterator it = llChildrenCopy.begin();
+         it != llChildrenCopy.end();
+         ++it)
+    {
+        Snapshot *pChild = *it;
+        rc = pChild->uninitRecursively(writeLock, llMedia, llFilenames);
+        if (FAILED(rc))
+            return rc;
+    }
+
+    // now call detachAllMedia on the snapshot machine
+    rc = m->pMachine->detachAllMedia(writeLock,
+                                     this /* pSnapshot */,
+                                     llMedia);
+    if (FAILED(rc))
+        return rc;
+
+    // now report the saved state file
+    if (!m->pMachine->mSSData->mStateFilePath.isEmpty())
+        llFilenames.push_back(m->pMachine->mSSData->mStateFilePath);
+
+    this->beginSnapshotDelete();
+    this->uninit();
+
+    return S_OK;
 }
 

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

@@ -778 +778 @@
     HRESULT detachDevice(MediumAttachment *pAttach,
                          AutoWriteLock &writeLock,
+                         Snapshot *pSnapshot,
                          bool *pfNeedsSaveSettings);
+    HRESULT detachAllMedia(AutoWriteLock &writeLock,
+                           Snapshot *pSnapshot,
+                           MediaList &llMedia);
 
     void commitMedia(bool aOnline = false);

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

@@ -33 +33 @@
 
 class ATL_NO_VTABLE Snapshot :
-    public VirtualBoxBase, // WithTypedChildren<Snapshot>,
+    public VirtualBoxBase,
     VBOX_SCRIPTABLE_IMPL(ISnapshot)
 {
@@ -124 +124 @@
     HRESULT saveSnapshotImpl(settings::Snapshot &data, bool aAttrsOnly);
 
+    HRESULT uninitRecursively(AutoWriteLock &writeLock,
+                              MediaList &llMedia,
+                              std::list<Utf8Str> &llFilenames);
+
 private:
     struct Data;            // opaque, defined in SnapshotImpl.cpp