Changeset 29873 in vbox for trunk/src

Timestamp:

May 28, 2010 5:14:53 PM (15 years ago)

Author:

vboxsync

Message:

OVF: fix incorrect disk UUIDs in machine XML on export

Location:

trunk/src/VBox

Files:

• Main/ApplianceImplExport.cpp (modified) (6 diffs)
• Main/ApplianceImplImport.cpp (modified) (9 diffs)
• Main/include/ApplianceImpl.h (modified) (1 diff)
• Main/xml/Settings.cpp (modified) (18 diffs)
• Runtime/r3/xml.cpp (modified) (2 diffs)

trunk/src/VBox/Main/ApplianceImplExport.cpp

@@ -661 +661 @@
  */
 void Appliance::buildXMLForOneVirtualSystem(xml::ElementNode &elmToAddVirtualSystemsTo,
+                                            std::list<xml::ElementNode*> *pllElementsWithUuidAttributes,
                                             ComObjPtr<VirtualSystemDescription> &vsdescThis,
                                             OVFFormat enFormat,
@@ -1298 +1299 @@
         pConfig->buildMachineXML(*pelmVBoxMachine,
                                    settings::MachineConfigFile::BuildMachineXML_WriteVboxVersionAttribute
-                                 | settings::MachineConfigFile::BuildMachineXML_SkipRemovableMedia);
+                                 | settings::MachineConfigFile::BuildMachineXML_SkipRemovableMedia,
                                         // but not BuildMachineXML_IncludeSnapshots
+                                 pllElementsWithUuidAttributes);
         delete pConfig;
     }
@@ -1425 +1427 @@
             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<xml::ElementNode*> llElementsWithUuidAttributes;
+
         list< ComObjPtr<VirtualSystemDescription> >::const_iterator it;
         /* Iterate throughs all virtual systems of that appliance */
@@ -1433 +1439 @@
             ComObjPtr<VirtualSystemDescription> vsdescThis = *it;
             buildXMLForOneVirtualSystem(*pelmToAddVirtualSystemsTo,
+                                        &llElementsWithUuidAttributes,
                                         vsdescThis,
                                         enFormat,
@@ -1537 +1544 @@
             // 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 +1574 @@
                                                                                     // 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<xml::ElementNode*>::iterator eit = llElementsWithUuidAttributes.begin();
+                 eit != llElementsWithUuidAttributes.end();
+                 ++eit)
+            {
+                xml::ElementNode *pelmImage = *eit;
+                // overwrite existing uuid attribute
+                pelmImage->setAttribute("uuid", guidTarget.toStringCurly());
+            }
         }
 

trunk/src/VBox/Main/ApplianceImplImport.cpp

@@ -50 +50 @@
 
 /**
- * Public method implementation.
+ * Public method implementation. This opens the OVF with ovfreader.cpp.
+ * Thread implementation is in Appliance::readImpl().
+ *
  * @param path
  * @return
@@ -100 +102 @@
 
 /**
- * Public method implementation.
+ * Public method implementation. This looks at the output of ovfreader.cpp and creates
+ * VirtualSystemDescription instances.
  * @return
  */
@@ -539 +542 @@
 
 /**
- * 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 +592 @@
  * 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 +641 @@
 /**
  * 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 +920 @@
  * 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 +1083 @@
 /**
  * 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 +1327 @@
             // 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 +1381 @@
  * 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,

trunk/src/VBox/Main/include/ApplianceImpl.h

@@ -168 +168 @@
     struct XMLStack;
     void buildXMLForOneVirtualSystem(xml::ElementNode &elmToAddVirtualSystemsTo,
+                                     std::list<xml::ElementNode*> *pllElementsWithUuidAttributes,
                                      ComObjPtr<VirtualSystemDescription> &vsdescThis,
                                      OVFFormat enFormat,

trunk/src/VBox/Main/xml/Settings.cpp

@@ -427 +427 @@
                       (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 +1149 @@
 {
     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 +1209 @@
         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 +1230 @@
         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 +1243 @@
         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 +3050 @@
          && (!hw.uuid.isEmpty())
        )
-        pelmHardware->setAttribute("uuid", makeString(hw.uuid));
+        pelmHardware->setAttribute("uuid", hw.uuid.toStringCurly());
 
     xml::ElementNode *pelmCPU      = pelmHardware->createChild("CPU");
@@ -3297 +3284 @@
                         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 +3300 @@
                     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 +3640 @@
  *   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<xml::ElementNode*> *pllElementsWithUuidAttributes)
 {
     xml::ElementNode *pelmStorageControllers = elmParent.createChild("StorageControllers");
@@ -3759 +3751 @@
             pelmDevice->setAttribute("device", att.lDevice);
 
+            // attached image, if any
             if (    !att.uuid.isEmpty()
                  && (    att.deviceType == DeviceType_HardDisk
@@ -3764 +3757 @@
                     )
                )
-                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 +3785 @@
     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 +3798 @@
     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 +3847 @@
  * @param elmMachine XML <Machine> 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<xml::ElementNode*> *pllElementsWithUuidAttributes)
 {
     if (fl & BuildMachineXML_WriteVboxVersionAttribute)
@@ -3854 +3858 @@
         setVersionAttribute(elmMachine);
 
-    elmMachine.setAttribute("uuid", makeString(uuid));
+    elmMachine.setAttribute("uuid", uuid.toStringCurly());
     elmMachine.setAttribute("name", strName);
     if (!fNameSync)
@@ -3865 +3869 @@
     if (    (fl & BuildMachineXML_IncludeSnapshots)
          && !uuidCurrentSnapshot.isEmpty())
-        elmMachine.setAttribute("currentSnapshot", makeString(uuidCurrentSnapshot));
+        elmMachine.setAttribute("currentSnapshot", uuidCurrentSnapshot.toStringCurly());
     if (strSnapshotFolder.length())
         elmMachine.setAttribute("snapshotFolder", strSnapshotFolder);
@@ -3897 +3901 @@
     buildStorageControllersXML(elmMachine,
                                storageMachine,
-                               !!(fl & BuildMachineXML_SkipRemovableMedia));
+                               !!(fl & BuildMachineXML_SkipRemovableMedia),
+                               pllElementsWithUuidAttributes);
 }
 
@@ -4085 +4090 @@
         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

trunk/src/VBox/Runtime/r3/xml.cpp

@@ -1011 +1011 @@
 AttributeNode* ElementNode::setAttribute(const char *pcszName, const char *pcszValue)
 {
+    AttributeNode *pattrReturn;
     Data::AttributesMap::const_iterator it;
 
@@ -1024 +1025 @@
         // 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<AttributeNode> pattr = it->second;
+        pattr->m_plibAttr = plibAttr;       // in case the xmlAttrPtr is different, I'm not sure
+
+        pattrReturn = pattr.get();
+    }
+
+    return pattrReturn;
 
 }