Changeset 28401 in vbox

Timestamp:

Apr 16, 2010 9:14:54 AM (15 years ago)

Author:

vboxsync

Message:

Main/Machine+Snapshot+Medium: Big medium locking cleanup and straightened up the responsibilities between Snapshot and Medium. Rewritten task handling and cleaned up task implementation for medium operations. Implemented IMedium::mergeTo (no way to call it via any frontend yet), making the method parameters similar to IMedium::cloneto. Plus lots of other minor cleanups.

Location:

trunk/src/VBox/Main

Files:

• MachineImpl.cpp (modified) (36 diffs)
• MediumImpl.cpp (modified) (83 diffs)
• SnapshotImpl.cpp (modified) (31 diffs)
• idl/VirtualBox.xidl (modified) (2 diffs)
• include/Logging.h (modified) (3 diffs)
• include/MachineImpl.h (modified) (5 diffs)
• include/MediumImpl.h (modified) (9 diffs, 1 prop)
• include/SnapshotImpl.h (modified) (3 diffs)

trunk/src/VBox/Main/MachineImpl.cpp

@@ -37 +37 @@
 #endif
 
+#include "Logging.h"
 #include "VirtualBoxImpl.h"
 #include "MachineImpl.h"
@@ -42 +43 @@
 #include "MediumAttachmentImpl.h"
 #include "MediumImpl.h"
+#include "MediumLock.h"
 #include "USBControllerImpl.h"
 #include "HostImpl.h"
@@ -55 +57 @@
 
 #include "AutoCaller.h"
-#include "Logging.h"
 #include "Performance.h"
 
@@ -2076 +2077 @@
     AutoCaller autoCaller(this);
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
-    MultiResult rc (S_OK);
+    MultiResult rc(S_OK);
 
 # ifdef VBOX_WITH_USB
@@ -3141 +3142 @@
         if (FAILED(rc)) return rc;
 
-        /* make sure the hard disk is not modified before createDiffStorage() */
-        rc = medium->LockRead(NULL);
+        /* Apply the normal locking logic to the entire chain. */
+        MediumLockList *pMediumLockList(new MediumLockList());
+        rc = diff->createMediumLockList(true, medium, *pMediumLockList);
         if (FAILED(rc)) return rc;
+        rc = pMediumLockList->Lock();
+        if (FAILED(rc))
+            return setError(rc,
+                            tr("Could not lock medium when creating diff '%s'"),
+                            diff->getLocationFull().c_str());
 
         /* will leave the lock before the potentially lengthy operation, so
@@ -3153 +3160 @@
         alock.leave();
 
-        rc = medium->createDiffStorageAndWait(diff, MediumVariant_Standard, &fNeedsSaveSettings);
+        rc = medium->createDiffStorage(diff, MediumVariant_Standard,
+                                       pMediumLockList, NULL /* aProgress */,
+                                       true /* aWait */, &fNeedsSaveSettings);
 
         alock.enter();
@@ -3160 +3169 @@
         setMachineState(oldState);
 
-        medium->UnlockRead(NULL);
+        /* Unlock the media and free the associated memory. */
+        delete pMediumLockList;
 
         if (FAILED(rc)) return rc;
@@ -3257 +3267 @@
         alock.leave();
 
-        rc = oldmedium->deleteStorageAndWait(NULL /*aProgress*/, &fNeedsSaveSettings);
+        rc = oldmedium->deleteStorage(NULL /*aProgress*/, true /*aWait*/,
+                                      &fNeedsSaveSettings);
 
         alock.enter();
@@ -4597 +4608 @@
 
     PSSMHANDLE pSSM;
-    int rc = SSMR3Open(pStateFilePath->raw(), 0 /*fFlags*/, &pSSM);
-    if (RT_SUCCESS(rc))
+    int vrc = SSMR3Open(pStateFilePath->raw(), 0 /*fFlags*/, &pSSM);
+    if (RT_SUCCESS(vrc))
     {
         uint32_t uVersion;
-        rc = SSMR3Seek(pSSM, "DisplayScreenshot", 1100 /*iInstance*/, &uVersion);
-        if (RT_SUCCESS(rc))
+        vrc = SSMR3Seek(pSSM, "DisplayScreenshot", 1100 /*iInstance*/, &uVersion);
+        if (RT_SUCCESS(vrc))
         {
             if (uVersion == sSSMDisplayScreenshotVer)
             {
                 uint32_t cBlocks;
-                rc = SSMR3GetU32(pSSM, &cBlocks);
-                AssertRCReturn(rc, rc);
+                vrc = SSMR3GetU32(pSSM, &cBlocks);
+                AssertRCReturn(vrc, vrc);
 
                 for (uint32_t i = 0; i < cBlocks; i++)
                 {
                     uint32_t cbBlock;
-                    rc = SSMR3GetU32(pSSM, &cbBlock);
-                    AssertRCBreak(rc);
+                    vrc = SSMR3GetU32(pSSM, &cbBlock);
+                    AssertRCBreak(vrc);
 
                     uint32_t typeOfBlock;
-                    rc = SSMR3GetU32(pSSM, &typeOfBlock);
-                    AssertRCBreak(rc);
+                    vrc = SSMR3GetU32(pSSM, &typeOfBlock);
+                    AssertRCBreak(vrc);
 
                     LogFlowFunc(("[%d] type %d, size %d bytes\n", i, typeOfBlock, cbBlock));
@@ -4630 +4641 @@
                             if (pu8Data == NULL)
                             {
-                                rc = VERR_NO_MEMORY;
+                                vrc = VERR_NO_MEMORY;
                                 break;
                             }
 
-                            rc = SSMR3GetU32(pSSM, &u32Width);
-                            AssertRCBreak(rc);
-                            rc = SSMR3GetU32(pSSM, &u32Height);
-                            AssertRCBreak(rc);
-                            rc = SSMR3GetMem(pSSM, pu8Data, cbData);
-                            AssertRCBreak(rc);
+                            vrc = SSMR3GetU32(pSSM, &u32Width);
+                            AssertRCBreak(vrc);
+                            vrc = SSMR3GetU32(pSSM, &u32Height);
+                            AssertRCBreak(vrc);
+                            vrc = SSMR3GetMem(pSSM, pu8Data, cbData);
+                            AssertRCBreak(vrc);
                         }
                         else
                         {
                             /* No saved state data. */
-                            rc = VERR_NOT_SUPPORTED;
+                            vrc = VERR_NOT_SUPPORTED;
                         }
 
@@ -4656 +4667 @@
                         if (cbBlock > 2 * sizeof (uint32_t))
                         {
-                            rc = SSMR3Skip(pSSM, cbBlock);
-                            AssertRCBreak(rc);
+                            vrc = SSMR3Skip(pSSM, cbBlock);
+                            AssertRCBreak(vrc);
                         }
                     }
@@ -4664 +4675 @@
             else
             {
-                rc = VERR_SSM_UNSUPPORTED_DATA_UNIT_VERSION;
+                vrc = VERR_SSM_UNSUPPORTED_DATA_UNIT_VERSION;
             }
         }
@@ -4671 +4682 @@
     }
 
-    if (RT_SUCCESS(rc))
+    if (RT_SUCCESS(vrc))
     {
         if (u32Type == 0 && cbData % 4 != 0)
         {
             /* Bitmap is 32bpp, so data is invalid. */
-            rc = VERR_SSM_UNEXPECTED_DATA;
-        }
-    }
-
-    if (RT_SUCCESS(rc))
+            vrc = VERR_SSM_UNEXPECTED_DATA;
+        }
+    }
+
+    if (RT_SUCCESS(vrc))
     {
         *ppu8Data = pu8Data;
@@ -4689 +4700 @@
     }
 
-    LogFlowFunc(("rc %Rrc\n", rc));
-    return rc;
+    LogFlowFunc(("vrc %Rrc\n", vrc));
+    return vrc;
 }
 
@@ -6334 +6345 @@
      * the VM after successfully re-checking the accessibility state. Note
      * that in case of normal Machine or SnapshotMachine uninitialization (as
-     * a result of unregistering or discarding the snapshot), outdated hard
+     * a result of unregistering or deleting the snapshot), outdated hard
      * disk attachments will already be uninitialized and deleted, so this
      * code will not affect them. */
@@ -8136 +8147 @@
  * machine and a new set of attachments to refer to created disks.
  *
- * Used when taking a snapshot or when discarding the current state.
+ * Used when taking a snapshot or when deleting the current state.
  *
  * This method assumes that mMediaData contains the original hard disk attachments
@@ -8189 +8200 @@
     HRESULT rc = S_OK;
 
-    MediaList lockedMedia;
+    MediumLockListMap lockedMediaOffline;
+    MediumLockListMap *lockedMediaMap;
+    if (aOnline)
+        lockedMediaMap = &mData->mSession.mLockedMedia;
+    else
+        lockedMediaMap = &lockedMediaOffline;
 
     try
@@ -8204 +8220 @@
                 if (pAtt->getType() == DeviceType_HardDisk)
                 {
-                    Medium* pHD = pAtt->getMedium();
-                    Assert(pHD);
-                    rc = pHD->LockRead(NULL);
-                    if (FAILED(rc)) throw rc;
-                    lockedMedia.push_back(pHD);
+                    Medium* pMedium = pAtt->getMedium();
+                    Assert(pMedium);
+
+                    MediumLockList *pMediumLockList(new MediumLockList());
+                    rc = pMedium->createMediumLockList(false, NULL,
+                                                       *pMediumLockList);
+                    if (FAILED(rc))
+                    {
+                        delete pMediumLockList;
+                        throw rc;
+                    }
+                    rc = lockedMediaMap->Insert(pAtt, pMediumLockList);
+                    if (FAILED(rc))
+                    {
+                        throw setError(rc,
+                                       tr("Collecting locking information for all attached media failed"));
+                    }
                 }
+            }
+
+            /* Now lock all media. If this fails, nothing is locked. */
+            rc = lockedMediaMap->Lock();
+            if (FAILED(rc))
+            {
+                throw setError(rc,
+                               tr("Locking of attached media failed"));
             }
         }
@@ -8229 +8265 @@
 
             DeviceType_T devType = pAtt->getType();
-            Medium* medium = pAtt->getMedium();
+            Medium* pMedium = pAtt->getMedium();
 
             if (   devType != DeviceType_HardDisk
-                || medium == NULL
-                || medium->getType() != MediumType_Normal)
+                || pMedium == NULL
+                || pMedium->getType() != MediumType_Normal)
             {
                 /* copy the attachment as is */
@@ -8242 +8278 @@
                 if (devType == DeviceType_HardDisk)
                 {
-                    if (medium == NULL)
+                    if (pMedium == NULL)
                         aProgress->SetNextOperation(Bstr(tr("Skipping attachment without medium")),
                                                     aWeight);        // weight
                     else
                         aProgress->SetNextOperation(BstrFmt(tr("Skipping medium '%s'"),
-                                                            medium->getBase()->getName().raw()),
+                                                            pMedium->getBase()->getName().raw()),
                                                     aWeight);        // weight
                 }
@@ -8257 +8293 @@
             /* need a diff */
             aProgress->SetNextOperation(BstrFmt(tr("Creating differencing hard disk for '%s'"),
-                                                medium->getBase()->getName().raw()),
+                                                pMedium->getBase()->getName().raw()),
                                         aWeight);        // weight
 
@@ -8263 +8299 @@
             diff.createObject();
             rc = diff->init(mParent,
-                            medium->preferredDiffFormat().raw(),
+                            pMedium->preferredDiffFormat().raw(),
                             BstrFmt("%ls"RTPATH_SLASH_STR,
                                     mUserData->mSnapshotFolderFull.raw()).raw(),
@@ -8269 +8305 @@
             if (FAILED(rc)) throw rc;
 
-            /* leave the lock before the potentially lengthy operation */
-            alock.leave();
-
-            rc = medium->createDiffStorageAndWait(diff,
-                                                  MediumVariant_Standard,
-                                                  pfNeedsSaveSettings);
-
             /** @todo r=bird: How is the locking and diff image cleaned up if we fail before
              *        the push_back?  Looks like we're going to leave medium with the
              *        wrong kind of lock (general issue with if we fail anywhere at all)
              *        and an orphaned VDI in the snapshots folder. */
-            // at this point, the old image is still locked for writing, but instead
-            // we need the new diff image locked for writing and lock the previously
-            // current one for reading only
+
+            /* update the appropriate lock list */
+            rc = lockedMediaMap->Unlock();
+            AssertComRCThrowRC(rc);
+            MediumLockList *pMediumLockList;
+            rc = lockedMediaMap->Get(pAtt, pMediumLockList);
+            AssertComRCThrowRC(rc);
             if (aOnline)
             {
-                diff->LockWrite(NULL);
-                mData->mSession.mLockedMedia.push_back(Data::Session::LockedMedia::value_type(ComPtr<IMedium>(diff), true));
-                medium->UnlockWrite(NULL);
-                medium->LockRead(NULL);
-                mData->mSession.mLockedMedia.push_back(Data::Session::LockedMedia::value_type(ComPtr<IMedium>(medium), false));
+                rc = pMediumLockList->Update(pMedium, false);
+                AssertComRCThrowRC(rc);
             }
-
+            rc = lockedMediaMap->Lock();
+            AssertComRCThrowRC(rc);
+
+            /* leave the lock before the potentially lengthy operation */
+            alock.leave();
+            rc = pMedium->createDiffStorage(diff, MediumVariant_Standard,
+                                            pMediumLockList,
+                                            NULL /* aProgress */,
+                                            true /* aWait */,
+                                            pfNeedsSaveSettings);
+            alock.enter();
             if (FAILED(rc)) throw rc;
 
-            alock.enter();
+            rc = lockedMediaMap->Unlock();
+            AssertComRCThrowRC(rc);
+            rc = pMediumLockList->Append(diff, true);
+            AssertComRCThrowRC(rc);
+            rc = lockedMediaMap->Lock();
+            AssertComRCThrowRC(rc);
 
             rc = diff->attachTo(mData->mUuid);
@@ -8311 +8356 @@
             if (FAILED(rc)) throw rc;
 
+            rc = lockedMediaMap->ReplaceKey(pAtt, attachment);
+            AssertComRCThrowRC(rc);
             mMediaData->mAttachments.push_back(attachment);
         }
@@ -8321 +8368 @@
         ErrorInfoKeeper eik;
 
-        for (MediaList::const_iterator it = lockedMedia.begin();
-             it != lockedMedia.end();
-             ++it)
-        {
-            HRESULT rc2 = (*it)->UnlockRead(NULL);
-            AssertComRC(rc2);
-        }
+        rc = lockedMediaMap->Clear();
+        AssertComRC(rc);
     }
 
@@ -8427 +8469 @@
             ComObjPtr<Medium> hd = (*it)->getMedium();
 
-            rc = hd->deleteStorageAndWait(NULL /*aProgress*/, pfNeedsSaveSettings);
-#if 1 /* HACK ALERT: Just make it kind of work */ /** @todo Fix this hack properly. The LockWrite / UnlockWrite / LockRead changes aren't undone! */
-            if (rc == VBOX_E_INVALID_OBJECT_STATE)
-            {
-                LogFlowFunc(("Applying unlock hack on '%s'! FIXME!\n", (*it)->getLogName()));
-                hd->UnlockWrite(NULL);
-                rc = hd->deleteStorageAndWait(NULL /*aProgress*/, pfNeedsSaveSettings);
-            }
-#endif
+            rc = hd->deleteStorage(NULL /*aProgress*/, true /*aWait*/,
+                                   pfNeedsSaveSettings);
             AssertMsg(SUCCEEDED(rc), ("rc=%Rhrc it=%s hd=%s\n", rc, (*it)->getLogName(), hd->getLocationFull().c_str() ));
             mrc = rc;
@@ -8563 +8598 @@
 
     MediaData::AttachmentList &oldAtts = mMediaData.backedUpData()->mAttachments;
+    bool fMediaNeedsLocking = false;
 
     /* enumerate new attachments */
@@ -8581 +8617 @@
 
         /** @todo convert all this Machine-based voodoo to MediumAttachment
-        * based commit logic. */
+         * based commit logic. */
         if (fImplicit)
         {
@@ -8592 +8628 @@
                 )
             {
-                rc = pMedium->LockWrite(NULL);
+                ComObjPtr<Medium> parent = pMedium->getParent();
+                AutoWriteLock parentLock(parent COMMA_LOCKVAL_SRC_POS);
+
+                /* update the appropriate lock list */
+                MediumLockList *pMediumLockList;
+                rc = mData->mSession.mLockedMedia.Get(pAttach, pMediumLockList);
                 AssertComRC(rc);
-
-                mData->mSession.mLockedMedia.push_back(
-                    Data::Session::LockedMedia::value_type(
-                        ComPtr<IMedium>(pMedium), true));
-
-                /* also, relock the old hard disk which is a base for the
-                * new diff for reading if the VM is online */
-
-                ComObjPtr<Medium> parent = pMedium->getParent();
-                /* make the relock atomic */
-                AutoWriteLock parentLock(parent COMMA_LOCKVAL_SRC_POS);
-                rc = parent->UnlockWrite(NULL);
-                AssertComRC(rc);
-                rc = parent->LockRead(NULL);
-                AssertComRC(rc);
-
-                /* XXX actually we should replace the old entry in that
-                * vector (write lock => read lock) but this would take
-                * some effort. So lets just ignore the error code in
-                * SessionMachine::unlockMedia(). */
-                mData->mSession.mLockedMedia.push_back(
-                    Data::Session::LockedMedia::value_type (
-                        ComPtr<IMedium>(parent), false));
+                if (pMediumLockList)
+                {
+                    /* unlock if there's a need to change the locking */
+                    if (!fMediaNeedsLocking)
+                    {
+                        rc = mData->mSession.mLockedMedia.Unlock();
+                        AssertComRC(rc);
+                        fMediaNeedsLocking = true;
+                    }
+                    rc = pMediumLockList->Update(parent, false);
+                    AssertComRC(rc);
+                    rc = pMediumLockList->Append(pMedium, true);
+                    AssertComRC(rc);
+                }
             }
 
@@ -8643 +8675 @@
 
     /* enumerate remaining old attachments and de-associate from the
-        * current machine state */
+     * current machine state */
     for (MediaData::AttachmentList::const_iterator it = oldAtts.begin();
-            it != oldAtts.end();
-            ++it)
+         it != oldAtts.end();
+         ++it)
     {
         MediumAttachment *pAttach = *it;
@@ -8652 +8684 @@
 
         /* Detach only hard disks, since DVD/floppy media is detached
-            * instantly in MountMedium. */
+         * instantly in MountMedium. */
         if (pAttach->getType() == DeviceType_HardDisk && pMedium)
         {
@@ -8661 +8693 @@
             AssertComRC(rc);
 
-            if (    aOnline
-                    && pAttach->getType() == DeviceType_HardDisk)
+            if (aOnline)
             {
-                /* unlock since not used anymore */
-                MediumState_T state;
-                rc = pMedium->UnlockWrite(&state);
-                /* the disk may be alredy relocked for reading above */
-                Assert(SUCCEEDED(rc) || state == MediumState_LockedRead);
+                /* unlock since medium is not used anymore */
+                MediumLockList *pMediumLockList;
+                rc = mData->mSession.mLockedMedia.Get(pAttach, pMediumLockList);
+                AssertComRC(rc);
+                if (pMediumLockList)
+                {
+                    rc = mData->mSession.mLockedMedia.Remove(pAttach);
+                    AssertComRC(rc);
+                }
             }
         }
+    }
+
+    /* take media locks again so that the locking state is consistent */
+    if (fMediaNeedsLocking)
+    {
+        Assert(aOnline);
+        rc = mData->mSession.mLockedMedia.Lock();
+        AssertComRC(rc);
     }
 
@@ -10774 +10817 @@
                  || mData->mMachineState == MachineState_Restoring
                  || mData->mMachineState == MachineState_TeleportingIn, E_FAIL);
-
-    try
-    {
-        HRESULT rc = S_OK;
-
-        ErrorInfoKeeper eik(true /* aIsNull */);
-        MultiResult mrc(S_OK);
-
-        /* Lock all medium objects attached to the VM.
-         * Get status for inaccessible media as well. */
-        for (MediaData::AttachmentList::const_iterator it = mMediaData->mAttachments.begin();
-             it != mMediaData->mAttachments.end();
-             ++it)
-        {
-            DeviceType_T devType = (*it)->getType();
-            ComObjPtr<Medium> medium = (*it)->getMedium();
-
-            bool first = true;
-
-            /** @todo split out the media locking, and put it into
-             * MediumImpl.cpp, as it needs this functionality too. */
-            while (!medium.isNull())
+    /* bail out if trying to lock things with already set up locking */
+    AssertReturn(mData->mSession.mLockedMedia.IsEmpty(), E_FAIL);
+
+    MultiResult mrc(S_OK);
+
+    /* Collect locking information for all medium objects attached to the VM. */
+    for (MediaData::AttachmentList::const_iterator it = mMediaData->mAttachments.begin();
+         it != mMediaData->mAttachments.end();
+         ++it)
+    {
+        MediumAttachment* pAtt = *it;
+        DeviceType_T devType = pAtt->getType();
+        Medium *pMedium = pAtt->getMedium();
+
+        MediumLockList *pMediumLockList(new MediumLockList());
+        // There can be attachments without a medium (floppy/dvd), and thus
+        // it's impossible to create a medium lock list. It still makes sense
+        // to have the empty medium lock list in the map in case a medium is
+        // attached later.
+        if (pMedium != NULL)
+        {
+            mrc = pMedium->createMediumLockList(devType != DeviceType_DVD,
+                                                NULL, *pMediumLockList);
+            if (FAILED(mrc))
             {
-                MediumState_T mediumState = medium->getState();
-
-                /* accessibility check must be first, otherwise locking
-                 * interferes with getting the medium state. */
-                if (mediumState == MediumState_Inaccessible)
-                {
-                    rc = medium->RefreshState(&mediumState);
-                    if (FAILED(rc)) throw rc;
-
-                    if (mediumState == MediumState_Inaccessible)
-                    {
-                        Bstr error;
-                        rc = medium->COMGETTER(LastAccessError)(error.asOutParam());
-                        if (FAILED(rc)) throw rc;
-
-                        Bstr loc;
-                        rc = medium->COMGETTER(Location)(loc.asOutParam());
-                        if (FAILED(rc)) throw rc;
-
-                        /* collect multiple errors */
-                        eik.restore();
-
-                        /* be in sync with MediumBase::setStateError() */
-                        Assert(!error.isEmpty());
-                        mrc = setError(E_FAIL,
-                                       tr("Medium '%ls' is not accessible. %ls"),
-                                       loc.raw(),
-                                       error.raw());
-
-                        eik.fetch();
-                    }
-                }
-
-                if (first)
-                {
-                    if (devType != DeviceType_DVD)
-                    {
-                        /* HardDisk and Floppy medium must be locked for writing */
-                        rc = medium->LockWrite(NULL);
-                        if (FAILED(rc)) throw rc;
-                    }
-                    else
-                    {
-                        /* DVD medium must be locked for reading */
-                        rc = medium->LockRead(NULL);
-                        if (FAILED(rc)) throw rc;
-                    }
-
-                    mData->mSession.mLockedMedia.push_back(
-                        Data::Session::LockedMedia::value_type(
-                            ComPtr<IMedium>(medium), true));
-
-                    first = false;
-                }
-                else
-                {
-                    rc = medium->LockRead(NULL);
-                    if (FAILED(rc)) throw rc;
-
-                    mData->mSession.mLockedMedia.push_back(
-                        Data::Session::LockedMedia::value_type(
-                            ComPtr<IMedium>(medium), false));
-                }
-
-
-                /* no locks or callers here since there should be no way to
-                 * change the hard disk parent at this point (as it is still
-                 * attached to the machine) */
-                medium = medium->getParent();
+                delete pMediumLockList;
+                mData->mSession.mLockedMedia.Clear();
+                break;
             }
         }
 
-        /* @todo r=dj is this correct? first restoring the eik and then throwing? */
-        eik.restore();
-        HRESULT rc2 = (HRESULT)mrc;
-        if (FAILED(rc2)) throw rc2;
-    }
-    catch (HRESULT aRC)
-    {
-        /* Unlock all locked media on failure */
-        unlockMedia();
-        return aRC;
-    }
-
-    return S_OK;
+        HRESULT rc = mData->mSession.mLockedMedia.Insert(pAtt, pMediumLockList);
+        if (FAILED(rc))
+        {
+            mData->mSession.mLockedMedia.Clear();
+            mrc = setError(rc,
+                           tr("Collecting locking information for all attached media failed"));
+            break;
+        }
+    }
+
+    if (SUCCEEDED(mrc))
+    {
+        /* Now lock all media. If this fails, nothing is locked. */
+        HRESULT rc = mData->mSession.mLockedMedia.Lock();
+        if (FAILED(rc))
+        {
+            mrc = setError(rc,
+                           tr("Locking of attached media failed"));
+        }
+    }
+
+    return mrc;
 }
 
@@ -10898 +10886 @@
     ErrorInfoKeeper eik;
 
-    HRESULT rc = S_OK;
-
-    for (Data::Session::LockedMedia::const_iterator
-         it = mData->mSession.mLockedMedia.begin();
-         it != mData->mSession.mLockedMedia.end(); ++it)
-    {
-        MediumState_T state;
-        if (it->second)
-            rc = it->first->UnlockWrite(&state);
-        else
-            rc = it->first->UnlockRead(&state);
-
-        /* The second can happen if an object was re-locked in
-         * Machine::fixupMedia(). The last can happen when e.g a DVD/Floppy
-         * image was unmounted at runtime. */
-        Assert(SUCCEEDED(rc) || state == MediumState_LockedRead || state == MediumState_Created);
-    }
-
-    mData->mSession.mLockedMedia.clear();
+    HRESULT rc = mData->mSession.mLockedMedia.Clear();
+    AssertComRC(rc);
 }
 
