Changeset 98123 in vbox for trunk/src/VBox/Main/src-client

Timestamp:

Jan 19, 2023 12:04:47 AM (2 years ago)

Author:

vboxsync

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

155292

Message:

Main/Console: Todo wrt MSD hotplugging and some coding style nits.

Location:

trunk/src/VBox/Main/src-client

Files:

• ConsoleImpl.cpp (modified) (5 diffs)
• ConsoleImpl2.cpp (modified) (4 diffs)

trunk/src/VBox/Main/src-client/ConsoleImpl.cpp

@@ -4128 +4128 @@
                                                  bool fSilent)
 {
-    LogFlowFunc(("pThis=%p uInstance=%u pszDevice=%p:{%s} enmBus=%u, pMediumAtt=%p\n",
+    LogRelFlowFunc(("pThis=%p uInstance=%u pszDevice=%p:{%s} enmBus=%u, pMediumAtt=%p\n",
                  pThis, uInstance, pcszDevice, pcszDevice, enmBus, pMediumAtt));
 
@@ -4143 +4143 @@
 
     /* Determine the base path for the device instance. */
-    PCFGMNODE pCtlInst;
-    pCtlInst = pVMM->pfnCFGMR3GetChildF(pVMM->pfnCFGMR3GetRootU(pUVM), "Devices/%s/%u/", pcszDevice, uInstance);
+    PCFGMNODE pCtlInst = pVMM->pfnCFGMR3GetChildF(pVMM->pfnCFGMR3GetRootU(pUVM), "Devices/%s/%u/", pcszDevice, uInstance);
     AssertReturn(pCtlInst || enmBus == StorageBus_USB, VERR_INTERNAL_ERROR);
 
@@ -4150 +4149 @@
 
     HRESULT hrc;
-    int rc = VINF_SUCCESS;
-    int rcRet = VINF_SUCCESS;
+    int vrc = VINF_SUCCESS;
+    LONG lDev;
+    hrc = pMediumAtt->COMGETTER(Device)(&lDev);                             H();
+    LONG lPort;
+    hrc = pMediumAtt->COMGETTER(Port)(&lPort);                              H();
+    DeviceType_T lType;
+    hrc = pMediumAtt->COMGETTER(Type)(&lType);                              H();
     unsigned uLUN;
-    LONG lDev;
-    LONG lPort;
-    DeviceType_T lType;
+    hrc = Console::i_storageBusPortDeviceToLun(enmBus, lPort, lDev, uLUN);  H();
+
+#undef H
+
     PCFGMNODE pLunL0 = NULL;
-
-    hrc = pMediumAtt->COMGETTER(Device)(&lDev);                             H();
-    hrc = pMediumAtt->COMGETTER(Port)(&lPort);                              H();
-    hrc = pMediumAtt->COMGETTER(Type)(&lType);                              H();
-    hrc = Console::i_storageBusPortDeviceToLun(enmBus, lPort, lDev, uLUN);  H();
-
-#undef H
-
     if (enmBus != StorageBus_USB)
     {
@@ -4172 +4169 @@
         {
             uint32_t fFlags = 0;
-
             if (fSilent)
                 fFlags |= PDM_TACH_FLAGS_NOT_HOT_PLUG;
 
-            rc = pVMM->pfnPDMR3DeviceDetach(pUVM, pcszDevice, uInstance, uLUN, fFlags);
-            if (rc == VERR_PDM_NO_DRIVER_ATTACHED_TO_LUN)
-                rc = VINF_SUCCESS;
-            AssertRCReturn(rc, rc);
+            vrc = pVMM->pfnPDMR3DeviceDetach(pUVM, pcszDevice, uInstance, uLUN, fFlags);
+            if (vrc == VERR_PDM_NO_DRIVER_ATTACHED_TO_LUN)
+                vrc = VINF_SUCCESS;
+            AssertLogRelRCReturn(vrc, vrc);
             pVMM->pfnCFGMR3RemoveNode(pLunL0);
 
             Utf8StrFmt devicePath("%s/%u/LUN#%u", pcszDevice, uInstance, uLUN);
             pThis->mapMediumAttachments.erase(devicePath);
-
         }
         else
-            AssertFailedReturn(VERR_INTERNAL_ERROR);
+            AssertLogRelFailedReturn(VERR_INTERNAL_ERROR);
 
         pVMM->pfnCFGMR3Dump(pCtlInst);
@@ -4197 +4192 @@
         USBStorageDeviceList::iterator it;
         for (it = pThis->mUSBStorageDevices.begin(); it != pThis->mUSBStorageDevices.end(); ++it)
-        {
             if (it->iPort == lPort)
                 break;
-        }
-
-        AssertReturn(it != pThis->mUSBStorageDevices.end(), VERR_INTERNAL_ERROR);
-        rc = pVMM->pfnPDMR3UsbDetachDevice(pUVM, &it->mUuid);
-        AssertRCReturn(rc, rc);
+        AssertLogRelReturn(it != pThis->mUSBStorageDevices.end(), VERR_INTERNAL_ERROR);
+
+        vrc = pVMM->pfnPDMR3UsbDetachDevice(pUVM, &it->mUuid);
+        AssertLogRelRCReturn(vrc, vrc);
         pThis->mUSBStorageDevices.erase(it);
     }
 #endif
 
-    LogFlowFunc(("Returning %Rrc\n", rcRet));
-    return rcRet;
+    LogFlowFunc(("Returning VINF_SUCCESS\n"));
+    return VINF_SUCCESS;
 }
 

trunk/src/VBox/Main/src-client/ConsoleImpl2.cpp

@@ -4943 +4943 @@
                 pCtlInst = pVMM->pfnCFGMR3GetChildF(pVMM->pfnCFGMR3GetRootU(pUVM), "USB/%s/", pcszDevice);
             else
-                pCtlInst = pVMM->pfnCFGMR3CreateTree(pUVM);
+                pCtlInst = pVMM->pfnCFGMR3CreateTree(pUVM); /** @todo r=bird: Leaked in error paths! */
         }
         AssertReturn(pCtlInst, VERR_INTERNAL_ERROR);
@@ -4965 +4965 @@
             uInstance = lPort; /* Overwrite uInstance with the correct one. */
 
+            /** @todo No LED after hotplugging. */
             if (!fHotplug && !fAttachDetach)
             {
                 USBStorageDevice UsbMsd;
-
                 UsbMsd.iPort = uInstance;
                 vrc = RTUuidCreate(&UsbMsd.mUuid);
@@ -4977 +4977 @@
                 mUSBStorageDevices.push_back(UsbMsd);
 
-                /** @todo No LED after hotplugging. */
+                /** @todo This LED set is not freed if the device is unplugged.  We could
+                 *        keep the LED set index in the UsbMsd structure and clean it up in
+                 *        i_detachStorageDevice. */
                 /* Attach the status driver */
                 i_attachStatusDriver(pCtlInst, RT_BIT_32(DeviceType_HardDisk),
@@ -5052 +5054 @@
          */
         if (   (fHotplug || !fAttachDetach)
-            && (   (enmBus == StorageBus_SCSI || enmBus == StorageBus_SAS || enmBus == StorageBus_USB || enmBus == StorageBus_VirtioSCSI)
+            && (   enmBus == StorageBus_SCSI
+                || enmBus == StorageBus_SAS
+                || enmBus == StorageBus_USB
+                || enmBus == StorageBus_VirtioSCSI
                 || (enmBus == StorageBus_SATA && enmType == DeviceType_DVD && !fPassthrough)))
         {