Index: /trunk/include/VBox/com/Guid.h =================================================================== --- /trunk/include/VBox/com/Guid.h (revision 29872) +++ /trunk/include/VBox/com/Guid.h (revision 29873) @@ -141,4 +141,17 @@ } + /** + * Like toString, but encloses the returned string in curly brackets. + * @return + */ + Utf8Str toStringCurly() const + { + char buf[RTUUID_STR_LENGTH + 2] = "{"; + ::RTUuidToStr(&uuid, buf + 1, RTUUID_STR_LENGTH); + buf[sizeof(buf) - 2] = '}'; + buf[sizeof(buf) - 1] = '\0'; + return Utf8Str(buf); + } + Bstr toUtf16() const { Index: /trunk/include/VBox/settings.h =================================================================== --- /trunk/include/VBox/settings.h (revision 29872) +++ /trunk/include/VBox/settings.h (revision 29873) @@ -93,5 +93,4 @@ com::Utf8Str makeString(const RTTIMESPEC &tm); - com::Utf8Str makeString(const com::Guid &guid); void readExtraData(const xml::ElementNode &elmExtraData, @@ -880,5 +879,7 @@ BuildMachineXML_SkipRemovableMedia = 0x02 }; - void buildMachineXML(xml::ElementNode &elmMachine, uint32_t fl); + void buildMachineXML(xml::ElementNode &elmMachine, + uint32_t fl, + std::list *pllElementsWithUuidAttributes); private: @@ -901,5 +902,8 @@ void buildHardwareXML(xml::ElementNode &elmParent, const Hardware &hw, const Storage &strg); void buildNetworkXML(NetworkAttachmentType_T mode, xml::ElementNode &elmParent, const NetworkAdapter &nic); - void buildStorageControllersXML(xml::ElementNode &elmParent, const Storage &st, bool fSkipRemovableMedia); + void buildStorageControllersXML(xml::ElementNode &elmParent, + const Storage &st, + bool fSkipRemovableMedia, + std::list *pllElementsWithUuidAttributes); void buildSnapshotXML(xml::ElementNode &elmParent, const Snapshot &snap); Index: /trunk/src/VBox/Main/ApplianceImplExport.cpp =================================================================== --- /trunk/src/VBox/Main/ApplianceImplExport.cpp (revision 29872) +++ /trunk/src/VBox/Main/ApplianceImplExport.cpp (revision 29873) @@ -661,4 +661,5 @@ */ void Appliance::buildXMLForOneVirtualSystem(xml::ElementNode &elmToAddVirtualSystemsTo, + std::list *pllElementsWithUuidAttributes, ComObjPtr &vsdescThis, OVFFormat enFormat, @@ -1298,6 +1299,7 @@ pConfig->buildMachineXML(*pelmVBoxMachine, settings::MachineConfigFile::BuildMachineXML_WriteVboxVersionAttribute - | settings::MachineConfigFile::BuildMachineXML_SkipRemovableMedia); + | settings::MachineConfigFile::BuildMachineXML_SkipRemovableMedia, // but not BuildMachineXML_IncludeSnapshots + pllElementsWithUuidAttributes); delete pConfig; } @@ -1425,4 +1427,8 @@ pelmToAddVirtualSystemsTo = pelmRoot; // add virtual system directly under root element + // this list receives pointers to the XML elements in the machine XML which + // might have UUIDs that need fixing after we know the UUIDs of the exported images + std::list llElementsWithUuidAttributes; + list< ComObjPtr >::const_iterator it; /* Iterate throughs all virtual systems of that appliance */ @@ -1433,4 +1439,5 @@ ComObjPtr vsdescThis = *it; buildXMLForOneVirtualSystem(*pelmToAddVirtualSystemsTo, + &llElementsWithUuidAttributes, vsdescThis, enFormat, @@ -1537,4 +1544,9 @@ // capacity is reported in megabytes, so... cbCapacity *= _1M; + + Bstr uuidTarget; + rc = pTargetDisk->COMGETTER(Id)(uuidTarget.asOutParam()); + if (FAILED(rc)) throw rc; + Guid guidTarget(uuidTarget); // upon success, close the disk as well @@ -1562,5 +1574,20 @@ // correct string as communicated to us by VMware (public bug #6612) ); - pelmDisk->setAttribute("vbox:uuid", Utf8StrFmt("%RTuuid", guidSource.raw()).c_str()); + + // add the UUID of the newly created target image to the OVF disk element, but in the + // vbox: namespace since it's not part of the standard + pelmDisk->setAttribute("vbox:uuid", Utf8StrFmt("%RTuuid", guidTarget.raw()).c_str()); + + // now, we might have other XML elements from vbox:Machine pointing to this image, + // but those would refer to the UUID of the _source_ image (which we created the + // export image from); those UUIDs need to be fixed to the export image + for (std::list::iterator eit = llElementsWithUuidAttributes.begin(); + eit != llElementsWithUuidAttributes.end(); + ++eit) + { + xml::ElementNode *pelmImage = *eit; + // overwrite existing uuid attribute + pelmImage->setAttribute("uuid", guidTarget.toStringCurly()); + } } Index: /trunk/src/VBox/Main/ApplianceImplImport.cpp =================================================================== --- /trunk/src/VBox/Main/ApplianceImplImport.cpp (revision 29872) +++ /trunk/src/VBox/Main/ApplianceImplImport.cpp (revision 29873) @@ -50,5 +50,7 @@ /** - * Public method implementation. + * Public method implementation. This opens the OVF with ovfreader.cpp. + * Thread implementation is in Appliance::readImpl(). + * * @param path * @return @@ -100,5 +102,6 @@ /** - * Public method implementation. + * Public method implementation. This looks at the output of ovfreader.cpp and creates + * VirtualSystemDescription instances. * @return */ @@ -539,5 +542,7 @@ /** - * Public method implementation. + * Public method implementation. This creates one or more new machines according to the + * VirtualSystemScription instances created by Appliance::Interpret(). + * Thread implementation is in Appliance::importImpl(). * @param aProgress * @return @@ -587,4 +592,5 @@ * Implementation for reading an OVF. This starts a new thread which will call * Appliance::taskThreadImportOrExport() which will then call readFS() or readS3(). + * This will then open the OVF with ovfreader.cpp. * * This is in a separate private method because it is used from two locations: @@ -635,5 +641,7 @@ /** * Actual worker code for reading an OVF from disk. This is called from Appliance::taskThreadImportOrExport() - * and therefore runs on the OVF read worker thread. This runs in two contexts: + * and therefore runs on the OVF read worker thread. This opens the OVF with ovfreader.cpp. + * + * This runs in two contexts: * * 1) in a first worker thread; in that case, Appliance::Read() called Appliance::readImpl(); @@ -912,4 +920,7 @@ * Appliance::taskThreadImportOrExport(). * + * This creates one or more new machines according to the VirtualSystemScription instances created by + * Appliance::Interpret(). + * * This is in a separate private method because it is used from two locations: * @@ -1072,5 +1083,8 @@ /** * Actual worker code for importing OVF data into VirtualBox. This is called from Appliance::taskThreadImportOrExport() - * and therefore runs on the OVF import worker thread. This runs in two contexts: + * and therefore runs on the OVF import worker thread. This creates one or more new machines according to the + * VirtualSystemScription instances created by Appliance::Interpret(). + * + * This runs in two contexts: * * 1) in a first worker thread; in that case, Appliance::ImportMachines() called Appliance::importImpl(); @@ -1313,10 +1327,10 @@ // First open the existing disk image rc = mVirtualBox->OpenHardDisk(Bstr(strSrcFilePath), - AccessMode_ReadOnly, - false, - NULL, - false, - NULL, - pSourceHD.asOutParam()); + AccessMode_ReadOnly, + false, + NULL, + false, + NULL, + pSourceHD.asOutParam()); if (FAILED(rc)) throw rc; fSourceHdNeedsClosing = true; @@ -1367,9 +1381,8 @@ * about what needs cleaning up (to support rollback). * - * @param locInfo - * @param vsysThis - * @param vsdescThis - * @param pNewMachine - * @param stack + * @param vsysThis OVF virtual system (machine) to import. + * @param vsdescThis Matching virtual system description (machine) to import. + * @param pNewMachine out: Newly created machine. + * @param stack Cleanup stack for when this throws. */ void Appliance::importMachineGeneric(const ovf::VirtualSystem &vsysThis, Index: /trunk/src/VBox/Main/include/ApplianceImpl.h =================================================================== --- /trunk/src/VBox/Main/include/ApplianceImpl.h (revision 29872) +++ /trunk/src/VBox/Main/include/ApplianceImpl.h (revision 29873) @@ -168,4 +168,5 @@ struct XMLStack; void buildXMLForOneVirtualSystem(xml::ElementNode &elmToAddVirtualSystemsTo, + std::list *pllElementsWithUuidAttributes, ComObjPtr &vsdescThis, OVFFormat enFormat, Index: /trunk/src/VBox/Main/xml/Settings.cpp =================================================================== --- /trunk/src/VBox/Main/xml/Settings.cpp (revision 29872) +++ /trunk/src/VBox/Main/xml/Settings.cpp (revision 29873) @@ -427,17 +427,4 @@ (uint16_t)time.u8Minute, (uint16_t)time.u8Second); -} - -/** - * Helper to create a string for a GUID. - * @param guid - * @return - */ -com::Utf8Str ConfigFileBase::makeString(const Guid &guid) -{ - Utf8Str str("{"); - str.append(guid.toString()); - str.append("}"); - return str; } @@ -1162,5 +1149,5 @@ { xml::ElementNode *pelmHardDisk = elmMedium.createChild("HardDisk"); - pelmHardDisk->setAttribute("uuid", makeString(mdm.uuid)); + pelmHardDisk->setAttribute("uuid", mdm.uuid.toStringCurly()); pelmHardDisk->setAttribute("location", mdm.strLocation); pelmHardDisk->setAttribute("format", mdm.strFormat); @@ -1222,5 +1209,5 @@ const MachineRegistryEntry &mre = *it; xml::ElementNode *pelmMachineEntry = pelmMachineRegistry->createChild("MachineEntry"); - pelmMachineEntry->setAttribute("uuid", makeString(mre.uuid)); + pelmMachineEntry->setAttribute("uuid", mre.uuid.toStringCurly()); pelmMachineEntry->setAttribute("src", mre.strSettingsFile); } @@ -1243,5 +1230,5 @@ const Medium &mdm = *it; xml::ElementNode *pelmMedium = pelmDVDImages->createChild("Image"); - pelmMedium->setAttribute("uuid", makeString(mdm.uuid)); + pelmMedium->setAttribute("uuid", mdm.uuid.toStringCurly()); pelmMedium->setAttribute("location", mdm.strLocation); if (mdm.strDescription.length()) @@ -1256,5 +1243,5 @@ const Medium &mdm = *it; xml::ElementNode *pelmMedium = pelmFloppyImages->createChild("Image"); - pelmMedium->setAttribute("uuid", makeString(mdm.uuid)); + pelmMedium->setAttribute("uuid", mdm.uuid.toStringCurly()); pelmMedium->setAttribute("location", mdm.strLocation); if (mdm.strDescription.length()) @@ -3063,5 +3050,5 @@ && (!hw.uuid.isEmpty()) ) - pelmHardware->setAttribute("uuid", makeString(hw.uuid)); + pelmHardware->setAttribute("uuid", hw.uuid.toStringCurly()); xml::ElementNode *pelmCPU = pelmHardware->createChild("CPU"); @@ -3297,5 +3284,5 @@ pelmDVD->setAttribute("passthrough", att.fPassThrough); if (!att.uuid.isEmpty()) - pelmDVD->createChild("Image")->setAttribute("uuid", makeString(att.uuid)); + pelmDVD->createChild("Image")->setAttribute("uuid", att.uuid.toStringCurly()); else if (att.strHostDriveSrc.length()) pelmDVD->createChild("HostDrive")->setAttribute("src", att.strHostDriveSrc); @@ -3313,5 +3300,5 @@ pelmFloppy->setAttribute("enabled", true); if (!att.uuid.isEmpty()) - pelmFloppy->createChild("Image")->setAttribute("uuid", makeString(att.uuid)); + pelmFloppy->createChild("Image")->setAttribute("uuid", att.uuid.toStringCurly()); else if (att.strHostDriveSrc.length()) pelmFloppy->createChild("HostDrive")->setAttribute("src", att.strHostDriveSrc); @@ -3653,8 +3640,13 @@ * This parameter is ignored unless the settings version is at least v1.9, which * is always the case when this gets called for OVF export. + * @param pllElementsWithUuidAttributes If not NULL, must point to a list of element node + * pointers to which we will append all allements that we created here that contain + * UUID attributes. This allows the OVF export code to quickly replace the internal + * media UUIDs with the UUIDs of the media that were exported. */ void MachineConfigFile::buildStorageControllersXML(xml::ElementNode &elmParent, const Storage &st, - bool fSkipRemovableMedia) + bool fSkipRemovableMedia, + std::list *pllElementsWithUuidAttributes) { xml::ElementNode *pelmStorageControllers = elmParent.createChild("StorageControllers"); @@ -3759,4 +3751,5 @@ pelmDevice->setAttribute("device", att.lDevice); + // attached image, if any if ( !att.uuid.isEmpty() && ( att.deviceType == DeviceType_HardDisk @@ -3764,5 +3757,12 @@ ) ) - pelmDevice->createChild("Image")->setAttribute("uuid", makeString(att.uuid)); + { + xml::ElementNode *pelmImage = pelmDevice->createChild("Image"); + pelmImage->setAttribute("uuid", att.uuid.toStringCurly()); + + // if caller wants a list of UUID elements, give it to them + if (pllElementsWithUuidAttributes) + pllElementsWithUuidAttributes->push_back(pelmImage); + } else if ( (m->sv >= SettingsVersion_v1_9) && (att.strHostDriveSrc.length()) @@ -3785,5 +3785,5 @@ xml::ElementNode *pelmSnapshot = elmParent.createChild("Snapshot"); - pelmSnapshot->setAttribute("uuid", makeString(snap.uuid)); + pelmSnapshot->setAttribute("uuid", snap.uuid.toStringCurly()); pelmSnapshot->setAttribute("name", snap.strName); pelmSnapshot->setAttribute("timeStamp", makeString(snap.timestamp)); @@ -3798,5 +3798,6 @@ buildStorageControllersXML(*pelmSnapshot, snap.storage, - false /* fSkipRemovableMedia */); + false /* fSkipRemovableMedia */, + NULL); /* pllElementsWithUuidAttributes */ // we only skip removable media for OVF, but we never get here for OVF // since snapshots never get written then @@ -3846,7 +3847,10 @@ * @param elmMachine XML element to add attributes and elements to. * @param fl Flags. + * @param pllElementsWithUuidAttributes pointer to list that should receive UUID elements or NULL; + * see buildStorageControllersXML() for details. */ void MachineConfigFile::buildMachineXML(xml::ElementNode &elmMachine, - uint32_t fl) + uint32_t fl, + std::list *pllElementsWithUuidAttributes) { if (fl & BuildMachineXML_WriteVboxVersionAttribute) @@ -3854,5 +3858,5 @@ setVersionAttribute(elmMachine); - elmMachine.setAttribute("uuid", makeString(uuid)); + elmMachine.setAttribute("uuid", uuid.toStringCurly()); elmMachine.setAttribute("name", strName); if (!fNameSync) @@ -3865,5 +3869,5 @@ if ( (fl & BuildMachineXML_IncludeSnapshots) && !uuidCurrentSnapshot.isEmpty()) - elmMachine.setAttribute("currentSnapshot", makeString(uuidCurrentSnapshot)); + elmMachine.setAttribute("currentSnapshot", uuidCurrentSnapshot.toStringCurly()); if (strSnapshotFolder.length()) elmMachine.setAttribute("snapshotFolder", strSnapshotFolder); @@ -3897,5 +3901,6 @@ buildStorageControllersXML(elmMachine, storageMachine, - !!(fl & BuildMachineXML_SkipRemovableMedia)); + !!(fl & BuildMachineXML_SkipRemovableMedia), + pllElementsWithUuidAttributes); } @@ -4085,6 +4090,7 @@ xml::ElementNode *pelmMachine = m->pelmRoot->createChild("Machine"); buildMachineXML(*pelmMachine, - MachineConfigFile::BuildMachineXML_IncludeSnapshots); + MachineConfigFile::BuildMachineXML_IncludeSnapshots, // but not BuildMachineXML_WriteVboxVersionAttribute + NULL); /* pllElementsWithUuidAttributes */ // now go write the XML Index: /trunk/src/VBox/Runtime/r3/xml.cpp =================================================================== --- /trunk/src/VBox/Runtime/r3/xml.cpp (revision 29872) +++ /trunk/src/VBox/Runtime/r3/xml.cpp (revision 29873) @@ -1011,4 +1011,5 @@ AttributeNode* ElementNode::setAttribute(const char *pcszName, const char *pcszValue) { + AttributeNode *pattrReturn; Data::AttributesMap::const_iterator it; @@ -1024,12 +1025,19 @@ // store m->attribs[pcszKey] = pNew; + pattrReturn = pNew.get(); } else { - // @todo - throw LogicError("Attribute exists"); - } - - return NULL; + // overwrite existing libxml attribute node + xmlAttrPtr plibAttr = xmlSetProp(m_plibNode, (xmlChar*)pcszName, (xmlChar*)pcszValue); + + // and fix our existing C++ side around it + boost::shared_ptr pattr = it->second; + pattr->m_plibAttr = plibAttr; // in case the xmlAttrPtr is different, I'm not sure + + pattrReturn = pattr.get(); + } + + return pattrReturn; }