@@ -11021 +10992 @@
     {
         /*
-         *  delete the saved state after Console::DiscardSavedState() is called
+         *  delete the saved state after Console::ForgetSavedState() is called
          *  or if the VM process (owning a direct VM session) crashed while the
          *  VM was Saved
@@ -11155 +11126 @@
         /* directControl may be already set to NULL here in #OnSessionEnd()
          * called too early by the direct session process while there is still
-         * some operation (like discarding the snapshot) in progress. The client
+         * some operation (like deleting the snapshot) in progress. The client
          * process in this case is waiting inside Session::close() for the
          * "end session" process object to complete, while #uninit() called by

trunk/src/VBox/Main/MediumImpl.cpp

@@ -190 +190 @@
 public:
     Task(Medium *aMedium, Progress *aProgress)
-        : mMedium(aMedium),
+        : mVDOperationIfaces(NULL),
+          m_pfNeedsSaveSettings(NULL),
+          mMedium(aMedium),
           mMediumCaller(aMedium),
-          m_pfNeedsSaveSettings(NULL),
-          mVDOperationIfaces(NULL),
           mThread(NIL_RTTHREAD),
           mProgress(aProgress)
@@ -229 +229 @@
     bool isAsync() { return mThread != NIL_RTTHREAD; }
 
-    const ComObjPtr<Medium> mMedium;
-    AutoCaller mMediumCaller;
+    PVDINTERFACE mVDOperationIfaces;
 
     // Whether the caller needs to call VirtualBox::saveSettings() after
@@ -237 +236 @@
     bool *m_pfNeedsSaveSettings;
 
-    PVDINTERFACE mVDOperationIfaces;
+    const ComObjPtr<Medium> mMedium;
+    AutoCaller mMediumCaller;
+
+    friend HRESULT Medium::runNow(Medium::Task*, bool*);
 
 protected:
@@ -279 +281 @@
                    Progress *aProgress,
                    Medium *aTarget,
-                   MediumVariant_T aVariant)
+                   MediumVariant_T aVariant,
+                   MediumLockList *aMediumLockList,
+                   bool fKeepMediumLockList = false)
         : Medium::Task(aMedium, aProgress),
+          mpMediumLockList(aMediumLockList),
           mTarget(aTarget),
           mVariant(aVariant),
-          mTargetCaller(aTarget)
+          mTargetCaller(aTarget),
+          mfKeepMediumLockList(fKeepMediumLockList)
     {
         AssertReturnVoidStmt(aTarget != NULL, mRC = E_FAIL);
@@ -291 +297 @@
     }
 
+    ~CreateDiffTask()
+    {
+        if (!mfKeepMediumLockList && mpMediumLockList)
+            delete mpMediumLockList;
+    }
+
+    MediumLockList *mpMediumLockList;
+
     const ComObjPtr<Medium> mTarget;
     MediumVariant_T mVariant;
@@ -298 +312 @@
 
     AutoCaller mTargetCaller;
+    bool mfKeepMediumLockList;
 };
 
@@ -306 +321 @@
               Progress *aProgress,
               Medium *aTarget,
+              MediumVariant_T aVariant,
               Medium *aParent,
-              ImageChain *aSourceChain,
-              ImageChain *aParentChain,
-              MediumVariant_T aVariant)
+              MediumLockList *aSourceMediumLockList,
+              MediumLockList *aTargetMediumLockList,
+              bool fKeepSourceMediumLockList = false,
+              bool fKeepTargetMediumLockList = false)
         : Medium::Task(aMedium, aProgress),
           mTarget(aTarget),
           mParent(aParent),
-          mSourceChain(aSourceChain),
-          mParentChain(aParentChain),
+          mpSourceMediumLockList(aSourceMediumLockList),
+          mpTargetMediumLockList(aTargetMediumLockList),
           mVariant(aVariant),
           mTargetCaller(aTarget),
-          mParentCaller(aParent)
+          mParentCaller(aParent),
+          mfKeepSourceMediumLockList(fKeepSourceMediumLockList),
+          mfKeepTargetMediumLockList(fKeepTargetMediumLockList)
     {
         AssertReturnVoidStmt(aTarget != NULL, mRC = E_FAIL);
@@ -327 +346 @@
         if (FAILED(mRC))
             return;
-        AssertReturnVoidStmt(aSourceChain != NULL, mRC = E_FAIL);
-        AssertReturnVoidStmt(aParentChain != NULL, mRC = E_FAIL);
+        AssertReturnVoidStmt(aSourceMediumLockList != NULL, mRC = E_FAIL);
+        AssertReturnVoidStmt(aTargetMediumLockList != NULL, mRC = E_FAIL);
+    }
+
+    ~CloneTask()
+    {
+        if (!mfKeepSourceMediumLockList && mpSourceMediumLockList)
+            delete mpSourceMediumLockList;
+        if (!mfKeepTargetMediumLockList && mpTargetMediumLockList)
+            delete mpTargetMediumLockList;
     }
 
     const ComObjPtr<Medium> mTarget;
     const ComObjPtr<Medium> mParent;
-    std::auto_ptr<ImageChain> mSourceChain;
-    std::auto_ptr<ImageChain> mParentChain;
+    MediumLockList *mpSourceMediumLockList;
+    MediumLockList *mpTargetMediumLockList;
     MediumVariant_T mVariant;
 
@@ -342 +369 @@
     AutoCaller mTargetCaller;
     AutoCaller mParentCaller;
+    bool mfKeepSourceMediumLockList;
+    bool mfKeepTargetMediumLockList;
 };
 
@@ -349 +378 @@
     CompactTask(Medium *aMedium,
                 Progress *aProgress,
-                ImageChain *aImageChain)
+                MediumLockList *aMediumLockList,
+                bool fKeepMediumLockList = false)
         : Medium::Task(aMedium, aProgress),
-          mImageChain(aImageChain)
-    {
-        AssertReturnVoidStmt(aImageChain != NULL, mRC = E_FAIL);
-    }
-
-    std::auto_ptr<ImageChain> mImageChain;
+          mpMediumLockList(aMediumLockList),
+          mfKeepMediumLockList(fKeepMediumLockList)
+    {
+        AssertReturnVoidStmt(aMediumLockList != NULL, mRC = E_FAIL);
+    }
+
+    ~CompactTask()
+    {
+        if (!mfKeepMediumLockList && mpMediumLockList)
+            delete mpMediumLockList;
+    }
+
+    MediumLockList *mpMediumLockList;
 
 private:
     virtual HRESULT handler();
+
+    bool mfKeepMediumLockList;
 };
 
@@ -366 +405 @@
 public:
     ResetTask(Medium *aMedium,
-              Progress *aProgress)
-        : Medium::Task(aMedium, aProgress)
+              Progress *aProgress,
+              MediumLockList *aMediumLockList,
+              bool fKeepMediumLockList = false)
+        : Medium::Task(aMedium, aProgress),
+          mpMediumLockList(aMediumLockList),
+          mfKeepMediumLockList(fKeepMediumLockList)
     {}
+
+    ~ResetTask()
+    {
+        if (!mfKeepMediumLockList && mpMediumLockList)
+            delete mpMediumLockList;
+    }
+
+    MediumLockList *mpMediumLockList;
 
 private:
     virtual HRESULT handler();
+
+    bool mfKeepMediumLockList;
 };
 
@@ -378 +431 @@
 public:
     DeleteTask(Medium *aMedium,
-               Progress *aProgress)
-        : Medium::Task(aMedium, aProgress)
+               Progress *aProgress,
+               MediumLockList *aMediumLockList,
+               bool fKeepMediumLockList = false)
+        : Medium::Task(aMedium, aProgress),
+          mpMediumLockList(aMediumLockList),
+          mfKeepMediumLockList(fKeepMediumLockList)
     {}
+
+    ~DeleteTask()
+    {
+        if (!mfKeepMediumLockList && mpMediumLockList)
+            delete mpMediumLockList;
+    }
+
+    MediumLockList *mpMediumLockList;
 
 private:
     virtual HRESULT handler();
+
+    bool mfKeepMediumLockList;
 };
 
@@ -390 +457 @@
 public:
     MergeTask(Medium *aMedium,
+              Medium *aTarget,
+              bool fMergeForward,
+              Medium *aParentForTarget,
+              const MediaList &aChildrenToReparent,
               Progress *aProgress,
-              MergeChain *aMergeChain)
+              MediumLockList *aMediumLockList,
+              bool fKeepMediumLockList = false)
         : Medium::Task(aMedium, aProgress),
-          mMergeChain(aMergeChain)
-    {
-        AssertReturnVoidStmt(aMergeChain != NULL, mRC = E_FAIL);
-    }
-
-    std::auto_ptr<MergeChain> mMergeChain;
+          mTarget(aTarget),
+          mfMergeForward(fMergeForward),
+          mParentForTarget(aParentForTarget),
+          mChildrenToReparent(aChildrenToReparent),
+          mpMediumLockList(aMediumLockList),
+          mTargetCaller(aTarget),
+          mParentForTargetCaller(aParentForTarget),
+          mfChildrenCaller(false),
+          mfKeepMediumLockList(fKeepMediumLockList)
+    {
+        AssertReturnVoidStmt(aMediumLockList != NULL, mRC = E_FAIL);
+        for (MediaList::const_iterator it = mChildrenToReparent.begin();
+             it != mChildrenToReparent.end();
+             ++it)
+        {
+            HRESULT rc2 = (*it)->addCaller();
+            if (FAILED(rc2))
+            {
+                mRC = E_FAIL;
+                for (MediaList::const_iterator it2 = mChildrenToReparent.begin();
+                     it2 != it;
+                     --it2)
+                {
+                    (*it2)->releaseCaller();
+                }
+                return;
+            }
+        }
+        mfChildrenCaller = true;
+    }
+
+    ~MergeTask()
+    {
+        if (!mfKeepMediumLockList && mpMediumLockList)
+            delete mpMediumLockList;
+        if (mfChildrenCaller)
+        {
+            for (MediaList::const_iterator it = mChildrenToReparent.begin();
+                 it != mChildrenToReparent.end();
+                 ++it)
+            {
+                (*it)->releaseCaller();
+            }
+        }
+    }
+
+    const ComObjPtr<Medium> mTarget;
+    bool mfMergeForward;
+    /* When mChildrenToReparent is empty then mParentForTarget is non-null.
+     * In other words: they are used in different cases. */
+    const ComObjPtr<Medium> mParentForTarget;
+    MediaList mChildrenToReparent;
+    MediumLockList *mpMediumLockList;
 
 private:
     virtual HRESULT handler();
+
+    AutoCaller mTargetCaller;
+    AutoCaller mParentForTargetCaller;
+    bool mfChildrenCaller;
+    bool mfKeepMediumLockList;
 };
 
@@ -469 +593 @@
 HRESULT Medium::CreateBaseTask::handler()
 {
-    return mMedium->taskThreadCreateBase(*this);
+    return mMedium->taskCreateBaseHandler(*this);
 }
 
@@ -477 +601 @@
 HRESULT Medium::CreateDiffTask::handler()
 {
-    return mMedium->taskThreadCreateDiff(*this);
+    return mMedium->taskCreateDiffHandler(*this);
 }
 
@@ -485 +609 @@
 HRESULT Medium::CloneTask::handler()
 {
-    return mMedium->taskThreadClone(*this);
+    return mMedium->taskCloneHandler(*this);
 }
 
@@ -493 +617 @@
 HRESULT Medium::CompactTask::handler()
 {
-    return mMedium->taskThreadCompact(*this);
+    return mMedium->taskCompactHandler(*this);
 }
 
@@ -501 +625 @@
 HRESULT Medium::ResetTask::handler()
 {
-    return mMedium->taskThreadReset(*this);
+    return mMedium->taskResetHandler(*this);
 }
 
@@ -509 +633 @@
 HRESULT Medium::DeleteTask::handler()
 {
-    return mMedium->taskThreadDelete(*this);
+    return mMedium->taskDeleteHandler(*this);
 }
 
@@ -517 +641 @@
 HRESULT Medium::MergeTask::handler()
 {
-    return mMedium->taskThreadMerge(*this);
-}
-
-
-////////////////////////////////////////////////////////////////////////////////
-//
-// Merge chain class
-//
-////////////////////////////////////////////////////////////////////////////////
-
-/**
- * Helper class for merge operations.
- *
- * @note It is assumed that when modifying methods of this class are called,
- *       the medium tree lock is held in read mode.
- */
-class Medium::MergeChain : public MediaList,
-                           public com::SupportErrorInfoBase
-{
-public:
-
-    MergeChain(bool aForward, bool aIgnoreAttachments)
-        : mForward(aForward)
-        , mIgnoreAttachments(aIgnoreAttachments) {}
-
-    ~MergeChain()
-    {
-        for (iterator it = mChildren.begin(); it != mChildren.end(); ++ it)
-        {
-            HRESULT rc = (*it)->UnlockWrite(NULL);
-            AssertComRC(rc);
-
-            (*it)->releaseCaller();
-        }
-
-        for (iterator it = begin(); it != end(); ++ it)
-        {
-            AutoWriteLock alock(*it COMMA_LOCKVAL_SRC_POS);
-            Assert((*it)->m->state == MediumState_LockedWrite ||
-                   (*it)->m->state == MediumState_LockedRead ||
-                   (*it)->m->state == MediumState_Deleting);
-            if ((*it)->m->state == MediumState_LockedWrite)
-                (*it)->UnlockWrite(NULL);
-            else if ((*it)->m->state == MediumState_LockedRead)
-                (*it)->UnlockRead(NULL);
-            else
-                (*it)->m->state = MediumState_Created;
-
-            (*it)->releaseCaller();
-        }
-
-        if (!mParent.isNull())
-            mParent->releaseCaller();
-    }
-
-    HRESULT addSource(Medium *aMedium)
-    {
-        HRESULT rc = aMedium->addCaller();
-        if (FAILED(rc)) return rc;
-
-        AutoWriteLock alock(aMedium COMMA_LOCKVAL_SRC_POS);
-
-        if (mForward)
-        {
-            rc = checkChildrenAndAttachmentsAndImmutable(aMedium);
-            if (FAILED(rc))
-            {
-                aMedium->releaseCaller();
-                return rc;
-            }
-        }
-
-        /* We have to fetch the state with the COM method, cause it's possible
-           that the medium isn't fully initialized yet. */
-        MediumState_T m;
-        rc = aMedium->RefreshState(&m);
-        if (FAILED(rc)) return rc;
-        /* go to Deleting */
-        switch (m)
-        {
-            case MediumState_Created:
-                aMedium->m->state = MediumState_Deleting;
-                break;
-            default:
-                aMedium->releaseCaller();
-                return aMedium->setStateError();
-        }
-
-        push_front(aMedium);
-
-        if (mForward)
-        {
-            /* we will need parent to reparent target */
-            if (!aMedium->m->pParent.isNull())
-            {
-                rc = aMedium->m->pParent->addCaller();
-                if (FAILED(rc)) return rc;
-
-                mParent = aMedium->m->pParent;
-            }
-
-            /* Include all images from base to source. */
-            ComObjPtr<Medium> pParent = aMedium->m->pParent;
-            while (!pParent.isNull())
-            {
-                rc = pParent->addCaller();
-                if (FAILED(rc)) return rc;
-
-                rc = pParent->LockRead(NULL);
-                if (FAILED(rc)) return rc;
-
-                push_front(pParent);
-                pParent = pParent->m->pParent;
-            }
-        }
-        else
-        {
-            /* we will need to reparent children */
-            for (MediaList::const_iterator it = aMedium->getChildren().begin();
-                 it != aMedium->getChildren().end();
-                 ++it)
-            {
-                ComObjPtr<Medium> pMedium = *it;
-                rc = pMedium->addCaller();
-                if (FAILED(rc)) return rc;
-
-                rc = pMedium->LockWrite(NULL);
-                if (FAILED(rc))
-                {
-                    pMedium->releaseCaller();
-                    return rc;
-                }
-
-                mChildren.push_back(pMedium);
-            }
-        }
-
-        mSource = aMedium;
-
-        return S_OK;
-    }
-
-    HRESULT addTarget(Medium *aMedium)
-    {
-        HRESULT rc = aMedium->addCaller();
-        if (FAILED(rc)) return rc;
-
-        AutoWriteLock alock(aMedium COMMA_LOCKVAL_SRC_POS);
-
-        if (!mForward)
-        {
-            rc = checkChildrenAndImmutable(aMedium);
-            if (FAILED(rc))
-            {
-                aMedium->releaseCaller();
-                return rc;
-            }
-        }
-
-        /* go to LockedWrite */
-        rc = aMedium->LockWrite(NULL);
-        if (FAILED(rc))
-        {
-            aMedium->releaseCaller();
-            return rc;
-        }
-
-        push_front(aMedium);
-
-        mTarget = aMedium;
-
-        return S_OK;
-    }
-
-    HRESULT addIntermediate(Medium *aMedium)
-    {
-        HRESULT rc = aMedium->addCaller();
-        if (FAILED(rc)) return rc;
-
-        AutoWriteLock alock(aMedium COMMA_LOCKVAL_SRC_POS);
-
-        rc = checkChildrenAndAttachments(aMedium);
-        if (FAILED(rc))
-        {
-            aMedium->releaseCaller();
-            return rc;
-        }
-
-        /* go to Deleting */
-        switch (aMedium->m->state)
-        {
-            case MediumState_Created:
-                aMedium->m->state = MediumState_Deleting;
-                break;
-            default:
-                aMedium->releaseCaller();
-                return aMedium->setStateError();
-        }
-
-        push_front(aMedium);
-
-        return S_OK;
-    }
-
-    int targetIdx()
-    {
-        Assert(!mTarget.isNull());
-        int idx = 0;
-
-        for (MediaList::const_iterator it = begin(); it != end(); ++it)
-        {
-            ComObjPtr<Medium> pMedium = *it;
-
-            /* Do we have the target? */
-            if (pMedium == mTarget)
-                break;
-
-            idx++;
-        }
-
-        return idx;
-    }
-
-    int sourceIdx()
-    {
-        Assert(!mSource.isNull());
-        int idx = 0;
-
-        for (MediaList::const_iterator it = begin(); it != end(); ++it)
-        {
-            ComObjPtr<Medium> pMedium = *it;
-
-            /* Do we have the source? */
-            if (pMedium == mSource)
-                break;
-
-            idx++;
-        }
-
-        return idx;
-    }
-
-    bool isForward() const { return mForward; }
-    Medium *parent() const { return mParent; }
-    const MediaList& children() const { return mChildren; }
-
-    Medium *source() const
-    { AssertReturn(size() > 0, NULL); return mSource; }
-
-    Medium *target() const
-    { AssertReturn(size() > 0, NULL); return mTarget; }
-
-protected:
-
-    // SupportErrorInfoBase interface
-    const GUID &mainInterfaceID() const { return COM_IIDOF(IMedium); }
-    const char *componentName() const { return Medium::ComponentName(); }
-
-private:
-
-    HRESULT check(Medium *aMedium, bool aChildren, bool aAttachments,
-                  bool aImmutable)
-    {
-        if (aChildren)
-        {
-            /* not going to multi-merge as it's too expensive */
-            if (aMedium->getChildren().size() > 1)
-            {
-                return setError(E_FAIL,
-                                tr("Medium '%s' involved in the merge operation has more than one child medium (%d)"),
-                                aMedium->m->strLocationFull.raw(),
-                                aMedium->getChildren().size());
-            }
-        }
-
-        if (aAttachments && !mIgnoreAttachments)
-        {
-            if (aMedium->m->backRefs.size() != 0)
-                return setError(E_FAIL,
-                                tr("Medium '%s' is attached to %d virtual machines"),
-                                aMedium->m->strLocationFull.raw(),
-                                aMedium->m->backRefs.size());
-        }
-
-        if (aImmutable)
-        {
-            if (aMedium->m->type == MediumType_Immutable)
-                return setError(E_FAIL,
-                                tr("Medium '%s' is immutable"),
-                                aMedium->m->strLocationFull.raw());
-        }
-
-        return S_OK;
-    }
-
-    HRESULT checkChildren(Medium *aMedium)
-    { return check(aMedium, true, false, false); }
-
-    HRESULT checkChildrenAndImmutable(Medium *aMedium)
-    { return check(aMedium, true, false, true); }
-
-    HRESULT checkChildrenAndAttachments(Medium *aMedium)
-    { return check(aMedium, true, true, false); }
-
-    HRESULT checkChildrenAndAttachmentsAndImmutable(Medium *aMedium)
-    { return check(aMedium, true, true, true); }
-
-    /** true if forward merge, false if backward */
-    bool mForward : 1;
-    /** true to not perform attachment checks */
-    bool mIgnoreAttachments : 1;
-
-    /** Parent of the source when forward merge (if any) */
-    ComObjPtr <Medium> mParent;
-    /** Children of the source when backward merge (if any) */
-    MediaList mChildren;
-    /** Source image */
-    ComObjPtr <Medium> mSource;
-    /** Target image */
-    ComObjPtr <Medium> mTarget;
-};
-
-////////////////////////////////////////////////////////////////////////////////
-//
-// ImageChain class
-//
-////////////////////////////////////////////////////////////////////////////////
-
-/**
- * Helper class for image operations involving the entire parent chain.
- *
- * @note It is assumed that when modifying methods of this class are called,
- *       the medium tree lock is held in read mode.
- */
-class Medium::ImageChain : public MediaList,
-                           public com::SupportErrorInfoBase
-{
-public:
-
-    ImageChain() {}
-
-    ~ImageChain()
-    {
-        /* empty? */
-        if (begin() != end())
-        {
-            MediaList::const_iterator last = end();
-            last--;
-            for (MediaList::const_iterator it = begin(); it != end(); ++ it)
-            {
-                AutoWriteLock alock(*it COMMA_LOCKVAL_SRC_POS);
-                if (it == last)
-                {
-                    Assert(   (*it)->m->state == MediumState_LockedRead
-                           || (*it)->m->state == MediumState_LockedWrite);
-                    if ((*it)->m->state == MediumState_LockedRead)
-                        (*it)->UnlockRead(NULL);
-                    else if ((*it)->m->state == MediumState_LockedWrite)
-                        (*it)->UnlockWrite(NULL);
-                }
-                else
-                {
-                    Assert((*it)->m->state == MediumState_LockedRead);
-                    if ((*it)->m->state == MediumState_LockedRead)
-                        (*it)->UnlockRead(NULL);
-                }
-
-                (*it)->releaseCaller();
-            }
-        }
-    }
-
-    HRESULT addImage(Medium *aMedium)
-    {
-        HRESULT rc = aMedium->addCaller();
-        if (FAILED(rc)) return rc;
-
-        push_front(aMedium);
-
-        return S_OK;
-    }
-
-    HRESULT lockImagesRead()
-    {
-        /* Lock all disks in the chain in {parent, child} order,
-         * and make sure they are accessible. */
-        /// @todo code duplication with SessionMachine::lockMedia, see below
-        ErrorInfoKeeper eik(true /* aIsNull */);
-        MultiResult mrc(S_OK);
-        for (MediaList::const_iterator it = begin(); it != end(); ++ it)
-        {
-            HRESULT rc = S_OK;
-            MediumState_T mediumState = (*it)->getState();
-
-            /* accessibility check must be first, otherwise locking
-             * interferes with getting the medium state. */
-            if (mediumState == MediumState_Inaccessible)
-            {
-                rc = (*it)->RefreshState(&mediumState);
-                if (FAILED(rc)) return rc;
-
-                if (mediumState == MediumState_Inaccessible)
-                {
-                    Bstr error;
-                    rc = (*it)->COMGETTER(LastAccessError)(error.asOutParam());
-                    if (FAILED(rc)) return rc;
-
-                    Bstr loc;
-                    rc = (*it)->COMGETTER(Location)(loc.asOutParam());
-                    if (FAILED(rc)) return rc;
-
-                    /* collect multiple errors */
-                    eik.restore();
-
-                    /* be in sync with Medium::setStateError() */
-                    Assert(!error.isEmpty());
-                    mrc = setError(E_FAIL,
-                                   tr("Medium '%ls' is not accessible. %ls"),
-                                   loc.raw(), error.raw());
-
-                    eik.fetch();
-                }
-            }
-
-            rc = (*it)->LockRead(&mediumState);
-            if (FAILED(rc)) return rc;
-        }
-
-        eik.restore();
-        HRESULT rc2 = (HRESULT)mrc;
-        if (FAILED(rc2)) return rc2;
-
-        return S_OK;
-    }
-
-    HRESULT lockImagesReadAndLastWrite()
-    {
-        /* Lock all disks in the chain in {parent, child} order,
-         * and make sure they are accessible. */
-        /// @todo code duplication with SessionMachine::lockMedia, see below
-        ErrorInfoKeeper eik(true /* aIsNull */);
-        MultiResult mrc(S_OK);
-        MediaList::const_iterator last = end();
-        last--;
-        for (MediaList::const_iterator it = begin(); it != end(); ++ it)
-        {
-            HRESULT rc = S_OK;
-            MediumState_T mediumState = (*it)->getState();
-
-            /* accessibility check must be first, otherwise locking
-             * interferes with getting the medium state. */
-            if (mediumState == MediumState_Inaccessible)
-            {
-                rc = (*it)->RefreshState(&mediumState);
-                if (FAILED(rc)) return rc;
-
-                if (mediumState == MediumState_Inaccessible)
-                {
-                    Bstr error;
-                    rc = (*it)->COMGETTER(LastAccessError)(error.asOutParam());
-                    if (FAILED(rc)) return rc;
-
-                    Bstr loc;
-                    rc = (*it)->COMGETTER(Location)(loc.asOutParam());
-                    if (FAILED(rc)) return rc;
-
-                    /* collect multiple errors */
-                    eik.restore();
-
-                    /* be in sync with Medium::setStateError() */
-                    Assert(!error.isEmpty());
-                    mrc = setError(E_FAIL,
-                                   tr("Medium '%ls' is not accessible. %ls"),
-                                   loc.raw(), error.raw());
-
-                    eik.fetch();
-                }
-            }
-
-            if (it == last)
-                rc = (*it)->LockWrite(&mediumState);
-            else
-                rc = (*it)->LockRead(&mediumState);
-        }
-
-        eik.restore();
-        HRESULT rc2 = (HRESULT)mrc;
-        if (FAILED(rc2)) return rc2;
-
-        return S_OK;
-    }
-
-protected:
-
-    // SupportErrorInfoBase interface
-    const GUID &mainInterfaceID() const { return COM_IIDOF(IMedium); }
-    const char *componentName() const { return Medium::ComponentName(); }
-
-private:
-
-};
+    return mMedium->taskMergeHandler(*this);
+}
 
 
@@ -1345 +970 @@
      * XML format version change if we wish) */
     for (settings::PropertiesMap::const_iterator it = data.properties.begin();
-         it != data.properties.end(); ++ it)
+         it != data.properties.end(); ++it)
     {
         const Utf8Str &name = it->first;
@@ -1496 +1121 @@
         /* we are being uninitialized after've been deleted by merge.
          * Reparenting has already been done so don't touch it here (we are
-         * now orphans and remoeDependentChild() will assert) */
+         * now orphans and removeDependentChild() will assert) */
         Assert(m->pParent.isNull());
     }
