Changeset 24345 in vbox for trunk/src/VBox

Timestamp:

Nov 4, 2009 4:50:07 PM (15 years ago)

Author:

vboxsync

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

54367

Message:

Main: make backrefs code a bit more readable + add backrefs logging, fix deleteSnapshot() progress bars, but deleteSnapshot() is still broken

Location:

trunk/src/VBox/Main

Files:

• MachineImpl.cpp (modified) (7 diffs)
• MediumImpl.cpp (modified) (15 diffs)
• SnapshotImpl.cpp (modified) (10 diffs)
• VirtualBoxImpl.cpp (modified) (3 diffs)
• include/MediumImpl.h (modified) (2 diffs)
• include/VirtualBoxImpl.h (modified) (2 diffs)

trunk/src/VBox/Main/MachineImpl.cpp

@@ -710 +710 @@
     CheckComRCReturnRC(autoCaller.rc());
 
+    LogFlowThisFunc(("ENTER\n"));
+
     AutoWriteLock alock(this);
 
@@ -720 +722 @@
         AutoReinitSpan autoReinitSpan(this);
         AssertReturn(autoReinitSpan.isOk(), E_FAIL);
+
+#ifdef DEBUG
+        LogFlowThisFunc(("Dumping media backreferences\n"));
+        mParent->dumpAllBackRefs();
+#endif
 
         if (mData->m_pMachineConfigFile)
@@ -744 +751 @@
     if (SUCCEEDED(rc))
         *aAccessible = mData->mAccessible;
+
+    LogFlowThisFuncLeave();
 
     return rc;
@@ -5508 +5517 @@
                                 data.uuid,
                                 strStateFile);
-    CheckComRCReturnRC (rc);
+    CheckComRCReturnRC(rc);
 
     /* create a snapshot object */
@@ -5543 +5552 @@
                           aCurSnapshotId,
                           pSnapshot);       // parent = the one we created above
-        CheckComRCBreakRC(rc);
+        CheckComRCReturnRC(rc);
     }
 
@@ -5973 +5982 @@
         }
 
-        if (rc)
+        if (FAILED(rc))
             break;
 
@@ -5994 +6003 @@
                 rc = medium->attachTo(mData->mUuid);
         }
-        AssertComRCBreakRC (rc);
+        if (FAILED(rc))
+            break;
 
         /* backup mMediaData to let registeredInit() properly rollback on failure

trunk/src/VBox/Main/MediumImpl.cpp

@@ -68 +68 @@
     typedef std::list<Guid> GuidList;
 
-    BackRef() : inCurState(false) {}
-
-    BackRef(const Guid &aMachineId, const Guid &aSnapshotId = Guid::Empty)
-        : machineId(aMachineId)
-        , inCurState(aSnapshotId.isEmpty())
+    BackRef(const Guid &aMachineId,
+            const Guid &aSnapshotId = Guid::Empty)
+        : machineId(aMachineId),
+          inCurState(aSnapshotId.isEmpty())
     {
         if (!aSnapshotId.isEmpty())
-            snapshotIds.push_back(aSnapshotId);
+            llSnapshotIds.push_back(aSnapshotId);
     }
 
     Guid machineId;
     bool inCurState : 1;
-    GuidList snapshotIds;
+    GuidList llSnapshotIds;
 };
 
@@ -1790 +1789 @@
         if (it->machineId == id)
         {
-            size_t size = it->snapshotIds.size();
+            size_t size = it->llSnapshotIds.size();
 
             /* if the medium is attached to the machine in the current state, we
@@ -1805 +1804 @@
                     it->machineId.toUtf16().detachTo(&snapshotIds [j ++]);
 
-                for (BackRef::GuidList::const_iterator jt =
-                        it->snapshotIds.begin();
-                     jt != it->snapshotIds.end(); ++ jt, ++ j)
+                for (BackRef::GuidList::const_iterator jt = it->llSnapshotIds.begin();
+                     jt != it->llSnapshotIds.end();
+                     ++jt, ++j)
                 {
                      (*jt).toUtf16().detachTo(&snapshotIds [j]);
@@ -2653 +2652 @@
     AssertReturn(!aMachineId.isEmpty(), E_FAIL);
 
+    LogFlowThisFunc(("ENTER, aMachineId: {%RTuuid}, aSnapshotId: {%RTuuid}\n", aMachineId.raw(), aSnapshotId.raw()));
+
     AutoCaller autoCaller(this);
     AssertComRCReturnRC(autoCaller.rc());
@@ -2672 +2673 @@
     if (m->numCreateDiffTasks > 0)
         return setError(E_FAIL,
-                        tr("One or more differencing child hard disks are being created for the hard disk '%s' (%u)"),
-                        m->strLocationFull.raw(), m->numCreateDiffTasks);
+                        tr("Cannot attach hard disk '%s' {%RTuuid}: %u differencing child hard disk(s) are being created"),
+                        m->strLocationFull.raw(),
+                        m->id.raw(),
+                        m->numCreateDiffTasks);
 
     BackRefList::iterator it =
@@ -2696 +2699 @@
 
     /* sanity: no duplicate attachments */
