Index: trunk/src/VBox/Main/src-server/SnapshotImpl.cpp
===================================================================
--- trunk/src/VBox/Main/src-server/SnapshotImpl.cpp	(revision 47401)
+++ trunk/src/VBox/Main/src-server/SnapshotImpl.cpp	(revision 48297)
@@ -2104,5 +2104,5 @@
 
 /**
- * Implementation for IInternalMachineControl::deleteSnapshot().
+ * Implementation for IInternalMachineControl::DeleteSnapshot().
  *
  * Gets called from Console::DeleteSnapshot(), and that's basically the
@@ -2294,7 +2294,8 @@
                     bool fMergeForward,
                     const ComObjPtr<Medium> &aParentForTarget,
-                    const MediaList &aChildrenToReparent,
+                    MediumLockList *aChildrenToReparent,
                     bool fNeedsOnlineMerge,
-                    MediumLockList *aMediumLockList)
+                    MediumLockList *aMediumLockList,
+                    const ComPtr<IToken> &aHDLockToken)
         : mpHD(aHd),
           mpSource(aSource),
@@ -2303,7 +2304,8 @@
           mfMergeForward(fMergeForward),
           mpParentForTarget(aParentForTarget),
-          mChildrenToReparent(aChildrenToReparent),
+          mpChildrenToReparent(aChildrenToReparent),
           mfNeedsOnlineMerge(fNeedsOnlineMerge),
-          mpMediumLockList(aMediumLockList)
+          mpMediumLockList(aMediumLockList),
+          mpHDLockToken(aHDLockToken)
     {}
 
@@ -2314,7 +2316,8 @@
                     bool fMergeForward,
                     const ComObjPtr<Medium> &aParentForTarget,
-                    const MediaList &aChildrenToReparent,
+                    MediumLockList *aChildrenToReparent,
                     bool fNeedsOnlineMerge,
                     MediumLockList *aMediumLockList,
+                    const ComPtr<IToken> &aHDLockToken,
                     const Guid &aMachineId,
                     const Guid &aSnapshotId)
@@ -2325,7 +2328,8 @@
           mfMergeForward(fMergeForward),
           mpParentForTarget(aParentForTarget),
-          mChildrenToReparent(aChildrenToReparent),
+          mpChildrenToReparent(aChildrenToReparent),
           mfNeedsOnlineMerge(fNeedsOnlineMerge),
           mpMediumLockList(aMediumLockList),
+          mpHDLockToken(aHDLockToken),
           mMachineId(aMachineId),
           mSnapshotId(aSnapshotId)
@@ -2338,7 +2342,9 @@
     bool mfMergeForward;
     ComObjPtr<Medium> mpParentForTarget;
-    MediaList mChildrenToReparent;
+    MediumLockList *mpChildrenToReparent;
     bool mfNeedsOnlineMerge;
     MediumLockList *mpMediumLockList;
+    /** optional lock token, used only in case mpHD is not merged/deleted */
+    ComPtr<IToken> mpHDLockToken;
     /* these are for reattaching the hard disk in case of a failure: */
     Guid mMachineId;
@@ -2450,9 +2456,10 @@
             bool fMergeForward = false;
             ComObjPtr<Medium> pParentForTarget;
-            MediaList childrenToReparent;
+            MediumLockList *pChildrenToReparent = NULL;
             bool fNeedsOnlineMerge = false;
             bool fOnlineMergePossible = aTask.m_fDeleteOnline;
             MediumLockList *pMediumLockList = NULL;
             MediumLockList *pVMMALockList = NULL;
+            ComPtr<IToken> pHDLockToken;
             ComObjPtr<MediumAttachment> pOnlineMediumAttachment;
             if (fOnlineMergePossible)
@@ -2487,7 +2494,8 @@
                                              pVMMALockList, pSource, pTarget,
                                              fMergeForward, pParentForTarget,
-                                             childrenToReparent,
+                                             pChildrenToReparent,
                                              fNeedsOnlineMerge,
-                                             pMediumLockList);
+                                             pMediumLockList,
+                                             pHDLockToken);
             treeLock.acquire();
             if (FAILED(rc))
@@ -2545,7 +2553,8 @@
                                                    fMergeForward,
                                                    pParentForTarget,
-                                                   childrenToReparent,
+                                                   pChildrenToReparent,
                                                    fNeedsOnlineMerge,
                                                    pMediumLockList,
+                                                   pHDLockToken,
                                                    replaceMachineId,
                                                    replaceSnapshotId));