@@ -1957 +1582 @@
         size_t i = 0;
         for (BackRefList::const_iterator it = m->backRefs.begin();
-             it != m->backRefs.end(); ++ it, ++ i)
+             it != m->backRefs.end(); ++it, ++i)
         {
              it->machineId.toUtf16().detachTo(&machineIds[i]);
@@ -2013 +1638 @@
     Guid id(aMachineId);
     for (BackRefList::const_iterator it = m->backRefs.begin();
-         it != m->backRefs.end(); ++ it)
+         it != m->backRefs.end(); ++it)
     {
         if (it->machineId == id)
@@ -2135 +1760 @@
             LogFlowThisFunc(("Failing - state=%d\n", m->state));
             rc = setError(VBOX_E_INVALID_OBJECT_STATE,
-                          tr ("Medium '%s' is not locked for reading"),
+                          tr("Medium '%s' is not locked for reading"),
                           m->strLocationFull.raw());
             break;
@@ -2220 +1845 @@
             LogFlowThisFunc(("Failing - state=%d locationFull=%s\n", m->state, getLocationFull().c_str()));
             rc = setError(VBOX_E_INVALID_OBJECT_STATE,
-                          tr ("Medium '%s' is not locked for writing"),
+                          tr("Medium '%s' is not locked for writing"),
                           m->strLocationFull.raw());
             break;
@@ -2433 +2058 @@
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-    aVariant = (MediumVariant_T)((unsigned)aVariant & (unsigned)~MediumVariant_Diff);
-    if (    !(aVariant & MediumVariant_Fixed)
-        &&  !(m->formatObj->capabilities() & MediumFormatCapabilities_CreateDynamic))
-        return setError(VBOX_E_NOT_SUPPORTED,
-                        tr("Hard disk format '%s' does not support dynamic storage creation"),
-                        m->strFormat.raw());
-    if (    (aVariant & MediumVariant_Fixed)
-        &&  !(m->formatObj->capabilities() & MediumFormatCapabilities_CreateDynamic))
-        return setError(VBOX_E_NOT_SUPPORTED,
-                        tr("Hard disk format '%s' does not support fixed storage creation"),
-                        m->strFormat.raw());
-
-    switch (m->state)
-    {
-        case MediumState_NotCreated:
-            break;
-        default:
-            return setStateError();
-    }
-
-    ComObjPtr <Progress> progress;
-    progress.createObject();
-    HRESULT rc = progress->init(m->pVirtualBox,
-                                static_cast<IMedium*>(this),
-                                (aVariant & MediumVariant_Fixed)
-                                        ? BstrFmt(tr("Creating fixed hard disk storage unit '%s'"), m->strLocationFull.raw())
-                                        : BstrFmt(tr("Creating dynamic hard disk storage unit '%s'"), m->strLocationFull.raw()),
-                                TRUE /* aCancelable */);
-    if (FAILED(rc)) return rc;
-
-    /* setup task object to carry out the operation asynchronously */
-    Medium::Task *pTask(new Medium::CreateBaseTask(this, progress,
-                                                   aLogicalSize, aVariant));
-    rc = pTask->rc();
-    if (FAILED(rc))
-    {
+    HRESULT rc = S_OK;
+    ComObjPtr <Progress> pProgress;
+    Medium::Task *pTask = NULL;
+
+    try
+    {
+        AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+        aVariant = (MediumVariant_T)((unsigned)aVariant & (unsigned)~MediumVariant_Diff);
+        if (    !(aVariant & MediumVariant_Fixed)
+            &&  !(m->formatObj->capabilities() & MediumFormatCapabilities_CreateDynamic))
+            throw setError(VBOX_E_NOT_SUPPORTED,
+                           tr("Hard disk format '%s' does not support dynamic storage creation"),
+                           m->strFormat.raw());
+        if (    (aVariant & MediumVariant_Fixed)
+            &&  !(m->formatObj->capabilities() & MediumFormatCapabilities_CreateDynamic))
+            throw setError(VBOX_E_NOT_SUPPORTED,
+                           tr("Hard disk format '%s' does not support fixed storage creation"),
+                           m->strFormat.raw());
+
+        if (m->state != MediumState_NotCreated)
+            throw setStateError();
+
+        pProgress.createObject();
+        rc = pProgress->init(m->pVirtualBox,
+                             static_cast<IMedium*>(this),
+                             (aVariant & MediumVariant_Fixed)
+                               ? BstrFmt(tr("Creating fixed hard disk storage unit '%s'"), m->strLocationFull.raw())
+                               : BstrFmt(tr("Creating dynamic hard disk storage unit '%s'"), m->strLocationFull.raw()),
+                             TRUE /* aCancelable */);
+        if (FAILED(rc)) throw rc;
+
+        /* setup task object to carry out the operation asynchronously */
+        pTask = new Medium::CreateBaseTask(this, pProgress, aLogicalSize,
+                                           aVariant);
+        rc = pTask->rc();
         AssertComRC(rc);
+        if (FAILED(rc)) throw rc;
+
+        m->state = MediumState_Creating;
+    }
+    catch (HRESULT aRC) { rc = aRC; }
+
+    if (SUCCEEDED(rc))
+    {
+        rc = startThread(pTask);
+
+        if (SUCCEEDED(rc))
+            pProgress.queryInterfaceTo(aProgress);
+    }
+    else if (pTask != NULL)
         delete pTask;
-        return rc;
-    }
-
-    rc = startThread(pTask);
-    if (FAILED(rc)) return rc;
-
-    /* go to Creating state on success */
-    m->state = MediumState_Creating;
-
-    progress.queryInterfaceTo(aProgress);
-
-    return S_OK;
+
+    return rc;
 }
 
@@ -2494 +2121 @@
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
-    ComObjPtr <Progress> progress;
-
-    HRESULT rc = deleteStorageNoWait(progress);
+    ComObjPtr <Progress> pProgress;
+
+    HRESULT rc = deleteStorage(&pProgress, false /* aWait */,
+                               NULL /* pfNeedsSaveSettings */);
     if (SUCCEEDED(rc))
-        progress.queryInterfaceTo(aProgress);
+        pProgress.queryInterfaceTo(aProgress);
 
     return rc;
@@ -2522 +2150 @@
                         m->strLocationFull.raw());
 
-    /* We want to be locked for reading as long as our diff child is being
-     * created */
-    HRESULT rc = LockRead(NULL);
-    if (FAILED(rc)) return rc;
-
-    ComObjPtr <Progress> progress;
-
-    rc = createDiffStorageNoWait(diff, aVariant, progress);
+    /* Apply the normal locking logic to the entire chain. */
+    MediumLockList *pMediumLockList(new MediumLockList());
+    HRESULT rc = createMediumLockList(true, this, *pMediumLockList);
     if (FAILED(rc))
     {
-        HRESULT rc2 = UnlockRead(NULL);
-        AssertComRC(rc2);
-        /* Note: on success, the task will unlock this */
-    }
+        delete pMediumLockList;
+        return rc;
+    }
+
+    ComObjPtr <Progress> pProgress;
+
+    rc = createDiffStorage(diff, aVariant, pMediumLockList, &pProgress,
+                           false /* aWait */, NULL /* pfNeedsSaveSettings*/);
+    if (FAILED(rc))
+        delete pMediumLockList;
     else
-        progress.queryInterfaceTo(aProgress);
+        pProgress.queryInterfaceTo(aProgress);
 
     return rc;
 }
 
-STDMETHODIMP Medium::MergeTo(IN_BSTR /* aTargetId */, IProgress ** /* aProgress */)
-{
+STDMETHODIMP Medium::MergeTo(IMedium *aTarget, IProgress **aProgress)
+{
+    CheckComArgNotNull(aTarget);
+    CheckComArgOutPointerValid(aProgress);
+    ComAssertRet(aTarget != this, E_INVALIDARG);
+
     AutoCaller autoCaller(this);
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
-    ReturnComNotImplemented();
+    ComObjPtr<Medium> pTarget = static_cast<Medium*>(aTarget);
+
+    bool fMergeForward = false;
+    ComObjPtr<Medium> pParentForTarget;
+    MediaList childrenToReparent;
+    MediumLockList *pMediumLockList = NULL;
+
+    HRESULT rc = S_OK;
+
+    rc = prepareMergeTo(pTarget, NULL, NULL, fMergeForward, pParentForTarget,
+                        childrenToReparent, pMediumLockList);
+    if (FAILED(rc)) return rc;
+
+    ComObjPtr <Progress> pProgress;
+
+    rc = mergeTo(pTarget, fMergeForward, pParentForTarget, childrenToReparent,
+                 pMediumLockList, &pProgress, false /* aWait */,
+                 NULL /* pfNeedsSaveSettings */);
+    if (FAILED(rc))
+        cancelMergeTo(childrenToReparent, pMediumLockList);
+    else
+        pProgress.queryInterfaceTo(aProgress);
+
+    return rc;
 }
 
@@ -2557 +2213 @@
     CheckComArgNotNull(aTarget);
     CheckComArgOutPointerValid(aProgress);
+    ComAssertRet(aTarget != this, E_INVALIDARG);
 
     AutoCaller autoCaller(this);
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
-    ComObjPtr<Medium> target = static_cast<Medium*>(aTarget);
-    ComObjPtr<Medium> parent;
+    ComObjPtr<Medium> pTarget = static_cast<Medium*>(aTarget);
+    ComObjPtr<Medium> pParent;
     if (aParent)
-        parent = static_cast<Medium*>(aParent);
-
-    ComObjPtr<Progress> progress;
+        pParent = static_cast<Medium*>(aParent);
+
     HRESULT rc = S_OK;
+    ComObjPtr<Progress> pProgress;
+    Medium::Task *pTask = NULL;
 
     try
@@ -2574 +2232 @@
         AutoReadLock treeLock(m->pVirtualBox->getMediaTreeLockHandle() COMMA_LOCKVAL_SRC_POS);
         // and we need to write-lock the images involved
-        AutoMultiWriteLock3 alock(this, target, parent COMMA_LOCKVAL_SRC_POS);
-
-        if (    target->m->state != MediumState_NotCreated
-            &&  target->m->state != MediumState_Created)
-            throw target->setStateError();
-
-        /** @todo separate out creating/locking an image chain from
-         * SessionMachine::lockMedia and use it from here too.
-         * logically this belongs into Medium functionality. */
-
-        /* Build the source chain and lock images in the proper order. */
-        std::auto_ptr<ImageChain> sourceChain(new ImageChain());
-
-        for (Medium *hd = this;
-             hd;
-             hd = hd->m->pParent)
-        {
-            rc = sourceChain->addImage(hd);
-            if (FAILED(rc)) throw rc;
-        }
-        rc = sourceChain->lockImagesRead();
+        AutoMultiWriteLock3 alock(this, pTarget, pParent COMMA_LOCKVAL_SRC_POS);
+
+        if (    pTarget->m->state != MediumState_NotCreated
+            &&  pTarget->m->state != MediumState_Created)
+            throw pTarget->setStateError();
+
+        /* Build the source lock list. */
+        MediumLockList *pSourceMediumLockList(new MediumLockList());
+        rc = createMediumLockList(false, NULL,
+                                  *pSourceMediumLockList);
+        if (FAILED(rc))
+        {
+            delete pSourceMediumLockList;
+            throw rc;
+        }
+
+        /* Build the target lock list (including the to-be parent chain). */
+        MediumLockList *pTargetMediumLockList(new MediumLockList());
+        rc = pTarget->createMediumLockList(true, pParent,
+                                           *pTargetMediumLockList);
+        if (FAILED(rc))
+        {
+            delete pSourceMediumLockList;
+            delete pTargetMediumLockList;
+            throw rc;
+        }
+
+        rc = pSourceMediumLockList->Lock();
+        if (FAILED(rc))
+        {
+            delete pSourceMediumLockList;
+            delete pTargetMediumLockList;
+            throw setError(rc,
+                           tr("Failed to lock source media '%ls'"),
+                           getLocationFull().raw());
+        }
+        rc = pTargetMediumLockList->Lock();
+        if (FAILED(rc))
+        {
+            delete pSourceMediumLockList;
+            delete pTargetMediumLockList;
+            throw setError(rc,
+                           tr("Failed to lock target media '%ls'"),
+                           pTarget->getLocationFull().raw());
+        }
+
+        pProgress.createObject();
+        rc = pProgress->init(m->pVirtualBox,
+                             static_cast <IMedium *>(this),
+                             BstrFmt(tr("Creating clone hard disk '%s'"), pTarget->m->strLocationFull.raw()),
+                             TRUE /* aCancelable */);
+        if (FAILED(rc))
+        {
+            delete pSourceMediumLockList;
+            delete pTargetMediumLockList;
+            throw rc;
+        }
+
+        /* setup task object to carry out the operation asynchronously */
+        pTask = new Medium::CloneTask(this, pProgress, pTarget, aVariant,
+                                      pParent, pSourceMediumLockList,
+                                      pTargetMediumLockList);
+        rc = pTask->rc();
+        AssertComRC(rc);
         if (FAILED(rc)) throw rc;
 
-        /* Build the parent chain and lock images in the proper order. */
-        std::auto_ptr<ImageChain> parentChain(new ImageChain());
-
-        for (Medium *hd = parent;
-             hd;
-             hd = hd->m->pParent)
-        {
-            rc = parentChain->addImage(hd);
-            if (FAILED(rc)) throw rc;
-        }
-        if (target->m->state == MediumState_Created)
-        {
-            /* If we're cloning to an existing image the parent chain also
-             * contains the target image, and it gets locked for writing. */
-            rc = parentChain->addImage(target);
-            if (FAILED(rc)) throw rc;
-            rc = parentChain->lockImagesReadAndLastWrite();
-            if (FAILED(rc)) throw rc;
-        }
-        else
-        {
-            rc = parentChain->lockImagesRead();
-            if (FAILED(rc)) throw rc;
-        }
-
-        progress.createObject();
-        rc = progress->init(m->pVirtualBox,
-                            static_cast <IMedium *>(this),
-                            BstrFmt(tr("Creating clone hard disk '%s'"), target->m->strLocationFull.raw()),
-                            TRUE /* aCancelable */);
-        if (FAILED(rc)) throw rc;
-
-        /* setup task object to carry out the operation asynchronously */
-        Medium::Task *pTask(new Medium::CloneTask(this, progress, target,
-                                                  parent,
-                                                  sourceChain.release(),
-                                                  parentChain.release(),
-                                                  aVariant));
-        rc = pTask->rc();
-        if (FAILED(rc))
-        {
-            AssertComRC(rc);
-            delete pTask;
-            throw rc;
-        }
-
+        if (pTarget->m->state == MediumState_NotCreated)
+            pTarget->m->state = MediumState_Creating;
+    }
+    catch (HRESULT aRC) { rc = aRC; }
+
+    if (SUCCEEDED(rc))
+    {
         rc = startThread(pTask);
-        if (FAILED(rc)) throw rc;
-
-        if (target->m->state == MediumState_NotCreated)
-        {
-            /* go to Creating state before leaving the lock */
-            target->m->state = MediumState_Creating;
-        }
-    }
-    catch (HRESULT aRC)
-    {
-        rc = aRC;
-    }
-
-    if (SUCCEEDED(rc))
-        progress.queryInterfaceTo(aProgress);
+
+        if (SUCCEEDED(rc))
+            pProgress.queryInterfaceTo(aProgress);
+    }
+    else if (pTask != NULL)
+        delete pTask;
 
     return rc;
@@ -2670 +2323 @@
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-    ComObjPtr <Progress> progress;
-
     HRESULT rc = S_OK;
+    ComObjPtr <Progress> pProgress;
+    Medium::Task *pTask = NULL;
 
     try
     {
-        /** @todo separate out creating/locking an image chain from
-         * SessionMachine::lockMedia and use it from here too.
-         * logically this belongs into Medium functionality. */
-
-        /* Build the image chain and lock images in the proper order. */
-        std::auto_ptr<ImageChain> imgChain(new ImageChain());
-
-        /* we walk the image tree */
-        AutoReadLock srcTreeLock(m->pVirtualBox->getMediaTreeLockHandle() COMMA_LOCKVAL_SRC_POS);
-        for (Medium *hd = this;
-             hd;
-             hd = hd->m->pParent)
-        {
-            rc = imgChain->addImage(hd);
-            if (FAILED(rc)) throw rc;
-        }
-        rc = imgChain->lockImagesReadAndLastWrite();
+        /* We need to lock both the current object, and the tree lock (would
+         * cause a lock order violation otherwise) for createMediumLockList. */
+        AutoMultiWriteLock2 multilock(&m->pVirtualBox->getMediaTreeLockHandle(),
+                                      this->lockHandle()
+                                      COMMA_LOCKVAL_SRC_POS);
+
+        /* Build the medium lock list. */
+        MediumLockList *pMediumLockList(new MediumLockList());
+        rc = createMediumLockList(true, NULL,
+                                  *pMediumLockList);
+        if (FAILED(rc))
+        {
+            delete pMediumLockList;
+            throw rc;
+        }
+
+        rc = pMediumLockList->Lock();
+        if (FAILED(rc))
+        {
+            delete pMediumLockList;
+            throw setError(rc,
+                           tr("Failed to lock media when compacting '%ls'"),
+                           getLocationFull().raw());
+        }
+
+        pProgress.createObject();
+        rc = pProgress->init(m->pVirtualBox,
+                             static_cast <IMedium *>(this),
+                             BstrFmt(tr("Compacting hard disk '%s'"), m->strLocationFull.raw()),
+                             TRUE /* aCancelable */);
+        if (FAILED(rc))
+        {
+            delete pMediumLockList;
+            throw rc;
+        }
+
+        /* setup task object to carry out the operation asynchronously */
+        pTask = new Medium::CompactTask(this, pProgress, pMediumLockList);
+        rc = pTask->rc();
+        AssertComRC(rc);
         if (FAILED(rc)) throw rc;
-
-        progress.createObject();
-        rc = progress->init(m->pVirtualBox,
-                            static_cast <IMedium *>(this),
-                            BstrFmt(tr("Compacting hard disk '%s'"), m->strLocationFull.raw()),
-                            TRUE /* aCancelable */);
-        if (FAILED(rc)) throw rc;
-
-        /* setup task object to carry out the operation asynchronously */
-        Medium::Task *pTask(new Medium::CompactTask(this, progress,
-                                                    imgChain.release()));
-        rc = pTask->rc();
-        if (FAILED(rc))
-        {
-            AssertComRC(rc);
-            delete pTask;
-            throw rc;
-        }
-
+    }
+    catch (HRESULT aRC) { rc = aRC; }
+
+    if (SUCCEEDED(rc))
+    {
         rc = startThread(pTask);
-        if (FAILED(rc)) throw rc;
-    }
-    catch (HRESULT aRC)
-    {
-        rc = aRC;
-    }
-
-    if (SUCCEEDED(rc))
-        progress.queryInterfaceTo(aProgress);
+
+        if (SUCCEEDED(rc))
+            pProgress.queryInterfaceTo(aProgress);
+    }
+    else if (pTask != NULL)
+        delete pTask;
 
     return rc;
@@ -2748 +2405 @@
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
-    /* canClose() needs the tree lock */
-    AutoMultiWriteLock2 multilock(&m->pVirtualBox->getMediaTreeLockHandle(),
-                                  this->lockHandle()
-                                  COMMA_LOCKVAL_SRC_POS);
-
-    LogFlowThisFunc(("ENTER for medium %s\n", m->strLocationFull.c_str()));
-
-    if (m->pParent.isNull())
-        return setError(VBOX_E_NOT_SUPPORTED,
-                        tr ("Hard disk '%s' is not differencing"),
-                        m->strLocationFull.raw());
-
-    HRESULT rc = canClose();
-    if (FAILED(rc)) return rc;
-
-    rc = LockWrite(NULL);
-    if (FAILED(rc)) return rc;
-
-    ComObjPtr <Progress> progress;
+    HRESULT rc = S_OK;
+    ComObjPtr <Progress> pProgress;
+    Medium::Task *pTask = NULL;
 
     try
     {
-        progress.createObject();
-        rc = progress->init(m->pVirtualBox,
-                            static_cast<IMedium*>(this),
-                            BstrFmt(tr("Resetting differencing hard disk '%s'"), m->strLocationFull.raw()),
-                            FALSE /* aCancelable */);
+        /* canClose() needs the tree lock */
+        AutoMultiWriteLock2 multilock(&m->pVirtualBox->getMediaTreeLockHandle(),
+                                      this->lockHandle()
+                                      COMMA_LOCKVAL_SRC_POS);
+
+        LogFlowThisFunc(("ENTER for medium %s\n", m->strLocationFull.c_str()));
+
+        if (m->pParent.isNull())
+            throw setError(VBOX_E_NOT_SUPPORTED,
+                           tr("Hard disk '%s' is not differencing"),
+                           m->strLocationFull.raw());
+
+        rc = canClose();
         if (FAILED(rc)) throw rc;
 
+        /* Build the medium lock list. */
+        MediumLockList *pMediumLockList(new MediumLockList());
+        rc = createMediumLockList(true, NULL,
+                                  *pMediumLockList);
+        if (FAILED(rc))
+        {
+            delete pMediumLockList;
+            throw rc;
+        }
+
+        rc = pMediumLockList->Lock();
+        if (FAILED(rc))
+        {
+            delete pMediumLockList;
+            throw setError(rc,
+                           tr("Failed to lock media when resetting '%ls'"),
+                           getLocationFull().raw());
+        }
+
+        pProgress.createObject();
+        rc = pProgress->init(m->pVirtualBox,
+                             static_cast<IMedium*>(this),
+                             BstrFmt(tr("Resetting differencing hard disk '%s'"), m->strLocationFull.raw()),
+                             FALSE /* aCancelable */);
+        if (FAILED(rc)) throw rc;
+
         /* setup task object to carry out the operation asynchronously */
-        Medium::Task *pTask(new Medium::ResetTask(this, progress));
+        pTask = new Medium::ResetTask(this, pProgress, pMediumLockList);
         rc = pTask->rc();
-        if (FAILED(rc))
-        {
-            AssertComRC(rc);
+        AssertComRC(rc);
+        if (FAILED(rc)) throw rc;
+    }
+    catch (HRESULT aRC) { rc = aRC; }
+
+    if (SUCCEEDED(rc))
+    {
+        rc = startThread(pTask);
+
+        if (SUCCEEDED(rc))
+            pProgress.queryInterfaceTo(aProgress);
+    }
+    else
+    {
+        /* Note: on success, the task will unlock this */
+        {
+            AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+            HRESULT rc2 = UnlockWrite(NULL);
+            AssertComRC(rc2);
+        }
+        if (pTask != NULL)
             delete pTask;
-            throw rc;
-        }
-
-        rc = startThread(pTask);
-        if (FAILED(rc)) throw rc;
-    }
-    catch (HRESULT aRC)
-    {
-        rc = aRC;
-    }
-
-    if (FAILED(rc))
-    {
-        HRESULT rc2 = UnlockWrite(NULL);
-        AssertComRC(rc2);
-        /* Note: on success, the task will unlock this */
-    }
-    else
-        progress.queryInterfaceTo(aProgress);
+    }
 
     LogFlowThisFunc(("LEAVE, rc=%Rhrc\n", rc));
@@ -3126 +2801 @@
     AssertComRCReturnVoid(autoCaller.rc());
 
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
     /* we access children() */
     AutoReadLock treeLock(m->pVirtualBox->getMediaTreeLockHandle() COMMA_LOCKVAL_SRC_POS);
+
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
     updatePath(aOldPath, aNewPath);
@@ -3136 +2811 @@
     for (MediaList::const_iterator it = getChildren().begin();
          it != getChildren().end();
-         ++ it)
+         ++it)
     {
         (*it)->updatePaths(aOldPath, aNewPath);
@@ -3214 +2889 @@
 
             for (BackRefList::const_iterator it = m->backRefs.begin();
-                 it != m->backRefs.end(); ++ it)
+                 it != m->backRefs.end(); ++it)
                 if (it->llSnapshotIds.size() != 0)
                     return true;
