Index: /trunk/src/VBox/Frontends/VBoxBalloonCtrl/VBoxBalloonCtrl.cpp =================================================================== --- /trunk/src/VBox/Frontends/VBoxBalloonCtrl/VBoxBalloonCtrl.cpp (revision 37102) +++ /trunk/src/VBox/Frontends/VBoxBalloonCtrl/VBoxBalloonCtrl.cpp (revision 37103) @@ -156,13 +156,12 @@ static bool machineIsRunning(MachineState_T enmState); -static mapVMIter machineGetByUUID(const Bstr &strUUID); -static int machineAdd(const ComPtr &rptrMachine); -static int machineUpdate(const ComPtr &rptrMachine, MachineState_T enmState); -static int machineUpdate(mapVMIter it, MachineState_T enmState); -static void machineRemove(mapVMIter it); +static bool machineHandled(const Bstr &strUuid); +static int machineAdd(const Bstr &strUuid); +static int machineRemove(const Bstr &strUuid); +static int machineUpdate(const Bstr &strUuid, MachineState_T enmState); static unsigned long balloonGetMaxSize(const ComPtr &rptrMachine); -static bool balloonIsRequired(mapVMIter it); -static int balloonUpdate(mapVMIterConst it); +static bool balloonIsRequired(PVBOXBALLOONCTRL_MACHINE pMachine); +static int balloonUpdate(const Bstr &strUuid, PVBOXBALLOONCTRL_MACHINE pMachine); static HRESULT balloonCtrlSetup(); @@ -220,20 +219,13 @@ if (RT_SUCCESS(rc)) { - if (fRegistered) - { - ComPtr machine; - hr = g_pVirtualBox->FindMachine(uuid.raw(), machine.asOutParam()); - if (SUCCEEDED(hr)) - rc = machineAdd(machine); - else - rc = VERR_NOT_FOUND; - } - else - machineRemove(machineGetByUUID(uuid)); - AssertRC(rc); + if (fRegistered && machineHandled(uuid)) + rc = machineAdd(uuid); + else if (!fRegistered) + rc = machineRemove(uuid); int rc2 = RTCritSectLeave(&g_MapCritSect); if (RT_SUCCESS(rc)) rc = rc2; + AssertRC(rc); } } @@ -258,21 +250,9 @@ if (RT_SUCCESS(rc)) { - mapVMIter it = machineGetByUUID(uuid); - if (it == g_mapVM.end()) - { - /* Use the machine object to figure out if we - * need to do something. */ - ComPtr machine; - hr = g_pVirtualBox->FindMachine(uuid.raw(), machine.asOutParam()); - if (SUCCEEDED(hr)) - rc = machineUpdate(machine, machineState); - } - else /* Update an existing machine. */ - rc = machineUpdate(it, machineState); - AssertRC(rc); - + rc = machineUpdate(uuid, machineState); int rc2 = RTCritSectLeave(&g_MapCritSect); if (RT_SUCCESS(rc)) rc = rc2; + AssertRC(rc); } } @@ -289,8 +269,7 @@ { serviceLog("VBoxSVC became unavailable\n"); - { - balloonCtrlShutdown(); - deleteGlobalObjects(); - } + + balloonCtrlShutdown(); + deleteGlobalObjects(); } else @@ -367,4 +346,12 @@ } +/** + * Retrieves the current delta value + * + * @return long Delta (MB) of the balloon to be deflated (<0) or inflated (>0). + * @param ulCurrentDesktopBalloonSize The balloon's current size. + * @param ulDesktopFreeMemory The VM's current free memory. + * @param ulMaxBalloonSize The maximum balloon size (MB) it can inflate to. + */ static long getlBalloonDelta(unsigned long ulCurrentDesktopBalloonSize, unsigned long ulDesktopFreeMemory, unsigned long ulMaxBalloonSize) { @@ -427,10 +414,44 @@ } -static mapVMIter machineGetByUUID(const Bstr &strUUID) -{ - return g_mapVM.find(strUUID); -} - -static int machineAdd(const ComPtr &rptrMachine) +/** + * Determines whether the specified machine needs to be handled + * by this service. + * + * @return bool True if the machine needs handling, false if not. + * @param strUuid UUID of the specified machine. + */ +static bool machineHandled(const Bstr &strUuid) +{ + bool fHandled = false; + + do + { + HRESULT rc; + + ComPtr machine; + CHECK_ERROR_BREAK(g_pVirtualBox, FindMachine(strUuid.raw(), machine.asOutParam())); + + MachineState_T machineState; + CHECK_ERROR_BREAK(machine, COMGETTER(State)(&machineState)); + + if ( balloonGetMaxSize(machine) + && machineIsRunning(machineState)) + { + serviceLogVerbose(("Handling machine \"%ls\"\n", strUuid.raw())); + fHandled = true; + } + } + while (0); + + return fHandled; +} + +/** + * Adds a specified machine to the list (map) of handled machines. + * + * @return IPRT status code. + * @param strUuid UUID of the specified machine. + */ +static int machineAdd(const Bstr &strUuid) { HRESULT rc; @@ -438,6 +459,19 @@ do { + ComPtr machine; + CHECK_ERROR_BREAK(g_pVirtualBox, FindMachine(strUuid.raw(), machine.asOutParam())); + + MachineState_T machineState; + CHECK_ERROR_BREAK(machine, COMGETTER(State)(&machineState)); + + if ( !balloonGetMaxSize(machine) + || !machineIsRunning(machineState)) + { + /* This machine does not need to be added, just skip it! */ + break; + } + VBOXBALLOONCTRL_MACHINE m; - m.machine = rptrMachine; + m.machine = machine; /* @@ -470,13 +504,10 @@ * Add machine to map. */ - Bstr strUUID; - CHECK_ERROR_BREAK(rptrMachine, COMGETTER(Id)(strUUID.asOutParam())); - - mapVMIter it = g_mapVM.find(strUUID); + mapVMIter it = g_mapVM.find(strUuid); Assert(it == g_mapVM.end()); - g_mapVM.insert(std::make_pair(strUUID, m)); - - serviceLogVerbose(("Added machine \"%ls\"\n", strUUID.raw())); + g_mapVM.insert(std::make_pair(strUuid, m)); + + serviceLogVerbose(("Added machine \"%ls\"\n", strUuid.raw())); } while (0); @@ -485,50 +516,115 @@ } -static void machineRemove(mapVMIter it) -{ +/** + * Removes a specified machine from the list of handled machines. + * + * @return IPRT status code. + * @param strUuid UUID of the specified machine. + */ +static int machineRemove(const Bstr &strUuid) +{ + int rc = RTCritSectEnter(&g_MapCritSect); + if (RT_SUCCESS(rc)) + { + mapVMIter it = g_mapVM.find(strUuid); + if (it != g_mapVM.end()) + { + /* Must log before erasing the iterator because of the UUID ref! */ + serviceLogVerbose(("Removing machine \"%ls\"\n", strUuid.raw())); + + /* + * Remove machine from map. + */ + g_mapVM.erase(it); + } + else + { + serviceLogVerbose(("Warning: Removing not added machine \"%ls\"\n", strUuid.raw())); + rc = VERR_NOT_FOUND; + } + + int rc2 = RTCritSectLeave(&g_MapCritSect); + if (RT_SUCCESS(rc)) + rc = rc2; + } + + return rc; +} + +/** + * Updates a specified machine according to its current machine state. + * That currently also could mean that a machine gets removed if it doesn't + * fit in our criteria anymore or a machine gets added if we need to handle + * it now (and didn't before). + * + * @return IPRT status code. + * @param strUuid UUID of the specified machine. + * @param enmState The machine's current state. + */ +static int machineUpdate(const Bstr &strUuid, MachineState_T enmState) +{ + int rc = VINF_SUCCESS; + + mapVMIter it = g_mapVM.find(strUuid); + if (it == g_mapVM.end()) + { + if (machineHandled(strUuid)) + { + rc = machineAdd(strUuid); + if (RT_SUCCESS(rc)) + it = g_mapVM.find(strUuid); + } + else + { + serviceLogVerbose(("Machine \"%ls\" (state: %u) does not need to be updated\n", + strUuid.raw(), enmState)); + } + } + if (it != g_mapVM.end()) { - /* Must log before erasing the iterator because of the UUID ref! */ - serviceLogVerbose(("Removing machine \"%ls\"\n", it->first.raw())); - /* - * Remove machine from map. + * Ballooning stuff - start. */ - g_mapVM.erase(it); - } -} - -static int machineUpdate(const ComPtr &rptrMachine, MachineState_T enmState) -{ - if ( !balloonGetMaxSize(rptrMachine) - || !machineIsRunning(enmState)) - { - return VINF_SUCCESS; /* Machine is not required to be added. */ - } - return machineAdd(rptrMachine); -} - -static int machineUpdate(mapVMIter it, MachineState_T enmState) -{ - Assert(it != g_mapVM.end()); - - if ( !balloonIsRequired(it) - || !machineIsRunning(enmState)) - { - machineRemove(it); - return VINF_SUCCESS; - } - - return balloonUpdate(it); -} - -static int getMetric(mapVMIterConst it, const Bstr& strName, LONG *pulData) -{ - AssertPtrReturn(pulData, VERR_INVALID_PARAMETER); + + /* Our actual ballooning criteria. */ + if ( !balloonIsRequired(&it->second) + || !machineIsRunning(enmState)) + { + /* Current machine is not suited for ballooning anymore - + * remove it from our map. */ + rc = machineRemove(strUuid); + } + else + { + rc = balloonUpdate(strUuid, &it->second); + AssertRC(rc); + } + } + + /* + * Ballooning stuff - end. + */ + + return rc; +} + +/** + * Retrieves a metric from a specified machine. + * + * @return IPRT status code. + * @param pMachine Pointer to the machine's internal structure. + * @param strName Name of metric to retrieve. + * @param pulData Pointer to value to retrieve the actual metric value. + */ +static int getMetric(PVBOXBALLOONCTRL_MACHINE pMachine, const Bstr& strName, LONG *pulData) +{ + AssertPtrReturn(pMachine, VERR_INVALID_POINTER); + AssertPtrReturn(pulData, VERR_INVALID_POINTER); /* Input. */ com::SafeArray metricNames(1); com::SafeIfaceArray metricObjects(1); - it->second.machine.queryInterfaceTo(&metricObjects[0]); + pMachine->machine.queryInterfaceTo(&metricObjects[0]); /* Output. */ @@ -545,7 +641,9 @@ strName.cloneTo(&metricNames[0]); #ifdef VBOX_BALLOONCTRL_GLOBAL_PERFCOL + Assert(!g_pPerfCollector.isNull()); HRESULT hrc = g_pPerfCollector->QueryMetricsData( #else - HRESULT hrc = it->second.collector->QueryMetricsData( + Assert(!pMachine->collector.isNull()); + HRESULT hrc = pMachine->collector->QueryMetricsData( #endif ComSafeArrayAsInParam(metricNames), @@ -590,5 +688,5 @@ * * @return unsigned long Balloon size (in MB) to set, 0 if no ballooning required. - * @param rptrMachine Pointer to specified machine. + * @param rptrMachine Pointer to interface of specified machine. */ static unsigned long balloonGetMaxSize(const ComPtr &rptrMachine) @@ -629,15 +727,18 @@ /** - * Determines whether ballooning is required to the spcified VM. + * Determines whether ballooning is required to the specified machine. * - * @return bool True if ballooning is required, false if not. - * @param it Iterator pointing to the VM to be processed. - */ -static bool balloonIsRequired(mapVMIter it) -{ + * @return bool True if ballooning is required, false if not. + * @param strUuid UUID of the specified machine. + */ +static bool balloonIsRequired(PVBOXBALLOONCTRL_MACHINE pMachine) +{ + AssertPtrReturn(pMachine, false); + /* Only do ballooning if we have a maximum balloon size set. */ - it->second.ulBalloonSizeMax = balloonGetMaxSize(it->second.machine); - - return it->second.ulBalloonSizeMax ? true : false; + pMachine->ulBalloonSizeMax = pMachine->machine.isNull() + ? 0 : balloonGetMaxSize(pMachine->machine); + + return pMachine->ulBalloonSizeMax ? true : false; } @@ -647,15 +748,18 @@ * * @return IPRT status code. - * @param it Iterator pointing to the VM to be processed. - */ -static int balloonUpdate(mapVMIterConst it) -{ + * @param strUuid UUID of the specified machine. + * @param pMachine Pointer to the machine's internal structure. + */ +static int balloonUpdate(const Bstr &strUuid, PVBOXBALLOONCTRL_MACHINE pMachine) +{ + AssertPtrReturn(pMachine, VERR_INVALID_POINTER); + /* * Get metrics collected at this point. */ LONG lMemFree, lBalloonCur; - int vrc = getMetric(it, L"Guest/RAM/Usage/Free", &lMemFree); + int vrc = getMetric(pMachine, L"Guest/RAM/Usage/Free", &lMemFree); if (RT_SUCCESS(vrc)) - vrc = getMetric(it, L"Guest/RAM/Usage/Balloon", &lBalloonCur); + vrc = getMetric(pMachine, L"Guest/RAM/Usage/Balloon", &lBalloonCur); if (RT_SUCCESS(vrc)) @@ -666,5 +770,5 @@ { #ifdef DEBUG - serviceLogVerbose(("%ls: No metrics available yet!\n", it->first.raw())); + serviceLogVerbose(("%ls: No metrics available yet!\n", strUuid.raw())); #endif return VINF_SUCCESS; @@ -675,9 +779,9 @@ serviceLogVerbose(("%ls: Balloon: %ld, Free mem: %ld, Max ballon: %ld\n", - it->first.raw(), - lBalloonCur, lMemFree, it->second.ulBalloonSizeMax)); + strUuid.raw(), + lBalloonCur, lMemFree, pMachine->ulBalloonSizeMax)); /* Calculate current balloon delta. */ - long lDelta = getlBalloonDelta(lBalloonCur, lMemFree, it->second.ulBalloonSizeMax); + long lDelta = getlBalloonDelta(lBalloonCur, lMemFree, pMachine->ulBalloonSizeMax); if (lDelta) /* Only do ballooning if there's really smth. to change ... */ { @@ -686,5 +790,5 @@ serviceLog("%ls: %s balloon by %ld to %ld ...\n", - it->first.raw(), + strUuid.raw(), lDelta > 0 ? "Inflating" : "Deflating", lDelta, lBalloonCur); @@ -692,5 +796,5 @@ /* Open a session for the VM. */ - CHECK_ERROR(it->second.machine, LockMachine(g_pSession, LockType_Shared)); + CHECK_ERROR(pMachine->machine, LockMachine(g_pSession, LockType_Shared)); do @@ -706,5 +810,5 @@ else serviceLog("Error: Unable to set new balloon size %ld for machine \"%ls\", rc=%Rhrc", - lBalloonCur, it->first.raw(), rc); + lBalloonCur, strUuid.raw(), rc); } while (0); @@ -715,5 +819,5 @@ else serviceLog("Error: Unable to retrieve metrics for machine \"%ls\", rc=%Rrc", - it->first.raw(), vrc); + strUuid.raw(), vrc); return vrc; } @@ -781,17 +885,7 @@ } - MachineState_T machineState; - CHECK_ERROR_BREAK(machines[i], COMGETTER(State)(&machineState)); - - if (g_fVerbose) - serviceLogVerbose(("Processing machine \"%ls\" (state: %ld)\n", - strUUID.raw(), machineState)); - - if (machineIsRunning(machineState)) - { - rc = machineAdd(machines[i]); - if (RT_FAILURE(rc)) - break; - } + rc = machineAdd(strUUID); + if (RT_FAILURE(rc)) + break; } } @@ -826,5 +920,5 @@ if (SUCCEEDED(hrc)) { - rc = machineUpdate(it, machineState); + rc = machineUpdate(it->first /* UUID */, machineState); if (RT_FAILURE(rc)) break;