VirtualBox

Changeset 36981 in vbox for trunk/src/VBox


Ignore:
Timestamp:
May 6, 2011 1:07:10 PM (14 years ago)
Author:
vboxsync
Message:

Main/MediumImpl: Fix memory leaks in some medium operation error cases. Clean up error handling, in particular with saving registries. Return all relevant error messages, i.e. both the failing operation and the reason for the failure to save the registry if applicable. Update the comments to match the current method names.

File:
1 edited

Legend:

Unmodified
Added
Removed

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

    r36975 r36981  
    239239    PVDINTERFACE mVDOperationIfaces;
    240240
    241     // Whether the caller needs to call VirtualBox::saveSettings() after
     241    // Whether the caller needs to call VirtualBox::saveRegistries() after
    242242    // the task function returns. Only used in synchronous (wait) mode;
    243243    // otherwise the task will save the settings itself.
    … …  
    13121312    m->strDescription = aDescription;
    13131313
    1314 /// @todo generate uuid (similarly to host network interface uuid) from location and device type
    1315 
    13161314    autoInitSpan.setSucceeded();
    13171315    return S_OK;
    … …  
    22572255
    22582256    GuidList llRegistriesThatNeedSaving;
    2259     HRESULT rc = close(&llRegistriesThatNeedSaving, autoCaller);
    2260 
    2261     pVirtualBox->saveRegistries(llRegistriesThatNeedSaving);
    2262 
    2263     return rc;
     2257    MultiResult mrc = close(&llRegistriesThatNeedSaving, autoCaller);
     2258    /* Must save the registries, since an entry was most likely removed. */
     2259    mrc = pVirtualBox->saveRegistries(llRegistriesThatNeedSaving);
     2260
     2261    return mrc;
    22642262}
    22652263
    … …  
    24762474
    24772475    GuidList llRegistriesThatNeedSaving;
    2478     HRESULT rc = deleteStorage(&pProgress,
    2479                                false /* aWait */,
    2480                                &llRegistriesThatNeedSaving);
    2481     m->pVirtualBox->saveRegistries(llRegistriesThatNeedSaving);
    2482 
    2483     if (SUCCEEDED(rc))
     2476    MultiResult mrc = deleteStorage(&pProgress,
     2477                                    false /* aWait */,
     2478                                    &llRegistriesThatNeedSaving);
     2479    /* Must save the registries in any case, since an entry was removed. */
     2480    mrc = m->pVirtualBox->saveRegistries(llRegistriesThatNeedSaving);
     2481
     2482    if (SUCCEEDED(mrc))
    24842483        pProgress.queryInterfaceTo(aProgress);
    24852484
    2486     return rc;
     2485    return mrc;
    24872486}
    24882487
    … …  
    37923791 *                          creating an asynchronous thread.
    37933792 * @param pllRegistriesThatNeedSaving Optional pointer to a list of UUIDs that will receive the registry IDs that need saving.
    3794  *                          This only works in "wait" mode; otherwise saveSettings is called automatically by the thread that
     3793 *                          This only works in "wait" mode; otherwise saveRegistries is called automatically by the thread that
    37953794 *                          was created, and this parameter is ignored.
    37963795 *
    … …  
    39293928/**
    39303929 * Implementation for the public Medium::Close() with the exception of calling
    3931  * VirtualBox::saveSettings(), in case someone wants to call this for several
     3930 * VirtualBox::saveRegistries(), in case someone wants to call this for several
    39323931 * media.
    39333932 *
    … …  
    40134012 *                      an asynchronous thread.
    40144013 * @param pfNeedsGlobalSaveSettings Optional pointer to a bool that must have been initialized to false and that will be set to true
    4015  *                by this function if the caller should invoke VirtualBox::saveSettings() because the global settings have changed.
    4016  *                This only works in "wait" mode; otherwise saveSettings gets called automatically by the thread that was created,
     4014 *                by this function if the caller should invoke VirtualBox::saveRegistries() because the global settings have changed.
     4015 *                This only works in "wait" mode; otherwise saveRegistries gets called automatically by the thread that was created,
    40174016 *                and this parameter is ignored.
    40184017 *
    … …  
    46314630 *                      an asynchronous thread.
    46324631 * @param pfNeedsGlobalSaveSettings Optional pointer to a bool that must have been initialized to false and that will be set to true
    4633  *                by this function if the caller should invoke VirtualBox::saveSettings() because the global settings have changed.
    4634  *                This only works in "wait" mode; otherwise saveSettings gets called automatically by the thread that was created,
     4632 *                by this function if the caller should invoke VirtualBox::saveRegistries() because the global settings have changed.
     4633 *                This only works in "wait" mode; otherwise saveRegistries gets called automatically by the thread that was created,
    46354634 *                and this parameter is ignored.
    46364635 *
    … …  
    48454844        catch (int aVRC)
    48464845        {
    4847             throw setError(E_FAIL,
    4848                             tr("Could not update medium UUID references to parent '%s' (%s)"),
    4849                             m->strLocationFull.c_str(),
    4850                             vdError(aVRC).c_str());
     4846            rc = setError(E_FAIL,
     4847                          tr("Could not update medium UUID references to parent '%s' (%s)"),
     4848                          m->strLocationFull.c_str(),
     4849                          vdError(aVRC).c_str());
    48514850        }
    48524851
    … …  
    62746273HRESULT Medium::taskCreateDiffHandler(Medium::CreateDiffTask &task)
    62756274{
    6276     HRESULT rc = S_OK;
     6275    HRESULT rcTmp = S_OK;
    62776276
    62786277    const ComObjPtr<Medium> &pTarget = task.mTarget;
    … …  
    63576356            if (capabilities & VD_CAP_FILE)
    63586357            {
    6359                 rc = VirtualBox::ensureFilePathExists(targetLocation);
     6358                HRESULT rc = VirtualBox::ensureFilePathExists(targetLocation);
    63606359                if (FAILED(rc))
    63616360                    throw rc;
    … …  
    63846383                variant = (MediumVariant_T)uImageFlags;
    63856384        }
    6386         catch (HRESULT aRC) { rc = aRC; }
     6385        catch (HRESULT aRC) { rcTmp = aRC; }
    63876386
    63886387        VDDestroy(hdd);
    63896388    }
    6390     catch (HRESULT aRC) { rc = aRC; }
    6391 
    6392     if (SUCCEEDED(rc))
     6389    catch (HRESULT aRC) { rcTmp = aRC; }
     6390
     6391    MultiResult mrc(rcTmp);
     6392
     6393    if (SUCCEEDED(mrc))
    63936394    {
    63946395        AutoWriteLock treeLock(m->pVirtualBox->getMediaTreeLockHandle() COMMA_LOCKVAL_SRC_POS);
    … …  
    64086409         * Created state only on success (leaving an orphan file is
    64096410         * better than breaking media registry consistency) */
    6410         rc = m->pVirtualBox->registerHardDisk(pTarget, &llRegistriesThatNeedSaving);
    6411 
    6412         if (FAILED(rc))
     6411        mrc = m->pVirtualBox->registerHardDisk(pTarget, &llRegistriesThatNeedSaving);
     6412
     6413        if (FAILED(mrc))
    64136414            /* break the parent association on failure to register */
    64146415            deparent();
    … …  
    64176418    AutoMultiWriteLock2 mediaLock(this, pTarget COMMA_LOCKVAL_SRC_POS);
    64186419
    6419     if (SUCCEEDED(rc))
     6420    if (SUCCEEDED(mrc))
    64206421    {
    64216422        pTarget->m->state = MediumState_Created;
    … …  
    64446445    {
    64456446        mediaLock.release();
    6446         m->pVirtualBox->saveRegistries(llRegistriesThatNeedSaving);
     6447        mrc = m->pVirtualBox->saveRegistries(llRegistriesThatNeedSaving);
    64476448    }
    64486449    else
    … …  
    64546455     * unlock the medium. */
    64556456
    6456     return rc;
     6457    return mrc;
    64576458}
    64586459
    … …  
    64716472HRESULT Medium::taskMergeHandler(Medium::MergeTask &task)
    64726473{
    6473     HRESULT rc = S_OK;
     6474    HRESULT rcTmp = S_OK;
    64746475
    64756476    const ComObjPtr<Medium> &pTarget = task.mTarget;
    … …  
    65986599            }
    65996600        }
    6600         catch (HRESULT aRC) { rc = aRC; }
     6601        catch (HRESULT aRC) { rcTmp = aRC; }
    66016602        catch (int aVRC)
    66026603        {
    6603             throw setError(VBOX_E_FILE_ERROR,
    6604                             tr("Could not merge the medium '%s' to '%s'%s"),
    6605                             m->strLocationFull.c_str(),
    6606                             pTarget->m->strLocationFull.c_str(),
    6607                             vdError(aVRC).c_str());
     6604            rcTmp = setError(VBOX_E_FILE_ERROR,
     6605                             tr("Could not merge the medium '%s' to '%s'%s"),
     6606                             m->strLocationFull.c_str(),
     6607                             pTarget->m->strLocationFull.c_str(),
     6608                             vdError(aVRC).c_str());
    66086609        }
    66096610
    66106611        VDDestroy(hdd);
    66116612    }
    6612     catch (HRESULT aRC) { rc = aRC; }
    6613 
     6613    catch (HRESULT aRC) { rcTmp = aRC; }
     6614
     6615    ErrorInfoKeeper eik;
     6616    MultiResult mrc(rcTmp);
    66146617    HRESULT rc2;
    66156618
    6616     if (SUCCEEDED(rc))
     6619    if (SUCCEEDED(mrc))
    66176620    {
    66186621        /* all media but the target were successfully deleted by
    … …  
    67316734        GuidList llRegistriesThatNeedSaving;
    67326735        addToRegistryIDList(llRegistriesThatNeedSaving);
    6733         rc = m->pVirtualBox->saveRegistries(llRegistriesThatNeedSaving);
     6736        /* collect multiple errors */
     6737        eik.restore();
     6738        mrc = m->pVirtualBox->saveRegistries(llRegistriesThatNeedSaving);
     6739        eik.fetch();
    67346740    }
    67356741    else
    … …  
    67386744            pTarget->addToRegistryIDList(*task.m_pllRegistriesThatNeedSaving);
    67396745
    6740     if (FAILED(rc))
     6746    if (FAILED(mrc))
    67416747    {
    67426748        /* Here we come if either VDMerge() failed (in which case we
    67436749         * assume that it tried to do everything to make a further
    67446750         * retry possible -- e.g. not deleted intermediate media
    6745          * and so on) or VirtualBox::saveSettings() failed (where we
     6751         * and so on) or VirtualBox::saveRegistries() failed (where we
    67466752         * should have the original tree but with intermediate storage
    67476753         * units deleted by VDMerge()). We have to only restore states
    … …  
    67576763    }
    67586764
    6759     return rc;
     6765    return mrc;
    67606766}
    67616767
    … …  
    67716777HRESULT Medium::taskCloneHandler(Medium::CloneTask &task)
    67726778{
    6773     HRESULT rc = S_OK;
     6779    HRESULT rcTmp = S_OK;
    67746780
    67756781    const ComObjPtr<Medium> &pTarget = task.mTarget;
    … …  
    68536859            if (capabilities & VD_CAP_FILE)
    68546860            {
    6855                 rc = VirtualBox::ensureFilePathExists(targetLocation);
     6861                HRESULT rc = VirtualBox::ensureFilePathExists(targetLocation);
    68566862                if (FAILED(rc))
    68576863                    throw rc;
    … …  
    69326938                    variant = (MediumVariant_T)uImageFlags;
    69336939            }
    6934             catch (HRESULT aRC) { rc = aRC; }
     6940            catch (HRESULT aRC) { rcTmp = aRC; }
    69356941
    69366942            VDDestroy(targetHdd);
    69376943        }
    6938         catch (HRESULT aRC) { rc = aRC; }
     6944        catch (HRESULT aRC) { rcTmp = aRC; }
    69396945
    69406946        VDDestroy(hdd);
    69416947    }
    6942     catch (HRESULT aRC) { rc = aRC; }
     6948    catch (HRESULT aRC) { rcTmp = aRC; }
     6949
     6950    ErrorInfoKeeper eik;
     6951    MultiResult mrc(rcTmp);
    69436952
    69446953    /* Only do the parent changes for newly created media. */
    6945     if (SUCCEEDED(rc) && fCreatingTarget)
     6954    if (SUCCEEDED(mrc) && fCreatingTarget)
    69466955    {
    69476956        /* we set mParent & children() */
    … …  
    69606969             * Created state only on success (leaving an orphan file is
    69616970             * better than breaking media registry consistency) */
    6962             rc = pParent->m->pVirtualBox->registerHardDisk(pTarget, NULL /* pllRegistriesThatNeedSaving */);
    6963 
    6964             if (FAILED(rc))
     6971            eik.restore();
     6972            mrc = pParent->m->pVirtualBox->registerHardDisk(pTarget, NULL /* pllRegistriesThatNeedSaving */);
     6973            eik.fetch();
     6974
     6975            if (FAILED(mrc))
    69656976                /* break parent association on failure to register */
    69666977                pTarget->deparent();     // removes target from parent
    … …  
    69696980        {
    69706981            /* just register  */
    6971             rc = m->pVirtualBox->registerHardDisk(pTarget, NULL /* pllRegistriesThatNeedSaving */);
     6982            eik.restore();
     6983            mrc = m->pVirtualBox->registerHardDisk(pTarget, NULL /* pllRegistriesThatNeedSaving */);
     6984            eik.fetch();
    69726985        }
    69736986    }
    … …  
    69776990        AutoWriteLock mLock(pTarget COMMA_LOCKVAL_SRC_POS);
    69786991
    6979         if (SUCCEEDED(rc))
     6992        if (SUCCEEDED(mrc))
    69806993        {
    69816994            pTarget->m->state = MediumState_Created;
    … …  
    70017014        GuidList llRegistriesThatNeedSaving;
    70027015        addToRegistryIDList(llRegistriesThatNeedSaving);
    7003         HRESULT rc1 = m->pVirtualBox->saveRegistries(llRegistriesThatNeedSaving);
     7016        /* collect multiple errors */
     7017        eik.restore();
     7018        mrc = m->pVirtualBox->saveRegistries(llRegistriesThatNeedSaving);
     7019        eik.fetch();
    70047020    }
    70057021
    … …  
    70127028    task.mpSourceMediumLockList->Clear();
    70137029
    7014     return rc;
     7030    return mrc;
    70157031}
    70167032
    … …  
    75537569HRESULT Medium::taskImportHandler(Medium::ImportTask &task)
    75547570{
    7555     HRESULT rc = S_OK;
     7571    HRESULT rcTmp = S_OK;
    75567572
    75577573    const ComObjPtr<Medium> &pParent = task.mParent;
    … …  
    76177633            if (capabilities & VD_CAP_FILE)
    76187634            {
    7619                 rc = VirtualBox::ensureFilePathExists(targetLocation);
     7635                HRESULT rc = VirtualBox::ensureFilePathExists(targetLocation);
    76207636                if (FAILED(rc))
    76217637                    throw rc;
    … …  
    76967712                    variant = (MediumVariant_T)uImageFlags;
    76977713            }
    7698             catch (HRESULT aRC) { rc = aRC; }
     7714            catch (HRESULT aRC) { rcTmp = aRC; }
    76997715
    77007716            VDDestroy(targetHdd);
    77017717        }
    7702         catch (HRESULT aRC) { rc = aRC; }
     7718        catch (HRESULT aRC) { rcTmp = aRC; }
    77037719
    77047720        VDDestroy(hdd);
    77057721    }
    7706     catch (HRESULT aRC) { rc = aRC; }
     7722    catch (HRESULT aRC) { rcTmp = aRC; }
     7723
     7724    ErrorInfoKeeper eik;
     7725    MultiResult mrc(rcTmp);
    77077726
    77087727    /* Only do the parent changes for newly created media. */
    7709     if (SUCCEEDED(rc) && fCreatingTarget)
     7728    if (SUCCEEDED(mrc) && fCreatingTarget)
    77107729    {
    77117730        /* we set mParent & children() */
    … …  
    77247743             * Created state only on success (leaving an orphan file is
    77257744             * better than breaking media registry consistency) */
    7726             rc = pParent->m->pVirtualBox->registerHardDisk(this, NULL /* llRegistriesThatNeedSaving */);
    7727 
    7728             if (FAILED(rc))
     7745            eik.restore();
     7746            mrc = pParent->m->pVirtualBox->registerHardDisk(this, NULL /* llRegistriesThatNeedSaving */);
     7747            eik.fetch();
     7748
     7749            if (FAILED(mrc))
    77297750                /* break parent association on failure to register */
    77307751                this->deparent();     // removes target from parent
    … …  
    77337754        {
    77347755            /* just register  */
    7735             rc = m->pVirtualBox->registerHardDisk(this, NULL /* pllRegistriesThatNeedSaving */);
     7756            eik.restore();
     7757            mrc = m->pVirtualBox->registerHardDisk(this, NULL /* pllRegistriesThatNeedSaving */);
     7758            eik.fetch();
    77367759        }
    77377760    }
    … …  
    77417764        AutoWriteLock mLock(this COMMA_LOCKVAL_SRC_POS);
    77427765
    7743         if (SUCCEEDED(rc))
     7766        if (SUCCEEDED(mrc))
    77447767        {
    77457768            m->state = MediumState_Created;
    … …  
    77657788        GuidList llRegistriesThatNeedSaving;
    77667789        addToRegistryIDList(llRegistriesThatNeedSaving);
    7767         HRESULT rc1 = m->pVirtualBox->saveRegistries(llRegistriesThatNeedSaving);
     7790        /* collect multiple errors */
     7791        eik.restore();
     7792        mrc = m->pVirtualBox->saveRegistries(llRegistriesThatNeedSaving);
     7793        eik.fetch();
    77687794    }
    77697795
    … …  
    77757801    task.mpTargetMediumLockList->Clear();
    77767802
    7777     return rc;
     7803    return mrc;
    77787804}
    77797805
Note: See TracChangeset for help on using the changeset viewer.

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