@@ -3352 +3027 @@
 
 /**
- * Checks that this hard disk may be discarded and performs necessary state
- * changes. Must not be called for writethrough disks because there is nothing
- * to discard then.
- *
- * This method is to be called prior to calling the #discard() to perform
- * necessary consistency checks and place involved hard disks to appropriate
- * states. If #discard() is not called or fails, the state modifications
- * performed by this method must be undone by #cancelDiscard().
- *
- * See #discard() for more info about discarding hard disks.
- *
- * @param aChain        Where to store the created merge chain (may return NULL
- *                      if no real merge is necessary).
- *
- * @note Caller must hold media tree lock for writing. This locks this object
- *       and every medium object on the merge chain for writing.
- */
-HRESULT Medium::prepareDiscard(MergeChain * &aChain)
-{
-    AutoCaller autoCaller(this);
-    AssertComRCReturnRC(autoCaller.rc());
-
-    aChain = NULL;
-
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-    Assert(m->pVirtualBox->getMediaTreeLockHandle().isWriteLockOnCurrentThread());
-
-    // Medium must not be writethrough at this point
-    AssertReturn(   m->type == MediumType_Normal
-                 || m->type == MediumType_Immutable, E_FAIL);
-
-    if (getChildren().size() == 0)
-    {
-        /* special treatment of the last hard disk in the chain: */
-
-        if (m->pParent.isNull())
-        {
-            /* lock only, to prevent any usage; discard() will unlock */
-            return LockWrite(NULL);
-        }
-
-        /* the differencing hard disk w/o children will be deleted, protect it
-         * from attaching to other VMs (this is why Deleting) */
-
-        switch (m->state)
-        {
-            case MediumState_Created:
-                m->state = MediumState_Deleting;
-                break;
-            default:
-                return setStateError();
-        }
-
-        /* aChain is intentionally NULL here */
-
-        return S_OK;
-    }
-
-    /* not going multi-merge as it's too expensive */
-    if (getChildren().size() > 1)
-        return setError(E_FAIL,
-                        tr ("Hard disk '%s' has more than one child hard disk (%d)"),
-                        m->strLocationFull.raw(), getChildren().size());
-
-    /* this is a read-only hard disk with children; it must be associated with
-     * exactly one snapshot (when the snapshot is being taken, none of the
-     * current VM's hard disks may be attached to other VMs). Note that by the
-     * time when discard() is called, there must be no any attachments at all
-     * (the code calling prepareDiscard() should detach). */
-    AssertReturn(m->backRefs.size() == 1, E_FAIL);
-    AssertReturn(!m->backRefs.front().fInCurState, E_FAIL);
-    AssertReturn(m->backRefs.front().llSnapshotIds.size() == 1, E_FAIL);
-
-    ComObjPtr<Medium> child = getChildren().front();
-
-    /* we keep this locked, so lock the affected child to make sure the lock
-     * order is correct when calling prepareMergeTo() */
-    AutoWriteLock childLock(child COMMA_LOCKVAL_SRC_POS);
-
-    /* delegate the rest to the profi */
-    if (m->pParent.isNull())
-    {
-        /* base hard disk, backward merge */
-        Assert(child->m->backRefs.size() == 1);
-        if (child->m->backRefs.front().machineId != m->backRefs.front().machineId)
-        {
-            /* backward merge is too tricky, we'll just detach on discard, so
-             * lock only, to prevent any usage; discard() will only unlock
-             * (since we return NULL in aChain) */
-            return LockWrite(NULL);
-        }
-
-        return child->prepareMergeTo(this, aChain, true /* aIgnoreAttachments */);
-    }
-    else
-    {
-        /* forward merge */
-        return prepareMergeTo(child, aChain, true /* aIgnoreAttachments */);
-    }
-}
-
-/**
- * Discards this hard disk.
- *
- * Discarding the hard disk is merging its contents to its differencing child
- * hard disk (forward merge) or contents of its child hard disk to itself
- * (backward merge) if this hard disk is a base hard disk. If this hard disk is
- * a differencing hard disk w/o children, then it will be simply deleted.
- * Calling this method on a base hard disk w/o children will do nothing and
- * silently succeed. If this hard disk has more than one child, the method will
- * currently return an error (since merging in this case would be too expensive
- * and result in data duplication).
- *
- * When the backward merge takes place (i.e. this hard disk is a target) then,
- * on success, this hard disk will automatically replace the differencing child
- * hard disk used as a source (which will then be deleted) in the attachment
- * this child hard disk is associated with. This will happen only if both hard
- * disks belong to the same machine because otherwise such a replace would be
- * too tricky and could be not expected by the other machine. Same relates to a
- * case when the child hard disk is not associated with any machine at all. When
- * the backward merge is not applied, the method behaves as if the base hard
- * disk were not attached at all -- i.e. simply detaches it from the machine but
- * leaves the hard disk chain intact.
- *
- * This method is basically a wrapper around #mergeTo() that selects the correct
- * merge direction and performs additional actions as described above and.
- *
- * Note that this method will not return until the merge operation is complete
- * (which may be quite time consuming depending on the size of the merged hard
- * disks).
- *
- * Note that #prepareDiscard() must be called before calling this method. If
- * this method returns a failure, the caller must call #cancelDiscard(). On
- * success, #cancelDiscard() must not be called (this method will perform all
- * necessary steps such as resetting states of all involved hard disks and
- * deleting @a aChain).
- *
- * @param aChain        Merge chain created by #prepareDiscard() (may be NULL if
- *                      no real merge takes place).
- *
- * @note Locks the hard disks from the chain for writing. Locks the machine
- *       object when the backward merge takes place. Locks medium tree for
- *       reading or writing.
- */
-HRESULT Medium::discard(ComObjPtr<Progress> &aProgress,
-                        ULONG ulWeight,
-                        MergeChain *aChain,
-                        bool *pfNeedsSaveSettings)
-{
-    AssertReturn(!aProgress.isNull(), E_FAIL);
-
-    ComObjPtr <Medium> hdFrom;
-
+ * Constructs a medium lock list for this medium. The lock is not taken.
+ *
+ * @note Locks the medium tree for reading.
+ *
+ * @param fMediumWritable   Whether to associate a write lock with this medium.
+ * @param pToBeParent       Medium which will become the parent of this medium.
+ * @param mediumLockList    Where to store the resulting list.
+ */
+HRESULT Medium::createMediumLockList(bool fMediumWritable,
+                                     Medium *pToBeParent,
+                                     MediumLockList &mediumLockList)
+{
     HRESULT rc = S_OK;
 
-    {
-        AutoCaller autoCaller(this);
-        AssertComRCReturnRC(autoCaller.rc());
-
-        aProgress->SetNextOperation(BstrFmt(tr("Merging differencing image '%s'"), getName().raw()),
-                                    ulWeight);        // weight
-
-        if (aChain == NULL)
-        {
-            /* we access mParent & children() */
-            AutoMultiWriteLock2 mLock(&m->pVirtualBox->getMediaTreeLockHandle(), this->lockHandle() COMMA_LOCKVAL_SRC_POS);
-
-            Assert(getChildren().size() == 0);
-
-            /* special treatment of the last hard disk in the chain: */
-
-            if (m->pParent.isNull())
+    /* we access parent medium objects */
+    AutoReadLock treeLock(m->pVirtualBox->getMediaTreeLockHandle() COMMA_LOCKVAL_SRC_POS);
+
+    /* paranoid sanity checking if the medium has a to-be parent medium */
+    if (pToBeParent)
+    {
+        AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+        ComAssertRet(getParent().isNull(), E_FAIL);
+        ComAssertRet(getChildren().size() == 0, E_FAIL);
+    }
+
+    ErrorInfoKeeper eik;
+    MultiResult mrc(S_OK);
+
+    ComObjPtr<Medium> pMedium = this;
+    while (!pMedium.isNull())
+    {
+        // need write lock for RefreshState if medium is inaccessible
+        AutoWriteLock alock(pMedium COMMA_LOCKVAL_SRC_POS);
+
+        /* Accessibility check must be first, otherwise locking interferes
+         * with getting the medium state. Lock lists are not created for
+         * fun, and thus getting the image status is no luxury. */
+        MediumState_T mediumState = pMedium->getState();
+        if (mediumState == MediumState_Inaccessible)
+        {
+            rc = pMedium->RefreshState(&mediumState);
+            if (FAILED(rc)) return rc;
+
+            if (mediumState == MediumState_Inaccessible)
             {
-                rc = UnlockWrite(NULL);
-                AssertComRC(rc);
-                return rc;
+                Bstr error;
+                rc = pMedium->COMGETTER(LastAccessError)(error.asOutParam());
+                if (FAILED(rc)) return rc;
+
+                Bstr loc;
+                rc = pMedium->COMGETTER(Location)(loc.asOutParam());
+                if (FAILED(rc)) return rc;
+
+                /* collect multiple errors */
+                eik.restore();
+
+                /* be in sync with MediumBase::setStateError() */
+                Assert(!error.isEmpty());
+                mrc = setError(E_FAIL,
+                               tr("Medium '%ls' is not accessible. %ls"),
+                               loc.raw(),
+                               error.raw());
+
+                eik.fetch();
             }
-
-            /* delete the differencing hard disk w/o children */
-
-            Assert(m->state == MediumState_Deleting);
-
-            /* go back to Created since deleteStorage() expects this state */
-            m->state = MediumState_Created;
-
-            hdFrom = this;
-
-            rc = deleteStorageAndWait(&aProgress, pfNeedsSaveSettings);
-        }
+        }
+
+        if (pMedium == this)
+            mediumLockList.Prepend(pMedium, fMediumWritable);
         else
-        {
-            hdFrom = aChain->source();
-
-            rc = hdFrom->mergeToAndWait(aChain, &aProgress, pfNeedsSaveSettings);
-        }
-    }
-
-    if (SUCCEEDED(rc))
-    {
-        /* mergeToAndWait() cannot uninitialize the initiator because of
-         * possible AutoCallers on the current thread, deleteStorageAndWait()
-         * doesn't do it either; do it ourselves */
-        hdFrom->uninit();
-    }
-
-    return rc;
-}
-
-/**
- * Undoes what #prepareDiscard() did. Must be called if #discard() is not called
- * or fails. Frees memory occupied by @a aChain.
- *
- * @param aChain        Merge chain created by #prepareDiscard() (may be NULL if
- *                      no real merge takes place).
- *
- * @note Locks the hard disks from the chain for writing. Locks medium tree for
- *       reading.
- */
-void Medium::cancelDiscard(MergeChain *aChain)
-{
-    AutoCaller autoCaller(this);
-    AssertComRCReturnVoid(autoCaller.rc());
-
-    if (aChain == NULL)
-    {
-        AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-        /* we access mParent & children() */
-        AutoReadLock treeLock(m->pVirtualBox->getMediaTreeLockHandle() COMMA_LOCKVAL_SRC_POS);
-
-        Assert(getChildren().size() == 0);
-
-        /* special treatment of the last hard disk in the chain: */
-
-        if (m->pParent.isNull())
-        {
-            HRESULT rc = UnlockWrite(NULL);
-            AssertComRC(rc);
-            return;
-        }
-
-        /* the differencing hard disk w/o children will be deleted, protect it
-         * from attaching to other VMs (this is why Deleting) */
-
-        Assert(m->state == MediumState_Deleting);
-        m->state = MediumState_Created;
-
-        return;
-    }
-
-    /* delegate the rest to the profi */
-    cancelMergeTo(aChain);
+            mediumLockList.Prepend(pMedium, false);
+
+        pMedium = pMedium->getParent();
+        if (pMedium.isNull() && pToBeParent)
+            pMedium = pToBeParent;
+    }
+
+    return mrc;
 }
 
@@ -3915 +3415 @@
     /* leave the lock before a lengthy operation */
     vrc = RTSemEventMultiReset(m->queryInfoSem);
-    ComAssertRCThrow(vrc, E_FAIL);
+    AssertRCReturn(vrc, E_FAIL);
     m->queryInfoRunning = true;
     alock.leave();
@@ -3935 +3435 @@
         {
             /** @todo This kind of opening of images is assuming that diff
-             * images can be opened as base images. Should be fixed ASAP. */
+             * images can be opened as base images. Should be documented if
+             * it must work for all medium format backends. */
             vrc = VDOpen(hdd,
                          format.c_str(),
@@ -4033 +3534 @@
 
                     Guid id = parentId;
-                    ComObjPtr<Medium> parent;
+                    ComObjPtr<Medium> pParent;
                     rc = m->pVirtualBox->findHardDisk(&id, NULL,
                                                    false /* aSetError */,
-                                                   &parent);
+                                                   &pParent);
                     if (FAILED(rc))
                     {
@@ -4050 +3551 @@
 
                     Assert(m->pParent.isNull());
-                    m->pParent = parent;
+                    m->pParent = pParent;
                     m->pParent->m->llChildren.push_back(this);
                 }
@@ -4253 +3754 @@
     AssertReturn(aProgress != NULL || aWait == true, E_FAIL);
 
-    /* we're accessing the media tree, and canClose() needs the tree lock too */
-    AutoMultiWriteLock2 multilock(&m->pVirtualBox->getMediaTreeLockHandle(),
-                                  this->lockHandle()
-                                  COMMA_LOCKVAL_SRC_POS);
-    LogFlowThisFunc(("aWait=%RTbool locationFull=%s\n", aWait, getLocationFull().c_str() ));
-
-    if (    !(m->formatObj->capabilities() & (   MediumFormatCapabilities_CreateDynamic
-                                               | MediumFormatCapabilities_CreateFixed)))
-        return setError(VBOX_E_NOT_SUPPORTED,
-                        tr("Hard disk format '%s' does not support storage deletion"),
-                        m->strFormat.raw());
-
-    /* Note that we are fine with Inaccessible state too: a) for symmetry with
-     * create calls and b) because it doesn't really harm to try, if it is
-     * really inaccessible, the delete operation will fail anyway. Accepting
-     * Inaccessible state is especially important because all registered hard
-     * disks are initially Inaccessible upon VBoxSVC startup until
-     * COMGETTER(State) is called. */
-
+    HRESULT rc = S_OK;
+    ComObjPtr<Progress> pProgress;
+    Medium::Task *pTask = NULL;
+
+    try
+    {
+        /* we're accessing the media tree, and canClose() needs it too */
+        AutoMultiWriteLock2 multilock(&m->pVirtualBox->getMediaTreeLockHandle(),
+                                      this->lockHandle()
+                                      COMMA_LOCKVAL_SRC_POS);
+        LogFlowThisFunc(("aWait=%RTbool locationFull=%s\n", aWait, getLocationFull().c_str() ));
+
+        if (    !(m->formatObj->capabilities() & (   MediumFormatCapabilities_CreateDynamic
+                                                   | MediumFormatCapabilities_CreateFixed)))
+            throw setError(VBOX_E_NOT_SUPPORTED,
+                           tr("Hard disk format '%s' does not support storage deletion"),
+                           m->strFormat.raw());
+
+        /* Note that we are fine with Inaccessible state too: a) for symmetry
+         * with create calls and b) because it doesn't really harm to try, if
+         * it is really inaccessible, the delete operation will fail anyway.
+         * Accepting Inaccessible state is especially important because all
+         * registered hard disks are initially Inaccessible upon VBoxSVC
+         * startup until COMGETTER(RefreshState) is called. Accept Deleting
+         * state because some callers need to put the image in this state early
+         * to prevent races. */
+        switch (m->state)
+        {
+            case MediumState_Created:
+            case MediumState_Deleting:
+            case MediumState_Inaccessible:
+                break;
+            default:
+                throw setStateError();
+        }
+
+        if (m->backRefs.size() != 0)
+        {
+            Utf8Str strMachines;
+            for (BackRefList::const_iterator it = m->backRefs.begin();
+                it != m->backRefs.end();
+                ++it)
+            {
+                const BackRef &b = *it;
+                if (strMachines.length())
+                    strMachines.append(", ");
+                strMachines.append(b.machineId.toString().c_str());
+            }
+#ifdef DEBUG
+            dumpBackRefs();
+#endif
+            throw setError(VBOX_E_OBJECT_IN_USE,
+                           tr("Cannot delete storage: hard disk '%s' is still attached to the following %d virtual machine(s): %s"),
+                           m->strLocationFull.c_str(),
+                           m->backRefs.size(),
+                           strMachines.c_str());
+        }
+
+        rc = canClose();
+        if (FAILED(rc)) throw rc;
+
+        /* go to Deleting state, so that the medium is not actually locked */
+        rc = markForDeletion();
+        if (FAILED(rc)) throw rc;
+
+        /* Build the medium lock list. */
+        MediumLockList *pMediumLockList(new MediumLockList());
+        rc = createMediumLockList(true, NULL,
+                                  *pMediumLockList);
+        if (FAILED(rc))
+        {
+            delete pMediumLockList;
+            throw rc;
+        }
+
+        rc = pMediumLockList->Lock();
+        if (FAILED(rc))
+        {
+            delete pMediumLockList;
+            throw setError(rc,
+                           tr("Failed to lock media when deleting '%ls'"),
+                           getLocationFull().raw());
+        }
+
+        /* try to remove from the list of known hard disks before performing
+         * actual deletion (we favor the consistency of the media registry in
+         * the first place which would have been broken if
+         * unregisterWithVirtualBox() failed after we successfully deleted the
+         * storage) */
+        rc = unregisterWithVirtualBox(pfNeedsSaveSettings);
+        if (FAILED(rc)) throw rc;
+
+        if (aProgress != NULL)
+        {
+            /* use the existing progress object... */
+            pProgress = *aProgress;
+
+            /* ...but create a new one if it is null */
+            if (pProgress.isNull())
+            {
+                pProgress.createObject();
+                rc = pProgress->init(m->pVirtualBox,
+                                     static_cast<IMedium*>(this),
+                                     BstrFmt(tr("Deleting hard disk storage unit '%s'"), m->strLocationFull.raw()),
+                                     FALSE /* aCancelable */);
+                if (FAILED(rc)) throw rc;
+            }
+        }
+
+        /* setup task object to carry out the operation sync/async */
+        pTask = new Medium::DeleteTask(this, pProgress, pMediumLockList);
+        rc = pTask->rc();
+        AssertComRC(rc);
+        if (FAILED(rc)) throw rc;
+    }
+    catch (HRESULT aRC) { rc = aRC; }
+
+    if (SUCCEEDED(rc))
+    {
+        if (aWait)
+            rc = runNow(pTask, NULL /* pfNeedsSaveSettings*/);
+        else
+            rc = startThread(pTask);
+
+        if (SUCCEEDED(rc) && aProgress != NULL)
+            *aProgress = pProgress;
+
+    }
+    else
+    {
+        if (pTask)
+            delete pTask;
+
+        /* Undo deleting state if necessary. */
+        AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+        unmarkForDeletion();
+    }
+
+    return rc;
+}
+
+/**
+ * Mark a medium for deletion.
+ *
+ * @note Caller must hold the write lock on this medium!
+ */
+HRESULT Medium::markForDeletion()
+{
+    ComAssertRet(this->lockHandle()->isWriteLockOnCurrentThread(), E_FAIL);
     switch (m->state)
     {
         case MediumState_Created:
         case MediumState_Inaccessible:
-            break;
+            m->preLockState = m->state;
+            m->state = MediumState_Deleting;
+            return S_OK;
         default:
             return setStateError();
     }
-
-    if (m->backRefs.size() != 0)
-    {
-        Utf8Str strMachines;
-        for (BackRefList::const_iterator it = m->backRefs.begin();
-            it != m->backRefs.end();
-            ++it)
-        {
-            const BackRef &b = *it;
-            if (strMachines.length())
-                strMachines.append(", ");
-            strMachines.append(b.machineId.toString().c_str());
-        }
-#ifdef DEBUG
-        dumpBackRefs();
-#endif
-        return setError(VBOX_E_OBJECT_IN_USE,
-                        tr("Cannot delete storage: hard disk '%s' is still attached to the following %d virtual machine(s): %s"),
-                        m->strLocationFull.c_str(),
-                        m->backRefs.size(),
-                        strMachines.c_str());
-    }
-
-    HRESULT rc = canClose();
-    if (FAILED(rc)) return rc;
-
-    /* go to Deleting state before leaving the lock */
-    m->state = MediumState_Deleting;
-
-    /* try to remove from the list of known hard disks before performing actual
-     * deletion (we favor the consistency of the media registry in the first
-     * place which would have been broken if unregisterWithVirtualBox() failed
-     * after we successfully deleted the storage) */
-    rc = unregisterWithVirtualBox(pfNeedsSaveSettings);
-
-    /* restore the state because we may fail below; we will set it later again*/
-    m->state = MediumState_Created;
-
-    if (FAILED(rc)) return rc;
-
-    ComObjPtr<Progress> progress;
-
-    if (aProgress != NULL)
-    {
-        /* use the existing progress object... */
-        progress = *aProgress;
-
-        /* ...but create a new one if it is null */
-        if (progress.isNull())
-        {
-            progress.createObject();
-            rc = progress->init(m->pVirtualBox,
-                                static_cast<IMedium*>(this),
-                                BstrFmt(tr("Deleting hard disk storage unit '%s'"), m->strLocationFull.raw()),
-                                FALSE /* aCancelable */);
-            if (FAILED(rc)) return rc;
-        }
-    }
-
-    /* setup task object to carry out the operation asynchronously */
-    Medium::Task *pTask(new Medium::DeleteTask(this, progress));
-    rc = pTask->rc();
-    if (FAILED(rc))
-    {
-        AssertComRC(rc);
-        delete pTask;
-        return rc;
-    }
-
-    if (aWait)
-    {
-        /* go to Deleting state before starting the task */
-        m->state = MediumState_Deleting;
-
-        rc = runNow(pTask, NULL /* pfNeedsSaveSettings*/ );        // there is no save settings to do in taskThreadDelete()
-        if (FAILED(rc)) return rc;
-    }
-    else
-    {
-        rc = startThread(pTask);
-        if (FAILED(rc)) return rc;
-
-        /* go to Deleting state before leaving the lock */
-        m->state = MediumState_Deleting;
-    }
-
-    if (aProgress != NULL)
-        *aProgress = progress;
-
-    return rc;
+}
+
+/**
+ * Removes the "mark for deletion".
+ *
+ * @note Caller must hold the write lock on this medium!
+ */
+HRESULT Medium::unmarkForDeletion()
+{
+    ComAssertRet(this->lockHandle()->isWriteLockOnCurrentThread(), E_FAIL);
+    switch (m->state)
+    {
+        case MediumState_Deleting:
+            m->state = m->preLockState;
+            return S_OK;
+        default:
+            return setStateError();
+    }
 }
 
@@ -4376 +3937 @@
  * format and the location. Note that @c aTarget must be NotCreated.
  *
- * As opposed to the CreateDiffStorage() method, this method doesn't try to lock
- * this hard disk for reading assuming that the caller has already done so. This
- * is used when taking an online snapshot (where all original hard disks are
- * locked for writing and must remain such). Note however that if @a aWait is
- * @c false and this method returns a success then the thread started by
- * this method will unlock the hard disk (unless it is in
- * MediumState_LockedWrite state) so make sure the hard disk is either in
- * MediumState_LockedWrite or call #LockRead() before calling this method! If @a
- * aWait is @c true then this method neither locks nor unlocks the hard disk, so
- * make sure you do it yourself as needed.
- *
- * If @a aProgress is not NULL but the object it points to is @c null then a new
- * progress object will be created and assigned to @a *aProgress on success,
- * otherwise the existing progress object is used. If @a aProgress is NULL, then no
- * progress object is created/used at all.
+ * The @a aMediumLockList parameter contains the associated medium lock list,
+ * which must be in locked state. If @a aWait is @c true then the caller is
+ * responsible for unlocking.
+ *
+ * If @a aProgress is not NULL but the object it points to is @c null then a
+ * new progress object will be created and assigned to @a *aProgress on
+ * success, otherwise the existing progress object is used. If @a aProgress is
+ * NULL, then no progress object is created/used at all.
  *
  * When @a aWait is @c false, this method will create a thread to perform the
@@ -4398 +3952 @@
  * NULL when @a aWait is @c false (this method will assert in this case).
  *
- * @param aTarget       Target hard disk.
- * @param aVariant      Precise image variant to create.
+ * @param aTarget           Target hard disk.
+ * @param aVariant          Precise image variant to create.
+ * @param aMediumLockList   List of media which should be locked.
+ * @param aProgress         Where to find/store a Progress object to track
+ *                          operation completion.
+ * @param aWait             @c true if this method should block instead of
+ *                          creating an asynchronous thread.
+ * @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. This only works in "wait"
+ *                          mode; otherwise saveSettings is called
+ *                          automatically by the thread that was created,
+ *                          and this parameter is ignored.
+ *
+ * @note Locks this object and @a aTarget for writing.
+ */
+HRESULT Medium::createDiffStorage(ComObjPtr<Medium> &aTarget,
+                                  MediumVariant_T aVariant,
+                                  MediumLockList *aMediumLockList,
+                                  ComObjPtr<Progress> *aProgress,
+                                  bool aWait,
+                                  bool *pfNeedsSaveSettings)
+{
+    AssertReturn(!aTarget.isNull(), E_FAIL);
+    AssertReturn(aMediumLockList, E_FAIL);
+    AssertReturn(aProgress != NULL || aWait == true, E_FAIL);
+
+    AutoCaller autoCaller(this);
+    if (FAILED(autoCaller.rc())) return autoCaller.rc();
+
+    AutoCaller targetCaller(aTarget);
+    if (FAILED(targetCaller.rc())) return targetCaller.rc();
+
+    HRESULT rc = S_OK;
+    ComObjPtr<Progress> pProgress;
+    Medium::Task *pTask = NULL;
+
+    try
+    {
+        AutoMultiWriteLock2 alock(this, aTarget COMMA_LOCKVAL_SRC_POS);
+
+        ComAssertThrow(m->type != MediumType_Writethrough, E_FAIL);
+        ComAssertThrow(m->state == MediumState_LockedRead, E_FAIL);
+
+        if (aTarget->m->state != MediumState_NotCreated)
+            throw aTarget->setStateError();
+
+        /* Check that the hard disk is not attached to the current state of
+         * any VM referring to it. */
+        for (BackRefList::const_iterator it = m->backRefs.begin();
+             it != m->backRefs.end();
+             ++it)
+        {
+            if (it->fInCurState)
+            {
+                /* Note: when a VM snapshot is being taken, all normal hard
+                 * disks attached to the VM in the current state will be, as an
+                 * exception, also associated with the snapshot which is about
+                 * to create (see SnapshotMachine::init()) before deassociating
+                 * them from the current state (which takes place only on
+                 * success in Machine::fixupHardDisks()), so that the size of
+                 * snapshotIds will be 1 in this case. The extra condition is
+                 * used to filter out this legal situation. */
+                if (it->llSnapshotIds.size() == 0)
+                    throw setError(VBOX_E_INVALID_OBJECT_STATE,
+                                   tr("Hard disk '%s' is attached to a virtual machine with UUID {%RTuuid}. No differencing hard disks based on it may be created until it is detached"),
+                                   m->strLocationFull.raw(), it->machineId.raw());
+
+                Assert(it->llSnapshotIds.size() == 1);
+            }
+        }
+
+        if (aProgress != NULL)
+        {
+            /* use the existing progress object... */
+            pProgress = *aProgress;
+
+            /* ...but create a new one if it is null */
+            if (pProgress.isNull())
+            {
+                pProgress.createObject();
+                rc = pProgress->init(m->pVirtualBox,
+                                     static_cast<IMedium*>(this),
+                                     BstrFmt(tr("Creating differencing hard disk storage unit '%s'"), aTarget->m->strLocationFull.raw()),
+                                     TRUE /* aCancelable */);
+                if (FAILED(rc)) throw rc;
+            }
+        }
+
+        /* setup task object to carry out the operation sync/async */
+        pTask = new Medium::CreateDiffTask(this, pProgress, aTarget, aVariant,
+                                           aMediumLockList,
+                                           aWait /* fKeepMediumLockList */);
+        rc = pTask->rc();
+        AssertComRC(rc);
+        if (FAILED(rc)) throw rc;
+
+        /* register a task (it will deregister itself when done) */
+        ++m->numCreateDiffTasks;
+        Assert(m->numCreateDiffTasks != 0); /* overflow? */
+
+        aTarget->m->state = MediumState_Creating;
+    }
+    catch (HRESULT aRC) { rc = aRC; }
+
+    if (SUCCEEDED(rc))
+    {
+        if (aWait)
+            rc = runNow(pTask, pfNeedsSaveSettings);
+        else
+            rc = startThread(pTask);
+
+        if (SUCCEEDED(rc) && aProgress != NULL)
+            *aProgress = pProgress;
+    }
+    else if (pTask != NULL)
+        delete pTask;
+
+    return rc;
+}
+
+/**
+ * Prepares this (source) hard disk, target hard disk and all intermediate hard
+ * disks for the merge operation.
+ *
+ * This method is to be called prior to calling the #mergeTo() to perform
+ * necessary consistency checks and place involved hard disks to appropriate
+ * states. If #mergeTo() is not called or fails, the state modifications
+ * performed by this method must be undone by #cancelMergeTo().
+ *
+ * See #mergeTo() for more information about merging.
+ *
+ * @param pTarget       Target hard disk.
+ * @param aMachineId    Allowed machine attachment. NULL means do not check.
+ * @param aSnapshotId   Allowed snapshot attachment. NULL or empty UUID means
+ *                      do not check.
+ * @param fMergeForward Resulting merge direction (out).
+ * @param pParentForTarget New parent for target medium after merge (out).
+ * @param aChildrenToReparent List of children of the source which will have
+ *                      to be reparented to the target after merge (out).
+ * @param aMediumLockList Medium locking information (out).
+ *
+ * @note Locks medium tree for reading. Locks this object, aTarget and all
+ *       intermediate hard disks for writing.
+ */
+HRESULT Medium::prepareMergeTo(const ComObjPtr<Medium> &pTarget,
+                               const Guid *aMachineId,
+                               const Guid *aSnapshotId,
+                               bool &fMergeForward,
+                               ComObjPtr<Medium> &pParentForTarget,
+                               MediaList &aChildrenToReparent,
+                               MediumLockList * &aMediumLockList)
+{
+    AssertReturn(pTarget != NULL, E_FAIL);
+    AssertReturn(pTarget != this, E_FAIL);
+
+    AutoCaller autoCaller(this);
+    AssertComRCReturnRC(autoCaller.rc());
+
+    AutoCaller targetCaller(pTarget);
+    AssertComRCReturnRC(targetCaller.rc());
+
+    HRESULT rc = S_OK;
+    fMergeForward = false;
+    pParentForTarget.setNull();
+    aChildrenToReparent.clear();
+    Assert(aMediumLockList == NULL);
+    aMediumLockList = NULL;
+
+    try
+    {
+        // locking: we need the tree lock first because we access parent pointers
+        AutoReadLock treeLock(m->pVirtualBox->getMediaTreeLockHandle() COMMA_LOCKVAL_SRC_POS);
+
+        /* more sanity checking and figuring out the merge direction */
+        ComObjPtr<Medium> pMedium = getParent();
+        while (!pMedium.isNull() && pMedium != pTarget)
+            pMedium = pMedium->getParent();
+        if (pMedium == pTarget)
+            fMergeForward = false;
+        else
+        {
+            pMedium = pTarget->getParent();
+            while (!pMedium.isNull() && pMedium != this)
+                pMedium = pMedium->getParent();
+            if (pMedium == this)
+                fMergeForward = true;
+            else
+            {
+                Utf8Str tgtLoc;
+                {
+                    AutoReadLock alock(pTarget COMMA_LOCKVAL_SRC_POS);
+                    tgtLoc = pTarget->getLocationFull();
+                }
+
+                AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+                throw setError(E_FAIL,
+                               tr("Hard disks '%s' and '%s' are unrelated"),
+                               m->strLocationFull.raw(), tgtLoc.raw());
+            }
+        }
+
+        /* Build the lock list. */
+        aMediumLockList = new MediumLockList();
+        if (fMergeForward)
+            rc = pTarget->createMediumLockList(true, NULL, *aMediumLockList);
+        else
+            rc = createMediumLockList(false, NULL, *aMediumLockList);
+        if (FAILED(rc))
+        {
+            delete aMediumLockList;
+            aMediumLockList = NULL;
+            throw rc;
+        }
+
+        /* sanity checking */
+        {
+            AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+            if (m->state != MediumState_Created)
+                throw setStateError();
+        }
+        {
+            AutoReadLock alock(pTarget COMMA_LOCKVAL_SRC_POS);
+            if (pTarget->m->state != MediumState_Created)
+                throw pTarget->setStateError();
+        }
+
+        /* check medium attachment and other sanity conditions */
+        if (fMergeForward)
+        {
+            AutoReadLock(this COMMA_LOCKVAL_SRC_POS);
+            if (getChildren().size() > 1)
+            {
+                throw setError(E_FAIL,
+                               tr("Medium '%s' involved in the merge operation has more than one child medium (%d)"),
+                               m->strLocationFull.raw(), getChildren().size());
+            }
+            /* One backreference is only allowed if the machine ID is not empty
+             * and it matches the machine the image is attached to (including
+             * the snapshot ID if not empty). */
+            if (   m->backRefs.size() != 0
+                && (   !aMachineId
+                    || m->backRefs.size() != 1
+                    || aMachineId->isEmpty()
+                    || *getFirstMachineBackrefId() != *aMachineId
+                    || (   (!aSnapshotId || !aSnapshotId->isEmpty())
+                        && *getFirstMachineBackrefSnapshotId() != *aSnapshotId)))
+                throw setError(E_FAIL,
+                               tr("Medium '%s' is attached to %d virtual machines"),
+                               m->strLocationFull.raw(), m->backRefs.size());
+            if (m->type == MediumType_Immutable)
+                throw setError(E_FAIL,
+                               tr("Medium '%s' is immutable"),
+                               m->strLocationFull.raw());
+        }
+        else
+        {
+            AutoReadLock alock(pTarget COMMA_LOCKVAL_SRC_POS);
+            if (pTarget->getChildren().size() > 1)
+            {
+                throw setError(E_FAIL,
+                               tr("Medium '%s' involved in the merge operation has more than one child medium (%d)"),
+                               pTarget->m->strLocationFull.raw(),
+                               pTarget->getChildren().size());
+            }
+            if (pTarget->m->type == MediumType_Immutable)
+                throw setError(E_FAIL,
+                               tr("Medium '%s' is immutable"),
+                               pTarget->m->strLocationFull.raw());
+        }
+        ComObjPtr<Medium> pLast(fMergeForward ? (Medium *)pTarget : this);
+        ComObjPtr<Medium> pLastIntermediate = pLast->getParent();
+        for (pLast = pLastIntermediate;
+             !pLast.isNull() && pLast != pTarget && pLast != this;
+             pLast = pLast->getParent())
+        {
+            AutoReadLock alock(pLast COMMA_LOCKVAL_SRC_POS);
+            if (pLast->getChildren().size() > 1)
+            {
+                throw setError(E_FAIL,
+                               tr("Medium '%s' involved in the merge operation has more than one child medium (%d)"),
+                               pLast->m->strLocationFull.raw(),
+                               pLast->getChildren().size());
+            }
+            if (pLast->m->backRefs.size() != 0)
+                throw setError(E_FAIL,
+                               tr("Medium '%s' is attached to %d virtual machines"),
+                               pLast->m->strLocationFull.raw(),
+                               pLast->m->backRefs.size());
+
+        }
+
+        /* Update medium states appropriately */
+        switch (m->state)
+        {
+            case MediumState_Created:
+                m->state = MediumState_Deleting;
+                break;
+            default:
+                throw setStateError();
+        }
+        if (fMergeForward)
+        {
+            /* we will need parent to reparent target */
+            pParentForTarget = m->pParent;
+        }
+        else
+        {
+            /* we will need to reparent children of the source */
+            for (MediaList::const_iterator it = getChildren().begin();
+                 it != getChildren().end();
+                 ++it)
+            {
+                pMedium = *it;
+                rc = pMedium->LockWrite(NULL);
+                if (FAILED(rc)) throw rc;
+
+                aChildrenToReparent.push_back(pMedium);
+            }
+        }
+        for (pLast = pLastIntermediate;
+             !pLast.isNull() && pLast != pTarget && pLast != this;
+             pLast = pLast->getParent())
+        {
+            AutoWriteLock alock(pLast COMMA_LOCKVAL_SRC_POS);
+            switch (pLast->m->state)
+            {
+                case MediumState_Created:
+                    pLast->m->state = MediumState_Deleting;
+                    break;
+                default:
+                    throw pLast->setStateError();
+            }
+        }
+
+        /* Tweak the lock list in the backward merge case, as the target
+         * isn't marked to be locked for writing yet. */
+        if (!fMergeForward)
+        {
+            MediumLockList::Base::iterator lockListBegin =
+                aMediumLockList->GetBegin();
+            MediumLockList::Base::iterator lockListEnd =
+                aMediumLockList->GetEnd();
+            lockListEnd--;
+            for (MediumLockList::Base::iterator it = lockListBegin;
+                 it != lockListEnd;
+                 ++it)
+            {
+                MediumLock &mediumLock = *it;
+                if (mediumLock.GetMedium() == pTarget)
+                {
+                    HRESULT rc2 = mediumLock.UpdateLock(true);
+                    AssertComRC(rc2);
+                    break;
+                }
+            }
+        }
+
+        rc = aMediumLockList->Lock();
+        if (FAILED(rc))
+        {
+            AutoReadLock alock(pTarget COMMA_LOCKVAL_SRC_POS);
+            throw setError(rc,
+                           tr("Failed to lock media when merging to '%ls'"),
+                           pTarget->getLocationFull().raw());
+        }
+    }
+    catch (HRESULT aRC) { rc = aRC; }
+
+    if (FAILED(rc))
+    {
+        delete aMediumLockList;
+        aMediumLockList = NULL;
+    }
+
+    return rc;
+}
+
+/**
+ * Merges this hard disk to the specified hard disk which must be either its
+ * direct ancestor or descendant.
+ *
+ * Given this hard disk is SOURCE and the specified hard disk is TARGET, we will
+ * get two varians of the merge operation:
+ *
+ *                forward merge
+ *                ------------------------->
+ *  [Extra] <- SOURCE <- Intermediate <- TARGET
+ *  Any        Del       Del             LockWr
+ *
+ *
+ *                            backward merge
+ *                <-------------------------
+ *             TARGET <- Intermediate <- SOURCE <- [Extra]
+ *             LockWr    Del             Del       LockWr
+ *
+ * Each diagram shows the involved hard disks on the hard disk chain where
+ * SOURCE and TARGET belong. Under each hard disk there is a state value which
+ * the hard disk must have at a time of the mergeTo() call.
+ *
+ * The hard disks in the square braces may be absent (e.g. when the forward
+ * operation takes place and SOURCE is the base hard disk, or when the backward
+ * merge operation takes place and TARGET is the last child in the chain) but if
+ * they present they are involved too as shown.
+ *
+ * Nor the source hard disk neither intermediate hard disks may be attached to
+ * any VM directly or in the snapshot, otherwise this method will assert.
+ *
+ * The #prepareMergeTo() method must be called prior to this method to place all
+ * involved to necessary states and perform other consistency checks.
+ *
+ * If @a aWait is @c true then this method will perform the operation on the
+ * calling thread and will not return to the caller until the operation is
+ * completed. When this method succeeds, all intermediate hard disk objects in
+ * the chain will be uninitialized, the state of the target hard disk (and all
+ * involved extra hard disks) will be restored. @a aMediumLockList will not be
+ * deleted, whether the operation is successful or not. The caller has to do
+ * this if appropriate. Note that this (source) hard disk is not uninitialized
+ * because of possible AutoCaller instances held by the caller of this method
+ * on the current thread. It's therefore the responsibility of the caller to
+ * call Medium::uninit() after releasing all callers.
+ *
+ * If @a aWait is @c false then this method will create a thread to perform the
+ * operation asynchronously and will return immediately. If the operation
+ * succeeds, the thread will uninitialize the source hard disk object and all
+ * intermediate hard disk objects in the chain, reset the state of the target
+ * hard disk (and all involved extra hard disks) and delete @a aMediumLockList.
+ * If the operation fails, the thread will only reset the states of all
+ * involved hard disks and delete @a aMediumLockList.
+ *
+ * When this method fails (regardless of the @a aWait mode), it is a caller's
+ * responsiblity to undo state changes and delete @a aMediumLockList using
+ * #cancelMergeTo().
+ *
+ * If @a aProgress is not NULL but the object it points to is @c null then a new
+ * progress object will be created and assigned to @a *aProgress on success,
+ * otherwise the existing progress object is used. If Progress is NULL, then no
+ * progress object is created/used at all. Note that @a aProgress cannot be
+ * NULL when @a aWait is @c false (this method will assert in this case).
+ *
+ * @param pTarget       Target hard disk.
+ * @param fMergeForward Merge direction.
+ * @param pParentForTarget New parent for target medium after merge.
+ * @param aChildrenToReparent List of children of the source which will have
+ *                      to be reparented to the target after merge.
+ * @param aMediumLockList Medium locking information.
  * @param aProgress     Where to find/store a Progress object to track operation
  *                      completion.
@@ -4409 +4409 @@
  *                and this parameter is ignored.
  *
- * @note Locks this object and @a aTarget for writing.
- */
-HRESULT Medium::createDiffStorage(ComObjPtr<Medium> &aTarget,
-                                  MediumVariant_T aVariant,
-                                  ComObjPtr<Progress> *aProgress,
-                                  bool aWait,
-                                  bool *pfNeedsSaveSettings)
-{
-    AssertReturn(!aTarget.isNull(), E_FAIL);
-    AssertReturn(aProgress != NULL || aWait == true, E_FAIL);
-
-    AutoCaller autoCaller(this);
-    if (FAILED(autoCaller.rc())) return autoCaller.rc();
-
-    AutoCaller targetCaller(aTarget);
-    if (FAILED(targetCaller.rc())) return targetCaller.rc();
-
-    AutoMultiWriteLock2 alock(this, aTarget COMMA_LOCKVAL_SRC_POS);
-
-    AssertReturn(m->type != MediumType_Writethrough, E_FAIL);
-
-    /* Note: MediumState_LockedWrite is ok when taking an online snapshot */
-    AssertReturn(    m->state == MediumState_LockedRead
-                  || m->state == MediumState_LockedWrite, E_FAIL);
-
-    if (aTarget->m->state != MediumState_NotCreated)
-        return aTarget->setStateError();
-
-    HRESULT rc = S_OK;
-
-    /* check that the hard disk is not attached to any VM in the current state*/
-    for (BackRefList::const_iterator it = m->backRefs.begin();
-         it != m->backRefs.end();
-         ++it)
-    {
-        if (it->fInCurState)
-        {
-            /* Note: when a VM snapshot is being taken, all normal hard disks
-             * attached to the VM in the current state will be, as an exception,
-             * also associated with the snapshot which is about to create  (see
-             * SnapshotMachine::init()) before deassociating them from the
-             * current state (which takes place only on success in
-             * Machine::fixupHardDisks()), so that the size of snapshotIds
-             * will be 1 in this case. The given condition is used to filter out
-             * this legal situatinon and do not report an error. */
-
-            if (it->llSnapshotIds.size() == 0)
-                return setError(VBOX_E_INVALID_OBJECT_STATE,
-                                tr("Hard disk '%s' is attached to a virtual machine with UUID {%RTuuid}. No differencing hard disks based on it may be created until it is detached"),
-                                m->strLocationFull.raw(), it->machineId.raw());
-
-            Assert(it->llSnapshotIds.size() == 1);
-        }
-    }
-
-    ComObjPtr<Progress> progress;
-
-    if (aProgress != NULL)
-    {
-        /* use the existing progress object... */
-        progress = *aProgress;
-
-        /* ...but create a new one if it is null */
-        if (progress.isNull())
-        {
-            progress.createObject();
-            rc = progress->init(m->pVirtualBox,
-                                static_cast<IMedium*>(this),
-                                BstrFmt(tr("Creating differencing hard disk storage unit '%s'"), aTarget->m->strLocationFull.raw()),
-                                TRUE /* aCancelable */);
-            if (FAILED(rc)) return rc;
-        }
-    }
-
-    /* setup task object to carry out the operation asynchronously */
-    Medium::Task *pTask(new Medium::CreateDiffTask(this, progress, aTarget,
-                                                   aVariant));
-    rc = pTask->rc();
-    if (FAILED(rc))
-    {
-        AssertComRC(rc);
-        delete pTask;
-        return rc;
-    }
-
-    /* register a task (it will deregister itself when done) */
-    ++m->numCreateDiffTasks;
-    Assert(m->numCreateDiffTasks != 0); /* overflow? */
-
-    if (aWait)
-    {
-        // go to Creating state before starting the task
-        aTarget->m->state = MediumState_Creating;
-
-        // release the locks because the task function will acquire other locks;
-        // this is safe because both this and the target are not protected by
-        // their states; we have asserted above that *this* is locked read or write!
-        alock.release();
-
-        rc = runNow(pTask, pfNeedsSaveSettings);
-        if (FAILED(rc)) return rc;
-    }
-    else
-    {
-        rc = startThread(pTask);
-        if (FAILED(rc)) return rc;
-
-        /* go to Creating state before leaving the lock */
-        aTarget->m->state = MediumState_Creating;
-    }
-
-    if (aProgress != NULL)
-        *aProgress = progress;
-
-    return rc;
-}
-
-/**
- * Prepares this (source) hard disk, target hard disk and all intermediate hard
- * disks for the merge operation.
- *
- * This method is to be called prior to calling the #mergeTo() to perform
- * necessary consistency checks and place involved hard disks to appropriate
- * states. If #mergeTo() is not called or fails, the state modifications
- * performed by this method must be undone by #cancelMergeTo().
- *
- * Note that when @a aIgnoreAttachments is @c true then it's the caller's
- * responsibility to detach the source and all intermediate hard disks before
- * calling #mergeTo() (which will fail otherwise).
- *
- * See #mergeTo() for more information about merging.
- *
- * @param aTarget       Target hard disk.
- * @param aChain        Where to store the created merge chain.
- * @param aIgnoreAttachments    Don't check if the source or any intermediate
- *                              hard disk is attached to any VM.
- *
- * @note Locks medium tree for reading. Locks this object, aTarget and all
- *       intermediate hard disks for writing.
- */
-HRESULT Medium::prepareMergeTo(Medium *aTarget,
-                               MergeChain * &aChain,
-                               bool aIgnoreAttachments /*= false*/)
-{
-    AssertReturn(aTarget != NULL, E_FAIL);
-
-    AutoCaller autoCaller(this);
-    AssertComRCReturnRC(autoCaller.rc());
-
-    AutoCaller targetCaller(aTarget);
-    AssertComRCReturnRC(targetCaller.rc());
-
-    aChain = NULL;
-
-    /* we walk the tree */
-    AutoReadLock treeLock(m->pVirtualBox->getMediaTreeLockHandle() COMMA_LOCKVAL_SRC_POS);
-
-    HRESULT rc = S_OK;
-
-    /* detect the merge direction */
-    bool forward;
-    {
-        Medium *parent = m->pParent;
-        while (parent != NULL && parent != aTarget)
-            parent = parent->m->pParent;
-        if (parent == aTarget)
-            forward = false;
-        else
-        {
-            parent = aTarget->m->pParent;
-            while (parent != NULL && parent != this)
-                parent = parent->m->pParent;
-            if (parent == this)
-                forward = true;
-            else
-            {
-                Utf8Str tgtLoc;
-                {
-                    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
-                    tgtLoc = aTarget->getLocationFull();
-                }
-
-                AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
-                return setError(E_FAIL,
-                                tr("Hard disks '%s' and '%s' are unrelated"),
-                                m->strLocationFull.raw(), tgtLoc.raw());
-            }
-        }
-    }
-
-    /* build the chain (will do necessary checks and state changes) */
-    std::auto_ptr<MergeChain> chain(new MergeChain(forward,
-                                                   aIgnoreAttachments));
-    {
-        Medium *last = forward ? aTarget : this;
-        Medium *first = forward ? this : aTarget;
-
-        for (;;)
-        {
-            if (last == aTarget)
-                rc = chain->addTarget(last);
-            else if (last == this)
-                rc = chain->addSource(last);
-            else
-                rc = chain->addIntermediate(last);
-            if (FAILED(rc)) return rc;
-
-            if (last == first)
-                break;
-
-            last = last->m->pParent;
-        }
-    }
-
-    aChain = chain.release();
-
-    return S_OK;
-}
-
-/**
- * Merges this hard disk to the specified hard disk which must be either its
- * direct ancestor or descendant.
- *
- * Given this hard disk is SOURCE and the specified hard disk is TARGET, we will
- * get two varians of the merge operation:
- *
- *                forward merge
- *                ------------------------->
- *  [Extra] <- SOURCE <- Intermediate <- TARGET
- *  Any        Del       Del             LockWr
- *
- *
- *                            backward merge
- *                <-------------------------
- *             TARGET <- Intermediate <- SOURCE <- [Extra]
- *             LockWr    Del             Del       LockWr
- *
- * Each scheme shows the involved hard disks on the hard disk chain where
- * SOURCE and TARGET belong. Under each hard disk there is a state value which
- * the hard disk must have at a time of the mergeTo() call.
- *
- * The hard disks in the square braces may be absent (e.g. when the forward
- * operation takes place and SOURCE is the base hard disk, or when the backward
- * merge operation takes place and TARGET is the last child in the chain) but if
- * they present they are involved too as shown.
- *
- * Nor the source hard disk neither intermediate hard disks may be attached to
- * any VM directly or in the snapshot, otherwise this method will assert.
- *
- * The #prepareMergeTo() method must be called prior to this method to place all
- * involved to necessary states and perform other consistency checks.
- *
- * If @a aWait is @c true then this method will perform the operation on the
- * calling thread and will not return to the caller until the operation is
- * completed. When this method succeeds, all intermediate hard disk objects in
- * the chain will be uninitialized, the state of the target hard disk (and all
- * involved extra hard disks) will be restored and @a aChain will be deleted.
- * Note that this (source) hard disk is not uninitialized because of possible
- * AutoCaller instances held by the caller of this method on the current thread.
- * It's therefore the responsibility of the caller to call Medium::uninit()
- * after releasing all callers in this case!
- *
- * If @a aWait is @c false then this method will crea,te a thread to perform the
- * create operation asynchronously and will return immediately. If the operation
- * succeeds, the thread will uninitialize the source hard disk object and all
- * intermediate hard disk objects in the chain, reset the state of the target
- * hard disk (and all involved extra hard disks) and delete @a aChain. If the
- * operation fails, the thread will only reset the states of all involved hard
- * disks and delete @a aChain.
- *
- * When this method fails (regardless of the @a aWait mode), it is a caller's
- * responsiblity to undo state changes and delete @a aChain using
- * #cancelMergeTo().
- *
- * If @a aProgress is not NULL but the object it points to is @c null then a new
- * progress object will be created and assigned to @a *aProgress on success,
- * otherwise the existing progress object is used. If Progress is NULL, then no
- * progress object is created/used at all. Note that @a aProgress cannot be
- * NULL when @a aWait is @c false (this method will assert in this case).
- *
- * @param aChain        Merge chain created by #prepareMergeTo().
- * @param aProgress     Where to find/store a Progress object to track operation
- *                      completion.
- * @param aWait         @c true if this method should block instead of creating
- *                      an asynchronous thread.
- * @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.
- *                This only works in "wait" mode; otherwise saveSettings gets called automatically by the thread that was created,
- *                and this parameter is ignored.
- *
- * @note Locks the branch lock for writing. Locks the hard disks from the chain
+ * @note Locks the tree lock for writing. Locks the hard disks from the chain
  *       for writing.
  */
-HRESULT Medium::mergeTo(MergeChain *aChain,
+HRESULT Medium::mergeTo(const ComObjPtr<Medium> &pTarget,
+                        bool &fMergeForward,
+                        ComObjPtr<Medium> pParentForTarget,
+                        const MediaList &aChildrenToReparent,
+                        MediumLockList *aMediumLockList,
                         ComObjPtr <Progress> *aProgress,
                         bool aWait,
                         bool *pfNeedsSaveSettings)
 {
-    AssertReturn(aChain != NULL, E_FAIL);
+    AssertReturn(pTarget != NULL, E_FAIL);
+    AssertReturn(pTarget != this, E_FAIL);
+    AssertReturn(aMediumLockList != NULL, E_FAIL);
     AssertReturn(aProgress != NULL || aWait == true, E_FAIL);
 
@@ -4714 +4430 @@
 
     HRESULT rc = S_OK;
-
-    ComObjPtr <Progress> progress;
-
-    if (aProgress != NULL)
-    {
-        /* use the existing progress object... */
-        progress = *aProgress;
-
-        /* ...but create a new one if it is null */
-        if (progress.isNull())
-        {
-            AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-            progress.createObject();
-            rc = progress->init(m->pVirtualBox,
-                                static_cast<IMedium*>(this),
-                                BstrFmt(tr("Merging hard disk '%s' to '%s'"),
-                                        getName().raw(),
-                                        aChain->target()->getName().raw()),
-                                TRUE /* aCancelable */);
-            if (FAILED(rc)) return rc;
-        }
-    }
-
-    /* setup task object to carry out the operation asynchronously */
-    Medium::Task *pTask(new Medium::MergeTask(this, progress, aChain));
-    rc = pTask->rc();
-    if (FAILED(rc))
-    {
+    ComObjPtr <Progress> pProgress;
+    Medium::Task *pTask = NULL;
+
+    try
+    {
+        if (aProgress != NULL)
+        {
+            /* use the existing progress object... */
+            pProgress = *aProgress;
+
+            /* ...but create a new one if it is null */
+            if (pProgress.isNull())
+            {
+                Utf8Str tgtName;
+                {
+                    AutoReadLock alock(pTarget COMMA_LOCKVAL_SRC_POS);
+                    tgtName = pTarget->getName();
+                }
+
+                AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+                pProgress.createObject();
+                rc = pProgress->init(m->pVirtualBox,
+                                     static_cast<IMedium*>(this),
+                                     BstrFmt(tr("Merging hard disk '%s' to '%s'"),
+                                             getName().raw(),
+                                             tgtName.raw()),
+                                     TRUE /* aCancelable */);
+                if (FAILED(rc)) throw rc;
+            }
+        }
+
+        /* setup task object to carry out the operation sync/async */
+        pTask = new Medium::MergeTask(this, pTarget, fMergeForward,
+                                      pParentForTarget, aChildrenToReparent,
+                                      pProgress, aMediumLockList,
+                                      aWait /* fKeepMediumLockList */);
+        rc = pTask->rc();
         AssertComRC(rc);
+        if (FAILED(rc)) throw rc;
+    }
+    catch (HRESULT aRC) { rc = aRC; }
+
+    if (SUCCEEDED(rc))
+    {
+        if (aWait)
+            rc = runNow(pTask, pfNeedsSaveSettings);
+        else
+            rc = startThread(pTask);
+
+        if (SUCCEEDED(rc) && aProgress != NULL)
+            *aProgress = pProgress;
+    }
+    else if (pTask != NULL)
         delete pTask;
-        return rc;
-    }
-
-    /* Note: task owns aChain (will delete it when not needed) in all cases
-     * except when @a aWait is @c true and runNow() fails -- in this case
-     * aChain will be left away because cancelMergeTo() will be applied by the
-     * caller on it as it is required in the documentation above */
-
-    if (aWait)
-    {
-        rc = runNow(pTask, pfNeedsSaveSettings);
-        if (FAILED(rc)) return rc;
-    }
-    else
-    {
-        rc = startThread(pTask);
-        if (FAILED(rc)) return rc;
-    }
-
-    if (aProgress != NULL)
-        *aProgress = progress;
 
     return rc;
@@ -4771 +4490 @@
 
 /**
- * Undoes what #prepareMergeTo() did. Must be called if #mergeTo() is not called
- * or fails. Frees memory occupied by @a aChain.
- *
- * @param aChain        Merge chain created by #prepareMergeTo().
+ * Undoes what #prepareMergeTo() did. Must be called if #mergeTo() is not
+ * called or fails. Frees memory occupied by @a aMediumLockList and unlocks
+ * the medium objects in @a aChildrenToReparent.
+ *
+ * @param aChildrenToReparent List of children of the source which will have
+ *                      to be reparented to the target after merge.
+ * @param aMediumLockList Medium locking information.
  *
  * @note Locks the hard disks from the chain for writing.
  */
-void Medium::cancelMergeTo(MergeChain *aChain)
+void Medium::cancelMergeTo(const MediaList &aChildrenToReparent,
+                           MediumLockList *aMediumLockList)
 {
     AutoCaller autoCaller(this);
     AssertComRCReturnVoid(autoCaller.rc());
 
-    AssertReturnVoid(aChain != NULL);
-
-    /* the destructor will do the thing */
-    delete aChain;
+    AssertReturnVoid(aMediumLockList != NULL);
+
+    /* the destructor will do the work */
+    delete aMediumLockList;
+
+    /* unlock the children which had to be reparented */
+    for (MediaList::const_iterator it = aChildrenToReparent.begin();
+         it != aChildrenToReparent.end();
+         ++it)
+    {
+        const ComObjPtr<Medium> &pMedium = *it;
+
+        AutoWriteLock alock(pMedium COMMA_LOCKVAL_SRC_POS);
+        pMedium->UnlockWrite(NULL);
+    }
 }
 
@@ -4808 +4542 @@
         if (m->formatObj.isNull())
             return setError(E_INVALIDARG,
-                            tr("Invalid hard disk storage format '%ls'"), aFormat);
+                            tr("Invalid hard disk storage format '%ls'"),
+                            aFormat);
 
         /* reference the format permanently to prevent its unexpected
@@ -4824 +4559 @@
                 m->formatObj->properties().begin();
              it != m->formatObj->properties().end();
-             ++ it)
+             ++it)
         {
             m->properties.insert(std::make_pair(it->name, Bstr::Null));
@@ -5047 +4782 @@
 HRESULT Medium::startThread(Medium::Task *pTask)
 {
+#ifdef VBOX_WITH_MAIN_LOCK_VALIDATION
+    /* Extreme paranoia: The calling thread should not hold the medium
+     * tree lock or any medium lock. Since there is no separate lock class
+     * for medium objects be even more strict: no other object locks. */
+    Assert(!AutoLockHoldsLocksInClass(LOCKCLASS_LISTOFMEDIA));
+    Assert(!AutoLockHoldsLocksInClass(getLockingClass()));
+#endif
+
     /// @todo use a more descriptive task name
     int vrc = RTThreadCreate(NULL, Medium::Task::fntMediumTask, pTask,
@@ -5081 +4824 @@
                        bool *pfNeedsSaveSettings)
 {
+#ifdef VBOX_WITH_MAIN_LOCK_VALIDATION
+    /* Extreme paranoia: The calling thread should not hold the medium
+     * tree lock or any medium lock. Since there is no separate lock class
+     * for medium objects be even more strict: no other object locks. */
+    Assert(!AutoLockHoldsLocksInClass(LOCKCLASS_LISTOFMEDIA));
+    Assert(!AutoLockHoldsLocksInClass(getLockingClass()));
+#endif
+
     pTask->m_pfNeedsSaveSettings = pfNeedsSaveSettings;
 
@@ -5097 +4848 @@
  * @return
  */
-HRESULT Medium::taskThreadCreateBase(Medium::CreateBaseTask &task)
+HRESULT Medium::taskCreateBaseHandler(Medium::CreateBaseTask &task)
 {
     HRESULT rc = S_OK;
@@ -5107 +4858 @@
     try
     {
-        /* The lock is also used as a signal from the task initiator (which
-        * releases it only after RTThreadCreate()) that we can start the job */
         AutoWriteLock thisLock(this COMMA_LOCKVAL_SRC_POS);
 
@@ -5122 +4871 @@
         }
 
+        Utf8Str format(m->strFormat);
+        Utf8Str location(m->strLocationFull);
+        uint64_t capabilities = m->formatObj->capabilities();
+        ComAssertThrow(capabilities & (  VD_CAP_CREATE_FIXED
+                                       | VD_CAP_CREATE_DYNAMIC), E_FAIL);
+        Assert(m->state == MediumState_Creating);
+
         PVBOXHDD hdd;
         int vrc = VDCreate(m->vdDiskIfaces, &hdd);
         ComAssertRCThrow(vrc, E_FAIL);
 
-        Utf8Str format(m->strFormat);
-        Utf8Str location(m->strLocationFull);
-        /* uint64_t capabilities = */ m->formatObj->capabilities();
-
         /* unlock before the potentially lengthy operation */
-        Assert(m->state == MediumState_Creating);
         thisLock.leave();
 
@@ -5214 +4965 @@
  *
  * This task always gets started from Medium::createDiffStorage() and can run
- * synchronously or asynchrously depending on the "wait" parameter passed to
+ * synchronously or asynchronously depending on the "wait" parameter passed to
  * that function. If we run synchronously, the caller expects the bool
  * *pfNeedsSaveSettings to be set before returning; otherwise (in asynchronous
@@ -5222 +4973 @@
  * @return
  */
-HRESULT Medium::taskThreadCreateDiff(Medium::CreateDiffTask &task)
+HRESULT Medium::taskCreateDiffHandler(Medium::CreateDiffTask &task)
 {
     HRESULT rc = S_OK;
@@ -5235 +4986 @@
     try
     {
-        /* Lock both in {parent,child} order. The lock is also used as a
-        * signal from the task initiator (which releases it only after
-        * RTThreadCreate()) that we can start the job*/
+        /* Lock both in {parent,child} order. */
         AutoMultiWriteLock2 mediaLock(this, pTarget COMMA_LOCKVAL_SRC_POS);
 
         /* The object may request a specific UUID (through a special form of
-        * the setLocation() argument). Otherwise we have to generate it */
+         * the setLocation() argument). Otherwise we have to generate it */
         Guid targetId = pTarget->m->id;
         fGenerateUuid = targetId.isEmpty();
@@ -5251 +5000 @@
         }
 
+        Guid id = m->id;
+
+        Utf8Str targetFormat(pTarget->m->strFormat);
+        Utf8Str targetLocation(pTarget->m->strLocationFull);
+        uint64_t capabilities = m->formatObj->capabilities();
+        ComAssertThrow(capabilities & VD_CAP_CREATE_DYNAMIC, E_FAIL);
+
+        Assert(pTarget->m->state == MediumState_Creating);
+        Assert(m->state == MediumState_LockedRead);
+
         PVBOXHDD hdd;
         int vrc = VDCreate(m->vdDiskIfaces, &hdd);
         ComAssertRCThrow(vrc, E_FAIL);
 
-        Guid id = m->id;
-        Utf8Str format(m->strFormat);
-        Utf8Str location(m->strLocationFull);
-
-        Utf8Str targetFormat(pTarget->m->strFormat);
-        Utf8Str targetLocation(pTarget->m->strLocationFull);
-
-        Assert(pTarget->m->state == MediumState_Creating);
-
-        /* Note: MediumState_LockedWrite is ok when taking an online
-         * snapshot */
-        Assert(    m->state == MediumState_LockedRead
-                || m->state == MediumState_LockedWrite);
-
         /* the two media are now protected by their non-default states;
-           unlock the media before the potentially lengthy operation */
+         * unlock the media before the potentially lengthy operation */
         mediaLock.leave();
 
         try
         {
-            vrc = VDOpen(hdd,
-                         format.c_str(),
-                         location.c_str(),
-                         VD_OPEN_FLAGS_READONLY | VD_OPEN_FLAGS_INFO,
-                         m->vdDiskIfaces);
-            if (RT_FAILURE(vrc))
-                throw setError(E_FAIL,
-                                tr("Could not open the hard disk storage unit '%s'%s"),
-                                location.raw(), vdError(vrc).raw());
+            /* Open all hard disk images in the target chain but the last. */
+            MediumLockList::Base::const_iterator targetListBegin =
+                task.mpMediumLockList->GetBegin();
+            MediumLockList::Base::const_iterator targetListEnd =
+                task.mpMediumLockList->GetEnd();
+            targetListEnd--;
+            for (MediumLockList::Base::const_iterator it = targetListBegin;
+                 it != targetListEnd;
+                 ++it)
+            {
+                const MediumLock &mediumLock = *it;
+                const ComObjPtr<Medium> &pMedium = mediumLock.GetMedium();
+
+                AutoReadLock alock(pMedium COMMA_LOCKVAL_SRC_POS);
+
+                /* sanity check */
+                Assert(pMedium->m->state == MediumState_LockedRead);
+
+                /* Open all images in appropriate mode. */
+                vrc = VDOpen(hdd,
+                             pMedium->m->strFormat.c_str(),
+                             pMedium->m->strLocationFull.c_str(),
+                             VD_OPEN_FLAGS_READONLY,
+                             pMedium->m->vdDiskIfaces);
+                if (RT_FAILURE(vrc))
+                    throw setError(E_FAIL,
+                                   tr("Could not open the hard disk storage unit '%s'%s"),
+                                   pMedium->m->strLocationFull.raw(),
+                                   vdError(vrc).raw());
+            }
 
             /* ensure the target directory exists */
@@ -5326 +5092 @@
             * potential race! */
         /* diffs for immutable hard disks are auto-reset by default */
-        pTarget->m->autoReset =   getBase()->m->type == MediumType_Immutable
-                               ? TRUE
-                               : FALSE;
+        pTarget->m->autoReset = (getBase()->m->type == MediumType_Immutable);
 
         /* register with mVirtualBox as the last step and move to
@@ -5363 +5127 @@
     if (task.isAsync())
     {
-        /* unlock ourselves when done (unless in MediumState_LockedWrite
-         * state because of taking the online snapshot*/
-        if (m->state != MediumState_LockedWrite)
-        {
-            HRESULT rc2 = UnlockRead(NULL);
-            AssertComRC(rc2);
-        }
-
         if (fNeedsSaveSettings)
         {
@@ -5405 +5161 @@
  * @return
  */
-HRESULT Medium::taskThreadMerge(Medium::MergeTask &task)
+HRESULT Medium::taskMergeHandler(Medium::MergeTask &task)
 {
     HRESULT rc = S_OK;
 
-    /* The lock is also used as a signal from the task initiator (which
-     * releases it only after RTThreadCreate()) that we can start the
-     * job. We don't actually need the lock for anything else since the
-     * object is protected by MediumState_Deleting and we don't modify
-     * its sensitive fields below */
-    {
-        AutoWriteLock thisLock(this COMMA_LOCKVAL_SRC_POS);
-    }
-
-    MergeChain *chain = task.mMergeChain.get();
+    const ComObjPtr<Medium> &pTarget = task.mTarget;
 
     try
@@ -5428 +5175 @@
         try
         {
-            /* Open all hard disks in the chain (they are in the
-             * {parent,child} order in there. Note that we don't lock
-             * objects in this chain since they must be in states
-             * (Deleting and LockedWrite) that prevent from changing
-             * their format and location fields from outside. */
-
-            for (MergeChain::const_iterator it = chain->begin();
-                 it != chain->end();
+            unsigned uTargetIdx = VD_LAST_IMAGE;
+            unsigned uSourceIdx = VD_LAST_IMAGE;
+            /* Open all hard disks in the chain. */
+            MediumLockList::Base::iterator lockListBegin =
+                task.mpMediumLockList->GetBegin();
+            MediumLockList::Base::iterator lockListEnd =
+                task.mpMediumLockList->GetEnd();
+            unsigned i = 0;
+            for (MediumLockList::Base::iterator it = lockListBegin;
+                 it != lockListEnd;
                  ++it)
             {
+                MediumLock &mediumLock = *it;
+                const ComObjPtr<Medium> &pMedium = mediumLock.GetMedium();
+
+                if (pMedium == this)
+                    uSourceIdx = i;
+                else if (pMedium == pTarget)
+                    uTargetIdx = i;
+
+                AutoReadLock alock(pMedium COMMA_LOCKVAL_SRC_POS);
+
                 /*
                  * complex sanity (sane complexity)
@@ -5445 +5204 @@
                  * If it is the target it must be in the LockedWrite state.
                  */
-                Assert(   (   *it != chain->target()
-                           && (   (*it)->m->state == MediumState_Deleting
-                               || (*it)->m->state == MediumState_LockedRead))
-                       || (   *it == chain->target()
-                           && (*it)->m->state == MediumState_LockedWrite));
+                Assert(   (   pMedium != pTarget
+                           && (   pMedium->m->state == MediumState_Deleting
+                               || pMedium->m->state == MediumState_LockedRead))
+                       || (   pMedium == pTarget
+                           && pMedium->m->state == MediumState_LockedWrite));
 
                 /*
@@ -5456 +5215 @@
                  * to a virtual machine.
                  */
-                Assert(   *it == chain->target()
-                       || (*it)->m->state == MediumState_LockedRead
-                       || (   (*it)->m->backRefs.size() == 0
-                           && (*it)->m->state == MediumState_Deleting)
-                      );
+                Assert(   pMedium == pTarget
+                       || pMedium->m->state == MediumState_LockedRead
+                       || (   pMedium->m->backRefs.size() == 0
+                           && pMedium->m->state == MediumState_Deleting));
+                /* The source medium must be in Deleting state. */
+                Assert(  pMedium != this
+                       || pMedium->m->state == MediumState_Deleting);
 
                 unsigned uOpenFlags = 0;
 
-                if (   (*it)->m->state == MediumState_LockedRead
-                    || (*it)->m->state == MediumState_Deleting)
+                if (   pMedium->m->state == MediumState_LockedRead
+                    || pMedium->m->state == MediumState_Deleting)
                     uOpenFlags = VD_OPEN_FLAGS_READONLY;
 
                 /* Open the image */
                 vrc = VDOpen(hdd,
-                             (*it)->m->strFormat.c_str(),
-                             (*it)->m->strLocationFull.c_str(),
+                             pMedium->m->strFormat.c_str(),
+                             pMedium->m->strLocationFull.c_str(),
                              uOpenFlags,
-                             (*it)->m->vdDiskIfaces);
+                             pMedium->m->vdDiskIfaces);
                 if (RT_FAILURE(vrc))
                     throw vrc;
+
+                i++;
             }
 
-            vrc = VDMerge(hdd, chain->sourceIdx(), chain->targetIdx(),
+            ComAssertThrow(   uSourceIdx != VD_LAST_IMAGE
+                           && uTargetIdx != VD_LAST_IMAGE, E_FAIL);
+
+            vrc = VDMerge(hdd, uSourceIdx, uTargetIdx,
                           task.mVDOperationIfaces);
             if (RT_FAILURE(vrc))
@@ -5484 +5250 @@
 
             /* update parent UUIDs */
-            if (!chain->isForward())
+            if (!task.mfMergeForward)
             {
                 /* we need to update UUIDs of all source's children
                  * which cannot be part of the container at once so
                  * add each one in there individually */
-                if (chain->children().size() > 0)
+                if (task.mChildrenToReparent.size() > 0)
                 {
-                    for (MediaList::const_iterator it = chain->children().begin();
-                         it != chain->children().end();
+                    for (MediaList::const_iterator it = task.mChildrenToReparent.begin();
+                         it != task.mChildrenToReparent.end();
                          ++it)
                     {
@@ -5505 +5271 @@
 
                         vrc = VDSetParentUuid(hdd, 1,
-                                              chain->target()->m->id);
+                                              pTarget->m->id);
                         if (RT_FAILURE(vrc))
                             throw vrc;
@@ -5521 +5287 @@
             throw setError(E_FAIL,
                             tr("Could not merge the hard disk '%s' to '%s'%s"),
-                            chain->source()->m->strLocationFull.raw(),
-                            chain->target()->m->strLocationFull.raw(),
+                            m->strLocationFull.raw(), m->strLocationFull.raw(),
                             vdError(aVRC).raw());
         }
@@ -5535 +5300 @@
     {
         /* all hard disks but the target were successfully deleted by
-         * VDMerge; reparent the last one and uninitialize deleted */
+         * VDMerge; reparent the last one and uninitialize deleted media. */
 
         AutoWriteLock treeLock(m->pVirtualBox->getMediaTreeLockHandle() COMMA_LOCKVAL_SRC_POS);
 
-        Medium *pSource = chain->source();
-        Medium *pTarget = chain->target();
-
-        if (chain->isForward())
+        if (task.mfMergeForward)
         {
             /* first, unregister the target since it may become a base
@@ -5552 +5314 @@
              * both ends (chain->parent() is source's parent) */
             pTarget->deparent();
-            pTarget->m->pParent = chain->parent();
+            pTarget->m->pParent = task.mParentForTarget;
             if (pTarget->m->pParent)
             {
                 pTarget->m->pParent->m->llChildren.push_back(pTarget);
-                pSource->deparent();
+                deparent();
             }
 
@@ -5571 +5333 @@
             targetChild->deparent();
 
-            const MediaList &children = chain->children();
-
             /* reparent source's chidren and disconnect the deleted
              * branch at the younger end m*/
-            if (children.size() > 0)
+            if (task.mChildrenToReparent.size() > 0)
             {
                 /* obey {parent,child} lock order */
-                AutoWriteLock sourceLock(pSource COMMA_LOCKVAL_SRC_POS);
-
-                for (MediaList::const_iterator it = children.begin();
-                     it != children.end();
-                     ++it)
+                AutoWriteLock sourceLock(this COMMA_LOCKVAL_SRC_POS);
+
+                for (MediaList::iterator it = task.mChildrenToReparent.begin();
+                     it != task.mChildrenToReparent.end();
+                     it++)
                 {
-                    AutoWriteLock childLock(*it COMMA_LOCKVAL_SRC_POS);
-
-                    Medium *p = *it;
-                    p->deparent();  // removes p from source
-                    pTarget->m->llChildren.push_back(p);
-                    p->m->pParent = pTarget;
+                    Medium *pMedium = *it;
+                    AutoWriteLock childLock(pMedium COMMA_LOCKVAL_SRC_POS);
+
+                    pMedium->deparent();  // removes pMedium from source
+                    pTarget->m->llChildren.push_back(pMedium);
+                    pMedium->m->pParent = pTarget;
                 }
             }
         }
 
-        /* unregister and uninitialize all hard disks in the chain but the target */
-        for (MergeChain::iterator it = chain->begin();
-             it != chain->end();
-            )
-        {
+        /* unregister and uninitialize all hard disks removed by the merge */
+        MediumLockList::Base::iterator lockListBegin =
+            task.mpMediumLockList->GetBegin();
+        MediumLockList::Base::iterator lockListEnd =
+            task.mpMediumLockList->GetEnd();
+        for (MediumLockList::Base::iterator it = lockListBegin;
+             it != lockListEnd;
+             )
+        {
+            MediumLock &mediumLock = *it;
+            /* Create a real copy of the medium pointer, as the medium
+             * lock deletion below would invalidate the referenced object. */
+            const ComObjPtr<Medium> pMedium = mediumLock.GetMedium();
+
             /* The target and all images not merged (readonly) are skipped */
-            if (   *it == chain->target()
-                || (*it)->m->state == MediumState_LockedRead)
+            if (   pMedium == pTarget
+                || pMedium->m->state == MediumState_LockedRead)
             {
                 ++it;
@@ -5607 +5376 @@
             }
 
-            rc2 = (*it)->m->pVirtualBox->unregisterHardDisk(*it, NULL /*pfNeedsSaveSettings*/);
+            rc2 = pMedium->m->pVirtualBox->unregisterHardDisk(pMedium,
+                                                              NULL /*pfNeedsSaveSettings*/);
             AssertComRC(rc2);
 
@@ -5622 +5392 @@
              * and therefore we cannot uninit() it (it's therefore
              * the caller's responsibility) */
-            if (*it == this)
+            if (pMedium == this)
+            {
+                Assert(getChildren().size() == 0);
+                Assert(m->backRefs.size() == 0);
                 task.mMediumCaller.release();
-
-            /* release the caller added by MergeChain before uninit() */
-            (*it)->releaseCaller();
-
-            if (task.isAsync() || *it != this)
-                (*it)->uninit();
-
-            /* delete (to prevent uninitialization in MergeChain
-             * dtor) and advance to the next item */
-            it = chain->erase(it);
+            }
+
+            /* Delete the medium lock list entry, which also releases the
+             * caller added by MergeChain before uninit() and updates the
+             * iterator to point to the right place. */
+            rc2 = task.mpMediumLockList->RemoveByIterator(it);
+            AssertComRC(rc2);
+
+            if (task.isAsync() || pMedium != this)
+                pMedium->uninit();
         }
     }
@@ -5660 +5433 @@
          * in the mergeTo() docs. The latter also implies that we
          * don't own the merge chain, so release it in this case. */
-
-        if (!task.isAsync())
-            task.mMergeChain.release();
+        if (task.isAsync())
+        {
+            Assert(task.mChildrenToReparent.size() == 0);
+            cancelMergeTo(task.mChildrenToReparent, task.mpMediumLockList);
+        }
     }
 
@@ -5677 +5452 @@
  * @return
  */
-HRESULT Medium::taskThreadClone(Medium::CloneTask &task)
+HRESULT Medium::taskCloneHandler(Medium::CloneTask &task)
 {
     HRESULT rc = S_OK;
@@ -5697 +5472 @@
 
         fCreatingTarget = pTarget->m->state == MediumState_Creating;
-
-        ImageChain *sourceChain = task.mSourceChain.get();
-        ImageChain *parentChain = task.mParentChain.get();
 
         /* The object may request a specific UUID (through a special form of
@@ -5719 +5491 @@
         {
             /* Open all hard disk images in the source chain. */
-            for (MediaList::const_iterator it = sourceChain->begin();
-                 it != sourceChain->end();
+            MediumLockList::Base::const_iterator sourceListBegin =
+                task.mpSourceMediumLockList->GetBegin();
+            MediumLockList::Base::const_iterator sourceListEnd =
+                task.mpSourceMediumLockList->GetEnd();
+            for (MediumLockList::Base::const_iterator it = sourceListBegin;
+                 it != sourceListEnd;
                  ++it)
             {
+                const MediumLock &mediumLock = *it;
+                const ComObjPtr<Medium> &pMedium = mediumLock.GetMedium();
+                AutoReadLock alock(pMedium COMMA_LOCKVAL_SRC_POS);
+
                 /* sanity check */
-                Assert((*it)->m->state == MediumState_LockedRead);
+                Assert(pMedium->m->state == MediumState_LockedRead);
 
                 /** Open all images in read-only mode. */
                 vrc = VDOpen(hdd,
-                             (*it)->m->strFormat.c_str(),
-                             (*it)->m->strLocationFull.c_str(),
+                             pMedium->m->strFormat.c_str(),
+                             pMedium->m->strLocationFull.c_str(),
                              VD_OPEN_FLAGS_READONLY,
-                             (*it)->m->vdDiskIfaces);
+                             pMedium->m->vdDiskIfaces);
                 if (RT_FAILURE(vrc))
                     throw setError(E_FAIL,
                                     tr("Could not open the hard disk storage unit '%s'%s"),
-                                    (*it)->m->strLocationFull.raw(),
+                                    pMedium->m->strLocationFull.raw(),
                                     vdError(vrc).raw());
             }
@@ -5760 +5540 @@
             try
             {
-                /* Open all hard disk images in the parent chain. */
-                for (MediaList::const_iterator it = parentChain->begin();
-                     it != parentChain->end();
+                /* Open all hard disk images in the target chain. */
+                MediumLockList::Base::const_iterator targetListBegin =
+                    task.mpTargetMediumLockList->GetBegin();
+                MediumLockList::Base::const_iterator targetListEnd =
+                    task.mpTargetMediumLockList->GetEnd();
+                for (MediumLockList::Base::const_iterator it = targetListBegin;
+                     it != targetListEnd;
                      ++it)
                 {
-                    /** @todo r=klaus (*it) is not locked, lots of
-                        * race opportunities below */
+                    const MediumLock &mediumLock = *it;
+                    const ComObjPtr<Medium> &pMedium = mediumLock.GetMedium();
+
+                    /* If the target medium is not created yet there's no
+                     * reason to open it. */
+                    if (pMedium == pTarget && fCreatingTarget)
+                        continue;
+
+                    AutoReadLock alock(pMedium COMMA_LOCKVAL_SRC_POS);
+
                     /* sanity check */
-                    Assert(    (*it)->m->state == MediumState_LockedRead
-                            || (*it)->m->state == MediumState_LockedWrite);
+                    Assert(    pMedium->m->state == MediumState_LockedRead
+                            || pMedium->m->state == MediumState_LockedWrite);
 
                     /* Open all images in appropriate mode. */
                     vrc = VDOpen(targetHdd,
-                                 (*it)->m->strFormat.c_str(),
-                                 (*it)->m->strLocationFull.c_str(),
-                                 ((*it)->m->state == MediumState_LockedWrite) ? VD_OPEN_FLAGS_NORMAL : VD_OPEN_FLAGS_READONLY,
-                                 (*it)->m->vdDiskIfaces);
+                                 pMedium->m->strFormat.c_str(),
+                                 pMedium->m->strLocationFull.c_str(),
+                                 (pMedium->m->state == MediumState_LockedWrite) ? VD_OPEN_FLAGS_NORMAL : VD_OPEN_FLAGS_READONLY,
+                                 pMedium->m->vdDiskIfaces);
                     if (RT_FAILURE(vrc))
                         throw setError(E_FAIL,
                                        tr("Could not open the hard disk storage unit '%s'%s"),
-                                       (*it)->m->strLocationFull.raw(),
+                                       pMedium->m->strLocationFull.raw(),
                                        vdError(vrc).raw());
                 }
@@ -5880 +5672 @@
      * that we get a deadlock in Appliance::Import when Medium::Close
      * is called & the source chain is released at the same time. */
-    task.mSourceChain.reset();
+    task.mpSourceMediumLockList->Clear();
 
     return rc;
@@ -5895 +5687 @@
  * @return
  */
-HRESULT Medium::taskThreadDelete(Medium::DeleteTask &task)
+HRESULT Medium::taskDeleteHandler(Medium::DeleteTask &task)
 {
     NOREF(task);
@@ -5961 +5753 @@
  * @return
  */
-HRESULT Medium::taskThreadReset(Medium::ResetTask &task)
+HRESULT Medium::taskResetHandler(Medium::ResetTask &task)
 {
     HRESULT rc = S_OK;
@@ -6071 +5863 @@
  * @return
  */
-HRESULT Medium::taskThreadCompact(Medium::CompactTask &task)
+HRESULT Medium::taskCompactHandler(Medium::CompactTask &task)
 {
     HRESULT rc = S_OK;
@@ -6080 +5872 @@
     AutoWriteLock thisLock(this COMMA_LOCKVAL_SRC_POS);
 
-    ImageChain *imgChain = task.mImageChain.get();
-
     try
     {
@@ -6091 +5881 @@
         {
             /* Open all hard disk images in the chain. */
-            MediaList::const_iterator last = imgChain->end();
-            last--;
-            for (MediaList::const_iterator it = imgChain->begin();
-                 it != imgChain->end();
+            MediumLockList::Base::const_iterator mediumListBegin =
+                task.mpMediumLockList->GetBegin();
+            MediumLockList::Base::const_iterator mediumListEnd =
+                task.mpMediumLockList->GetEnd();
+            MediumLockList::Base::const_iterator mediumListLast =
+                mediumListEnd;
+            mediumListLast--;
+            for (MediumLockList::Base::const_iterator it = mediumListBegin;
+                 it != mediumListEnd;
                  ++it)
             {
+                const MediumLock &mediumLock = *it;
+                const ComObjPtr<Medium> &pMedium = mediumLock.GetMedium();
+                AutoReadLock alock(pMedium COMMA_LOCKVAL_SRC_POS);
+
                 /* sanity check */
-                if (it == last)
-                    Assert((*it)->m->state == MediumState_LockedWrite);
+                if (it == mediumListLast)
+                    Assert(pMedium->m->state == MediumState_LockedWrite);
                 else
-                    Assert((*it)->m->state == MediumState_LockedRead);
+                    Assert(pMedium->m->state == MediumState_LockedRead);
 
                 /** Open all images but last in read-only mode. */
                 vrc = VDOpen(hdd,
-                             (*it)->m->strFormat.c_str(),
-                             (*it)->m->strLocationFull.c_str(),
-                             (it == last) ? VD_OPEN_FLAGS_NORMAL : VD_OPEN_FLAGS_READONLY,
-                             (*it)->m->vdDiskIfaces);
+                             pMedium->m->strFormat.c_str(),
+                             pMedium->m->strLocationFull.c_str(),
+                             (it == mediumListLast) ? VD_OPEN_FLAGS_NORMAL : VD_OPEN_FLAGS_READONLY,
+                             pMedium->m->vdDiskIfaces);
                 if (RT_FAILURE(vrc))
                     throw setError(E_FAIL,
                                    tr("Could not open the hard disk storage unit '%s'%s"),
-                                   (*it)->m->strLocationFull.raw(),
+                                   pMedium->m->strLocationFull.raw(),
                                    vdError(vrc).raw());
             }

trunk/src/VBox/Main/SnapshotImpl.cpp

@@ +1 @@
+/* $Id$ */
+
 /** @file
  *
- * COM class implementation for Snapshot and SnapshotMachine.
+ * COM class implementation for Snapshot and SnapshotMachine in VBoxSVC.
  */
 
 /*
- * Copyright (C) 2006-2007 Sun Microsystems, Inc.
+ * Copyright (C) 2006-2010 Sun Microsystems, Inc.
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
@@ -20 +22 @@
  */
 
+#include "Logging.h"
 #include "SnapshotImpl.h"
 
@@ -34 +37 @@
 
 #include "AutoCaller.h"
-#include "Logging.h"
 
 #include <iprt/path.h>
@@ -107 +109 @@
 HRESULT Snapshot::FinalConstruct()
 {
-    LogFlowMember(("Snapshot::FinalConstruct()\n"));
+    LogFlowThisFunc(("\n"));
     return S_OK;
 }
@@ -113 +115 @@
 void Snapshot::FinalRelease()
 {
-    LogFlowMember(("Snapshot::FinalRelease()\n"));
+    LogFlowThisFunc(("\n"));
     uninit();
 }
@@ -135 +137 @@
                        Snapshot *aParent)
 {
-    LogFlowMember(("Snapshot::init(uuid: %s, aParent->uuid=%s)\n", aId.toString().c_str(), (aParent) ? aParent->m->uuid.toString().c_str() : ""));
+    LogFlowThisFunc(("uuid=%s aParent->uuid=%s\n", aId.toString().c_str(), (aParent) ? aParent->m->uuid.toString().c_str() : ""));
 
     ComAssertRet(!aId.isEmpty() && !aName.isEmpty() && aMachine, E_INVALIDARG);
@@ -175 +177 @@
 void Snapshot::uninit()
 {
-    LogFlowMember(("Snapshot::uninit()\n"));
+    LogFlowThisFunc(("\n"));
 
     /* Enclose the state transition Ready->InUninit->NotReady */
@@ -210 +212 @@
 
 /**
- *  Discards the current snapshot by removing it from the tree of snapshots
+ *  Delete the current snapshot by removing it from the tree of snapshots
  *  and reparenting its children.
  *
@@ -221 +223 @@
  *  lock in write mode AND the machine state must be DeletingSnapshot.
  */
-void Snapshot::beginDiscard()
+void Snapshot::beginSnapshotDelete()
 {
     AutoCaller autoCaller(this);
@@ -231 +233 @@
     Assert(m->pMachine->isWriteLockOnCurrentThread());
 
-    // the snapshot must have only one child when discarded or no children at all
+    // the snapshot must have only one child when being deleted or no children at all
     AssertReturnVoid(m->llChildren.size() <= 1);
 
@@ -237 +239 @@
 
     /// @todo (dmik):
-    //  when we introduce clones later, discarding the snapshot
-    //  will affect the current and first snapshots of clones, if they are
-    //  direct children of this snapshot. So we will need to lock machines
-    //  associated with child snapshots as well and update mCurrentSnapshot
-    //  and/or mFirstSnapshot fields.
+    //  when we introduce clones later, deleting the snapshot will affect
+    //  the current and first snapshots of clones, if they are direct children
+    //  of this snapshot. So we will need to lock machines associated with
+    //  child snapshots as well and update mCurrentSnapshot and/or
+    //  mFirstSnapshot fields.
 
     if (this == m->pMachine->mData->mCurrentSnapshot)
@@ -1148 +1150 @@
 };
 
-/** Discard snapshot task */
+/** Delete snapshot task */
 struct SessionMachine::DeleteSnapshotTask
     : public SessionMachine::SnapshotTask
@@ -1685 +1687 @@
     try
     {
-        /* discard the saved state file if the machine was Saved prior to this
+        /* Delete the saved state file if the machine was Saved prior to this
          * operation */
         if (aTask.machineStateBackup == MachineState_Saved)
@@ -1811 +1813 @@
         int saveFlags = 0;
 
-        /* we have already discarded the current state, so set the execution
-         * state accordingly no matter of the discard snapshot result */
+        /* we have already deleted the current state, so set the execution
+         * state accordingly no matter of the delete snapshot result */
         if (!mSSData->mStateFilePath.isEmpty())
             setMachineState(MachineState_Saved);
@@ -1872 +1874 @@
             LogFlowThisFunc(("Deleting old current state in differencing image '%s'\n", pMedium->getName().raw()));
 
-            HRESULT rc2 = pMedium->deleteStorageAndWait(NULL /*aProgress*/, &fNeedsSaveSettings);
+            HRESULT rc2 = pMedium->deleteStorage(NULL /* aProgress */,
+                                                 true /* aWait */,
+                                                 &fNeedsSaveSettings);
             // ignore errors here because we cannot roll back after saveSettings() above
             if (SUCCEEDED(rc2))
@@ -1971 +1975 @@
                         childrenCount);
 
-    /* If the snapshot being discarded is the current one, ensure current
+    /* If the snapshot being deleted is the current one, ensure current
      * settings are committed and saved.
      */
@@ -2017 +2021 @@
             if (type != MediumType_Writethrough) // writethrough images are unaffected by snapshots, so do nothing for them
             {
-                // normal or immutable: then this will need to be discarded
+                // normal or immutable media need attention
                 ++ulOpCount;
                 ulTotalWeight += (ULONG)(pHD->getSize() / _1M);
@@ -2069 +2073 @@
  * Helper struct for SessionMachine::deleteSnapshotHandler().
  */
-struct MediumDiscardRec
-{
-    MediumDiscardRec()
-        : chain(NULL)
+struct MediumDeleteRec
+{
+    MediumDeleteRec()
+        : mpMediumLockList(NULL)
     {}
 
-    MediumDiscardRec(const ComObjPtr<Medium> &aHd,
-                     Medium::MergeChain *aChain = NULL)
-        : hd(aHd),
-          chain(aChain)
+    MediumDeleteRec(const ComObjPtr<Medium> &aHd,
+                    const ComObjPtr<Medium> &aSource,
+                    const ComObjPtr<Medium> &aTarget,
+                    bool fMergeForward,
+                    const ComObjPtr<Medium> &aParentForTarget,
+                    const MediaList &aChildrenToReparent,
+                    MediumLockList *aMediumLockList)
+        : mpHD(aHd),
+          mpSource(aSource),
+          mpTarget(aTarget),
+          mfMergeForward(fMergeForward),
+          mpParentForTarget(aParentForTarget),
+          mChildrenToReparent(aChildrenToReparent),
+          mpMediumLockList(aMediumLockList)
     {}
 
-    MediumDiscardRec(const ComObjPtr<Medium> &aHd,
-                     Medium::MergeChain *aChain,
-                     const ComObjPtr<Medium> &aReplaceHd,
-                     const ComObjPtr<MediumAttachment> &aReplaceHda,
-                     const Guid &aSnapshotId)
-        : hd(aHd),
-          chain(aChain),
-          replaceHd(aReplaceHd),
-          replaceHda(aReplaceHda),
-          snapshotId(aSnapshotId)
+    MediumDeleteRec(const ComObjPtr<Medium> &aHd,
+                    const ComObjPtr<Medium> &aSource,
+                    const ComObjPtr<Medium> &aTarget,
+                    bool fMergeForward,
+                    const ComObjPtr<Medium> &aParentForTarget,
+                    const MediaList &aChildrenToReparent,
+                    MediumLockList *aMediumLockList,
+                    const ComObjPtr<MediumAttachment> &aReplaceHda,
+                    const Guid &aSnapshotId)
+        : mpHD(aHd),
+          mpSource(aSource),
+          mpTarget(aTarget),
+          mfMergeForward(fMergeForward),
+          mpParentForTarget(aParentForTarget),
+          mChildrenToReparent(aChildrenToReparent),
+          mpMediumLockList(aMediumLockList),
+          mpReplaceHda(aReplaceHda),
+          mSnapshotId(aSnapshotId)
     {}
 
-    ComObjPtr<Medium> hd;
-    Medium::MergeChain *chain;
+    ComObjPtr<Medium> mpHD;
+    ComObjPtr<Medium> mpSource;
+    ComObjPtr<Medium> mpTarget;
+    bool mfMergeForward;
+    ComObjPtr<Medium> mpParentForTarget;
+    MediaList mChildrenToReparent;
+    MediumLockList *mpMediumLockList;
     /* these are for the replace hard disk case: */
-    ComObjPtr<Medium> replaceHd;
-    ComObjPtr<MediumAttachment> replaceHda;
-    Guid snapshotId;
+    ComObjPtr<MediumAttachment> mpReplaceHda;
+    Guid mSnapshotId;
 };
 
-typedef std::list <MediumDiscardRec> MediumDiscardRecList;
+typedef std::list<MediumDeleteRec> MediumDeleteRecList;
 
 /**
@@ -2138 +2164 @@
     }
 
-    MediumDiscardRecList toDiscard;
+    MediumDeleteRecList toDelete;
 
     HRESULT rc = S_OK;
@@ -2145 +2171 @@
     bool fNeedsSaveSettings = false;            // VirtualBox.xml
 
-    Guid snapshotId1;
+    Guid snapshotId;
 
     try
@@ -2159 +2185 @@
         ComObjPtr<SnapshotMachine> pSnapMachine = aTask.pSnapshot->getSnapshotMachine();
         // no need to lock the snapshot machine since it is const by definiton
+        Guid machineId = pSnapMachine->getId();
 
         // save the snapshot ID (for callbacks)
-        snapshotId1 = aTask.pSnapshot->getId();
+        snapshotId = aTask.pSnapshot->getId();
 
         // first pass:
@@ -2177 +2204 @@
             ComObjPtr<MediumAttachment> &pAttach = *it;
             AutoReadLock attachLock(pAttach COMMA_LOCKVAL_SRC_POS);
-            if (pAttach->getType() == DeviceType_HardDisk)
+            if (pAttach->getType() != DeviceType_HardDisk)
+                continue;
+
+            ComObjPtr<Medium> pHD = pAttach->getMedium();
+            Assert(!pHD.isNull());
+
             {
-                Assert(pAttach->getMedium());
-                ComObjPtr<Medium> pHD = pAttach->getMedium();
-
+                // writethrough images are unaffected by snapshots, skip them
+                AutoReadLock medlock(pHD COMMA_LOCKVAL_SRC_POS);
+                MediumType_T type = pHD->getType();
+                if (type == MediumType_Writethrough)
+                    continue;
+            }
+
+#ifdef DEBUG
+            pHD->dumpBackRefs();
+#endif
+
+            // needs to be merged with child or deleted, check prerequisites
+            ComObjPtr<Medium> pTarget;
+            ComObjPtr<Medium> pSource;
+            bool fMergeForward = false;
+            ComObjPtr<Medium> pParentForTarget;
+            MediaList childrenToReparent;
+            MediumLockList *pMediumLockList = NULL;
+            rc = prepareDeleteSnapshotMedium(pHD, machineId, snapshotId,
+                                             pSource, pTarget, fMergeForward,
+                                             pParentForTarget,
+                                             childrenToReparent,
+                                             pMediumLockList);
+            if (FAILED(rc)) throw rc;
+
+            // for simplicity, we merge pHd onto its child (forward merge), not
+            // the other way round, because that saves us from updating the
+            // attachments for the machine that follows the snapshot (next
+            // snapshot or real machine), unless it's a base image:
+
+            if (pMediumLockList != NULL && !fMergeForward)
+            {
+                // parent is null -> this disk is a base hard disk: we will
+                // then do a backward merge, i.e. merge its only child onto the
+                // base disk. Here we need then to update the attachment that
+                // refers to the child and have it point to the parent instead
+                Assert(pHD->getParent().isNull());
+                Assert(pHD->getChildren().size() == 1);
+
+                ComObjPtr<Medium> pReplaceHD = pHD->getChildren().front();
+
+                ComAssertThrow(pReplaceHD == pSource, E_FAIL);
+
+                const Guid *pReplaceMachineId = pSource->getFirstMachineBackrefId();
+                NOREF(pReplaceMachineId);
+                Assert(pReplaceMachineId);
+                Assert(*pReplaceMachineId == mData->mUuid);
+
+                Guid snapshotId2;
+                const Guid *pSnapshotId = pSource->getFirstMachineBackrefSnapshotId();
+                if (pSnapshotId)
+                    snapshotId2 = *pSnapshotId;
+
+                HRESULT rc2 = S_OK;
+
+                attachLock.release();
+
+                // First we must detach the child otherwise mergeTo() will
+                // will assert because it is going to delete the child.
+                // Adjust the backreferences:
+                // 1) detach the first child hard disk
+                rc2 = pSource->detachFrom(mData->mUuid, snapshotId2);
+                AssertComRC(rc2);
+                // 2) attach to machine and snapshot
+                rc2 = pHD->attachTo(mData->mUuid, snapshotId2);
+                AssertComRC(rc2);
+
+                /* replace the hard disk in the attachment object */
+                if (snapshotId2.isEmpty())
                 {
-                    // writethrough images are unaffected by snapshots, so do nothing for them
-                    AutoReadLock medlock(pHD COMMA_LOCKVAL_SRC_POS);
-                    MediumType_T type = pHD->getType();
-                    if (type == MediumType_Writethrough)
-                        continue;
+                    /* in current state */
+                    AssertBreak(pAttach = findAttachment(mMediaData->mAttachments, pSource));
                 }
-
-                // image is normal or immutable: then this will need to be discarded
-
-                // do not lock medium now, prepareDiscard() has a write lock which will hang otherwise
-
-#ifdef DEBUG
-                pHD->dumpBackRefs();
-#endif
-
-                Medium::MergeChain *chain = NULL;
-
-                // needs to be discarded (merged with the child if any), check prerequisites
-                rc = pHD->prepareDiscard(chain);
-                if (FAILED(rc)) throw rc;
-
-                // for simplicity, we merge pHd onto its child (forward merge), not the
-                // other way round, because that saves us from updating the attachments
-                // for the machine that follows the snapshot (next snapshot or real machine),
-                // unless it's a base image:
-
-                if (    pHD->getParent().isNull()
-                     && chain != NULL
-                   )
+                else
                 {
-                    // parent is null -> this disk is a base hard disk: we will
-                    // then do a backward merge, i.e. merge its only child onto
-                    // the base disk; prepareDiscard() does necessary checks.
-                    // So here we need then to update the attachment that refers
-                    // to the child and have it point to the parent instead
-
-                    /* The below assert would be nice but I don't want to move
-                     * Medium::MergeChain to the header just for that
-                     * Assert(!chain->isForward()); */
-
-                    // prepareDiscard() should have raised an error already
-                    // if there was more than one child
-                    Assert(pHD->getChildren().size() == 1);
-
-                    ComObjPtr<Medium> pReplaceHD = pHD->getChildren().front();
-
-                    const Guid *pReplaceMachineId = pReplaceHD->getFirstMachineBackrefId();
-                    NOREF(pReplaceMachineId);
-                    Assert(pReplaceMachineId);
-                    Assert(*pReplaceMachineId == mData->mUuid);
-
-                    Guid snapshotId;
-                    const Guid *pSnapshotId = pReplaceHD->getFirstMachineBackrefSnapshotId();
-                    if (pSnapshotId)
-                        snapshotId = *pSnapshotId;
-
-                    HRESULT rc2 = S_OK;
-
-                    attachLock.release();
-
-                    // First we must detach the child (otherwise mergeTo() called
-                    // by discard() will assert because it will be going to delete
-                    // the child), so adjust the backreferences:
-                    // 1) detach the first child hard disk
-                    rc2 = pReplaceHD->detachFrom(mData->mUuid, snapshotId);
+                    /* in snapshot */
+                    ComObjPtr<Snapshot> snapshot;
+                    rc2 = findSnapshot(snapshotId2, snapshot);
                     AssertComRC(rc2);
-                    // 2) attach to machine and snapshot
-                    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, pReplaceHD));
-                    }
-                    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, pReplaceHD));
-                    }
-
-                    AutoWriteLock attLock(pAttach COMMA_LOCKVAL_SRC_POS);
-                    pAttach->updateMedium(pHD, false /* aImplicit */);
-
-                    toDiscard.push_back(MediumDiscardRec(pHD,
-                                                         chain,
-                                                         pReplaceHD,
-                                                         pAttach,
-                                                         snapshotId));
-                    continue;
+
+                    /* don't lock the snapshot; cannot be modified outside */
+                    MediaData::AttachmentList &snapAtts = snapshot->getSnapshotMachine()->mMediaData->mAttachments;
+                    AssertBreak(pAttach = findAttachment(snapAtts, pSource));
                 }
 
-                toDiscard.push_back(MediumDiscardRec(pHD, chain));
+                AutoWriteLock attLock(pAttach COMMA_LOCKVAL_SRC_POS);
+                pAttach->updateMedium(pHD, false /* aImplicit */);
+
+                toDelete.push_back(MediumDeleteRec(pHD, pSource, pTarget,
+                                                   fMergeForward,
+                                                   pParentForTarget,
+                                                   childrenToReparent,
+                                                   pMediumLockList,
+                                                   pAttach, snapshotId2));
             }
+            else
+                toDelete.push_back(MediumDeleteRec(pHD, pSource, pTarget,
+                                                   fMergeForward,
+                                                   pParentForTarget,
+                                                   childrenToReparent,
+                                                   pMediumLockList));
         }
 
@@ -2289 +2317 @@
         multiLock.release();
 
+        /** @todo r=klaus the comment below makes only limited sense to me.
+         * Deleting the snapshot early has some benefits, but also the drawback
+         * that a normal user surely has difficulties to perform the not
+         * completed merge operations later. Before IMedium::mergeTo is added
+         * to VBoxManage it actually is completely impossible. */
+
         /* Now we checked that we can successfully merge all normal hard disks
          * (unless a runtime error like end-of-disc happens). Prior to
-         * performing the actual merge, we want to discard the snapshot itself
+         * performing the actual merge, we want to delete the snapshot itself
          * and remove it from the XML file to make sure that a possible merge
-         * ruintime error will not make this snapshot inconsistent because of
+         * runtime error will not make this snapshot inconsistent because of
          * the partially merged or corrupted hard disks */
 
         /* second pass: */
-        LogFlowThisFunc(("2: Discarding snapshot...\n"));
+        LogFlowThisFunc(("2: Deleting snapshot...\n"));
 
         {
@@ -2307 +2341 @@
             Utf8Str stateFilePath = aTask.pSnapshot->stateFilePath();
 
-            // Note that discarding the snapshot will deassociate it from the
+            // Note that deleting the snapshot will deassociate it from the
             // hard disks which will allow the merge+delete operation for them
-            aTask.pSnapshot->beginDiscard();
+            aTask.pSnapshot->beginSnapshotDelete();
             aTask.pSnapshot->uninit();
                     // this requests the machine lock in turn when deleting all the children
@@ -2320 +2354 @@
             if (!stateFilePath.isEmpty())
             {
-                aTask.pProgress->SetNextOperation(Bstr(tr("Discarding the execution state")),
+                aTask.pProgress->SetNextOperation(Bstr(tr("Deleting the execution state")),
                                                   1);        // weight
 
@@ -2330 +2364 @@
             /// saveSnapshotSettings fails. Actually, we may call
             /// #saveSnapshotSettings() with a special flag that will tell it to
-            /// skip the given snapshot as if it would have been discarded and
-            /// only actually discard it if the save operation succeeds.
+            /// skip the given snapshot as if it would have been deleted and
+            /// only actually delete it if the save operation succeeds.
         }
 
-        /* here we come when we've irrevesibly discarded the snapshot which
+        /* here we come when we've irreversibly eleted the snapshot which
          * means that the VM settigns (our relevant changes to mData) need to be
          * saved too */
@@ -2347 +2381 @@
         /// snapshot itself has been already deleted (and interpret these
         /// warnings properly on the GUI side)
-        for (MediumDiscardRecList::iterator it = toDiscard.begin();
-             it != toDiscard.end();)
+        for (MediumDeleteRecList::iterator it = toDelete.begin();
+             it != toDelete.end();)
         {
-            rc = it->hd->discard(aTask.pProgress,
-                                 (ULONG)(it->hd->getSize() / _1M),          // weight
-                                 it->chain,
-                                 &fNeedsSaveSettings);
+            const ComObjPtr<Medium> &pMedium(it->mpHD);
+            ULONG ulWeight;
+
+            {
+                AutoReadLock alock(pMedium COMMA_LOCKVAL_SRC_POS);
+                ulWeight = (ULONG)(pMedium->getSize() / _1M);
+            }
+
+            aTask.pProgress->SetNextOperation(BstrFmt(tr("Merging differencing image '%s'"),
+                                              pMedium->getName().raw()),
+                                              ulWeight);
+
+            if (it->mpMediumLockList == NULL)
+            {
+                /* no real merge needed, just updating state and delete
+                 * diff files if necessary */
+                AutoMultiWriteLock2 mLock(&mParent->getMediaTreeLockHandle(), pMedium->lockHandle() COMMA_LOCKVAL_SRC_POS);
+
+                Assert(   !it->mfMergeForward
+                       || pMedium->getChildren().size() == 0);
+
+                /* Delete the differencing hard disk (has no children). Two
+                 * exceptions: if it's the last medium in the chain or if it's
+                 * a backward merge we don't want to handle due to complextity.
+                 * In both cases leave the image in place. If it's the first
+                 * exception the user can delete it later if he wants. */
+                if (!pMedium->getParent().isNull())
+                {
+                    Assert(pMedium->getState() == MediumState_Deleting);
+                    rc = pMedium->deleteStorage(&aTask.pProgress,
+                                                true /* aWait */,
+                                                &fNeedsSaveSettings);
+                    if (SUCCEEDED(rc))
+                        pMedium->uninit();
+                }
+            }
+            else
+            {
+                /* Normal merge operation, in the direction decided earlier. */
+                rc = it->mpSource->mergeTo(it->mpTarget, it->mfMergeForward,
+                                           it->mpParentForTarget,
+                                           it->mChildrenToReparent,
+                                           it->mpMediumLockList,
+                                           &aTask.pProgress, true /* aWait */,
+                                           &fNeedsSaveSettings);
+                if (SUCCEEDED(rc))
+                {
+                    /* On success delete the no longer needed medium lock
+                     * list. This unlocks the media as well. */
+                    delete it->mpMediumLockList;
+                    it->mpMediumLockList = NULL;
+                    it->mpSource->uninit();
+                }
+            }
+
             if (FAILED(rc)) throw rc;
 
-            /* prevent from calling cancelDiscard() */
-            it = toDiscard.erase(it);
+            /* prevent from calling cancelDeleteSnapshotMedium() */
+            it = toDelete.erase(it);
         }
     }
@@ -2364 +2449 @@
     if (FAILED(rc))
     {
-        HRESULT rc2 = S_OK;
-
         AutoMultiWriteLock2 multiLock(this->lockHandle(),                   // machine
                                       &mParent->getMediaTreeLockHandle()    // media tree
@@ -2371 +2454 @@
 
         // un-prepare the remaining hard disks
-        for (MediumDiscardRecList::const_iterator it = toDiscard.begin();
-             it != toDiscard.end();
+        for (MediumDeleteRecList::const_iterator it = toDelete.begin();
+             it != toDelete.end();
              ++it)
         {
-            it->hd->cancelDiscard(it->chain);
-
-            if (!it->replaceHd.isNull())
-            {
-                rc2 = it->replaceHd->attachTo(mData->mUuid, it->snapshotId);
-                AssertComRC(rc2);
-
-                rc2 = it->hd->detachFrom(mData->mUuid, it->snapshotId);
-                AssertComRC(rc2);
-
-                AutoWriteLock attLock(it->replaceHda COMMA_LOCKVAL_SRC_POS);
-                it->replaceHda->updateMedium(it->replaceHd, false /* aImplicit */);
-            }
+            cancelDeleteSnapshotMedium(it->mpHD, it->mpSource, it->mpTarget,
+                                       it->mChildrenToReparent,
+                                       it->mpMediumLockList,
+                                       it->mpReplaceHda, it->mSnapshotId);
         }
     }
@@ -2423 +2497 @@
 
     if (SUCCEEDED(rc))
-        mParent->onSnapshotDeleted(mData->mUuid, snapshotId1);
+        mParent->onSnapshotDeleted(mData->mUuid, snapshotId);
 
     LogFlowThisFunc(("Done deleting snapshot (rc=%08X)\n", rc));
@@ -2429 +2503 @@
 }
 
+/**
+ * Checks that this hard disk (part of a snapshot) may be deleted/merged and
+ * performs necessary state changes. Must not be called for writethrough disks
+ * because there is nothing to delete/merge then.
+ *
+ * This method is to be called prior to calling #deleteSnapshotMedium().
+ * If #deleteSnapshotMedium() is not called or fails, the state modifications
+ * performed by this method must be undone by #cancelDeleteSnapshotMedium().
+ *
+ * @param aHD           Hard disk which is connected to the snapshot.
+ * @param aMachineId    UUID of machine this hard disk is attached to.
+ * @param aSnapshotId   UUID of snapshot this hard disk is attached to. May
+ *                      be a zero UUID if no snapshot is applicable.
+ * @param aSource       Source hard disk for merge (out).
+ * @param aTarget       Target hard disk for merge (out).
+ * @param aMergeForward Merge direction decision (out).
+ * @param aParentForTarget New parent if target needs to be reparented (out).
+ * @param aChildrenToReparent Children which have to be reparented to the
+ *                      target (out).
+ * @param aMediumLockList Where to store the created medium lock list (may
+ *                      return NULL if no real merge is necessary).
+ *
+ * @note Caller must hold media tree lock for writing. This locks this object
+ *       and every medium object on the merge chain for writing.
+ */
+HRESULT SessionMachine::prepareDeleteSnapshotMedium(const ComObjPtr<Medium> &aHD,
+                                                    const Guid &aMachineId,
+                                                    const Guid &aSnapshotId,
+                                                    ComObjPtr<Medium> &aSource,
+                                                    ComObjPtr<Medium> &aTarget,
+                                                    bool &aMergeForward,
+                                                    ComObjPtr<Medium> &aParentForTarget,
+                                                    MediaList &aChildrenToReparent,
+                                                    MediumLockList * &aMediumLockList)
+{
+    Assert(mParent->getMediaTreeLockHandle().isWriteLockOnCurrentThread());
+
+    AutoWriteLock alock(aHD COMMA_LOCKVAL_SRC_POS);
+
+    // Medium must not be writethrough at this point
+    AssertReturn(aHD->getType() != MediumType_Writethrough, E_FAIL);
+
+    aMediumLockList = NULL;
+
+    if (aHD->getChildren().size() == 0)
+    {
+        /* special treatment of the last hard disk in the chain: */
+        if (aHD->getParent().isNull())
+        {
+            /* lock only, to prevent any usage until the snapshot deletion
+             * is completed */
+            return aHD->LockWrite(NULL);
+        }
+
+        /* the differencing hard disk w/o children will be deleted, protect it
+         * from attaching to other VMs (this is why Deleting) */
+        return aHD->markForDeletion();
+   }
+
+    /* not going multi-merge as it's too expensive */
+    if (aHD->getChildren().size() > 1)
+        return setError(E_FAIL,
+                        tr ("Hard disk '%s' has more than one child hard disk (%d)"),
+                        aHD->getLocationFull().raw(),
+                        aHD->getChildren().size());
+
+    ComObjPtr<Medium> pChild = aHD->getChildren().front();
+
+    /* we keep this locked, so lock the affected child to make sure the lock
+     * order is correct when calling prepareMergeTo() */
+    AutoWriteLock childLock(pChild COMMA_LOCKVAL_SRC_POS);
+
+    /* the rest is a normal merge setup */
+    if (aHD->getParent().isNull())
+    {
+        /* base hard disk, backward merge */
+        const Guid *pMachineId1 = pChild->getFirstMachineBackrefId();
+        const Guid *pMachineId2 = aHD->getFirstMachineBackrefId();
+        if (pMachineId1 && pMachineId2 && *pMachineId1 != *pMachineId2)
+        {
+            /* backward merge is too tricky, we'll just detach on snapshot
+             * deletion, so lock only, to prevent any usage */
+            return aHD->LockWrite(NULL);
+        }
+
+        aSource = pChild;
+        aTarget = aHD;
+    }
+    else
+    {
+        /* forward merge */
+        aSource = aHD;
+        aTarget = pChild;
+    }
+
+    return aSource->prepareMergeTo(aTarget, &aMachineId, &aSnapshotId,
+                                   aMergeForward, aParentForTarget,
+                                   aChildrenToReparent, aMediumLockList);
+}
+
+/**
+ * Cancels the deletion/merging of this hard disk (part of a snapshot). Undoes
+ * what #prepareDeleteSnapshotMedium() did. Must be called if
+ * #deleteSnapshotMedium() is not called or fails.
+ *
+ * @param aHD           Hard disk which is connected to the snapshot.
+ * @param aSource       Source hard disk for merge.
+ * @param aTarget       Source hard disk for merge.
+ * @param aChildrenToReparent Children to unlock.
+ * @param aMediumLockList Medium locks to cancel.
+ * @param aReplaceHda   Hard disk attachment to restore.
+ * @param aSnapshotId   Snapshot id to attach the medium to.
+ *
+ * @note Locks the medium tree and the hard disks in the chain for writing.
+ */
+void SessionMachine::cancelDeleteSnapshotMedium(const ComObjPtr<Medium> &aHD,
+                                                const ComObjPtr<Medium> &aSource,
+                                                const ComObjPtr<Medium> &aTarget,
+                                                const MediaList &aChildrenToReparent,
+                                                MediumLockList *aMediumLockList,
+                                                const ComObjPtr<MediumAttachment> &aReplaceHda,
+                                                const Guid &aSnapshotId)
+{
+    if (aMediumLockList == NULL)
+    {
+        AutoMultiWriteLock2 mLock(&mParent->getMediaTreeLockHandle(), aHD->lockHandle() COMMA_LOCKVAL_SRC_POS);
+
+        Assert(aHD->getChildren().size() == 0);
+
+        if (aHD->getParent().isNull())
+        {
+            HRESULT rc = aHD->UnlockWrite(NULL);;
+            AssertComRC(rc);
+        }
+        else
+        {
+            HRESULT rc = aHD->unmarkForDeletion();
+            AssertComRC(rc);
+        }
+    }
+    else
+        aSource->cancelMergeTo(aChildrenToReparent, aMediumLockList);
+
+    if (!aReplaceHda.isNull())
+    {
+        HRESULT rc = aTarget->attachTo(mData->mUuid, aSnapshotId);
+        AssertComRC(rc);
+
+        rc = aHD->detachFrom(mData->mUuid, aSnapshotId);
+        AssertComRC(rc);
+
+        AutoWriteLock attLock(aReplaceHda COMMA_LOCKVAL_SRC_POS);
+        aReplaceHda->updateMedium(aTarget, false /* aImplicit */);
+    }
+}

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

@@ -9159 +9159 @@
   <interface
     name="IMedium" extends="$unknown"
-    uuid="aa8167ba-df72-4738-b740-9b84377ba9f1"
+    uuid="d709160c-303f-4ead-b7ef-53ffa26aa861"
     wsmap="managed"
   >
@@ -10351 +10351 @@
         </note>
       </desc>
-      <param name="targetId" type="uuid" mod="string" dir="in">
-        <desc>UUID of the target ancestor or descendant medium.</desc>
+      <param name="target" type="IMedium" dir="in">
+        <desc>Target medium.</desc>
       </param>
       <param name="progress" type="IProgress" dir="return">

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

@@ +1 @@
+/* $Id$ */
+
 /** @file
  *
@@ -5 +7 @@
 
 /*
- * Copyright (C) 2006-2007 Sun Microsystems, Inc.
+ * Copyright (C) 2006-2010 Sun Microsystems, Inc.
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
@@ -54 +56 @@
 #include <iprt/assert.h>
 
-/** @deprecated Please use LogFlowThisFunc instead! */
-#define LogFlowMember(m)     \
-    do { LogFlow (("{%p} ", this)); LogFlow (m); } while (0)
-
 /** @def MyLogIt
  * Copy of LogIt that works even when logging is completely disabled (e.g. in

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

@@ -3 +3 @@
 /** @file
  *
- * VirtualBox COM class declaration
+ * VirtualBox COM class implementation
  */
 
@@ -29 +29 @@
 #include "VRDPServerImpl.h"
 #include "MediumAttachmentImpl.h"
+#include "MediumLock.h"
 #include "NetworkAdapterImpl.h"
 #include "AudioAdapterImpl.h"
@@ -148 +149 @@
             ComObjPtr<SessionMachine> mMachine;
 
-            /**
-             * Successfully locked media list. The 2nd value in the pair is true
-             * if the medium is locked for writing and false if locked for
-             * reading.
-             */
-            typedef std::list<std::pair<ComPtr<IMedium>, bool > > LockedMedia;
-            LockedMedia mLockedMedia;
+            /** Medium object lock collection. */
+            MediumLockListMap mLockedMedia;
         };
 
@@ -316 +312 @@
      *  The usage policy is the same as for HWData, but a separate structure
      *  is necessary because hard disk data requires different procedures when
-     *  taking or discarding snapshots, etc.
+     *  taking or deleting snapshots, etc.
      *
      *  The data variable is |mMediaData|.
@@ -997 +993 @@
     void restoreSnapshotHandler(RestoreSnapshotTask &aTask);
 
+    HRESULT prepareDeleteSnapshotMedium(const ComObjPtr<Medium> &aHD,
+                                        const Guid &machineId,
+                                        const Guid &snapshotId,
+                                        ComObjPtr<Medium> &aSource,
+                                        ComObjPtr<Medium> &aTarget,
+                                        bool &fMergeForward,
+                                        ComObjPtr<Medium> &pParentForTarget,
+                                        MediaList &aChildrenToReparent,
+                                        MediumLockList * &aMediumLockList);
+    void cancelDeleteSnapshotMedium(const ComObjPtr<Medium> &aHD,
+                                    const ComObjPtr<Medium> &aSource,
+                                    const ComObjPtr<Medium> &aTarget,
+                                    const MediaList &aChildrenToReparent,
+                                    MediumLockList *aMediumLockList,
+                                    const ComObjPtr<MediumAttachment> &aReplaceHda,
+                                    const Guid &aSnapshotId);
+
     HRESULT lockMedia();
     void unlockMedia();

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

@@ -1 +1 @@
 /* $Id$ */
+
 /** @file
  *
@@ -25 +26 @@
 
 #include "VirtualBoxBase.h"
+#include "MediumLock.h"
 
 class Progress;
@@ -45 +47 @@
 {
 public:
-    class MergeChain;
-    class ImageChain;
-
     VIRTUALBOXBASE_ADD_ERRORINFO_SUPPORT(Medium)
 
@@ -144 +143 @@
                                  MediumVariant_T aVariant,
                                  IProgress **aProgress);
-    STDMETHOD(MergeTo)(IN_BSTR aTargetId, IProgress **aProgress);
+    STDMETHOD(MergeTo)(IMedium *aTarget, IProgress **aProgress);
     STDMETHOD(CloneTo)(IMedium *aTarget, MediumVariant_T aVariant,
                         IMedium *aParent, IProgress **aProgress);
@@ -155 +154 @@
     const MediaList& getChildren() const;
 
+    // unsafe methods for internal purposes only (ensure there is
+    // a caller and a read lock before calling them!)
     const Guid& getId() const;
     MediumState_T getState() const;
@@ -160 +161 @@
     const Utf8Str& getLocationFull() const;
     uint64_t getSize() const;
+    MediumType_T getType() const;
+    Utf8Str getName();
 
     HRESULT attachTo(const Guid &aMachineId,
@@ -184 +187 @@
     HRESULT compareLocationTo(const char *aLocation, int &aResult);
 
-    /**
-     * Shortcut to #deleteStorage() that doesn't wait for operation completion
-     * and implies the progress object will be used for waiting.
-     */
-    HRESULT deleteStorageNoWait(ComObjPtr<Progress> &aProgress)
-    { return deleteStorage(&aProgress, false /* aWait */, NULL /* pfNeedsSaveSettings */); }
-
-    /**
-     * Shortcut to #deleteStorage() that wait for operation completion by
-     * blocking the current thread.
-     */
-    HRESULT deleteStorageAndWait(ComObjPtr<Progress> *aProgress, bool *pfNeedsSaveSettings)
-    { return deleteStorage(aProgress, true /* aWait */, pfNeedsSaveSettings); }
-
-    /**
-     * Shortcut to #createDiffStorage() that doesn't wait for operation
-     * completion and implies the progress object will be used for waiting.
-     */
-    HRESULT createDiffStorageNoWait(ComObjPtr<Medium> &aTarget,
-                                    MediumVariant_T aVariant,
-                                    ComObjPtr<Progress> &aProgress)
-    { return createDiffStorage(aTarget, aVariant, &aProgress, false /* aWait */, NULL /* pfNeedsSaveSettings*/ ); }
-
-    /**
-     * Shortcut to #createDiffStorage() that wait for operation completion by
-     * blocking the current thread.
-     */
-    HRESULT createDiffStorageAndWait(ComObjPtr<Medium> &aTarget,
-                                     MediumVariant_T aVariant,
-                                     bool *pfNeedsSaveSettings)
-    { return createDiffStorage(aTarget, aVariant, NULL /*aProgress*/, true /* aWait */, pfNeedsSaveSettings); }
-
-    HRESULT prepareMergeTo(Medium *aTarget, MergeChain * &aChain,
-                            bool aIgnoreAttachments = false);
-
-    /**
-     * Shortcut to #mergeTo() that doesn't wait for operation completion and
-     * implies the progress object will be used for waiting.
-     */
-    HRESULT mergeToNoWait(MergeChain *aChain,
-                          ComObjPtr<Progress> &aProgress)
-    { return mergeTo(aChain, &aProgress, false /* aWait */, NULL /*pfNeedsSaveSettings*/); }
-
-    /**
-     * Shortcut to #mergeTo() that wait for operation completion by
-     * blocking the current thread.
-     */
-    HRESULT mergeToAndWait(MergeChain *aChain,
-                           ComObjPtr<Progress> *aProgress,
-                           bool *pfNeedsSaveSettings)
-    { return mergeTo(aChain, aProgress, true /* aWait */, pfNeedsSaveSettings); }
-
-    void cancelMergeTo(MergeChain *aChain);
-
-    Utf8Str getName();
-
-    HRESULT prepareDiscard(MergeChain * &aChain);
-    HRESULT discard(ComObjPtr<Progress> &aProgress, ULONG ulWeight, MergeChain *aChain, bool *pfNeedsSaveSettings);
-    void cancelDiscard(MergeChain *aChain);
+    HRESULT createMediumLockList(bool fMediumWritable, Medium *pToBeParent, MediumLockList &mediumLockList);
+
+    HRESULT createDiffStorage(ComObjPtr<Medium> &aTarget,
+                              MediumVariant_T aVariant,
+                              MediumLockList *pMediumLockList,
+                              ComObjPtr<Progress> *aProgress,
+                              bool aWait,
+                              bool *pfNeedsSaveSettings);
+
+    HRESULT deleteStorage(ComObjPtr<Progress> *aProgress, bool aWait, bool *pfNeedsSaveSettings);
+    HRESULT markForDeletion();
+    HRESULT unmarkForDeletion();
+
+    HRESULT prepareMergeTo(const ComObjPtr<Medium> &pTarget,
+                           const Guid *aMachineId,
+                           const Guid *aSnapshotId,
+                           bool &fMergeForward,
+                           ComObjPtr<Medium> &pParentForTarget,
+                           MediaList &aChildrenToReparent,
+                           MediumLockList * &aMediumLockList);
+    HRESULT mergeTo(const ComObjPtr<Medium> &pTarget,
+                    bool &fMergeForward,
+                    ComObjPtr<Medium> pParentForTarget,
+                    const MediaList &aChildrenToReparent,
+                    MediumLockList *aMediumLockList,
+                    ComObjPtr<Progress> *aProgress,
+                    bool aWait,
+                    bool *pfNeedsSaveSettings);
+    void cancelMergeTo(const MediaList &aChildrenToReparent,
+                       MediumLockList *aMediumLockList);
 
     /** Returns a preferred format for a differencing hard disk. */
     Bstr preferredDiffFormat();
-
-    // unsafe inline public methods for internal purposes only (ensure there is
-    // a caller and a read lock before calling them!)
-    MediumType_T getType() const;
 
     /** For com::SupportErrorInfoImpl. */
@@ -272 +242 @@
 
     HRESULT setStateError();
-
-    HRESULT deleteStorage(ComObjPtr<Progress> *aProgress, bool aWait, bool *pfNeedsSaveSettings);
-
-    HRESULT createDiffStorage(ComObjPtr<Medium> &aTarget,
-                              MediumVariant_T aVariant,
-                              ComObjPtr<Progress> *aProgress,
-                              bool aWait,
-                              bool *pfNeedsSaveSettings);
-
-    HRESULT mergeTo(MergeChain *aChain,
-                    ComObjPtr<Progress> *aProgress,
-                    bool aWait,
-                    bool *pfNeedsSaveSettings);
 
     HRESULT setLocation(const Utf8Str &aLocation, const Utf8Str &aFormat = Utf8Str());
@@ -321 +278 @@
     HRESULT runNow(Medium::Task *pTask, bool *pfNeedsSaveSettings);
 
-    HRESULT taskThreadCreateBase(Medium::CreateBaseTask &task);
-    HRESULT taskThreadCreateDiff(Medium::CreateDiffTask &task);
-    HRESULT taskThreadMerge(Medium::MergeTask &task);
-    HRESULT taskThreadClone(Medium::CloneTask &task);
-    HRESULT taskThreadDelete(Medium::DeleteTask &task);
-    HRESULT taskThreadReset(Medium::ResetTask &task);
-    HRESULT taskThreadCompact(Medium::CompactTask &task);
+    HRESULT taskCreateBaseHandler(Medium::CreateBaseTask &task);
+    HRESULT taskCreateDiffHandler(Medium::CreateDiffTask &task);
+    HRESULT taskMergeHandler(Medium::MergeTask &task);
+    HRESULT taskCloneHandler(Medium::CloneTask &task);
+    HRESULT taskDeleteHandler(Medium::DeleteTask &task);
+    HRESULT taskResetHandler(Medium::ResetTask &task);
+    HRESULT taskCompactHandler(Medium::CompactTask &task);
 
     struct Data;            // opaque data struct, defined in MediumImpl.cpp

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

@@ +1 @@
+/* $Id$ */
+
 /** @file
  *
@@ -5 +7 @@
 
 /*
- * Copyright (C) 2006-2009 Sun Microsystems, Inc.
+ * Copyright (C) 2006-2010 Sun Microsystems, Inc.
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
@@ -70 +72 @@
     void uninit();
 
-    void beginDiscard();
+    void beginSnapshotDelete();
 
     void deparent();