VirtualBox

Ignore:
Timestamp:
Sep 5, 2013 9:57:44 AM (12 years ago)
Author:
vboxsync
svn:sync-xref-src-repo-rev:
88714
Message:

Main/Medium: redesign API level medium locking, needed conversions from MediaList to MediumLockLists in several places, forced cleanups elsewhere, too
Main/Token: introduced token objects for controlling the unlocking, will be used as a general concept in the future
Main/Snapshot: snapshot deletion needed significant cleanups as it was still using many shortcuts, directly calling the API to lock media instead of using lock lists. Now much better, and the online snapshot deletion is also a lot cleaner as it no longer passes unnecessary parameters around which are already known in the machine/snapshot code
Main/MediumLock: small improvements, now has a mode which skips locking already locked media, needed by the Snapshot code where we have overlapping lock lists and have to update the original one instead
Main/Console+Session+Machine: follow-up changes for the online snapshot merging parameter passing simplification, plus an unrelated lock order violation fix in Machine which happens only for inaccessible machines
Main/testcase: update correspondingly
doc: update SDK reference

File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/VBox/Main/src-server/SnapshotImpl.cpp

    r47401 r48297  
    21042104
    21052105/**
    2106  * Implementation for IInternalMachineControl::deleteSnapshot().
     2106 * Implementation for IInternalMachineControl::DeleteSnapshot().
    21072107 *
    21082108 * Gets called from Console::DeleteSnapshot(), and that's basically the
    … …  
    22942294                    bool fMergeForward,
    22952295                    const ComObjPtr<Medium> &aParentForTarget,
    2296                     const MediaList &aChildrenToReparent,
     2296                    MediumLockList *aChildrenToReparent,
    22972297                    bool fNeedsOnlineMerge,
    2298                     MediumLockList *aMediumLockList)
     2298                    MediumLockList *aMediumLockList,
     2299                    const ComPtr<IToken> &aHDLockToken)
    22992300        : mpHD(aHd),
    23002301          mpSource(aSource),
    … …  
    23032304          mfMergeForward(fMergeForward),
    23042305          mpParentForTarget(aParentForTarget),
    2305           mChildrenToReparent(aChildrenToReparent),
     2306          mpChildrenToReparent(aChildrenToReparent),
    23062307          mfNeedsOnlineMerge(fNeedsOnlineMerge),
    2307           mpMediumLockList(aMediumLockList)
     2308          mpMediumLockList(aMediumLockList),
     2309          mpHDLockToken(aHDLockToken)
    23082310    {}
    23092311
    … …  
    23142316                    bool fMergeForward,
    23152317                    const ComObjPtr<Medium> &aParentForTarget,
    2316                     const MediaList &aChildrenToReparent,
     2318                    MediumLockList *aChildrenToReparent,
    23172319                    bool fNeedsOnlineMerge,
    23182320                    MediumLockList *aMediumLockList,
     2321                    const ComPtr<IToken> &aHDLockToken,
    23192322                    const Guid &aMachineId,
    23202323                    const Guid &aSnapshotId)
    … …  
    23252328          mfMergeForward(fMergeForward),
    23262329          mpParentForTarget(aParentForTarget),
    2327           mChildrenToReparent(aChildrenToReparent),
     2330          mpChildrenToReparent(aChildrenToReparent),
    23282331          mfNeedsOnlineMerge(fNeedsOnlineMerge),
    23292332          mpMediumLockList(aMediumLockList),
     2333          mpHDLockToken(aHDLockToken),
    23302334          mMachineId(aMachineId),
    23312335          mSnapshotId(aSnapshotId)
    … …  
    23382342    bool mfMergeForward;
    23392343    ComObjPtr<Medium> mpParentForTarget;
    2340     MediaList mChildrenToReparent;
     2344    MediumLockList *mpChildrenToReparent;
    23412345    bool mfNeedsOnlineMerge;
    23422346    MediumLockList *mpMediumLockList;
     2347    /** optional lock token, used only in case mpHD is not merged/deleted */
     2348    ComPtr<IToken> mpHDLockToken;
    23432349    /* these are for reattaching the hard disk in case of a failure: */
    23442350    Guid mMachineId;
    … …  
    24502456            bool fMergeForward = false;
    24512457            ComObjPtr<Medium> pParentForTarget;
    2452             MediaList childrenToReparent;
     2458            MediumLockList *pChildrenToReparent = NULL;
    24532459            bool fNeedsOnlineMerge = false;
    24542460            bool fOnlineMergePossible = aTask.m_fDeleteOnline;
    24552461            MediumLockList *pMediumLockList = NULL;
    24562462            MediumLockList *pVMMALockList = NULL;
     2463            ComPtr<IToken> pHDLockToken;
    24572464            ComObjPtr<MediumAttachment> pOnlineMediumAttachment;
    24582465            if (fOnlineMergePossible)
    … …  
    24872494                                             pVMMALockList, pSource, pTarget,
    24882495                                             fMergeForward, pParentForTarget,
    2489                                              childrenToReparent,
     2496                                             pChildrenToReparent,
    24902497                                             fNeedsOnlineMerge,
    2491                                              pMediumLockList);
     2498                                             pMediumLockList,
     2499                                             pHDLockToken);
    24922500            treeLock.acquire();
    24932501            if (FAILED(rc))
    … …  
    25452553                                                   fMergeForward,
    25462554                                                   pParentForTarget,
    2547                                                    childrenToReparent,
     2555                                                   pChildrenToReparent,
    25482556                                                   fNeedsOnlineMerge,
    25492557                                                   pMediumLockList,
     2558                                                   pHDLockToken,
    25502559                                                   replaceMachineId,
    25512560                                                   replaceSnapshotId));
    … …  
    25562565                                                   fMergeForward,
    25572566                                                   pParentForTarget,
    2558                                                    childrenToReparent,
     2567                                                   pChildrenToReparent,
    25592568                                                   fNeedsOnlineMerge,
    2560                                                    pMediumLockList));
     2569                                                   pMediumLockList,
     2570                                                   pHDLockToken));
    25612571        }
    25622572
    … …  
    27562766                if (it->mfNeedsOnlineMerge)
    27572767                {
     2768                    // Put the medium merge information (MediumDeleteRec) where
     2769                    // SessionMachine::FinishOnlineMergeMedium can get at it.
     2770                    // This callback will arrive while onlineMergeMedium is
     2771                    // still executing, and there can't be two tasks.
     2772                    mConsoleTaskData.mDeleteSnapshotInfo = (void *)&(*it);
    27582773                    // online medium merge, in the direction decided earlier
    27592774                    rc = onlineMergeMedium(it->mpOnlineMediumAttachment,
    … …  
    27622777                                           it->mfMergeForward,
    27632778                                           it->mpParentForTarget,
    2764                                            it->mChildrenToReparent,
     2779                                           it->mpChildrenToReparent,
    27652780                                           it->mpMediumLockList,
    27662781                                           aTask.pProgress,
    27672782                                           &fNeedsSave);
     2783                    mConsoleTaskData.mDeleteSnapshotInfo = NULL;
    27682784                }
    27692785                else
    … …  
    27732789                                               it->mfMergeForward,
    27742790                                               it->mpParentForTarget,
    2775                                                it->mChildrenToReparent,
     2791                                               it->mpChildrenToReparent,
    27762792                                               it->mpMediumLockList,
    27772793                                               &aTask.pProgress,
    … …  
    29122928        {
    29132929            cancelDeleteSnapshotMedium(it->mpHD, it->mpSource,
    2914                                        it->mChildrenToReparent,
     2930                                       it->mpChildrenToReparent,
    29152931                                       it->mfNeedsOnlineMerge,
    2916                                        it->mpMediumLockList, it->mMachineId,
    2917                                        it->mSnapshotId);
     2932                                       it->mpMediumLockList, it->mpHDLockToken,
     2933                                       it->mMachineId, it->mSnapshotId);
    29182934        }
    29192935    }
    … …  
    29662982 * @param aMergeForward Merge direction decision (out).
    29672983 * @param aParentForTarget New parent if target needs to be reparented (out).
    2968  * @param aChildrenToReparent Children which have to be reparented to the
    2969  *                      target (out).
     2984 * @param aChildrenToReparent MediumLockList with children which have to be
     2985 *                      reparented to the target (out).
    29702986 * @param fNeedsOnlineMerge Whether this merge needs to be done online (out).
    29712987 *                      If this is set to @a true then the @a aVMMALockList
    … …  
    29742990 * @param aMediumLockList Where to store the created medium lock list (may
    29752991 *                      return NULL if no real merge is necessary).
     2992 * @param aHDLockToken  Where to store the write lock token for aHD, in case
     2993 *                      it is not merged or deleted (out).
    29762994 *
    29772995 * @note Caller must hold media tree lock for writing. This locks this object
    … …  
    29873005                                                    bool &aMergeForward,
    29883006                                                    ComObjPtr<Medium> &aParentForTarget,
    2989                                                     MediaList &aChildrenToReparent,
     3007                                                    MediumLockList * &aChildrenToReparent,
    29903008                                                    bool &fNeedsOnlineMerge,
    2991                                                     MediumLockList * &aMediumLockList)
     3009                                                    MediumLockList * &aMediumLockList,
     3010                                                    ComPtr<IToken> &aHDLockToken)
    29923011{
    29933012    Assert(!mParent->getMediaTreeLockHandle().isWriteLockOnCurrentThread());
    … …  
    30023021                 && type != MediumType_Readonly, E_FAIL);
    30033022
     3023    aChildrenToReparent = NULL;
    30043024    aMediumLockList = NULL;
    30053025    fNeedsOnlineMerge = false;
    … …  
    30183038             * is completed */
    30193039            alock.release();
    3020             return aHD->LockWrite(NULL);
     3040            return aHD->LockWrite(aHDLockToken.asOutParam());
    30213041        }
    30223042
    … …  
    30493069            childLock.release();
    30503070            alock.release();
    3051             return aHD->LockWrite(NULL);
     3071            return aHD->LockWrite(aHDLockToken.asOutParam());
    30523072        }
    30533073
    … …  
    31463166            {
    31473167                /* we will lock the children of the source for reparenting */
    3148                 for (MediaList::const_iterator it = aChildrenToReparent.begin();
    3149                      it != aChildrenToReparent.end();
    3150                      ++it)
     3168                if (aChildrenToReparent && !aChildrenToReparent->IsEmpty())
    31513169                {
    3152                     ComObjPtr<Medium> pMedium = *it;
    3153                     AutoReadLock mediumLock(pMedium COMMA_LOCKVAL_SRC_POS);
    3154                     if (pMedium->getState() == MediumState_Created)
     3170                    /* Cannot just call aChildrenToReparent->Lock(), as one of
     3171                     * the children is the one under which the current state of
     3172                     * the VM is located, and this means it is already locked
     3173                     * (for reading). Note that no special unlocking is needed,
     3174                     * because cancelMergeTo will unlock everything locked in
     3175                     * its context (using the unlock on destruction), and both
     3176                     * cancelDeleteSnapshotMedium (in case something fails) and
     3177                     * FinishOnlineMergeMedium re-define the read/write lock
     3178                     * state of everything which the VM need, search for the
     3179                     * UpdateLock method calls. */
     3180                    childLock.release();
     3181                    alock.release();
     3182                    rc = aChildrenToReparent->Lock(true /* fSkipOverLockedMedia */);
     3183                    alock.acquire();
     3184                    childLock.acquire();
     3185                    MediumLockList::Base::iterator childrenToReparentBegin = aChildrenToReparent->GetBegin();
     3186                    MediumLockList::Base::iterator childrenToReparentEnd = aChildrenToReparent->GetEnd();
     3187                    for (MediumLockList::Base::iterator it = childrenToReparentBegin;
     3188                         it != childrenToReparentEnd;
     3189                         ++it)
    31553190                    {
    3156                         mediumLock.release();
    3157                         childLock.release();
    3158                         alock.release();
    3159                         rc = pMedium->LockWrite(NULL);
    3160                         alock.acquire();
    3161                         childLock.acquire();
    3162                         mediumLock.acquire();
    3163                         if (FAILED(rc))
    3164                             throw rc;
    3165                     }
    3166                     else
    3167                     {
    3168                         mediumLock.release();
    3169                         childLock.release();
    3170                         alock.release();
    3171                         rc = aVMMALockList->Update(pMedium, true);
    3172                         alock.acquire();
    3173                         childLock.acquire();
    3174                         mediumLock.acquire();
    3175                         if (FAILED(rc))
     3191                        ComObjPtr<Medium> pMedium = it->GetMedium();
     3192                        AutoReadLock mediumLock(pMedium COMMA_LOCKVAL_SRC_POS);
     3193                        if (!it->IsLocked())
    31763194                        {
    31773195                            mediumLock.release();
    31783196                            childLock.release();
    31793197                            alock.release();
    3180                             rc = pMedium->LockWrite(NULL);
     3198                            rc = aVMMALockList->Update(pMedium, true);
    31813199                            alock.acquire();
    31823200                            childLock.acquire();
    … …  
    32653283 * @param fNeedsOnlineMerge Whether this merge needs to be done online.
    32663284 * @param aMediumLockList Medium locks to cancel.
     3285 * @param aHDLockToken  Optional write lock token for aHD.
    32673286 * @param aMachineId    Machine id to attach the medium to.
    32683287 * @param aSnapshotId   Snapshot id to attach the medium to.
    … …  
    32723291void SessionMachine::cancelDeleteSnapshotMedium(const ComObjPtr<Medium> &aHD,
    32733292                                                const ComObjPtr<Medium> &aSource,
    3274                                                 const MediaList &aChildrenToReparent,
     3293                                                MediumLockList *aChildrenToReparent,
    32753294                                                bool fNeedsOnlineMerge,
    32763295                                                MediumLockList *aMediumLockList,
     3296                                                const ComPtr<IToken> &aHDLockToken,
    32773297                                                const Guid &aMachineId,
    32783298                                                const Guid &aSnapshotId)
    … …  
    32863306        if (aHD->getParent().isNull())
    32873307        {
    3288             HRESULT rc = aHD->UnlockWrite(NULL);
    3289             AssertComRC(rc);
     3308            Assert(!aHDLockToken.isNull());
     3309            if (!aHDLockToken.isNull())
     3310            {
     3311                HRESULT rc = aHDLockToken->Abandon();
     3312                AssertComRC(rc);
     3313            }
    32903314        }
    32913315        else
    … …  
    33543378 * @param aMergeForward Merge direction.
    33553379 * @param aParentForTarget New parent if target needs to be reparented.
    3356  * @param aChildrenToReparent Children which have to be reparented to the
    3357  *                      target.
     3380 * @param aChildrenToReparent Medium lock list with children which have to be
     3381 *                      reparented to the target.
    33583382 * @param aMediumLockList Where to store the created medium lock list (may
    33593383 *                      return NULL if no real merge is necessary).
    … …  
    33663390                                          bool fMergeForward,
    33673391                                          const ComObjPtr<Medium> &aParentForTarget,
    3368                                           const MediaList &aChildrenToReparent,
     3392                                          MediumLockList *aChildrenToReparent,
    33693393                                          MediumLockList *aMediumLockList,
    33703394                                          ComObjPtr<Progress> &aProgress,
    … …  
    34153439                       && uTargetIdx != (unsigned)-1, E_FAIL);
    34163440
    3417         // For forward merges, tell the VM what images need to have their
    3418         // parent UUID updated. This cannot be done in VBoxSVC, as opening
    3419         // the required parent images is not safe while the VM is running.
    3420         // For backward merges this will be simply an array of size 0.
    3421         com::SafeIfaceArray<IMedium> childrenToReparent(aChildrenToReparent);
    3422 
    34233441        ComPtr<IInternalSessionControl> directControl;
    34243442        {
    … …  
    34363454        rc = directControl->OnlineMergeMedium(aMediumAttachment,
    34373455                                              uSourceIdx, uTargetIdx,
    3438                                               aSource, aTarget,
    3439                                               fMergeForward, aParentForTarget,
    3440                                               ComSafeArrayAsInParam(childrenToReparent),
    34413456                                              aProgress);
    34423457        if (FAILED(rc))
    … …  
    34543469
    34553470/**
    3456  * Implementation for IInternalMachineControl::finishOnlineMergeMedium().
     3471 * Implementation for IInternalMachineControl::FinishOnlineMergeMedium().
    34573472 *
    34583473 * Gets called after the successful completion of an online merge from
    … …  
    34633478 * can continue with the updated state of the medium chain.
    34643479 */
    3465 STDMETHODIMP SessionMachine::FinishOnlineMergeMedium(IMediumAttachment *aMediumAttachment,
    3466                                                      IMedium *aSource,
    3467                                                      IMedium *aTarget,
    3468                                                      BOOL aMergeForward,
    3469                                                      IMedium *aParentForTarget,
    3470                                                      ComSafeArrayIn(IMedium *, aChildrenToReparent))
     3480STDMETHODIMP SessionMachine::FinishOnlineMergeMedium()
    34713481{
    34723482    HRESULT rc = S_OK;
    3473     ComObjPtr<Medium> pSource(static_cast<Medium *>(aSource));
    3474     ComObjPtr<Medium> pTarget(static_cast<Medium *>(aTarget));
    3475     ComObjPtr<Medium> pParentForTarget(static_cast<Medium *>(aParentForTarget));
     3483    MediumDeleteRec *pDeleteRec = (MediumDeleteRec *)mConsoleTaskData.mDeleteSnapshotInfo;
     3484    AssertReturn(pDeleteRec, E_FAIL);
    34763485    bool fSourceHasChildren = false;
    34773486
    … …  
    34863495    ComObjPtr<Medium> targetChild;
    34873496
    3488     if (aMergeForward)
     3497    if (pDeleteRec->mfMergeForward)
    34893498    {
    34903499        // first, unregister the target since it may become a base
    34913500        // hard disk which needs re-registration
    3492         rc = mParent->unregisterMedium(pTarget);
     3501        rc = mParent->unregisterMedium(pDeleteRec->mpTarget);
    34933502        AssertComRC(rc);
    34943503
    34953504        // then, reparent it and disconnect the deleted branch at
    34963505        // both ends (chain->parent() is source's parent)
    3497         pTarget->deparent();
    3498         pTarget->setParent(pParentForTarget);
    3499         if (pParentForTarget)
    3500             pSource->deparent();
     3506        pDeleteRec->mpTarget->deparent();
     3507        pDeleteRec->mpTarget->setParent(pDeleteRec->mpParentForTarget);
     3508        if (pDeleteRec->mpParentForTarget)
     3509            pDeleteRec->mpSource->deparent();
    35013510
    35023511        // then, register again
    3503         rc = mParent->registerMedium(pTarget, &pTarget, DeviceType_HardDisk);
     3512        rc = mParent->registerMedium(pDeleteRec->mpTarget, &pDeleteRec->mpTarget, DeviceType_HardDisk);
    35043513        AssertComRC(rc);
    35053514    }
    35063515    else
    35073516    {
    3508         Assert(pTarget->getChildren().size() == 1);
    3509         targetChild = pTarget->getChildren().front();
     3517        Assert(pDeleteRec->mpTarget->getChildren().size() == 1);
     3518        targetChild = pDeleteRec->mpTarget->getChildren().front();
    35103519
    35113520        // disconnect the deleted branch at the elder end
    … …  
    35143523        // Update parent UUIDs of the source's children, reparent them and
    35153524        // disconnect the deleted branch at the younger end
    3516         com::SafeIfaceArray<IMedium> childrenToReparent(ComSafeArrayInArg(aChildrenToReparent));
    3517         if (childrenToReparent.size() > 0)
     3525        if (pDeleteRec->mpChildrenToReparent && !pDeleteRec->mpChildrenToReparent->IsEmpty())
    35183526        {
    35193527            fSourceHasChildren = true;
    … …  
    35233531            // we must continue. The worst possible result is that the images
    35243532            // need manual fixing via VBoxManage to adjust the parent UUID.
    3525             MediaList toReparent;
    3526             for (size_t i = 0; i < childrenToReparent.size(); i++)
     3533            treeLock.release();
     3534            pDeleteRec->mpTarget->fixParentUuidOfChildren(pDeleteRec->mpChildrenToReparent);
     3535            // The childen are still write locked, unlock them now and don't
     3536            // rely on the destructor doing it very late.
     3537            pDeleteRec->mpChildrenToReparent->Unlock();
     3538            treeLock.acquire();
     3539
     3540            // obey {parent,child} lock order
     3541            AutoWriteLock sourceLock(pDeleteRec->mpSource COMMA_LOCKVAL_SRC_POS);
     3542
     3543            MediumLockList::Base::iterator childrenBegin = pDeleteRec->mpChildrenToReparent->GetBegin();
     3544            MediumLockList::Base::iterator childrenEnd = pDeleteRec->mpChildrenToReparent->GetEnd();
     3545            for (MediumLockList::Base::iterator it = childrenBegin;
     3546                 it != childrenEnd;
     3547                 ++it)
    35273548            {
    3528                 Medium *pMedium = static_cast<Medium *>(childrenToReparent[i]);
    3529                 toReparent.push_back(pMedium);
    3530             }
    3531             treeLock.release();
    3532             pTarget->fixParentUuidOfChildren(toReparent);
    3533             treeLock.acquire();
    3534 
    3535             // obey {parent,child} lock order
    3536             AutoWriteLock sourceLock(pSource COMMA_LOCKVAL_SRC_POS);
    3537 
    3538             for (size_t i = 0; i < childrenToReparent.size(); i++)
    3539             {
    3540                 Medium *pMedium = static_cast<Medium *>(childrenToReparent[i]);
     3549                Medium *pMedium = it->GetMedium();
    35413550                AutoWriteLock childLock(pMedium COMMA_LOCKVAL_SRC_POS);
    35423551
    35433552                pMedium->deparent();  // removes pMedium from source
    3544                 pMedium->setParent(pTarget);
     3553                pMedium->setParent(pDeleteRec->mpTarget);
    35453554            }
    35463555        }
    … …  
    35493558    /* unregister and uninitialize all hard disks removed by the merge */
    35503559    MediumLockList *pMediumLockList = NULL;
    3551     MediumAttachment *pMediumAttachment = static_cast<MediumAttachment *>(aMediumAttachment);
    3552     rc = mData->mSession.mLockedMedia.Get(pMediumAttachment, pMediumLockList);
    3553     const ComObjPtr<Medium> &pLast = aMergeForward ? pTarget : pSource;
     3560    rc = mData->mSession.mLockedMedia.Get(pDeleteRec->mpOnlineMediumAttachment, pMediumLockList);
     3561    const ComObjPtr<Medium> &pLast = pDeleteRec->mfMergeForward ? pDeleteRec->mpTarget : pDeleteRec->mpSource;
    35543562    AssertReturn(SUCCEEDED(rc) && pMediumLockList, E_FAIL);
    35553563    MediumLockList::Base::iterator lockListBegin =
    … …  
    35673575
    35683576        /* The target and all images not merged (readonly) are skipped */
    3569         if (   pMedium == pTarget
     3577        if (   pMedium == pDeleteRec->mpTarget
    35703578            || pMedium->getState() == MediumState_LockedRead)
    35713579        {
    … …  
    35893597             * and therefore we cannot uninit() it (it's therefore
    35903598             * the caller's responsibility) */
    3591             if (pMedium == aSource)
     3599            if (pMedium == pDeleteRec->mpSource)
    35923600            {
    3593                 Assert(pSource->getChildren().size() == 0);
    3594                 Assert(pSource->getFirstMachineBackrefId() == NULL);
     3601                Assert(pDeleteRec->mpSource->getChildren().size() == 0);
     3602                Assert(pDeleteRec->mpSource->getFirstMachineBackrefId() == NULL);
    35953603            }
    35963604
    … …  
    36283636     * attachment, as the previously associated one (source) is now deleted.
    36293637     * Without the immediate update the VM could not continue running. */
    3630     if (!aMergeForward && !fSourceHasChildren)
    3631     {
    3632         AutoWriteLock attLock(pMediumAttachment COMMA_LOCKVAL_SRC_POS);
    3633         pMediumAttachment->updateMedium(pTarget);
     3638    if (!pDeleteRec->mfMergeForward && !fSourceHasChildren)
     3639    {
     3640        AutoWriteLock attLock(pDeleteRec->mpOnlineMediumAttachment COMMA_LOCKVAL_SRC_POS);
     3641        pDeleteRec->mpOnlineMediumAttachment->updateMedium(pDeleteRec->mpTarget);
    36343642    }
    36353643
Note: See TracChangeset for help on using the changeset viewer.

© 2025 Oracle Support Privacy / Do Not Sell My Info Terms of Use Trademark Policy Automated Access Etiquette