-    BackRef::GuidList::const_iterator jt =
-        std::find(it->snapshotIds.begin(), it->snapshotIds.end(), aSnapshotId);
-    AssertReturn(jt == it->snapshotIds.end(), E_FAIL);
-
-    it->snapshotIds.push_back(aSnapshotId);
+    for (BackRef::GuidList::const_iterator jt = it->llSnapshotIds.begin();
+         jt != it->llSnapshotIds.end();
+         ++jt)
+    {
+        const Guid &idOldSnapshot = *jt;
+
+        if (idOldSnapshot == aSnapshotId)
+            return setError(E_FAIL,
+                            tr("Cannot attach medium '%s' {%RTuuid} from snapshot '%RTuuid': medium is already in use by snapshot '%RTuuid'"),
+                            m->strLocationFull.raw(),
+                            m->id.raw(),
+                            aSnapshotId.raw(),
+                            idOldSnapshot.raw());
+    }
+
+    it->llSnapshotIds.push_back(aSnapshotId);
+
+    LogFlowThisFuncLeave();
 
     return S_OK;
@@ -2737 +2753 @@
         /* remove the snapshot attachment */
         BackRef::GuidList::iterator jt =
-            std::find(it->snapshotIds.begin(), it->snapshotIds.end(), aSnapshotId);
-
-        AssertReturn(jt != it->snapshotIds.end(), E_FAIL);
-        it->snapshotIds.erase(jt);
+            std::find(it->llSnapshotIds.begin(), it->llSnapshotIds.end(), aSnapshotId);
+
+        AssertReturn(jt != it->llSnapshotIds.end(), E_FAIL);
+        it->llSnapshotIds.erase(jt);
     }
 
     /* if the backref becomes empty, remove it */
-    if (it->inCurState == false && it->snapshotIds.size() == 0)
+    if (it->inCurState == false && it->llSnapshotIds.size() == 0)
         m->backRefs.erase(it);
 
     return S_OK;
 }
+
+#ifdef DEBUG
+/**
+ * Debugging helper that gets called after VirtualBox initialization that writes all
+ * machine backreferences to the debug log.
+ */
+void Medium::dumpBackRefs()
+{
+    AutoCaller autoCaller(this);
+    AutoReadLock(this);
+
+    LogFlowThisFunc(("Dumping backrefs for medium '%s':\n", m->strLocationFull.raw()));
+
+    for (BackRefList::iterator it2 = m->backRefs.begin();
+         it2 != m->backRefs.end();
+         ++it2)
+    {
+        const BackRef &ref = *it2;
+        LogFlowThisFunc(("  Backref from machine {%RTuuid}\n", ref.machineId.raw()));
+
+        for (BackRef::GuidList::const_iterator jt2 = it2->llSnapshotIds.begin();
+             jt2 != it2->llSnapshotIds.end();
+             ++jt2)
+        {
+            const Guid &id = *jt2;
+            LogFlowThisFunc(("  Backref from snapshot {%RTuuid}\n", id.raw()));
+        }
+    }
+}
+#endif
 
 /**
@@ -2787 +2833 @@
 
 /**
+ * Internal method to return the medium's size. Must have caller + locking!
+ * @return
+ */
+uint64_t Medium::size() const
+{
+    return m->size;
+}
+
+/**
  * Internal method to return the medium's list of backrefs. Must have caller + locking!
  * @return
  */
