Changeset 98091 in vbox for trunk/src/VBox/Main

Timestamp:

Jan 16, 2023 10:18:24 AM (2 years ago)

Author:

vboxsync

Message:

Main/Console: LED related cleanups. bugref:9892

Location:

trunk/src/VBox/Main

Files:

• include/ConsoleImpl.h (modified) (2 diffs)
• src-client/ConsoleImpl.cpp (modified) (7 diffs)
• src-client/ConsoleImpl2.cpp (modified) (14 diffs)

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

@@ -809 +809 @@
     struct LEDSET;
     typedef struct LEDSET *PLEDSET;
-    PPDMLED *i_getLedSet(uint32_t iLedSet);
+    PPDMLED volatile *i_getLedSet(uint32_t iLedSet);
     uint32_t i_allocateDriverLeds(uint32_t cLeds, uint32_t fTypes, DeviceType_T **ppSubTypes);
     void i_attachStatusDriver(PCFGMNODE pCtlInst, DeviceType_T enmType);
@@ -1069 +1069 @@
      * @{ */
     /** Read/write lock separating LED allocations (write) from queries (read). */
-    RWLockHandle mLedLock;
+    RWLockHandle            mLedLock;
     /** Number of LED sets in use in maLedSets. */
-    uint32_t          mcLedSets;
+    uint32_t                mcLedSets;
     /** LED sets. */
     struct LEDSET
     {
-        PPDMLED        *papLeds;
-        uint32_t        cLeds;
-        uint32_t        fTypes;     /**< Bitmask of possible DeviceType_T values (e.g. RT_BIT_32(DeviceType_Network)). */
-        DeviceType_T   *paSubTypes; /**< Optionally, device types for each individual LED. Runs parallel to papLeds. */
-    }                 maLedSets[32];
+        /** Bitmask of possible DeviceType_T values (e.g. RT_BIT_32(DeviceType_Network)). */
+        uint32_t            fTypes;
+        /** Number of LEDs.   */
+        uint32_t            cLeds;
+        /** Array of PDMLED pointers.  The pointers in the array can be changed at any
+         *  time by Console::i_drvStatus_UnitChanged(). */
+        PPDMLED volatile   *papLeds;
+        /** Optionally, device types for each individual LED. Runs parallel to papLeds. */
+        DeviceType_T       *paSubTypes;
+    } maLedSets[32];
     /** @} */
 

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

@@ -883 +883 @@
     for (size_t idxSet = 0; idxSet < mcLedSets; idxSet++)
     {
-        RTMemFree(maLedSets[idxSet].papLeds);
+        RTMemFree((void *)maLedSets[idxSet].papLeds);
         maLedSets[idxSet].papLeds = NULL;
         maLedSets[idxSet].paSubTypes = NULL;
@@ -2862 +2862 @@
             if (pLS->fTypes & fWanted)
             {
-                uint32_t const        cLeds      = pLS->cLeds;
-                PPDMLED const * const papSrcLeds = pLS->papLeds;
+                uint32_t const           cLeds      = pLS->cLeds;
+                PPDMLED volatile * const papSrcLeds = pLS->papLeds;
 
                 /* Multi-type drivers (e.g. SCSI) have a subtype array which must be matched. */
@@ -11515 +11515 @@
  * @param   iLedSet      Index of LED set to fetch
  */
-PPDMLED *
+PPDMLED volatile *
 Console::i_getLedSet(uint32_t iLedSet)
 {
@@ -11681 +11681 @@
     PPDMILEDPORTS       pLedPorts;
     /** Pointer to the array of LED pointers. */
-    PPDMLED            *papLeds;
+    PPDMLED volatile   *papLeds;
     /** The unit number corresponding to the first entry in the LED array. */
     uint32_t            iFirstLUN;
@@ -11697 +11697 @@
     /** Pointer to the Console object, for driver triggered activities. */
     Console             *pConsole;
-} DRVMAINSTATUS, *PDRVMAINSTATUS;
+} DRVMAINSTATUS;
+/** Pointer the instance data for a Main status driver. */
+typedef DRVMAINSTATUS *PDRVMAINSTATUS;
 
 
@@ -11714 +11716 @@
     if (iLUN >= pThis->iFirstLUN && iLUN <= pThis->iLastLUN)
     {
+        /*
+         * Query the pointer to the PDMLED field inside the target device
+         * structure (owned by the virtual hardware device).
+         */
         PPDMLED pLed;
         int rc = pThis->pLedPorts->pfnQueryStatusLed(pThis->pLedPorts, iLUN, &pLed);
-        /*
-         * pLed now points directly to the per-unit struct PDMLED field
-         * inside the target device struct owned by the hardware driver.
-         */
         if (RT_FAILURE(rc))
             pLed = NULL;
-        ASMAtomicWritePtr(&pThis->papLeds[iLUN - pThis->iFirstLUN], pLed);
+
         /*
+         * Update the corresponding papLeds[] entry.
+         *
          * papLeds[] points to the struct PDMLED of each of this driver's
          * units.  The entries are initialized here, called out of a loop
@@ -11733 +11737 @@
          * src/VBox/Frontends/VirtualBox/src/runtime/UIIndicatorsPool.cpp
          */
+        /** @todo acquire Console::mLedLock here in exclusive mode? */
+        ASMAtomicWritePtr(&pThis->papLeds[iLUN - pThis->iFirstLUN], pLed);
         Log(("drvStatus_UnitChanged: iLUN=%d pLed=%p\n", iLUN, pLed));
     }

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

@@ -676 +676 @@
     /* Preallocate the arrays we need, bunching them together. */
     AssertCompile((unsigned)DeviceType_Null == 0);
-    PPDMLED *papLeds = (PPDMLED *)RTMemAllocZ((sizeof(PPDMLED) + (ppaSubTypes ? sizeof(**ppaSubTypes) : 0)) * cLeds);
+    PPDMLED volatile *papLeds = (PPDMLED volatile *)RTMemAllocZ(  (sizeof(PPDMLED) + (ppaSubTypes ? sizeof(**ppaSubTypes) : 0))
+                                                                * cLeds);
     AssertStmt(papLeds, throw E_OUTOFMEMORY);
 
@@ -701 +702 @@
     }
 