@@ -2556,7 +2565,8 @@
                                                    fMergeForward,
                                                    pParentForTarget,
-                                                   childrenToReparent,
+                                                   pChildrenToReparent,
                                                    fNeedsOnlineMerge,
-                                                   pMediumLockList));
+                                                   pMediumLockList,
+                                                   pHDLockToken));
         }
 
@@ -2756,4 +2766,9 @@
                 if (it->mfNeedsOnlineMerge)
                 {
+                    // Put the medium merge information (MediumDeleteRec) where
+                    // SessionMachine::FinishOnlineMergeMedium can get at it.
+                    // This callback will arrive while onlineMergeMedium is
+                    // still executing, and there can't be two tasks.
+                    mConsoleTaskData.mDeleteSnapshotInfo = (void *)&(*it);
                     // online medium merge, in the direction decided earlier
                     rc = onlineMergeMedium(it->mpOnlineMediumAttachment,
@@ -2762,8 +2777,9 @@
                                            it->mfMergeForward,
                                            it->mpParentForTarget,
-                                           it->mChildrenToReparent,
+                                           it->mpChildrenToReparent,
                                            it->mpMediumLockList,
                                            aTask.pProgress,
                                            &fNeedsSave);
+                    mConsoleTaskData.mDeleteSnapshotInfo = NULL;
                 }
                 else
@@ -2773,5 +2789,5 @@
                                                it->mfMergeForward,
                                                it->mpParentForTarget,
-                                               it->mChildrenToReparent,
+                                               it->mpChildrenToReparent,
                                                it->mpMediumLockList,
                                                &aTask.pProgress,
@@ -2912,8 +2928,8 @@
         {
             cancelDeleteSnapshotMedium(it->mpHD, it->mpSource,
-                                       it->mChildrenToReparent,
+                                       it->mpChildrenToReparent,
                                        it->mfNeedsOnlineMerge,
-                                       it->mpMediumLockList, it->mMachineId,
-                                       it->mSnapshotId);
+                                       it->mpMediumLockList, it->mpHDLockToken,
+                                       it->mMachineId, it->mSnapshotId);
         }
     }
@@ -2966,6 +2982,6 @@
  * @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 aChildrenToReparent MediumLockList with children which have to be
+ *                      reparented to the target (out).
  * @param fNeedsOnlineMerge Whether this merge needs to be done online (out).
  *                      If this is set to @a true then the @a aVMMALockList
@@ -2974,4 +2990,6 @@
  * @param aMediumLockList Where to store the created medium lock list (may
  *                      return NULL if no real merge is necessary).
+ * @param aHDLockToken  Where to store the write lock token for aHD, in case
+ *                      it is not merged or deleted (out).
  *
  * @note Caller must hold media tree lock for writing. This locks this object
@@ -2987,7 +3005,8 @@
                                                     bool &aMergeForward,
                                                     ComObjPtr<Medium> &aParentForTarget,
-                                                    MediaList &aChildrenToReparent,
+                                                    MediumLockList * &aChildrenToReparent,
                                                     bool &fNeedsOnlineMerge,
-                                                    MediumLockList * &aMediumLockList)
+                                                    MediumLockList * &aMediumLockList,
+                                                    ComPtr<IToken> &aHDLockToken)
 {
     Assert(!mParent->getMediaTreeLockHandle().isWriteLockOnCurrentThread());
@@ -3002,4 +3021,5 @@
                  && type != MediumType_Readonly, E_FAIL);
 
+    aChildrenToReparent = NULL;
     aMediumLockList = NULL;
     fNeedsOnlineMerge = false;
@@ -3018,5 +3038,5 @@
              * is completed */
             alock.release();
-            return aHD->LockWrite(NULL);
+            return aHD->LockWrite(aHDLockToken.asOutParam());
         }
 
@@ -3049,5 +3069,5 @@
             childLock.release();
             alock.release();
-            return aHD->LockWrite(NULL);
+            return aHD->LockWrite(aHDLockToken.asOutParam());
         }
 
