VirtualBox

Changeset 31296 in vbox


Ignore:
Timestamp:
Aug 2, 2010 1:13:14 PM (15 years ago)
Author:
vboxsync
svn:sync-xref-src-repo-rev:
64323
Message:

Main: cleanup host drives management: do not return different IMedium objects every time Host::GetDVDDrives() or HOst::GetFloppyDrives() are called; refresh the internal list only when a public API is called, but not for every single internal use such as loading machine settings file; fix code duplication

Location:
trunk/src/VBox/Main
Files:
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/VBox/Main/HostImpl.cpp

    r30764 r31296  
    155155{
    156156    Data()
     157        :
    157158#ifdef VBOX_WITH_USB
    158         : usbListsLock(LOCKCLASS_USBLIST)
     159          usbListsLock(LOCKCLASS_USBLIST),
    159160#endif
     161          fDVDDrivesListBuilt(false),
     162          fFloppyDrivesListBuilt(false)
    160163    {};
    161164
     
    171174    USBProxyService         *pUSBProxyService;
    172175#endif /* VBOX_WITH_USB */
     176
     177    // list of host drives; lazily created by getDVDDrives() and getFloppyDrives()
     178    MediaList               llDVDDrives,
     179                            llFloppyDrives;
     180    bool                    fDVDDrivesListBuilt,
     181                            fFloppyDrivesListBuilt;
    173182
    174183#if defined(RT_OS_LINUX) || defined(RT_OS_FREEBSD)
     
    409418    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
    410419
    411     MediaList list;
    412     HRESULT rc = getDVDDrives(list);
     420    MediaList *pList;
     421    HRESULT rc = getDrives(DeviceType_DVD, true /* fRefresh */, pList);
    413422    if (SUCCEEDED(rc))
    414423    {
    415         SafeIfaceArray<IMedium> array(list);
     424        SafeIfaceArray<IMedium> array(*pList);
    416425        array.detachTo(ComSafeArrayOutArg(aDrives));
    417426    }
     
    435444    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
    436445
    437     MediaList list;
    438     HRESULT rc = getFloppyDrives(list);
     446    MediaList *pList;
     447    HRESULT rc = getDrives(DeviceType_Floppy, true /* fRefresh */, pList);
    439448    if (SUCCEEDED(rc))
    440449    {
    441         SafeIfaceArray<IMedium> collection(list);
     450        SafeIfaceArray<IMedium> collection(*pList);
    442451        collection.detachTo(ComSafeArrayOutArg(aDrives));
    443452    }
     
    15101519}
    15111520
    1512 HRESULT Host::getDVDDrives(MediaList &list)
     1521/**
     1522 * Sets the given pointer to point to the static list of DVD or floppy
     1523 * drives in the Host instance data, depending on the @a mediumType
     1524 * parameter.
     1525 *
     1526 * This builds the list on the first call; it adds or removes host drives
     1527 * that may have changed if fRefresh == true.
     1528 *
     1529 * The caller must hold the Host write lock before calling this.
     1530 * To protect the list to which the caller's pointer points, the caller
     1531 * must also hold the Host lock.
     1532 *
     1533 * @param mediumType Must be DeviceType_Floppy or DeviceType_DVD.
     1534 * @param fRefresh Whether to refresh the host drives list even if this is not the first call.
     1535 * @param pll Caller's pointer which gets set to the static list of host drives.
     1536 * @return
     1537 */
     1538HRESULT Host::getDrives(DeviceType_T mediumType,
     1539                        bool fRefresh,
     1540                        MediaList *&pll)
     1541{
     1542    HRESULT rc = S_OK;
     1543    Assert(isWriteLockOnCurrentThread());
     1544
     1545    MediaList llNew;
     1546    MediaList *pllCached;
     1547    bool *pfListBuilt = NULL;
     1548
     1549    switch (mediumType)
     1550    {
     1551        case DeviceType_DVD:
     1552            if (!m->fDVDDrivesListBuilt || fRefresh)
     1553            {
     1554                rc = buildDVDDrivesList(llNew);
     1555                if (FAILED(rc))
     1556                    return rc;
     1557                pfListBuilt = &m->fDVDDrivesListBuilt;
     1558            }
     1559            pllCached = &m->llDVDDrives;
     1560        break;
     1561
     1562        case DeviceType_Floppy:
     1563            if (!m->fFloppyDrivesListBuilt || fRefresh)
     1564            {
     1565                rc = buildFloppyDrivesList(llNew);
     1566                if (FAILED(rc))
     1567                    return rc;
     1568                pfListBuilt = &m->fFloppyDrivesListBuilt;
     1569            }
     1570            pllCached = &m->llFloppyDrives;
     1571        break;
     1572
     1573        default:
     1574            return E_INVALIDARG;
     1575    }
     1576
     1577    if (pfListBuilt)
     1578    {
     1579        // a list was built in llNew above:
     1580        if (!*pfListBuilt)
     1581        {
     1582            // this was the first call (instance bool is still false): then just copy the whole list and return
     1583            *pllCached = llNew;
     1584            // and mark the instance data as "built"
     1585            *pfListBuilt = true;
     1586        }
     1587        else
     1588        {
     1589            // list was built, and this was a subsequent call: then compare the old and the new lists
     1590
     1591            // remove drives from the cached list which are no longer present
     1592            for (MediaList::iterator itCached = pllCached->begin();
     1593                 itCached != pllCached->end();
     1594                 ++itCached)
     1595            {
     1596                Medium *pCached = *itCached;
     1597                const Utf8Str strLocationCached = pCached->getLocation();
     1598                bool fFound = false;
     1599                for (MediaList::iterator itNew = llNew.begin();
     1600                     itNew != llNew.end();
     1601                     ++itNew)
     1602                {
     1603                    Medium *pNew = *itNew;
     1604                    const Utf8Str strLocationNew = pNew->getLocation();
     1605                    if (strLocationNew == strLocationCached)
     1606                    {
     1607                        fFound = true;
     1608                        break;
     1609                    }
     1610                }
     1611                if (!fFound)
     1612                    itCached = pllCached->erase(itCached);
     1613            }
     1614
     1615            // add drives to the cached list that are not on there yet
     1616            for (MediaList::iterator itNew = llNew.begin();
     1617                 itNew != llNew.end();
     1618                 ++itNew)
     1619            {
     1620                Medium *pNew = *itNew;
     1621                const Utf8Str strLocationNew = pNew->getLocation();
     1622                bool fFound = false;
     1623                for (MediaList::iterator itCached = pllCached->begin();
     1624                     itCached != pllCached->end();
     1625                     ++itCached)
     1626                {
     1627                    Medium *pCached = *itCached;
     1628                    const Utf8Str strLocationCached = pCached->getLocation();
     1629                    if (strLocationNew == strLocationCached)
     1630                    {
     1631                        fFound = true;
     1632                        break;
     1633                    }
     1634                }
     1635
     1636                if (!fFound)
     1637                    pllCached->push_back(pNew);
     1638            }
     1639        }
     1640    }
     1641
     1642    // return cached list to caller
     1643    pll = pllCached;
     1644
     1645    return rc;
     1646}
     1647
     1648/**
     1649 * Goes through the list of host drives that would be returned by getDrives()
     1650 * and looks for a host drive with the given UUID. If found, it sets pMedium
     1651 * to that drive; otherwise returns VBOX_E_OBJECT_NOT_FOUND.
     1652 * @param mediumType Must be DeviceType_DVD or DeviceType_Floppy.
     1653 * @param uuid Medium UUID of host drive to look for.
     1654 * @param fRefresh Whether to refresh the host drives list (see getDrives())
     1655 * @param pMedium Medium object, if found…
     1656 * @return VBOX_E_OBJECT_NOT_FOUND if not found, or S_OK if found, or errors from getDrives().
     1657 */
     1658HRESULT Host::findHostDrive(DeviceType_T mediumType,
     1659                            const Guid &uuid,
     1660                            bool fRefresh,
     1661                            ComObjPtr<Medium> &pMedium)
     1662{
     1663    MediaList *pllMedia;
     1664
     1665    AutoWriteLock wlock(this COMMA_LOCKVAL_SRC_POS);
     1666    HRESULT rc = getDrives(mediumType, fRefresh, pllMedia);
     1667    if (SUCCEEDED(rc))
     1668    {
     1669        for (MediaList::iterator it = pllMedia->begin();
     1670             it != pllMedia->end();
     1671             ++it)
     1672        {
     1673            Medium *pThis = *it;
     1674            if (pThis->getId() == uuid)
     1675            {
     1676                pMedium = pThis;
     1677                return S_OK;
     1678            }
     1679        }
     1680    }
     1681
     1682    return VBOX_E_OBJECT_NOT_FOUND;
     1683}
     1684
     1685/**
     1686 * Called from getDrives() to build the DVD drives list.
     1687 * @param pll
     1688 * @return
     1689 */
     1690HRESULT Host::buildDVDDrivesList(MediaList &list)
    15131691{
    15141692    HRESULT rc = S_OK;
     
    16231801
    16241802/**
    1625  * Internal implementation for COMGETTER(FloppyDrives) which can be called
    1626  * from elsewhere. Caller must hold the Host object write lock!
     1803 * Called from getDrives() to build the floppy drives list.
    16271804 * @param list
    16281805 * @return
    16291806 */
    1630 HRESULT Host::getFloppyDrives(MediaList &list)
     1807HRESULT Host::buildFloppyDrivesList(MediaList &list)
    16311808{
    16321809    HRESULT rc = S_OK;
  • trunk/src/VBox/Main/MachineImpl.cpp

    r31287 r31296  
    32073207            break;
    32083208
    3209         case DeviceType_DVD: // @todo r=dj eliminate this, replace with findDVDImage
    3210             if (!uuid.isEmpty())
    3211             {
    3212                 /* first search for host drive */
    3213                 SafeIfaceArray<IMedium> drivevec;
    3214                 rc = mParent->host()->COMGETTER(DVDDrives)(ComSafeArrayAsOutParam(drivevec));
    3215                 if (SUCCEEDED(rc))
    3216                 {
    3217                     for (size_t i = 0; i < drivevec.size(); ++i)
    3218                     {
    3219                         /// @todo eliminate this conversion
    3220                         ComObjPtr<Medium> med = (Medium *)drivevec[i];
    3221                         if (med->getId() == uuid)
    3222                         {
    3223                             medium = med;
    3224                             break;
    3225                         }
    3226                     }
    3227                 }
    3228 
    3229                 if (medium.isNull())
    3230                 {
    3231                     /* find a DVD image by UUID */
    3232                     rc = mParent->findDVDOrFloppyImage(DeviceType_DVD, &uuid, Utf8Str::Empty, true /* aSetError */, &medium);
    3233                     if (FAILED(rc)) return rc;
    3234                 }
    3235             }
    3236             else
    3237             {
    3238                 /* null UUID means null medium, which needs no code */
    3239             }
    3240             break;
    3241 
    3242         case DeviceType_Floppy: // @todo r=dj eliminate this, replace with findFloppyImage
    3243             if (!uuid.isEmpty())
    3244             {
    3245                 /* first search for host drive */
    3246                 SafeIfaceArray<IMedium> drivevec;
    3247                 rc = mParent->host()->COMGETTER(FloppyDrives)(ComSafeArrayAsOutParam(drivevec));
    3248                 if (SUCCEEDED(rc))
    3249                 {
    3250                     for (size_t i = 0; i < drivevec.size(); ++i)
    3251                     {
    3252                         /// @todo eliminate this conversion
    3253                         ComObjPtr<Medium> med = (Medium *)drivevec[i];
    3254                         if (med->getId() == uuid)
    3255                         {
    3256                             medium = med;
    3257                             break;
    3258                         }
    3259                     }
    3260                 }
    3261 
    3262                 if (medium.isNull())
    3263                 {
    3264                     /* find a floppy image by UUID */
    3265                     rc = mParent->findDVDOrFloppyImage(DeviceType_Floppy, &uuid, Utf8Str::Empty, true /* aSetError */, &medium);
    3266                     if (FAILED(rc)) return rc;
    3267                 }
    3268             }
    3269             else
    3270             {
    3271                 /* null UUID means null medium, which needs no code */
    3272             }
    3273             break;
     3209        case DeviceType_DVD:
     3210        case DeviceType_Floppy:
     3211            rc = mParent->findRemoveableMedium(aType, uuid, true /* fRefresh */, medium);
     3212        break;
    32743213
    32753214        default:
     
    37083647        case DeviceType_DVD:
    37093648        case DeviceType_Floppy:
    3710             if (!uuid.isEmpty())
    3711             {
    3712                 // check if the UUID refers to a host DVD or floppy drive
    3713                 MediaList llHostDrives;
    3714                 rc = (mediumType == DeviceType_DVD)
    3715                         ? mParent->host()->getDVDDrives(llHostDrives)
    3716                         : mParent->host()->getFloppyDrives(llHostDrives);
    3717                 if (SUCCEEDED(rc))
    3718                 {
    3719                     for (MediaList::iterator it = llHostDrives.begin();
    3720                          it != llHostDrives.end();
    3721                          ++it)
    3722                     {
    3723                         ComObjPtr<Medium> &p = *it;
    3724                         if (uuid == p->getId())
    3725                         {
    3726                             medium = p;
    3727                             break;
    3728                         }
    3729                     }
    3730                 }
    3731 
    3732                 if (medium.isNull())
    3733                     // UUID was not a host drive:
    3734                     rc = mParent->findDVDOrFloppyImage(mediumType, &uuid, Utf8Str::Empty, true /* aDoSetError */, &medium);
    3735             }
     3649            rc = mParent->findRemoveableMedium(mediumType, uuid, true /* fRefresh */, medium);
    37363650            if (FAILED(rc)) return rc;
    37373651        break;
     
    71597073        {
    71607074            case DeviceType_Floppy:
    7161                 /* find a floppy by UUID */
    7162                 if (!dev.uuid.isEmpty())
    7163                     rc = mParent->findDVDOrFloppyImage(DeviceType_Floppy, &dev.uuid, Utf8Str::Empty, true /* aDoSetError */, &medium);
    7164                 /* find a floppy by host device name */
    7165                 else if (!dev.strHostDriveSrc.isEmpty())
    7166                 {
    7167                     SafeIfaceArray<IMedium> drivevec;
    7168                     rc = mParent->host()->COMGETTER(FloppyDrives)(ComSafeArrayAsOutParam(drivevec));
    7169                     if (SUCCEEDED(rc))
    7170                     {
    7171                         for (size_t i = 0; i < drivevec.size(); ++i)
    7172                         {
    7173                             /// @todo eliminate this conversion
    7174                             ComObjPtr<Medium> med = (Medium *)drivevec[i];
    7175                             if (    dev.strHostDriveSrc == med->getName()
    7176                                 ||  dev.strHostDriveSrc == med->getLocation())
    7177                             {
    7178                                 medium = med;
    7179                                 break;
    7180                             }
    7181                         }
    7182                     }
    7183                 }
    7184                 break;
    7185 
    71867075            case DeviceType_DVD:
    7187                 /* find a DVD by UUID */
    7188                 if (!dev.uuid.isEmpty())
    7189                     rc = mParent->findDVDOrFloppyImage(DeviceType_DVD, &dev.uuid, Utf8Str::Empty, true /* aDoSetError */, &medium);
    7190                 /* find a DVD by host device name */
    7191                 else if (!dev.strHostDriveSrc.isEmpty())
    7192                 {
    7193                     SafeIfaceArray<IMedium> drivevec;
    7194                     rc = mParent->host()->COMGETTER(DVDDrives)(ComSafeArrayAsOutParam(drivevec));
    7195                     if (SUCCEEDED(rc))
    7196                     {
    7197                         for (size_t i = 0; i < drivevec.size(); ++i)
    7198                         {
    7199                             Bstr hostDriveSrc(dev.strHostDriveSrc);
    7200                             /// @todo eliminate this conversion
    7201                             ComObjPtr<Medium> med = (Medium *)drivevec[i];
    7202                             if (    hostDriveSrc == med->getName()
    7203                                 ||  hostDriveSrc == med->getLocation())
    7204                             {
    7205                                 medium = med;
    7206                                 break;
    7207                             }
    7208                         }
    7209                     }
    7210                 }
    7211                 break;
     7076                rc = mParent->findRemoveableMedium(dev.deviceType, dev.uuid, false /* fRefresh */, medium);
     7077                if (FAILED(rc))
     7078                    return rc;
     7079            break;
    72127080
    72137081            case DeviceType_HardDisk:
  • trunk/src/VBox/Main/VirtualBoxImpl.cpp

    r31281 r31296  
    27702770 * @note Locks the media tree for reading.
    27712771 */
    2772 /**
    2773  *
    2774  * @param aId
    2775  * @param aLocation
    2776  * @param aSetError
    2777  * @param aImage
    2778  * @return
    2779  */
    27802772HRESULT VirtualBox::findDVDOrFloppyImage(DeviceType_T mediumType,
    27812773                                         const Guid *aId,
     
    28642856                     m->strSettingsFilePath.raw());
    28652857    }
     2858
     2859    return rc;
     2860}
     2861
     2862/**
     2863 * Searches for an IMedium object that represents the given UUID.
     2864 *
     2865 * If the UUID is empty (indicating an empty drive), this sets pMedium
     2866 * to NULL and returns S_OK.
     2867 *
     2868 * If the UUID refers to a host drive of the given device type, this
     2869 * sets pMedium to the object from the list in IHost and returns S_OK.
     2870 *
     2871 * If the UUID is an image file, this sets pMedium to the object that
     2872 * findDVDOrFloppyImage() returned.
     2873 *
     2874 * If none of the above apply, this returns VBOX_E_OBJECT_NOT_FOUND.
     2875 *
     2876 * @param mediumType Must be DeviceType_DVD or DeviceType_Floppy.
     2877 * @param uuid UUID to search for; must refer to a host drive or an image file or be null.
     2878 * @param fRefresh Whether to refresh the list of host drives in IHost (see Host::getDrives())
     2879 * @param pMedium out: IMedium object found.
     2880 * @return
     2881 */
     2882HRESULT VirtualBox::findRemoveableMedium(DeviceType_T mediumType,
     2883                                         const Guid &uuid,
     2884                                         bool fRefresh,
     2885                                         ComObjPtr<Medium> &pMedium)
     2886{
     2887    if (uuid.isEmpty())
     2888    {
     2889        // that's easy
     2890        pMedium.setNull();
     2891        return S_OK;
     2892    }
     2893
     2894    // first search for host drive with that UUID
     2895    HRESULT rc = m->pHost->findHostDrive(mediumType,
     2896                                         uuid,
     2897                                         fRefresh,
     2898                                         pMedium);
     2899    if (rc == VBOX_E_OBJECT_NOT_FOUND)
     2900                // then search for an image with that UUID
     2901        rc = findDVDOrFloppyImage(mediumType, &uuid, Utf8Str::Empty, true /* aSetError */, &pMedium);
    28662902
    28672903    return rc;
  • trunk/src/VBox/Main/include/HostImpl.h

    r30764 r31296  
    108108    HRESULT saveSettings(settings::Host &data);
    109109
    110     HRESULT getDVDDrives(MediaList &ll);
    111     HRESULT getFloppyDrives(MediaList &ll);
     110    HRESULT getDrives(DeviceType_T mediumType, bool fRefresh, MediaList *&pll);
     111    HRESULT findHostDrive(DeviceType_T mediumType, const Guid &uuid, bool fRefresh, ComObjPtr<Medium> &pMedium);
    112112
    113113#ifdef VBOX_WITH_USB
     
    127127
    128128private:
     129
     130    HRESULT buildDVDDrivesList(MediaList &list);
     131    HRESULT buildFloppyDrivesList(MediaList &list);
    129132
    130133#if defined(RT_OS_SOLARIS) && defined(VBOX_USE_LIBHAL)
  • trunk/src/VBox/Main/include/VirtualBoxImpl.h

    r31281 r31296  
    228228                                 bool aSetError,
    229229                                 ComObjPtr<Medium> *aImage = NULL);
     230    HRESULT findRemoveableMedium(DeviceType_T mediumType,
     231                                 const Guid &uuid,
     232                                 bool fRefresh,
     233                                 ComObjPtr<Medium> &pMedium);
    230234
    231235    HRESULT findGuestOSType(const Bstr &bstrOSType,
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