Changeset 29422 in vbox

Timestamp:

May 12, 2010 2:08:52 PM (15 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

61537

Message:

Main/OVF: fix IDE channel confusion introduced with earlier OVF two-channel IDE fix (trunk regression)

Location:

trunk/src/VBox/Main

Files:

• ApplianceImplImport.cpp (modified) (14 diffs)
• MachineImpl.cpp (modified) (1 diff)
• include/ApplianceImpl.h (modified) (1 diff)
• include/ApplianceImplPrivate.h (modified) (1 diff)
• include/ovfreader.h (modified) (2 diffs)
• xml/ovfreader.cpp (modified) (3 diffs)

trunk/src/VBox/Main/ApplianceImplImport.cpp

@@ -821 +821 @@
  * @param ulAddressOnParent in: the AddressOnParent parameter from OVF.
  * @param controllerType out: the name of the hard disk controller to attach to (e.g. "IDE Controller").
- * @param lChannel out: the channel (controller port) of the controller to attach to.
+ * @param lControllerPort out: the channel (controller port) of the controller to attach to.
  * @param lDevice out: the device number to attach to.
  */
@@ -827 +827 @@
                                             uint32_t ulAddressOnParent,
                                             Bstr &controllerType,
-                                            int32_t &lChannel,
+                                            int32_t &lControllerPort,
                                             int32_t &lDevice)
 {
+    Log(("Appliance::convertDiskAttachmentValues: hdc.system=%d, hdc.fPrimary=%d, ulAddressOnParent=%d\n", hdc.system, hdc.fPrimary, ulAddressOnParent));
+
     switch (hdc.system)
     {
         case ovf::HardDiskController::IDE:
-            // For the IDE bus, the channel parameter can be either 0 or 1, to specify the primary
+            // For the IDE bus, the port parameter can be either 0 or 1, to specify the primary
             // or secondary IDE controller, respectively. For the primary controller of the IDE bus,
             // the device number can be either 0 or 1, to specify the master or the slave device,
@@ -842 +844 @@
             {
                 case 0: // master
-                    if (hdc.ulAddress == 1)
+                    if (!hdc.fPrimary)
                     {
-                        // IDE controller has address 1: then it was exported from VMware and is the secondary controller
-                        lChannel = (long)1;
+                        // secondary master
+                        lControllerPort = (long)1;
                         lDevice = (long)0;
                     }
-                    else // interpret this as primary master
+                    else // primary master
                     {
-                        lChannel = (long)0;
+                        lControllerPort = (long)0;
                         lDevice = (long)0;
                     }
@@ -856 +858 @@
 
                 case 1: // slave
-                    if (hdc.ulAddress == 1)
+                    if (!hdc.fPrimary)
                     {
-                        // IDE controller has address 1: then it was exported from VMware and is the secondary controller
-                        lChannel = (long)1;
+                        // secondary slave
+                        lControllerPort = (long)1;
                         lDevice = (long)1;
                     }
-                    else // interpret this as primary slave
+                    else // primary slave
                     {
-                        lChannel = (long)0;
+                        lControllerPort = (long)0;
                         lDevice = (long)1;
                     }
@@ -871 +873 @@
                 // used by older VBox exports
                 case 2:     // interpret this as secondary master
-                    lChannel = (long)1;
+                    lControllerPort = (long)1;
                     lDevice = (long)0;
                 break;
@@ -877 +879 @@
                 // used by older VBox exports
                 case 3:     // interpret this as secondary slave
-                    lChannel = (long)1;
+                    lControllerPort = (long)1;
                     lDevice = (long)1;
                 break;
@@ -890 +892 @@
         case ovf::HardDiskController::SATA:
             controllerType = Bstr("SATA Controller");
-            lChannel = (long)ulAddressOnParent;
+            lControllerPort = (long)ulAddressOnParent;
             lDevice = (long)0;
         break;
@@ -896 +898 @@
         case ovf::HardDiskController::SCSI:
             controllerType = Bstr("SCSI Controller");
-            lChannel = (long)ulAddressOnParent;
+            lControllerPort = (long)ulAddressOnParent;
             lDevice = (long)0;
         break;
@@ -902 +904 @@
         default: break;
     }
+
+    Log(("=> lControllerPort=%d, lDevice=%d\n", lControllerPort, lDevice));
 }
 
@@ -1176 +1180 @@
                 if (SUCCEEDED(rc2))
                 {
-                    rc2 = sMachine->DetachDevice(Bstr(mhda.controllerType), mhda.lChannel, mhda.lDevice);
+                    rc2 = sMachine->DetachDevice(Bstr(mhda.controllerType), mhda.lControllerPort, mhda.lDevice);
                     rc2 = sMachine->SaveSettings();
                 }
@@ -1740 +1744 @@
                 mhda.pMachine = pNewMachine;
                 mhda.controllerType = bstrName;
-                mhda.lChannel = 0;
+                mhda.lControllerPort = 0;
                 mhda.lDevice = 0;
 
@@ -1746 +1750 @@
 
                 rc = sMachine->AttachDevice(mhda.controllerType,
-                                            mhda.lChannel,
+                                            mhda.lControllerPort,
                                             mhda.lDevice,
                                             DeviceType_Floppy,
@@ -1789 +1793 @@
                                             2,     // interpreted as secondary master
                                             mhda.controllerType,        // Bstr
-                                            mhda.lChannel,
+                                            mhda.lControllerPort,
                                             mhda.lDevice);
 
-                Log(("Attaching CD-ROM to channel %d on device %d\n", mhda.lChannel, mhda.lDevice));
+                Log(("Attaching CD-ROM to port %d on device %d\n", mhda.lControllerPort, mhda.lDevice));
 
                 rc = sMachine->AttachDevice(mhda.controllerType,
-                                            mhda.lChannel,
+                                            mhda.lControllerPort,
                                             mhda.lDevice,
                                             DeviceType_DVD,
@@ -1882 +1886 @@
                                             ovfVdisk.ulAddressOnParent,
                                             mhda.controllerType,        // Bstr
-                                            mhda.lChannel,
+                                            mhda.lControllerPort,
                                             mhda.lDevice);
 
-                Log(("Attaching disk %s to channel %d on device %d\n", vsdeHD->strVbox.c_str(), mhda.lChannel, mhda.lDevice));
+                Log(("Attaching disk %s to port %d on device %d\n", vsdeHD->strVbox.c_str(), mhda.lControllerPort, mhda.lDevice));
 
                 rc = sMachine->AttachDevice(mhda.controllerType,    // wstring name
-                                            mhda.lChannel,          // long controllerPort
+                                            mhda.lControllerPort,          // long controllerPort
                                             mhda.lDevice,           // long device
                                             DeviceType_HardDisk,    // DeviceType_T type

trunk/src/VBox/Main/MachineImpl.cpp

@@ -2766 +2766 @@
             AutoReadLock mediumLock(pMedium COMMA_LOCKVAL_SRC_POS);
             return setError(VBOX_E_OBJECT_IN_USE,
-                            tr("Medium '%s' is already attached to device slot %d on port %d of controller '%ls' of this virtual machine"),
+                            tr("Medium '%s' is already attached to port %d, device %d of controller '%ls' of this virtual machine"),
                             pMedium->getLocationFull().raw(),
+                            aControllerPort,
                             aDevice,
-                            aControllerPort,
                             aControllerName);
         }
         else
             return setError(VBOX_E_OBJECT_IN_USE,
-                            tr("Device is already attached to slot %d on port %d of controller '%ls' of this virtual machine"),
-                            aDevice, aControllerPort, aControllerName);
+                            tr("Device is already attached to port %d, device %d of controller '%ls' of this virtual machine"),
+                            aControllerPort, aDevice, aControllerName);
     }
 

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