-// const Medium::BackRefList& Medium::backRefs() const
-// {
-//     return m->backRefs;
-// }
-
 const Guid* Medium::getFirstMachineBackrefId() const
 {
@@ -2809 +2859 @@
 
     const BackRef &ref = m->backRefs.front();
-    if (!ref.snapshotIds.size())
+    if (!ref.llSnapshotIds.size())
         return NULL;
 
-    return &ref.snapshotIds.front();
+    return &ref.llSnapshotIds.front();
 }
 
@@ -2934 +2984 @@
             for (BackRefList::const_iterator it = m->backRefs.begin();
                  it != m->backRefs.end(); ++ it)
-                if (it->snapshotIds.size() != 0)
+                if (it->llSnapshotIds.size() != 0)
                     return true;
 
@@ -3139 +3189 @@
      * time when discard() is called, there must be no any attachments at all
      * (the code calling prepareDiscard() should detach). */
-    AssertReturn(m->backRefs.size() == 1 &&
-                  !m->backRefs.front().inCurState &&
-                  m->backRefs.front().snapshotIds.size() == 1, E_FAIL);
+    AssertReturn(    m->backRefs.size() == 1
+                  && !m->backRefs.front().inCurState
+                  && m->backRefs.front().llSnapshotIds.size() == 1,
+                 E_FAIL);
 
     ComObjPtr<Medium> child = children().front();
@@ -3215 +3266 @@
  *       reading or writing.
  */
-HRESULT Medium::discard(ComObjPtr<Progress> &aProgress, MergeChain *aChain)
+HRESULT Medium::discard(ComObjPtr<Progress> &aProgress, ULONG ulWeight, MergeChain *aChain)
 {
     AssertReturn(!aProgress.isNull(), E_FAIL);
@@ -3228 +3279 @@
 
         aProgress->SetNextOperation(BstrFmt(tr("Discarding hard disk '%s'"), name().raw()),
-                                    1);        // weight
+                                    ulWeight);        // weight
 
         if (aChain == NULL)
@@ -4175 +4226 @@
              * this legal situatinon and do not report an error. */
 
-            if (it->snapshotIds.size() == 0)
+            if (it->llSnapshotIds.size() == 0)
             {
                 return setError(VBOX_E_INVALID_OBJECT_STATE,
@@ -4182 +4233 @@
             }
 
-            Assert(it->snapshotIds.size() == 1);
+            Assert(it->llSnapshotIds.size() == 1);
         }
     }

trunk/src/VBox/Main/SnapshotImpl.cpp

@@ -1506 +1506 @@
 
     ComObjPtr<Snapshot> pSnapshot(static_cast<Snapshot*>(aSnapshot));
+    ComPtr<SnapshotMachine> pSnapMachine = pSnapshot->getSnapshotMachine();
 
     // create a progress object. The number of operations is:
     // 1 (preparing) + # of hard disks + 1 (if we need to copy the saved state file) */
-    ComObjPtr<Progress> progress;
-    progress.createObject();
-
     LogFlowThisFunc(("Going thru snapshot machine attachments to determine progress setup\n"));
 
     ULONG ulOpCount = 1;            // one for preparations
     ULONG ulTotalWeight = 1;        // one for preparations
