Index: /trunk/src/VBox/Main/include/ApplianceImpl.h =================================================================== --- /trunk/src/VBox/Main/include/ApplianceImpl.h (revision 73131) +++ /trunk/src/VBox/Main/include/ApplianceImpl.h (revision 73132) @@ -124,5 +124,7 @@ bool i_isApplianceIdle(); HRESULT i_searchUniqueVMName(Utf8Str& aName) const; - HRESULT i_searchUniqueDiskImageFilePath(const Utf8Str &aMachineFolder, Utf8Str &aName) const; + HRESULT i_searchUniqueImageFilePath(const Utf8Str &aMachineFolder, + DeviceType_T aDeviceType, + Utf8Str &aName) const; HRESULT i_setUpProgress(ComObjPtr &pProgress, const Utf8Str &strDescription, Index: /trunk/src/VBox/Main/src-server/ApplianceImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-server/ApplianceImpl.cpp (revision 73131) +++ /trunk/src/VBox/Main/src-server/ApplianceImpl.cpp (revision 73132) @@ -896,5 +896,4 @@ char *tmpName = RTStrDup(aName.c_str()); int i = 1; - /** @todo Maybe too cost-intensive; try to find a lighter way */ while (mVirtualBox->FindMachine(Bstr(tmpName).raw(), &machine) != VBOX_E_OBJECT_NOT_FOUND) { @@ -909,16 +908,14 @@ } -HRESULT Appliance::i_searchUniqueDiskImageFilePath(const Utf8Str &aMachineFolder, Utf8Str &aName) const -{ - IMedium *harddisk = NULL; +HRESULT Appliance::i_searchUniqueImageFilePath(const Utf8Str &aMachineFolder, DeviceType_T aDeviceType, Utf8Str &aName) const +{ + IMedium *pMedium = NULL; char *tmpName = RTStrDup(aName.c_str()); char *tmpAbsName = RTPathAbsExDup(aMachineFolder.c_str(), tmpName); int i = 1; - /* Check if the file exists or if a file with this path is registered - * already */ - /** @todo Maybe too cost-intensive; try to find a lighter way */ + /* Check if the file exists or if a medium with this path is registered already */ while ( RTPathExists(tmpAbsName) - || mVirtualBox->OpenMedium(Bstr(tmpAbsName).raw(), DeviceType_HardDisk, AccessMode_ReadWrite, - FALSE /* fForceNewUuid */, &harddisk) != VBOX_E_OBJECT_NOT_FOUND) + || mVirtualBox->OpenMedium(Bstr(tmpAbsName).raw(), aDeviceType, AccessMode_ReadWrite, + FALSE /* fForceNewUuid */, &pMedium) != VBOX_E_OBJECT_NOT_FOUND) { RTStrFree(tmpAbsName); @@ -928,5 +925,8 @@ RTPathStripSuffix(tmpFile); const char *pszTmpSuff = RTPathSuffix(aName.c_str()); - RTStrAPrintf(&tmpName, "%s%c%s_%d%s", tmpDir, RTPATH_DELIMITER, tmpFile, i, pszTmpSuff); + if (!strcmp(tmpDir, ".")) + RTStrAPrintf(&tmpName, "%s_%d%s", tmpFile, i, pszTmpSuff); + else + RTStrAPrintf(&tmpName, "%s%c%s_%d%s", tmpDir, RTPATH_DELIMITER, tmpFile, i, pszTmpSuff); tmpAbsName = RTPathAbsExDup(aMachineFolder.c_str(), tmpName); RTStrFree(tmpFile); @@ -1157,5 +1157,5 @@ { ComObjPtr vsdescThis = (*it); - /* One for every hard disk of the Virtual System */ + /* One for every medium of the Virtual System */ std::list avsdeHDs = vsdescThis->i_findByType(VirtualSystemDescriptionType_HardDiskImage); std::list::const_iterator itH; @@ -1550,5 +1550,5 @@ * Internal method; adds a new description item to the member list. * @param aType Type of description for the new item. - * @param strRef Reference item; only used with hard disk controllers. + * @param strRef Reference item; only used with storage controllers. * @param aOvfValue Corresponding original value from OVF. * @param aVBoxValue Initial configuration value (can be overridden by caller with setFinalValues). Index: /trunk/src/VBox/Main/src-server/ApplianceImplImport.cpp =================================================================== --- /trunk/src/VBox/Main/src-server/ApplianceImplImport.cpp (revision 73131) +++ /trunk/src/VBox/Main/src-server/ApplianceImplImport.cpp (revision 73132) @@ -134,5 +134,5 @@ // Change the appliance state so we can safely leave the lock while doing time-consuming - // disk imports; also the below method calls do all kinds of locking which conflicts with + // medium imports; also the below method calls do all kinds of locking which conflicts with // the appliance object lock m->state = Data::ApplianceImporting; @@ -549,10 +549,10 @@ pNewDesc->i_addEntry(VirtualSystemDescriptionType_CDROM, "", "", ""); - /* Hard disk Controller */ + /* Storage Controller */ uint16_t cIDEused = 0; uint16_t cSATAused = 0; NOREF(cSATAused); uint16_t cSCSIused = 0; NOREF(cSCSIused); ovf::ControllersMap::const_iterator hdcIt; - /* Iterate through all hard disk controllers */ + /* Iterate through all storage controllers */ for (hdcIt = vsysThis.mapControllers.begin(); hdcIt != vsysThis.mapControllers.end(); @@ -644,9 +644,9 @@ } - /* Hard disks */ + /* Storage devices (hard disks/DVDs/...) */ if (vsysThis.mapVirtualDisks.size() > 0) { ovf::VirtualDisksMap::const_iterator itVD; - /* Iterate through all hard disks ()*/ + /* Iterate through all storage devices */ for (itVD = vsysThis.mapVirtualDisks.begin(); itVD != vsysThis.mapVirtualDisks.end(); @@ -654,5 +654,5 @@ { const ovf::VirtualDisk &hd = itVD->second; - /* Get the associated disk image */ + /* Get the associated image */ ovf::DiskImage di; std::map::iterator foundDisk; @@ -667,8 +667,7 @@ /* - * Figure out from URI which format the image of disk has. - * URI must have inside section . - * But there aren't strong requirements about correspondence one URI for one disk virtual format. - * So possibly, we aren't able to recognize some URIs. + * Figure out from URI which format the image has. + * There is no strict mapping of image URI to image format. + * It's possible we aren't able to recognize some URIs. */ @@ -690,4 +689,5 @@ Utf8Str strFilename = di.strHref; + DeviceType_T devType = DeviceType_Null; if (vdf.compare("VMDK", Utf8Str::CaseInsensitive) == 0) { @@ -695,4 +695,5 @@ if (!strFilename.length()) strFilename = Utf8StrFmt("%s.vmdk", hd.strDiskId.c_str()); + devType = DeviceType_HardDisk; } else if (vdf.compare("RAW", Utf8Str::CaseInsensitive) == 0) @@ -701,4 +702,5 @@ if (!strFilename.length()) strFilename = Utf8StrFmt("%s.iso", hd.strDiskId.c_str()); + devType = DeviceType_DVD; } else @@ -714,13 +716,13 @@ strFilename.stripSuffix(); - i_searchUniqueDiskImageFilePath(strMachineFolder, strFilename); - - /* find the description for the hard disk controller + i_searchUniqueImageFilePath(strMachineFolder, devType, strFilename); + + /* find the description for the storage controller * that has the same ID as hd.idController */ const VirtualSystemDescriptionEntry *pController; if (!(pController = pNewDesc->i_findControllerFromID(hd.idController))) throw setError(E_FAIL, - tr("Cannot find hard disk controller with OVF instance ID %RI32 " - "to which disk \"%s\" should be attached"), + tr("Cannot find storage controller with OVF instance ID %RI32 " + "to which medium \"%s\" should be attached"), hd.idController, di.strHref.c_str()); @@ -2032,5 +2034,5 @@ /* Change the appliance state so we can safely leave the lock while doing - * time-consuming disk imports; also the below method calls do all kinds of + * time-consuming image imports; also the below method calls do all kinds of * locking which conflicts with the appliance object lock. */ AutoWriteLock writeLock(this COMMA_LOCKVAL_SRC_POS); @@ -2053,5 +2055,5 @@ { /* With _whatever_ error we've had, do a complete roll-back of - * machines and disks we've created */ + * machines and images we've created */ writeLock.release(); ErrorInfoKeeper eik; @@ -2274,5 +2276,5 @@ * @param hdc in: the HardDiskController structure to attach to. * @param ulAddressOnParent in: the AddressOnParent parameter from OVF. - * @param controllerName out: the name of the hard disk controller to attach to (e.g. "IDE"). + * @param controllerName out: the name of the storage controller to attach to (e.g. "IDE"). * @param lControllerPort out: the channel (controller port) of the controller to attach to. * @param lDevice out: the device number to attach to. @@ -2372,5 +2374,5 @@ /** - * Imports one disk image. + * Imports one image. * * This is common code shared between @@ -2383,18 +2385,18 @@ * the caller needs to pass in the ovf::DiskImage structure from ovfreader.cpp. * - * As a result, in both cases, if di.strHref is empty, we create a new disk as per the OVF + * As a result, in both cases, if di.strHref is empty, we create a new image as per the OVF * spec, even though this cannot really happen in the vbox:Machine case since such data * would never have been exported. * - * This advances stack.pProgress by one operation with the disk's weight. - * - * @param di ovfreader.cpp structure describing the disk image from the OVF that is to be imported + * This advances stack.pProgress by one operation with the image's weight. + * + * @param di ovfreader.cpp structure describing the image from the OVF that is to be imported * @param strDstPath Where to create the target image. - * @param pTargetHD out: The newly created target disk. This also gets pushed on stack.llHardDisksCreated for cleanup. + * @param pTargetMedium out: The newly created target medium. This also gets pushed on stack.llHardDisksCreated for cleanup. * @param stack */ void Appliance::i_importOneDiskImage(const ovf::DiskImage &di, const Utf8Str &strDstPath, - ComObjPtr &pTargetHD, + ComObjPtr &pTargetMedium, ImportStack &stack) { @@ -2432,11 +2434,15 @@ /* First of all check if the original (non-absolute) destination path is - * a valid hard disk UUID. If so, the user wants to import the disk into + * a valid medium UUID. If so, the user wants to import the image into * an existing path. This is useful for iSCSI for example. */ + /** @todo r=klaus the code structure after this point is totally wrong, + * full of unnecessary code duplication and other issues. 4.2 still had + * the right structure for importing into existing medium objects, which + * the current code can't possibly handle. */ RTUUID uuid; int vrc = RTUuidFromStr(&uuid, strDstPath.c_str()); if (vrc == VINF_SUCCESS) { - rc = mVirtualBox->i_findHardDiskById(Guid(uuid), true, &pTargetHD); + rc = mVirtualBox->i_findHardDiskById(Guid(uuid), true, &pTargetMedium); if (FAILED(rc)) throw rc; } @@ -2446,5 +2452,5 @@ /* check read file to GZIP compression */ - bool const fGzipped = di.strCompression.compare("gzip",Utf8Str::CaseInsensitive) == 0; + bool const fGzipped = di.strCompression.compare("gzip", Utf8Str::CaseInsensitive) == 0; Utf8Str strDeleteTemp; try @@ -2514,7 +2520,4 @@ } - /* Create an IMedium object. */ - pTargetHD.createObject(); - /*CD/DVD case*/ if (strTrgFormat.compare("RAW", Utf8Str::CaseInsensitive) == 0) @@ -2526,4 +2529,16 @@ else i_importCopyFile(stack, strSrcFilePath, strAbsDstPath, strSourceOVF.c_str()); + + ComPtr pTmp; + rc = mVirtualBox->OpenMedium(Bstr(strAbsDstPath).raw(), + DeviceType_DVD, + AccessMode_ReadWrite, + false, + pTmp.asOutParam()); + if (FAILED(rc)) + throw rc; + + IMedium *iM = pTmp; + pTargetMedium = static_cast(iM); } catch (HRESULT /*arc*/) @@ -2540,16 +2555,12 @@ else/* HDD case*/ { - rc = pTargetHD->init(mVirtualBox, - strTrgFormat, - strAbsDstPath, - Guid::Empty /* media registry: none yet */, - DeviceType_HardDisk); - if (FAILED(rc)) throw rc; - - /* Now create an empty hard disk. */ - rc = mVirtualBox->CreateMedium(Bstr(strTrgFormat).raw(), - Bstr(strAbsDstPath).raw(), - AccessMode_ReadWrite, DeviceType_HardDisk, - ComPtr(pTargetHD).asOutParam()); + /* Create an IMedium object. */ + pTargetMedium.createObject(); + + rc = pTargetMedium->init(mVirtualBox, + strTrgFormat, + strAbsDstPath, + Guid::Empty /* media registry: none yet */, + DeviceType_HardDisk); if (FAILED(rc)) throw rc; @@ -2560,6 +2571,6 @@ mediumVariant.push_back(MediumVariant_Standard); - /* Kick of the creation of a dynamic growing disk image with the given capacity. */ - rc = pTargetHD->CreateBaseStorage(di.iCapacity / _1M, + /* Kick off the creation of a dynamic growing disk image with the given capacity. */ + rc = pTargetMedium->CreateBaseStorage(di.iCapacity / _1M, ComSafeArrayAsInParam(mediumVariant), ComPtr(pProgress).asOutParam()); @@ -2622,10 +2633,10 @@ /* Start the source image cloning operation. */ ComObjPtr nullParent; - rc = pTargetHD->i_importFile(strSrcFilePath.c_str(), - srcFormat, - MediumVariant_Standard, - hVfsIosReadAhead, - nullParent, - pProgress); + rc = pTargetMedium->i_importFile(strSrcFilePath.c_str(), + srcFormat, + MediumVariant_Standard, + hVfsIosReadAhead, + nullParent, + pProgress); RTVfsIoStrmRelease(hVfsIosReadAhead); hVfsIosSrc = NIL_RTVFSIOSTREAM; @@ -2640,5 +2651,5 @@ } - /* Now wait for the background disk operation to complete; this throws + /* Now wait for the background import operation to complete; this throws * HRESULTs on error. */ ComPtr pp(pProgress); @@ -2930,5 +2941,5 @@ } - // IDE Hard disk controller + // Storage controller IDE std::list vsdeHDCIDE = vsdescThis->i_findByType(VirtualSystemDescriptionType_HardDiskControllerIDE); @@ -2963,5 +2974,5 @@ } - /* Hard disk controller SATA */ + /* Storage controller SATA */ std::list vsdeHDCSATA = vsdescThis->i_findByType(VirtualSystemDescriptionType_HardDiskControllerSATA); @@ -2986,5 +2997,5 @@ } - /* Hard disk controller SCSI */ + /* Storage controller SCSI */ std::list vsdeHDCSCSI = vsdescThis->i_findByType(VirtualSystemDescriptionType_HardDiskControllerSCSI); @@ -3021,5 +3032,5 @@ } - /* Hard disk controller SAS */ + /* Storage controller SAS */ std::list vsdeHDCSAS = vsdescThis->i_findByType(VirtualSystemDescriptionType_HardDiskControllerSAS); @@ -3038,5 +3049,5 @@ } - /* Now its time to register the machine before we add any hard disks */ + /* Now its time to register the machine before we add any storage devices */ rc = mVirtualBox->RegisterMachine(pNewMachine); if (FAILED(rc)) throw rc; @@ -3108,5 +3119,5 @@ if (FAILED(rc)) throw rc; - // only now that we're done with all disks, close the session + // only now that we're done with all storage devices, close the session rc = stack.pSession->UnlockMachine(); if (FAILED(rc)) throw rc; @@ -3127,5 +3138,5 @@ } - // create the hard disks & connect them to the appropriate controllers + // create the storage devices & connect them to the appropriate controllers std::list avsdeHDs = vsdescThis->i_findByType(VirtualSystemDescriptionType_HardDiskImage); if (!avsdeHDs.empty()) @@ -3173,6 +3184,6 @@ Log(("diCurrent.strDiskId=%s diCurrent.strHref=%s\n", diCurrent.strDiskId.c_str(), diCurrent.strHref.c_str())); - /* Iterate over all given disk images of the virtual system - * disks description. We need to find the target disk path, + /* Iterate over all given images of the virtual system + * description. We need to find the target image path, * which could be changed by the user. */ VirtualSystemDescriptionEntry *vsdeTargetHD = NULL; @@ -3190,5 +3201,5 @@ if (!vsdeTargetHD) { - /* possible case if a disk image belongs to other virtual system (OVF package with multiple VMs inside) */ + /* possible case if an image belongs to other virtual system (OVF package with multiple VMs inside) */ Log1Warning(("OVA/OVF import: Disk image %s was missed during import of VM %s\n", oit->first.c_str(), vmNameEntry->strOvf.c_str())); @@ -3198,6 +3209,6 @@ } - //diCurrent.strDiskId contains the disk identifier (e.g. "vmdisk1"), which should exist - //in the virtual system's disks map under that ID and also in the global images map + //diCurrent.strDiskId contains the image identifier (e.g. "vmdisk1"), which should exist + //in the virtual system's images map under that ID and also in the global images map ovf::VirtualDisksMap::const_iterator itVDisk = vsysThis.mapVirtualDisks.find(diCurrent.strDiskId); if (itVDisk == vsysThis.mapVirtualDisks.end()) @@ -3216,5 +3227,5 @@ if (h != disksResolvedNames.end()) { - /* Yes, disk name was found, we can skip it*/ + /* Yes, image name was found, we can skip it*/ ++oit; continue; @@ -3227,7 +3238,7 @@ if (RTStrICmp(diCurrent.strHref.c_str(), stack.pszOvaLookAheadName) != 0) { - /* availableImage contains the disk file reference (e.g. "disk1.vmdk"), which should + /* availableImage contains the image file reference (e.g. "disk1.vmdk"), which should * exist in the global images map. - * And find the disk from the OVF's disk list */ + * And find the image from the OVF's disk list */ ovf::DiskImagesMap::const_iterator itDiskImage; for (itDiskImage = stack.mapDisks.begin(); @@ -3244,10 +3255,10 @@ } - /* replace with a new found disk image */ + /* replace with a new found image */ diCurrent = *(&itDiskImage->second); /* - * Again iterate over all given disk images of the virtual system - * disks description using the found disk image + * Again iterate over all given images of the virtual system + * description using the found image */ for (list::const_iterator itHD = avsdeHDs.begin(); @@ -3295,19 +3306,16 @@ } - /* very important to store disk name for the next checks */ + /* very important to store image name for the next checks */ disksResolvedNames.insert(diCurrent.strHref); ////// end of duplicated code. const ovf::VirtualDisk &ovfVdisk = itVDisk->second; - ComObjPtr pTargetHD; - - Utf8Str savedVBoxCurrent = vsdeTargetHD->strVBoxCurrent; - + ComObjPtr pTargetMedium; i_importOneDiskImage(diCurrent, vsdeTargetHD->strVBoxCurrent, - pTargetHD, + pTargetMedium, stack); - // now use the new uuid to attach the disk image to our new machine + // now use the new uuid to attach the medium to our new machine ComPtr sMachine; rc = stack.pSession->COMGETTER(Machine)(sMachine.asOutParam()); @@ -3331,48 +3339,16 @@ vsdeTargetHD->strVBoxCurrent.c_str(), mhda.lControllerPort, mhda.lDevice)); - ComObjPtr mediumFormat; - rc = i_findMediumFormatFromDiskImage(diCurrent, mediumFormat); + DeviceType_T devType = DeviceType_Null; + rc = pTargetMedium->COMGETTER(DeviceType)(&devType); if (FAILED(rc)) throw rc; - Bstr bstrFormatName; - rc = mediumFormat->COMGETTER(Name)(bstrFormatName.asOutParam()); + rc = sMachine->AttachDevice(Bstr(mhda.controllerName).raw(),// name + mhda.lControllerPort, // long controllerPort + mhda.lDevice, // long device + devType, // DeviceType_T type + pTargetMedium); if (FAILED(rc)) throw rc; - - Utf8Str vdf = Utf8Str(bstrFormatName); - - if (vdf.compare("RAW", Utf8Str::CaseInsensitive) == 0) - { - ComPtr dvdImage(pTargetHD); - - rc = mVirtualBox->OpenMedium(Bstr(vsdeTargetHD->strVBoxCurrent).raw(), - DeviceType_DVD, - AccessMode_ReadWrite, - false, - dvdImage.asOutParam()); - - if (FAILED(rc)) - throw rc; - - rc = sMachine->AttachDevice(Bstr(mhda.controllerName).raw(),// name - mhda.lControllerPort, // long controllerPort - mhda.lDevice, // long device - DeviceType_DVD, // DeviceType_T type - dvdImage); - if (FAILED(rc)) - throw rc; - } - else - { - rc = sMachine->AttachDevice(Bstr(mhda.controllerName).raw(),// name - mhda.lControllerPort, // long controllerPort - mhda.lDevice, // long device - DeviceType_HardDisk, // DeviceType_T type - pTargetHD); - - if (FAILED(rc)) - throw rc; - } stack.llHardDiskAttachments.push_back(mhda); @@ -3381,7 +3357,4 @@ if (FAILED(rc)) throw rc; - - /* restore */ - vsdeTargetHD->strVBoxCurrent = savedVBoxCurrent; ++cImportedDisks; @@ -3817,23 +3790,4 @@ settings::StorageController &sc = *sit; - // find the OVF virtual system description entry for this storage controller -/** @todo - * r=bird: What on earh this is switch supposed to do? (I've added the default:break;, so don't - * get confused by it.) Kind of looks like it's supposed to do something error handling related - * in the default case... - */ - switch (sc.storageBus) - { - case StorageBus_SATA: - break; - case StorageBus_SCSI: - break; - case StorageBus_IDE: - break; - case StorageBus_SAS: - break; - default: break; /* Shut up MSC. */ - } - // for each medium attachment to this controller... for (settings::AttachedDevicesList::iterator dit = sc.llAttachedDevices.begin(); @@ -3873,53 +3827,15 @@ * step 3: import disk */ - Utf8Str savedVBoxCurrent = vsdeTargetHD->strVBoxCurrent; - ComObjPtr pTargetHD; - + ComObjPtr pTargetMedium; i_importOneDiskImage(diCurrent, vsdeTargetHD->strVBoxCurrent, - pTargetHD, + pTargetMedium, stack); + // ... and replace the old UUID in the machine config with the one of + // the imported disk that was just created Bstr hdId; - - ComObjPtr mediumFormat; - rc = i_findMediumFormatFromDiskImage(diCurrent, mediumFormat); - if (FAILED(rc)) - throw rc; - - Bstr bstrFormatName; - rc = mediumFormat->COMGETTER(Name)(bstrFormatName.asOutParam()); - if (FAILED(rc)) - throw rc; - - Utf8Str vdf = Utf8Str(bstrFormatName); - - if (vdf.compare("RAW", Utf8Str::CaseInsensitive) == 0) - { - ComPtr dvdImage(pTargetHD); - - rc = mVirtualBox->OpenMedium(Bstr(vsdeTargetHD->strVBoxCurrent).raw(), - DeviceType_DVD, - AccessMode_ReadWrite, - false, - dvdImage.asOutParam()); - - if (FAILED(rc)) throw rc; - - // ... and replace the old UUID in the machine config with the one of - // the imported disk that was just created - rc = dvdImage->COMGETTER(Id)(hdId.asOutParam()); - if (FAILED(rc)) throw rc; - } - else - { - // ... and replace the old UUID in the machine config with the one of - // the imported disk that was just created - rc = pTargetHD->COMGETTER(Id)(hdId.asOutParam()); - if (FAILED(rc)) throw rc; - } - - /* restore */ - vsdeTargetHD->strVBoxCurrent = savedVBoxCurrent; + rc = pTargetMedium->COMGETTER(Id)(hdId.asOutParam()); + if (FAILED(rc)) throw rc; /*