@@ -141 +141 @@
                                      uint32_t ulAddressOnParent,
                                      Bstr &controllerType,
-                                     int32_t &lChannel,
+                                     int32_t &lControllerPort,
                                      int32_t &lDevice);
 

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

@@ -127 +127 @@
     ComPtr<IMachine>    pMachine;
     Bstr                controllerType;
-    int32_t             lChannel;
-    int32_t             lDevice;
+    int32_t             lControllerPort;        // 0-29 for SATA
+    int32_t             lDevice;                // IDE: 0 or 1, otherwise 0 always
 };
 

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

@@ -217 +217 @@
                                         // a network adapter depending on the network name.
     iprt::MiniString strAddress;                 // "Device-specific. For an Ethernet adapter, this specifies the MAC address."
+    int32_t lAddress;                   // strAddress as an integer, if applicable.
     iprt::MiniString strAddressOnParent;         // "For a device, this specifies its location on the controller."
     iprt::MiniString strAllocationUnits;         // "Specifies the units of allocation used. For example, “byte * 2^20”."
@@ -262 +263 @@
             // treats it as a fourth class besides IDE, SATA, SCSI
 
-    uint32_t                ulAddress;          // controller index; this is determined heuristically by the OVF reader and will
-                                                // be 0 for the first controller of this type (e.g. IDE primary ctler), 1 for the
-                                                // next (e.g. IDE secondary ctler)
+    int32_t                 lAddress;           // value from OVF "Address" element
+    bool                    fPrimary;           // controller index; this is determined heuristically by the OVF reader and will
+                                                // be true for the first controller of this type (e.g. IDE primary ctler) or
+                                                // false for the next (e.g. IDE secondary ctler)
 
     HardDiskController()
         : idController(0),