-    for (MediaData::AttachmentList::iterator it = pSnapshot->getSnapshotMachine()->mMediaData->mAttachments.begin();
-         it != pSnapshot->getSnapshotMachine()->mMediaData->mAttachments.end();
+    for (MediaData::AttachmentList::iterator it = pSnapMachine->mMediaData->mAttachments.begin();
+         it != pSnapMachine->mMediaData->mAttachments.end();
          ++it)
     {
@@ -1551 +1549 @@
     }
 
-    progress->init(mParent, aInitiator,
-                   Bstr(tr("Restoring snapshot")),
-                   FALSE /* aCancelable */,
-                   ulOpCount,
-                   ulTotalWeight,
-                   Bstr(tr("Restoring machine settings")),
-                   1);
+    ComObjPtr<Progress> pProgress;
+    pProgress.createObject();
+    pProgress->init(mParent, aInitiator,
+                    BstrFmt(tr("Restoring snapshot '%s'"), pSnapshot->getName().c_str()),
+                    FALSE /* aCancelable */,
+                    ulOpCount,
+                    ulTotalWeight,
+                    Bstr(tr("Restoring machine settings")),
+                    1);
 
     /* create and start the task on a separate thread (note that it will not
      * start working until we release alock) */
     RestoreSnapshotTask *task = new RestoreSnapshotTask(this,
-                                                        progress,
+                                                        pProgress,
                                                         pSnapshot,
                                                         ulStateFileSizeMB);
@@ -1582 +1582 @@
 
     /* return the progress to the caller */
-    progress.queryInterfaceTo(aProgress);
+    pProgress.queryInterfaceTo(aProgress);
 
     /* return the new state to the caller */
@@ -1909 +1909 @@
     AutoWriteLock treeLock(snapshotsTreeLockHandle());
 
-    ComObjPtr<Snapshot> snapshot;
-    HRESULT rc = findSnapshot(id, snapshot, true /* aSetError */);
+    ComObjPtr<Snapshot> pSnapshot;
+    HRESULT rc = findSnapshot(id, pSnapshot, true /* aSetError */);
     CheckComRCReturnRC(rc);
 
-    AutoWriteLock snapshotLock(snapshot);
-
-    size_t childrenCount = snapshot->getChildrenCount();
+    AutoWriteLock snapshotLock(pSnapshot);
+
+    size_t childrenCount = pSnapshot->getChildrenCount();
     if (childrenCount > 1)
         return setError(VBOX_E_INVALID_OBJECT_STATE,
                         tr("Snapshot '%s' of the machine '%ls' cannot be deleted. because it has %d child snapshots, which is more than the one snapshot allowed for deletion"),
-                        snapshot->getName().c_str(),
+                        pSnapshot->getName().c_str(),
                         mUserData->mName.raw(),
                         childrenCount);
@@ -1926 +1926 @@
      * settings are committed and saved.
      */
-    if (snapshot == mData->mCurrentSnapshot)
+    if (pSnapshot == mData->mCurrentSnapshot)
     {
         if (isModified())
@@ -1935 +1935 @@
     }
 
+    ComPtr<SnapshotMachine> pSnapMachine = pSnapshot->getSnapshotMachine();
+
     /* create a progress object. The number of operations is:
-     *   1 (preparing) + # of hard disks + 1 if the snapshot is online
+     *   1 (preparing) + 1 if the snapshot is online + # of normal hard disks
      */
-    ComObjPtr<Progress> progress;
-    progress.createObject();
-    rc = progress->init(mParent, aInitiator,
-                        BstrFmt(tr("Discarding snapshot '%s'"),
-                                snapshot->getName().c_str()),
-                        FALSE /* aCancelable */,
-                        1 + (ULONG)snapshot->getSnapshotMachine()->mMediaData->mAttachments.size()
-                          + (snapshot->stateFilePath().length() ? 1 : 0),
-                        Bstr(tr("Preparing to discard snapshot")));
-    AssertComRCReturn(rc, rc);
+    LogFlowThisFunc(("Going thru snapshot machine attachments to determine progress setup\n"));
+
+    ULONG ulOpCount = 1;            // one for preparations
+    ULONG ulTotalWeight = 1;        // one for preparations
+
+    if (pSnapshot->stateFilePath().length())
+    {
+        ++ulOpCount;
+        ++ulTotalWeight;            // assume 1 MB for deleting the state file
+    }
+
+    // count normal hard disks and add their sizes to the weight
+    for (MediaData::AttachmentList::iterator it = pSnapMachine->mMediaData->mAttachments.begin();
+         it != pSnapMachine->mMediaData->mAttachments.end();
+         ++it)
+    {
+        ComObjPtr<MediumAttachment> &pAttach = *it;
+        AutoReadLock attachLock(pAttach);
+        if (pAttach->type() == DeviceType_HardDisk)
+        {
+            Assert(pAttach->medium());
+            ComObjPtr<Medium> pHD = pAttach->medium();
+            AutoReadLock mlock(pHD);
+            if (pHD->type() == MediumType_Normal)
+            {
+                ++ulOpCount;
+                ulTotalWeight += pHD->size() / _1M;
+            }
+            LogFlowThisFunc(("op %d: considering hard disk attachment %s\n", ulOpCount, pHD->name().c_str()));
+        }
+    }
+
+    ComObjPtr<Progress> pProgress;
+    pProgress.createObject();
+    pProgress->init(mParent, aInitiator,
+                    BstrFmt(tr("Deleting snapshot '%s'"), pSnapshot->getName().c_str()),
+                    FALSE /* aCancelable */,
+                    ulOpCount,
+                    ulTotalWeight,
+                    Bstr(tr("Setting up")),
+                    1);
 
     /* create and start the task on a separate thread */
-    DeleteSnapshotTask *task = new DeleteSnapshotTask(this, progress, snapshot);
+    DeleteSnapshotTask *task = new DeleteSnapshotTask(this, pProgress, pSnapshot);
     int vrc = RTThreadCreate(NULL,
                              taskHandler,
@@ -1968 +2001 @@
 
     /* return the progress to the caller */
-    progress.queryInterfaceTo(aProgress);
+    pProgress.queryInterfaceTo(aProgress);
 
     /* return the new state to the caller */
@@ -2051 +2084 @@
                               aTask.pSnapshot->lockHandle());
 
-    ComPtr<SnapshotMachine> sm = aTask.pSnapshot->getSnapshotMachine();
+    ComPtr<SnapshotMachine> pSnapMachine = aTask.pSnapshot->getSnapshotMachine();
     /* no need to lock the snapshot machine since it is const by definiton */
 
@@ -2068 +2101 @@
         LogFlowThisFunc(("1: Checking hard disk merge prerequisites...\n"));
 
-        for (MediaData::AttachmentList::const_iterator it = sm->mMediaData->mAttachments.begin();
-             it != sm->mMediaData->mAttachments.end();
-             ++it)
+        for (MediaData::AttachmentList::iterator it = pSnapMachine->mMediaData->mAttachments.begin();
+            it != pSnapMachine->mMediaData->mAttachments.end();
+            ++it)
         {
-            ComObjPtr<MediumAttachment> hda = *it;
-            ComObjPtr<Medium> hd = hda->medium();
-
-            // medium can be NULL only for non-hard-disk types
-            Assert(    !hd.isNull()
-                    || hda->type() != DeviceType_HardDisk);
-            if (hd.isNull())
-                continue;
-
-            /* Medium::prepareDiscard() reqiuires a write lock */
-            AutoWriteLock hdLock(hd);
-
-            if (hd->type() != MediumType_Normal)
+            ComObjPtr<MediumAttachment> &pAttach = *it;
+            AutoReadLock attachLock(pAttach);
+            if (pAttach->type() == DeviceType_HardDisk)
             {
-                /* skip writethrough hard disks */
-                Assert(hd->type() == MediumType_Writethrough);
-                rc = aTask.pProgress->SetNextOperation(BstrFmt(tr("Skipping writethrough hard disk '%s'"),
-                                                               hd->base()->name().raw()),
-                                                       1); // weight
+                Assert(pAttach->medium());
+                ComObjPtr<Medium> pHD = pAttach->medium();
+                AutoReadLock mlock(pHD);
+
+                Medium::MergeChain *chain = NULL;
+
+                /* needs to be discarded (merged with the child if any), check
+                * prerequisites */
+                rc = pHD->prepareDiscard(chain);
                 CheckComRCThrowRC(rc);
-                continue;
+
+                if (pHD->parent().isNull() && chain != NULL)
+                {
+                    /* it's a base hard disk so it will be a backward merge of its
+                    * only child to it (prepareDiscard() does necessary checks). We
+                    * need then to update the attachment that refers to the child
+                    * to refer to the parent instead. Don't forget to detach the
+                    * child (otherwise mergeTo() called by discard() will assert
+                    * because it will be going to delete the child) */
+
+                    /* The below assert would be nice but I don't want to move
+                    * Medium::MergeChain to the header just for that
+                    * Assert (!chain->isForward()); */
+
+                    Assert(pHD->children().size() == 1);
+
+                    ComObjPtr<Medium> replaceHd = pHD->children().front();
+
+                    const Guid *pReplaceMachineId = replaceHd->getFirstMachineBackrefId();
+                    Assert(pReplaceMachineId  && *pReplaceMachineId == mData->mUuid);
+
+                    Guid snapshotId;
+                    const Guid *pSnapshotId = replaceHd->getFirstMachineBackrefSnapshotId();
+                    if (pSnapshotId)
+                        snapshotId = *pSnapshotId;
+
+                    HRESULT rc2 = S_OK;
+
+                    /* adjust back references */
+                    rc2 = replaceHd->detachFrom(mData->mUuid, snapshotId);
+                    AssertComRC(rc2);
+
+                    rc2 = pHD->attachTo(mData->mUuid, snapshotId);
+                    AssertComRC(rc2);
+
+                    /* replace the hard disk in the attachment object */
+                    if (snapshotId.isEmpty())
+                    {
+                        /* in current state */
+                        AssertBreak(pAttach = findAttachment(mMediaData->mAttachments, replaceHd));
+                    }
+                    else
+                    {
+                        /* in snapshot */
+                        ComObjPtr<Snapshot> snapshot;
+                        rc2 = findSnapshot(snapshotId, snapshot);
+                        AssertComRC(rc2);
+
+                        /* don't lock the snapshot; cannot be modified outside */
+                        MediaData::AttachmentList &snapAtts = snapshot->getSnapshotMachine()->mMediaData->mAttachments;
+                        AssertBreak(pAttach = findAttachment(snapAtts, replaceHd));
+                    }
+
+                    attachLock.unlock();
+                    AutoWriteLock attLock(pAttach);
+                    pAttach->updateMedium(pHD, false /* aImplicit */);
+
+                    toDiscard.push_back(MediumDiscardRec(pHD,
+                                                         chain,
+                                                         replaceHd,
+                                                         pAttach,
+                                                         snapshotId));
+                    continue;
+                }
+
+                toDiscard.push_back(MediumDiscardRec(pHD, chain));
             }
-
-            Medium::MergeChain *chain = NULL;
-
-            /* needs to be discarded (merged with the child if any), check
-             * prerequisites */
-            rc = hd->prepareDiscard(chain);
-            CheckComRCThrowRC(rc);
-
-            if (hd->parent().isNull() && chain != NULL)
-            {
-                /* it's a base hard disk so it will be a backward merge of its
-                 * only child to it (prepareDiscard() does necessary checks). We
-                 * need then to update the attachment that refers to the child
-                 * to refer to the parent instead. Don't forget to detach the
-                 * child (otherwise mergeTo() called by discard() will assert
-                 * because it will be going to delete the child) */
-
-                /* The below assert would be nice but I don't want to move
-                 * Medium::MergeChain to the header just for that
-                 * Assert (!chain->isForward()); */
-
-                Assert(hd->children().size() == 1);
-
-                ComObjPtr<Medium> replaceHd = hd->children().front();
-
-                const Guid *pReplaceMachineId = replaceHd->getFirstMachineBackrefId();
-                Assert(pReplaceMachineId  && *pReplaceMachineId == mData->mUuid);
-
-                Guid snapshotId;
-                const Guid *pSnapshotId = replaceHd->getFirstMachineBackrefSnapshotId();
-                if (pSnapshotId)
-                    snapshotId = *pSnapshotId;
-
-                HRESULT rc2 = S_OK;
-
-                /* adjust back references */
-                rc2 = replaceHd->detachFrom (mData->mUuid, snapshotId);
-                AssertComRC(rc2);
-
-                rc2 = hd->attachTo (mData->mUuid, snapshotId);
-                AssertComRC(rc2);
-
-                /* replace the hard disk in the attachment object */
-                if (snapshotId.isEmpty())
-                {
-                    /* in current state */
-                    AssertBreak(hda = findAttachment(mMediaData->mAttachments, replaceHd));
-                }
-                else
-                {
-                    /* in snapshot */
-                    ComObjPtr<Snapshot> snapshot;
-                    rc2 = findSnapshot(snapshotId, snapshot);
-                    AssertComRC(rc2);
-
-                    /* don't lock the snapshot; cannot be modified outside */
-                    MediaData::AttachmentList &snapAtts = snapshot->getSnapshotMachine()->mMediaData->mAttachments;
-                    AssertBreak(hda = findAttachment(snapAtts, replaceHd));
-                }
-
-                AutoWriteLock attLock(hda);
-                hda->updateMedium(hd, false /* aImplicit */);
-
-                toDiscard.push_back(MediumDiscardRec(hd,
-                                                     chain,
-                                                     replaceHd,
-                                                     hda,
-                                                     snapshotId));
-                continue;
-            }
-
-            toDiscard.push_back(MediumDiscardRec(hd, chain));
         }
 
@@ -2229 +2249 @@
              it != toDiscard.end();)
         {
-            rc = it->hd->discard(aTask.pProgress, it->chain);
+            rc = it->hd->discard(aTask.pProgress,
+                                 it->hd->size() / _1M,          // weight
+                                 it->chain);
             CheckComRCBreakRC(rc);
 
             /* prevent from calling cancelDiscard() */
-            it = toDiscard.erase (it);
+            it = toDiscard.erase(it);
         }
 