@@ -3146,37 +3166,35 @@
             {
                 /* we will lock the children of the source for reparenting */
-                for (MediaList::const_iterator it = aChildrenToReparent.begin();
-                     it != aChildrenToReparent.end();
-                     ++it)
+                if (aChildrenToReparent && !aChildrenToReparent->IsEmpty())
                 {
-                    ComObjPtr<Medium> pMedium = *it;
-                    AutoReadLock mediumLock(pMedium COMMA_LOCKVAL_SRC_POS);
-                    if (pMedium->getState() == MediumState_Created)
+                    /* Cannot just call aChildrenToReparent->Lock(), as one of
+                     * the children is the one under which the current state of
+                     * the VM is located, and this means it is already locked
+                     * (for reading). Note that no special unlocking is needed,
+                     * because cancelMergeTo will unlock everything locked in
+                     * its context (using the unlock on destruction), and both
+                     * cancelDeleteSnapshotMedium (in case something fails) and
+                     * FinishOnlineMergeMedium re-define the read/write lock
+                     * state of everything which the VM need, search for the
+                     * UpdateLock method calls. */
+                    childLock.release();
+                    alock.release();
+                    rc = aChildrenToReparent->Lock(true /* fSkipOverLockedMedia */);
+                    alock.acquire();
+                    childLock.acquire();
+                    MediumLockList::Base::iterator childrenToReparentBegin = aChildrenToReparent->GetBegin();
+                    MediumLockList::Base::iterator childrenToReparentEnd = aChildrenToReparent->GetEnd();
+                    for (MediumLockList::Base::iterator it = childrenToReparentBegin;
+                         it != childrenToReparentEnd;
+                         ++it)
                     {
-                        mediumLock.release();
-                        childLock.release();
-                        alock.release();
-                        rc = pMedium->LockWrite(NULL);
-                        alock.acquire();
-                        childLock.acquire();
-                        mediumLock.acquire();
-                        if (FAILED(rc))
-                            throw rc;
-                    }
-                    else
-                    {
-                        mediumLock.release();
-                        childLock.release();
-                        alock.release();
-                        rc = aVMMALockList->Update(pMedium, true);
-                        alock.acquire();
-                        childLock.acquire();
-                        mediumLock.acquire();
-                        if (FAILED(rc))
+                        ComObjPtr<Medium> pMedium = it->GetMedium();
+                        AutoReadLock mediumLock(pMedium COMMA_LOCKVAL_SRC_POS);
+                        if (!it->IsLocked())
                         {
                             mediumLock.release();
                             childLock.release();
                             alock.release();
-                            rc = pMedium->LockWrite(NULL);
+                            rc = aVMMALockList->Update(pMedium, true);
                             alock.acquire();
                             childLock.acquire();
@@ -3265,4 +3283,5 @@
  * @param fNeedsOnlineMerge Whether this merge needs to be done online.
  * @param aMediumLockList Medium locks to cancel.
+ * @param aHDLockToken  Optional write lock token for aHD.
  * @param aMachineId    Machine id to attach the medium to.
  * @param aSnapshotId   Snapshot id to attach the medium to.
@@ -3272,7 +3291,8 @@
 void SessionMachine::cancelDeleteSnapshotMedium(const ComObjPtr<Medium> &aHD,
                                                 const ComObjPtr<Medium> &aSource,
-                                                const MediaList &aChildrenToReparent,
+                                                MediumLockList *aChildrenToReparent,
                                                 bool fNeedsOnlineMerge,
                                                 MediumLockList *aMediumLockList,
+                                                const ComPtr<IToken> &aHDLockToken,
                                                 const Guid &aMachineId,
                                                 const Guid &aSnapshotId)
@@ -3286,6 +3306,10 @@
         if (aHD->getParent().isNull())
         {
-            HRESULT rc = aHD->UnlockWrite(NULL);
-            AssertComRC(rc);
+            Assert(!aHDLockToken.isNull());
+            if (!aHDLockToken.isNull())
+            {
+                HRESULT rc = aHDLockToken->Abandon();
+                AssertComRC(rc);
+            }
         }
         else
@@ -3354,6 +3378,6 @@
  * @param aMergeForward Merge direction.
  * @param aParentForTarget New parent if target needs to be reparented.
- * @param aChildrenToReparent Children which have to be reparented to the
- *                      target.
+ * @param aChildrenToReparent Medium lock list with children which have to be
+ *                      reparented to the target.
  * @param aMediumLockList Where to store the created medium lock list (may
  *                      return NULL if no real merge is necessary).
@@ -3366,5 +3390,5 @@
                                           bool fMergeForward,
                                           const ComObjPtr<Medium> &aParentForTarget,
-                                          const MediaList &aChildrenToReparent,
+                                          MediumLockList *aChildrenToReparent,
                                           MediumLockList *aMediumLockList,
                                           ComObjPtr<Progress> &aProgress,
@@ -3415,10 +3439,4 @@
                        && uTargetIdx != (unsigned)-1, E_FAIL);
 
-        // For forward merges, tell the VM what images need to have their
-        // parent UUID updated. This cannot be done in VBoxSVC, as opening
-        // the required parent images is not safe while the VM is running.
-        // For backward merges this will be simply an array of size 0.
-        com::SafeIfaceArray<IMedium> childrenToReparent(aChildrenToReparent);
-
         ComPtr<IInternalSessionControl> directControl;
         {
@@ -3436,7 +3454,4 @@
         rc = directControl->OnlineMergeMedium(aMediumAttachment,
                                               uSourceIdx, uTargetIdx,
-                                              aSource, aTarget,
-                                              fMergeForward, aParentForTarget,
-                                              ComSafeArrayAsInParam(childrenToReparent),
                                               aProgress);
         if (FAILED(rc))
@@ -3454,5 +3469,5 @@
 
 /**
- * Implementation for IInternalMachineControl::finishOnlineMergeMedium().
+ * Implementation for IInternalMachineControl::FinishOnlineMergeMedium().
  *
  * Gets called after the successful completion of an online merge from
@@ -3463,15 +3478,9 @@
  * can continue with the updated state of the medium chain.
  */
-STDMETHODIMP SessionMachine::FinishOnlineMergeMedium(IMediumAttachment *aMediumAttachment,
-                                                     IMedium *aSource,
-                                                     IMedium *aTarget,
-                                                     BOOL aMergeForward,
-                                                     IMedium *aParentForTarget,
-                                                     ComSafeArrayIn(IMedium *, aChildrenToReparent))
+STDMETHODIMP SessionMachine::FinishOnlineMergeMedium()
 {
     HRESULT rc = S_OK;
-    ComObjPtr<Medium> pSource(static_cast<Medium *>(aSource));
-    ComObjPtr<Medium> pTarget(static_cast<Medium *>(aTarget));
-    ComObjPtr<Medium> pParentForTarget(static_cast<Medium *>(aParentForTarget));
+    MediumDeleteRec *pDeleteRec = (MediumDeleteRec *)mConsoleTaskData.mDeleteSnapshotInfo;
+    AssertReturn(pDeleteRec, E_FAIL);
     bool fSourceHasChildren = false;
 
@@ -3486,26 +3495,26 @@
     ComObjPtr<Medium> targetChild;
 
-    if (aMergeForward)
+    if (pDeleteRec->mfMergeForward)
     {
         // first, unregister the target since it may become a base
         // hard disk which needs re-registration
-        rc = mParent->unregisterMedium(pTarget);
+        rc = mParent->unregisterMedium(pDeleteRec->mpTarget);
         AssertComRC(rc);
 
         // then, reparent it and disconnect the deleted branch at
         // both ends (chain->parent() is source's parent)
-        pTarget->deparent();
-        pTarget->setParent(pParentForTarget);
-        if (pParentForTarget)
-            pSource->deparent();
+        pDeleteRec->mpTarget->deparent();
+        pDeleteRec->mpTarget->setParent(pDeleteRec->mpParentForTarget);
+        if (pDeleteRec->mpParentForTarget)
+            pDeleteRec->mpSource->deparent();
 
         // then, register again
-        rc = mParent->registerMedium(pTarget, &pTarget, DeviceType_HardDisk);
+        rc = mParent->registerMedium(pDeleteRec->mpTarget, &pDeleteRec->mpTarget, DeviceType_HardDisk);
         AssertComRC(rc);
     }
     else
     {
-        Assert(pTarget->getChildren().size() == 1);
-        targetChild = pTarget->getChildren().front();
+        Assert(pDeleteRec->mpTarget->getChildren().size() == 1);
+        targetChild = pDeleteRec->mpTarget->getChildren().front();
 
         // disconnect the deleted branch at the elder end
@@ -3514,6 +3523,5 @@
         // Update parent UUIDs of the source's children, reparent them and
         // disconnect the deleted branch at the younger end
-        com::SafeIfaceArray<IMedium> childrenToReparent(ComSafeArrayInArg(aChildrenToReparent));
-        if (childrenToReparent.size() > 0)
+        if (pDeleteRec->mpChildrenToReparent && !pDeleteRec->mpChildrenToReparent->IsEmpty())
         {
             fSourceHasChildren = true;
@@ -3523,24 +3531,25 @@
             // we must continue. The worst possible result is that the images
             // need manual fixing via VBoxManage to adjust the parent UUID.
-            MediaList toReparent;
-            for (size_t i = 0; i < childrenToReparent.size(); i++)
+            treeLock.release();
+            pDeleteRec->mpTarget->fixParentUuidOfChildren(pDeleteRec->mpChildrenToReparent);
+            // The childen are still write locked, unlock them now and don't
+            // rely on the destructor doing it very late.
+            pDeleteRec->mpChildrenToReparent->Unlock();
+            treeLock.acquire();
+
+            // obey {parent,child} lock order
+            AutoWriteLock sourceLock(pDeleteRec->mpSource COMMA_LOCKVAL_SRC_POS);
+
+            MediumLockList::Base::iterator childrenBegin = pDeleteRec->mpChildrenToReparent->GetBegin();
+            MediumLockList::Base::iterator childrenEnd = pDeleteRec->mpChildrenToReparent->GetEnd();
+            for (MediumLockList::Base::iterator it = childrenBegin;
+                 it != childrenEnd;
+                 ++it)
             {
-                Medium *pMedium = static_cast<Medium *>(childrenToReparent[i]);
-                toReparent.push_back(pMedium);
-            }
-            treeLock.release();
-            pTarget->fixParentUuidOfChildren(toReparent);
-            treeLock.acquire();
-
-            // obey {parent,child} lock order
-            AutoWriteLock sourceLock(pSource COMMA_LOCKVAL_SRC_POS);
-
-            for (size_t i = 0; i < childrenToReparent.size(); i++)
-            {
-                Medium *pMedium = static_cast<Medium *>(childrenToReparent[i]);
+                Medium *pMedium = it->GetMedium();
                 AutoWriteLock childLock(pMedium COMMA_LOCKVAL_SRC_POS);
 
                 pMedium->deparent();  // removes pMedium from source
-                pMedium->setParent(pTarget);
+                pMedium->setParent(pDeleteRec->mpTarget);
             }
         }
@@ -3549,7 +3558,6 @@
     /* unregister and uninitialize all hard disks removed by the merge */
     MediumLockList *pMediumLockList = NULL;
-    MediumAttachment *pMediumAttachment = static_cast<MediumAttachment *>(aMediumAttachment);
-    rc = mData->mSession.mLockedMedia.Get(pMediumAttachment, pMediumLockList);
-    const ComObjPtr<Medium> &pLast = aMergeForward ? pTarget : pSource;
+    rc = mData->mSession.mLockedMedia.Get(pDeleteRec->mpOnlineMediumAttachment, pMediumLockList);
+    const ComObjPtr<Medium> &pLast = pDeleteRec->mfMergeForward ? pDeleteRec->mpTarget : pDeleteRec->mpSource;
     AssertReturn(SUCCEEDED(rc) && pMediumLockList, E_FAIL);
     MediumLockList::Base::iterator lockListBegin =
@@ -3567,5 +3575,5 @@
 
         /* The target and all images not merged (readonly) are skipped */
-        if (   pMedium == pTarget
+        if (   pMedium == pDeleteRec->mpTarget
             || pMedium->getState() == MediumState_LockedRead)
         {
@@ -3589,8 +3597,8 @@
              * and therefore we cannot uninit() it (it's therefore
              * the caller's responsibility) */
-            if (pMedium == aSource)
+            if (pMedium == pDeleteRec->mpSource)
             {
-                Assert(pSource->getChildren().size() == 0);
-                Assert(pSource->getFirstMachineBackrefId() == NULL);
+                Assert(pDeleteRec->mpSource->getChildren().size() == 0);
+                Assert(pDeleteRec->mpSource->getFirstMachineBackrefId() == NULL);
             }
 
@@ -3628,8 +3636,8 @@
      * attachment, as the previously associated one (source) is now deleted.
      * Without the immediate update the VM could not continue running. */
-    if (!aMergeForward && !fSourceHasChildren)
-    {
-        AutoWriteLock attLock(pMediumAttachment COMMA_LOCKVAL_SRC_POS);
-        pMediumAttachment->updateMedium(pTarget);
+    if (!pDeleteRec->mfMergeForward && !fSourceHasChildren)
+    {
+        AutoWriteLock attLock(pDeleteRec->mpOnlineMediumAttachment COMMA_LOCKVAL_SRC_POS);
+        pDeleteRec->mpOnlineMediumAttachment->updateMedium(pDeleteRec->mpTarget);
     }
 