-    RTMemFree(papLeds);
+    RTMemFree((void *)papLeds);
     AssertFailed();
     throw ConfigError("AllocateDriverPapLeds", VERR_OUT_OF_RANGE, "Too many LED sets");
@@ -707 +708 @@
 
 
+/**
+ * @throws ConfigError and std::bad_alloc.
+ */
 void Console::i_attachStatusDriver(PCFGMNODE pCtlInst, uint32_t fTypes, uint32_t cLeds, DeviceType_T **ppaSubTypes,
                                    Console::MediumAttachmentMap *pmapMediumAttachments,
@@ -731 +735 @@
 
 
+/**
+ * @throws ConfigError and std::bad_alloc.
+ */
 void Console::i_attachStatusDriver(PCFGMNODE pCtlInst, DeviceType_T enmType)
 {
@@ -4880 +4887 @@
         LONG lPort;
         hrc = pMediumAtt->COMGETTER(Port)(&lPort);                                          H();
-        DeviceType_T lType;
-        hrc = pMediumAtt->COMGETTER(Type)(&lType);                                          H();
+        DeviceType_T enmType;
+        hrc = pMediumAtt->COMGETTER(Type)(&enmType);                                        H();
         BOOL fNonRotational;
         hrc = pMediumAtt->COMGETTER(NonRotational)(&fNonRotational);                        H();
@@ -4887 +4894 @@
         hrc = pMediumAtt->COMGETTER(Discard)(&fDiscard);                                    H();
 
-        if (lType == DeviceType_DVD)
+        if (enmType == DeviceType_DVD)
             fInsertDiskIntegrityDrv = false;
 
@@ -4948 +4955 @@
 
         vrc = i_removeMediumDriverFromVm(pCtlInst, pcszDevice, uInstance, uLUN, enmBus, fAttachDetach,
-                                         fHotplug, fForceUnmount, pUVM, pVMM, lType, &pLunL0);
+                                         fHotplug, fForceUnmount, pUVM, pVMM, enmType, &pLunL0);
         if (RT_FAILURE(vrc))
             return vrc;
@@ -4965 +4972 @@
          *    taking an online snapshot!
          */
-        if (   lType == DeviceType_HardDisk
+        if (   enmType == DeviceType_HardDisk
             && (   aMachineState == MachineState_Starting
                 || aMachineState == MachineState_Restoring))
@@ -4979 +4986 @@
             BOOL fHostDrive;
             hrc = ptrMedium->COMGETTER(HostDrive)(&fHostDrive);                             H();
-            if (  (   lType == DeviceType_DVD
-                   || lType == DeviceType_Floppy)
+            if (  (   enmType == DeviceType_DVD
+                   || enmType == DeviceType_Floppy)
                 && !fHostDrive)
             {
@@ -4992 +4999 @@
                 (void)RTFsQueryType(strFile.c_str(), &enmFsTypeFile);
                 LogRel(("File system of '%s' (%s) is %s\n",
-                       strFile.c_str(), lType == DeviceType_DVD ? "DVD" : "Floppy", RTFsTypeName(enmFsTypeFile)));
+                       strFile.c_str(), enmType == DeviceType_DVD ? "DVD" : "Floppy", RTFsTypeName(enmFsTypeFile)));
             }
 
@@ -5016 +5023 @@
         if (   (fHotplug || !fAttachDetach)
             && (   (enmBus == StorageBus_SCSI || enmBus == StorageBus_SAS || enmBus == StorageBus_USB || enmBus == StorageBus_VirtioSCSI)
-                || (enmBus == StorageBus_SATA && lType == DeviceType_DVD && !fPassthrough)))
+                || (enmBus == StorageBus_SATA && enmType == DeviceType_DVD && !fPassthrough)))
         {
             InsertConfigString(pLunL0, "Driver", "SCSI");
@@ -5024 +5031 @@
         vrc = i_configMedium(pLunL0,
                              !!fPassthrough,
-                             lType,
+                             enmType,
                              fUseHostIOCache,
                              fBuiltinIOCache,
@@ -5060 +5067 @@
             else if (   !fHotplug
                      && (   (enmBus == StorageBus_SAS || enmBus == StorageBus_SCSI || enmBus == StorageBus_VirtioSCSI)
-                         || (enmBus == StorageBus_SATA && lType == DeviceType_DVD)))
+                         || (enmBus == StorageBus_SATA && enmType == DeviceType_DVD)))
                 vrc = pVMM->pfnPDMR3DriverAttach(pUVM, pcszDevice, uInstance, uLUN,
                                                  fHotplug ? 0 : PDM_TACH_FLAGS_NOT_HOT_PLUG, NULL /*ppBase*/);
@@ -5090 +5097 @@
 
         if (paLedDevType)
-            paLedDevType[uLUN] = lType;
+            paLedDevType[uLUN] = enmType;
 
         /* Dump the changed LUN if possible, dump the complete device otherwise */