Index: /trunk/src/VBox/Main/HostImpl.cpp =================================================================== --- /trunk/src/VBox/Main/HostImpl.cpp (revision 31295) +++ /trunk/src/VBox/Main/HostImpl.cpp (revision 31296) @@ -155,7 +155,10 @@ { Data() + : #ifdef VBOX_WITH_USB - : usbListsLock(LOCKCLASS_USBLIST) + usbListsLock(LOCKCLASS_USBLIST), #endif + fDVDDrivesListBuilt(false), + fFloppyDrivesListBuilt(false) {}; @@ -171,4 +174,10 @@ USBProxyService *pUSBProxyService; #endif /* VBOX_WITH_USB */ + + // list of host drives; lazily created by getDVDDrives() and getFloppyDrives() + MediaList llDVDDrives, + llFloppyDrives; + bool fDVDDrivesListBuilt, + fFloppyDrivesListBuilt; #if defined(RT_OS_LINUX) || defined(RT_OS_FREEBSD) @@ -409,9 +418,9 @@ AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - MediaList list; - HRESULT rc = getDVDDrives(list); + MediaList *pList; + HRESULT rc = getDrives(DeviceType_DVD, true /* fRefresh */, pList); if (SUCCEEDED(rc)) { - SafeIfaceArray array(list); + SafeIfaceArray array(*pList); array.detachTo(ComSafeArrayOutArg(aDrives)); } @@ -435,9 +444,9 @@ AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - MediaList list; - HRESULT rc = getFloppyDrives(list); + MediaList *pList; + HRESULT rc = getDrives(DeviceType_Floppy, true /* fRefresh */, pList); if (SUCCEEDED(rc)) { - SafeIfaceArray collection(list); + SafeIfaceArray collection(*pList); collection.detachTo(ComSafeArrayOutArg(aDrives)); } @@ -1510,5 +1519,174 @@ } -HRESULT Host::getDVDDrives(MediaList &list) +/** + * Sets the given pointer to point to the static list of DVD or floppy + * drives in the Host instance data, depending on the @a mediumType + * parameter. + * + * This builds the list on the first call; it adds or removes host drives + * that may have changed if fRefresh == true. + * + * The caller must hold the Host write lock before calling this. + * To protect the list to which the caller's pointer points, the caller + * must also hold the Host lock. + * + * @param mediumType Must be DeviceType_Floppy or DeviceType_DVD. + * @param fRefresh Whether to refresh the host drives list even if this is not the first call. + * @param pll Caller's pointer which gets set to the static list of host drives. + * @return + */ +HRESULT Host::getDrives(DeviceType_T mediumType, + bool fRefresh, + MediaList *&pll) +{ + HRESULT rc = S_OK; + Assert(isWriteLockOnCurrentThread()); + + MediaList llNew; + MediaList *pllCached; + bool *pfListBuilt = NULL; + + switch (mediumType) + { + case DeviceType_DVD: + if (!m->fDVDDrivesListBuilt || fRefresh) + { + rc = buildDVDDrivesList(llNew); + if (FAILED(rc)) + return rc; + pfListBuilt = &m->fDVDDrivesListBuilt; + } + pllCached = &m->llDVDDrives; + break; + + case DeviceType_Floppy: + if (!m->fFloppyDrivesListBuilt || fRefresh) + { + rc = buildFloppyDrivesList(llNew); + if (FAILED(rc)) + return rc; + pfListBuilt = &m->fFloppyDrivesListBuilt; + } + pllCached = &m->llFloppyDrives; + break; + + default: + return E_INVALIDARG; + } + + if (pfListBuilt) + { + // a list was built in llNew above: + if (!*pfListBuilt) + { + // this was the first call (instance bool is still false): then just copy the whole list and return + *pllCached = llNew; + // and mark the instance data as "built" + *pfListBuilt = true; + } + else + { + // list was built, and this was a subsequent call: then compare the old and the new lists + + // remove drives from the cached list which are no longer present + for (MediaList::iterator itCached = pllCached->begin(); + itCached != pllCached->end(); + ++itCached) + { + Medium *pCached = *itCached; + const Utf8Str strLocationCached = pCached->getLocation(); + bool fFound = false; + for (MediaList::iterator itNew = llNew.begin(); + itNew != llNew.end(); + ++itNew) + { + Medium *pNew = *itNew; + const Utf8Str strLocationNew = pNew->getLocation(); + if (strLocationNew == strLocationCached) + { + fFound = true; + break; + } + } + if (!fFound) + itCached = pllCached->erase(itCached); + } + + // add drives to the cached list that are not on there yet + for (MediaList::iterator itNew = llNew.begin(); + itNew != llNew.end(); + ++itNew) + { + Medium *pNew = *itNew; + const Utf8Str strLocationNew = pNew->getLocation(); + bool fFound = false; + for (MediaList::iterator itCached = pllCached->begin(); + itCached != pllCached->end(); + ++itCached) + { + Medium *pCached = *itCached; + const Utf8Str strLocationCached = pCached->getLocation(); + if (strLocationNew == strLocationCached) + { + fFound = true; + break; + } + } + + if (!fFound) + pllCached->push_back(pNew); + } + } + } + + // return cached list to caller + pll = pllCached; + + return rc; +} + +/** + * Goes through the list of host drives that would be returned by getDrives() + * and looks for a host drive with the given UUID. If found, it sets pMedium + * to that drive; otherwise returns VBOX_E_OBJECT_NOT_FOUND. + * @param mediumType Must be DeviceType_DVD or DeviceType_Floppy. + * @param uuid Medium UUID of host drive to look for. + * @param fRefresh Whether to refresh the host drives list (see getDrives()) + * @param pMedium Medium object, if found… + * @return VBOX_E_OBJECT_NOT_FOUND if not found, or S_OK if found, or errors from getDrives(). + */ +HRESULT Host::findHostDrive(DeviceType_T mediumType, + const Guid &uuid, + bool fRefresh, + ComObjPtr &pMedium) +{ + MediaList *pllMedia; + + AutoWriteLock wlock(this COMMA_LOCKVAL_SRC_POS); + HRESULT rc = getDrives(mediumType, fRefresh, pllMedia); + if (SUCCEEDED(rc)) + { + for (MediaList::iterator it = pllMedia->begin(); + it != pllMedia->end(); + ++it) + { + Medium *pThis = *it; + if (pThis->getId() == uuid) + { + pMedium = pThis; + return S_OK; + } + } + } + + return VBOX_E_OBJECT_NOT_FOUND; +} + +/** + * Called from getDrives() to build the DVD drives list. + * @param pll + * @return + */ +HRESULT Host::buildDVDDrivesList(MediaList &list) { HRESULT rc = S_OK; @@ -1623,10 +1801,9 @@ /** - * Internal implementation for COMGETTER(FloppyDrives) which can be called - * from elsewhere. Caller must hold the Host object write lock! + * Called from getDrives() to build the floppy drives list. * @param list * @return */ -HRESULT Host::getFloppyDrives(MediaList &list) +HRESULT Host::buildFloppyDrivesList(MediaList &list) { HRESULT rc = S_OK; Index: /trunk/src/VBox/Main/MachineImpl.cpp =================================================================== --- /trunk/src/VBox/Main/MachineImpl.cpp (revision 31295) +++ /trunk/src/VBox/Main/MachineImpl.cpp (revision 31296) @@ -3207,69 +3207,8 @@ break; - case DeviceType_DVD: // @todo r=dj eliminate this, replace with findDVDImage - if (!uuid.isEmpty()) - { - /* first search for host drive */ - SafeIfaceArray drivevec; - rc = mParent->host()->COMGETTER(DVDDrives)(ComSafeArrayAsOutParam(drivevec)); - if (SUCCEEDED(rc)) - { - for (size_t i = 0; i < drivevec.size(); ++i) - { - /// @todo eliminate this conversion - ComObjPtr med = (Medium *)drivevec[i]; - if (med->getId() == uuid) - { - medium = med; - break; - } - } - } - - if (medium.isNull()) - { - /* find a DVD image by UUID */ - rc = mParent->findDVDOrFloppyImage(DeviceType_DVD, &uuid, Utf8Str::Empty, true /* aSetError */, &medium); - if (FAILED(rc)) return rc; - } - } - else - { - /* null UUID means null medium, which needs no code */ - } - break; - - case DeviceType_Floppy: // @todo r=dj eliminate this, replace with findFloppyImage - if (!uuid.isEmpty()) - { - /* first search for host drive */ - SafeIfaceArray drivevec; - rc = mParent->host()->COMGETTER(FloppyDrives)(ComSafeArrayAsOutParam(drivevec)); - if (SUCCEEDED(rc)) - { - for (size_t i = 0; i < drivevec.size(); ++i) - { - /// @todo eliminate this conversion - ComObjPtr med = (Medium *)drivevec[i]; - if (med->getId() == uuid) - { - medium = med; - break; - } - } - } - - if (medium.isNull()) - { - /* find a floppy image by UUID */ - rc = mParent->findDVDOrFloppyImage(DeviceType_Floppy, &uuid, Utf8Str::Empty, true /* aSetError */, &medium); - if (FAILED(rc)) return rc; - } - } - else - { - /* null UUID means null medium, which needs no code */ - } - break; + case DeviceType_DVD: + case DeviceType_Floppy: + rc = mParent->findRemoveableMedium(aType, uuid, true /* fRefresh */, medium); + break; default: @@ -3708,30 +3647,5 @@ case DeviceType_DVD: case DeviceType_Floppy: - if (!uuid.isEmpty()) - { - // check if the UUID refers to a host DVD or floppy drive - MediaList llHostDrives; - rc = (mediumType == DeviceType_DVD) - ? mParent->host()->getDVDDrives(llHostDrives) - : mParent->host()->getFloppyDrives(llHostDrives); - if (SUCCEEDED(rc)) - { - for (MediaList::iterator it = llHostDrives.begin(); - it != llHostDrives.end(); - ++it) - { - ComObjPtr &p = *it; - if (uuid == p->getId()) - { - medium = p; - break; - } - } - } - - if (medium.isNull()) - // UUID was not a host drive: - rc = mParent->findDVDOrFloppyImage(mediumType, &uuid, Utf8Str::Empty, true /* aDoSetError */, &medium); - } + rc = mParent->findRemoveableMedium(mediumType, uuid, true /* fRefresh */, medium); if (FAILED(rc)) return rc; break; @@ -7159,55 +7073,9 @@ { case DeviceType_Floppy: - /* find a floppy by UUID */ - if (!dev.uuid.isEmpty()) - rc = mParent->findDVDOrFloppyImage(DeviceType_Floppy, &dev.uuid, Utf8Str::Empty, true /* aDoSetError */, &medium); - /* find a floppy by host device name */ - else if (!dev.strHostDriveSrc.isEmpty()) - { - SafeIfaceArray drivevec; - rc = mParent->host()->COMGETTER(FloppyDrives)(ComSafeArrayAsOutParam(drivevec)); - if (SUCCEEDED(rc)) - { - for (size_t i = 0; i < drivevec.size(); ++i) - { - /// @todo eliminate this conversion - ComObjPtr med = (Medium *)drivevec[i]; - if ( dev.strHostDriveSrc == med->getName() - || dev.strHostDriveSrc == med->getLocation()) - { - medium = med; - break; - } - } - } - } - break; - case DeviceType_DVD: - /* find a DVD by UUID */ - if (!dev.uuid.isEmpty()) - rc = mParent->findDVDOrFloppyImage(DeviceType_DVD, &dev.uuid, Utf8Str::Empty, true /* aDoSetError */, &medium); - /* find a DVD by host device name */ - else if (!dev.strHostDriveSrc.isEmpty()) - { - SafeIfaceArray drivevec; - rc = mParent->host()->COMGETTER(DVDDrives)(ComSafeArrayAsOutParam(drivevec)); - if (SUCCEEDED(rc)) - { - for (size_t i = 0; i < drivevec.size(); ++i) - { - Bstr hostDriveSrc(dev.strHostDriveSrc); - /// @todo eliminate this conversion - ComObjPtr med = (Medium *)drivevec[i]; - if ( hostDriveSrc == med->getName() - || hostDriveSrc == med->getLocation()) - { - medium = med; - break; - } - } - } - } - break; + rc = mParent->findRemoveableMedium(dev.deviceType, dev.uuid, false /* fRefresh */, medium); + if (FAILED(rc)) + return rc; + break; case DeviceType_HardDisk: Index: /trunk/src/VBox/Main/VirtualBoxImpl.cpp =================================================================== --- /trunk/src/VBox/Main/VirtualBoxImpl.cpp (revision 31295) +++ /trunk/src/VBox/Main/VirtualBoxImpl.cpp (revision 31296) @@ -2770,12 +2770,4 @@ * @note Locks the media tree for reading. */ -/** - * - * @param aId - * @param aLocation - * @param aSetError - * @param aImage - * @return - */ HRESULT VirtualBox::findDVDOrFloppyImage(DeviceType_T mediumType, const Guid *aId, @@ -2864,4 +2856,48 @@ m->strSettingsFilePath.raw()); } + + return rc; +} + +/** + * Searches for an IMedium object that represents the given UUID. + * + * If the UUID is empty (indicating an empty drive), this sets pMedium + * to NULL and returns S_OK. + * + * If the UUID refers to a host drive of the given device type, this + * sets pMedium to the object from the list in IHost and returns S_OK. + * + * If the UUID is an image file, this sets pMedium to the object that + * findDVDOrFloppyImage() returned. + * + * If none of the above apply, this returns VBOX_E_OBJECT_NOT_FOUND. + * + * @param mediumType Must be DeviceType_DVD or DeviceType_Floppy. + * @param uuid UUID to search for; must refer to a host drive or an image file or be null. + * @param fRefresh Whether to refresh the list of host drives in IHost (see Host::getDrives()) + * @param pMedium out: IMedium object found. + * @return + */ +HRESULT VirtualBox::findRemoveableMedium(DeviceType_T mediumType, + const Guid &uuid, + bool fRefresh, + ComObjPtr &pMedium) +{ + if (uuid.isEmpty()) + { + // that's easy + pMedium.setNull(); + return S_OK; + } + + // first search for host drive with that UUID + HRESULT rc = m->pHost->findHostDrive(mediumType, + uuid, + fRefresh, + pMedium); + if (rc == VBOX_E_OBJECT_NOT_FOUND) + // then search for an image with that UUID + rc = findDVDOrFloppyImage(mediumType, &uuid, Utf8Str::Empty, true /* aSetError */, &pMedium); return rc; Index: /trunk/src/VBox/Main/include/HostImpl.h =================================================================== --- /trunk/src/VBox/Main/include/HostImpl.h (revision 31295) +++ /trunk/src/VBox/Main/include/HostImpl.h (revision 31296) @@ -108,6 +108,6 @@ HRESULT saveSettings(settings::Host &data); - HRESULT getDVDDrives(MediaList &ll); - HRESULT getFloppyDrives(MediaList &ll); + HRESULT getDrives(DeviceType_T mediumType, bool fRefresh, MediaList *&pll); + HRESULT findHostDrive(DeviceType_T mediumType, const Guid &uuid, bool fRefresh, ComObjPtr &pMedium); #ifdef VBOX_WITH_USB @@ -127,4 +127,7 @@ private: + + HRESULT buildDVDDrivesList(MediaList &list); + HRESULT buildFloppyDrivesList(MediaList &list); #if defined(RT_OS_SOLARIS) && defined(VBOX_USE_LIBHAL) Index: /trunk/src/VBox/Main/include/VirtualBoxImpl.h =================================================================== --- /trunk/src/VBox/Main/include/VirtualBoxImpl.h (revision 31295) +++ /trunk/src/VBox/Main/include/VirtualBoxImpl.h (revision 31296) @@ -228,4 +228,8 @@ bool aSetError, ComObjPtr *aImage = NULL); + HRESULT findRemoveableMedium(DeviceType_T mediumType, + const Guid &uuid, + bool fRefresh, + ComObjPtr &pMedium); HRESULT findGuestOSType(const Bstr &bstrOSType,