+        LogFlowThisFunc(("Entering locks again...\n"));
         alock.enter();
+        LogFlowThisFunc(("Entered locks OK\n"));
 
         CheckComRCThrowRC(rc);

trunk/src/VBox/Main/VirtualBoxImpl.cpp

@@ -389 +389 @@
             throw rc;
 
+
+#ifdef DEBUG
+        LogFlowThisFunc(("Dumping media backreferences\n"));
+        dumpAllBackRefs();
+#endif
+
         /* net services */
         for (settings::DHCPServersList::const_iterator it = m->pMainConfigFile->llDhcpServers.begin();
@@ -1908 +1914 @@
 // public methods only for internal purposes
 /////////////////////////////////////////////////////////////////////////////
+
+#ifdef DEBUG
+void VirtualBox::dumpAllBackRefs()
+{
+    for (HardDiskList::const_iterator mt = m->llHardDisks.begin();
+         mt != m->llHardDisks.end();
+         ++mt)
+    {
+        ComObjPtr<Medium> pMedium = *mt;
+        pMedium->dumpBackRefs();
+    }
+    for (DVDImageList::const_iterator mt = m->llDVDImages.begin();
+         mt != m->llDVDImages.end();
+         ++mt)
+    {
+        ComObjPtr<Medium> pMedium = *mt;
+        pMedium->dumpBackRefs();
+    }
+}
+#endif
 
 /**
@@ -3274 +3300 @@
  *
  * @param aHardDisk     Hard disk object to remember.
- * @param aSaveRegistry @c true to save hard disk registry to disk (default).
+ * @param aSaveRegistry Whether to save the registry after adding the new disk; this is @c false when this gets called during VirtualBox initialization.
  *
  * When @a aSaveRegistry is @c true, this operation may fail because of the

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

@@ -163 +163 @@
                        const Guid &aSnapshotId = Guid::Empty);
 
+#ifdef DEBUG
+    void dumpBackRefs();
+#endif
+
     const Guid& id() const;
     MediumState_T state() const;
     const Utf8Str& location() const;
     const Utf8Str& locationFull() const;
+    uint64_t size() const;
 
     const Guid* getFirstMachineBackrefId() const;
@@ -244 +249 @@
 
     HRESULT prepareDiscard(MergeChain * &aChain);
-    HRESULT discard(ComObjPtr<Progress> &aProgress, MergeChain *aChain);
+    HRESULT discard(ComObjPtr<Progress> &aProgress, ULONG ulWeight, MergeChain *aChain);
     void cancelDiscard(MergeChain *aChain);
 

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

@@ -78 +78 @@
     friend class CallbackEvent;
 
-    VIRTUALBOXBASE_ADD_ERRORINFO_SUPPORT (VirtualBox)
+    VIRTUALBOXBASE_ADD_ERRORINFO_SUPPORT(VirtualBox)
 
     DECLARE_CLASSFACTORY_SINGLETON(VirtualBox)
@@ -179 +179 @@
 
     /* public methods only for internal purposes */
+#ifdef DEBUG
+    void dumpAllBackRefs();
+#endif
 
     HRESULT postEvent(Event *event);