-          ulAddress(0)
+          lAddress(0),
+          fPrimary(true)
     { }
 };

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

@@ -401 +401 @@
                         i.strConnection = pelmItemChild->getValue();
                     else if (!strcmp(pcszItemChildName, "Address"))
+                    {
                         i.strAddress = pelmItemChild->getValue();
+                        pelmItemChild->copyValue(i.lAddress);
+                    }
                     else if (!strcmp(pcszItemChildName, "AddressOnParent"))
                         i.strAddressOnParent = pelmItemChild->getValue();
@@ -433 +436 @@
             }
 
+            HardDiskController *pPrimaryIDEController = NULL;       // will be set once found
+
             // now go thru all hardware items and handle them according to their type;
             // in this first loop we handle all items _except_ hard disk images,
@@ -491 +496 @@
                         hdc.strControllerType = i.strResourceSubType;
 
-                        // if there is a numeric address tag for the IDE controller, use that;
-                        // VMware uses "0" and "1" to keep the two OVF IDE controllers apart;
-                        // otherwise use the "bus number" field which was specified in some old
-                        // OVF files (but not the standard)
-                        if (i.strAddress == "0")
-                            hdc.ulAddress = 0;
-                        else if (i.strAddress == "1")
-                            hdc.ulAddress = 1;
-                        else if (i.strAddress == "2")     // just to be sure, this doesn't seem to be used by VMware
-                            hdc.ulAddress = 2;
-                        else if (i.strAddress == "3")
-                            hdc.ulAddress = 3;
+                        hdc.lAddress = i.lAddress;
+
+                        if (!pPrimaryIDEController)
+                            // this is the first IDE controller found: then mark it as "primary"
+                            hdc.fPrimary = true;
                         else
-                            hdc.ulAddress = i.ulBusNumber;
+                        {
+                            // this is the second IDE controller found: If VMware exports two
+                            // IDE controllers, it seems that they are given an "Address" of 0
+                            // an 1, respectively, so assume address=0 means primary controller
+                            if (    pPrimaryIDEController->lAddress == 0
+                                 && hdc.lAddress == 1
+                               )
+                            {
+                                pPrimaryIDEController->fPrimary = true;
+                                hdc.fPrimary = false;
+                            }
+                            else if (    pPrimaryIDEController->lAddress == 1
+                                      && hdc.lAddress == 0
+                                    )
+                            {
+                                pPrimaryIDEController->fPrimary = false;
+                                hdc.fPrimary = false;
+                            }
+                            else
+                                // then we really can't tell, just hope for the best
+                                hdc.fPrimary = false;
+                        }
 
                         vsys.mapControllers[i.ulInstanceID] = hdc;
+                        if (!pPrimaryIDEController)
+                            pPrimaryIDEController = &vsys.mapControllers[i.ulInstanceID];
                     }
                     break;