Changeset 40358 in vbox

Timestamp:

Mar 5, 2012 2:40:52 PM (13 years ago)

Author:

vboxsync

Message:

Main/Metrics: All calls to VM processes are done in separate thread (#6029)

Location:

trunk/src/VBox

Files:

• Frontends/VBoxManage/VBoxManageMetrics.cpp (modified) (4 diffs)
• Main/include/Performance.h (modified) (16 diffs)
• Main/include/PerformanceImpl.h (modified) (2 diffs)
• Main/src-server/Performance.cpp (modified) (15 diffs)
• Main/src-server/PerformanceImpl.cpp (modified) (10 diffs)

trunk/src/VBox/Frontends/VBoxManage/VBoxManageMetrics.cpp

@@ -303 +303 @@
                      ComSafeArrayAsInParam(objects), period, samples,
                      ComSafeArrayAsOutParam(affectedMetrics)));
+    if (FAILED(rc))
+        return 2;
+
     if (listMatches)
         listAffectedMetrics(aVirtualBox,
@@ -475 +478 @@
                      ComSafeArrayAsInParam(objects), period, samples,
                      ComSafeArrayAsOutParam(affectedMetrics)));
+    if (FAILED(rc))
+        return 2;
+
     if (listMatches)
         listAffectedMetrics(aVirtualBox,
@@ -581 +587 @@
                       ComSafeArrayAsInParam(objects),
                       ComSafeArrayAsOutParam(affectedMetrics)));
+    if (FAILED(rc))
+        return 2;
+
     if (listMatches)
         listAffectedMetrics(aVirtualBox,
@@ -623 +632 @@
                        ComSafeArrayAsInParam(objects),
                        ComSafeArrayAsOutParam(affectedMetrics)));
+    if (FAILED(rc))
+        return 2;
+
     if (listMatches)
         listAffectedMetrics(aVirtualBox,

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

@@ -25 +25 @@
 #include <iprt/types.h>
 #include <iprt/err.h>
+#include <iprt/cpp/lock.h>
 
 #include <algorithm>
@@ -30 +31 @@
 #include <list>
 #include <vector>
+#include <queue>
 
 /* Forward decl. */
@@ -171 +173 @@
         GUESTSTATMASK_ALLOCVMM|GUESTSTATMASK_FREEVMM|
         GUESTSTATMASK_BALOONVMM|GUESTSTATMASK_SHAREDVMM;
+    const ULONG GUESTSTATS_ALL = GUESTSTATS_CPULOAD|GUESTSTATS_RAMUSAGE|GUESTSTATS_VMMRAM;
+
+    class CollectorGuest;
+
+    class CollectorGuestRequest
+    {
+    public:
+        CollectorGuestRequest()
+            : mCGuest(0) {};
+        virtual ~CollectorGuestRequest() {};
+        void setGuest(CollectorGuest *aGuest) { mCGuest = aGuest; };
+        CollectorGuest *getGuest() { return mCGuest; };
+        virtual int execute() = 0;
+
+        virtual void debugPrint(void *aObject, const char *aFunction, const char *aText) = 0;
+    protected:
+        CollectorGuest *mCGuest;
+        const char *mDebugName;
+    };
+
+    class CGRQEnable : public CollectorGuestRequest
+    {
+    public:
+        CGRQEnable(ULONG aMask)
+            : mMask(aMask) {};
+        int execute();
+
+        void debugPrint(void *aObject, const char *aFunction, const char *aText);
+    private:
+        ULONG mMask;
+    };
+
+    class CGRQDisable : public CollectorGuestRequest
+    {
+    public:
+        CGRQDisable(ULONG aMask)
+            : mMask(aMask) {};
+        int execute();
+
+        void debugPrint(void *aObject, const char *aFunction, const char *aText);
+    private:
+        ULONG mMask;
+    };
+
+    class CGRQAbort : public CollectorGuestRequest
+    {
+    public:
+        CGRQAbort() {};
+        int execute();
+
+        void debugPrint(void *aObject, const char *aFunction, const char *aText);
+    };
+
+    class CollectorGuestQueue
+    {
+    public:
+        CollectorGuestQueue();
+        ~CollectorGuestQueue();
+        void push(CollectorGuestRequest* rq);
+        CollectorGuestRequest* pop();
+    private:
+        RTCLockMtx mLockMtx;
+        RTSEMEVENT mEvent;
+        std::queue<CollectorGuestRequest*> mQueue;
+    };
+
+    class CollectorGuestManager;
 
     class CollectorGuest
@@ -178 +247 @@
         ~CollectorGuest();
 
-        bool isUnregistered()   { return mUnregistered; };
-        bool isEnabled()        { return mEnabled; };
-        bool isValid(ULONG mask){ return (mValid & mask) == mask; };
-        void invalidateStats()  { mValid = 0; };
-        void unregister()       { mUnregistered = true; };
+        void setManager(CollectorGuestManager *aManager)
+                                    { mManager = aManager; };
+        bool isUnregistered()       { return mUnregistered; };
+        bool isEnabled()            { return mEnabled != 0; };
+        bool isValid(ULONG mask)    { return (mValid & mask) == mask; };
+        void invalidate(ULONG mask) { mValid &= ~mask; };
+        void unregister()           { mUnregistered = true; };
         void updateStats(ULONG aValidStats, ULONG aCpuUser,
                          ULONG aCpuKernel, ULONG aCpuIdle,
@@ -190 +261 @@
                          ULONG aAllocVMM, ULONG aFreeVMM,
                          ULONG aBalloonedVMM, ULONG aSharedVMM);
-        int enable();
-        int disable();
-        int enableVMMStats(bool mCollectVMMStats);
+        int enable(ULONG mask);
+        int disable(ULONG mask);
+
+        int enqueueRequest(CollectorGuestRequest *aRequest);
+        int enableInternal(ULONG mask);
+        int disableInternal(ULONG mask);
+
+        const com::Utf8Str& getVMName() const { return mMachineName; };
 
         RTPROCESS getProcess()  { return mProcess; };
@@ -210 +286 @@
 
     private:
+        int enableVMMStats(bool mCollectVMMStats);
+
+        CollectorGuestManager *mManager;
+
         bool                 mUnregistered;
-        bool                 mEnabled;
+        ULONG                mEnabled;
         ULONG                mValid;
         Machine             *mMachine;
+        com::Utf8Str         mMachineName;
         RTPROCESS            mProcess;
         ComPtr<IConsole>     mConsole;
@@ -236 +317 @@
     {
     public:
-        CollectorGuestManager() : mVMMStatsProvider(NULL) {};
-        ~CollectorGuestManager() { Assert(mGuests.size() == 0); };
+        CollectorGuestManager();
+        ~CollectorGuestManager();
         void registerGuest(CollectorGuest* pGuest);
         void unregisterGuest(CollectorGuest* pGuest);
@@ -243 +324 @@
         void preCollect(CollectorHints& hints, uint64_t iTick);
         void destroyUnregistered();
-    private:
-        CollectorGuestList mGuests;
-        CollectorGuest    *mVMMStatsProvider;
+        int enqueueRequest(CollectorGuestRequest *aRequest);
+
+        CollectorGuest *getBlockedGuest() { return mGuestBeingCalled; };
+
+        static DECLCALLBACK(int) requestProcessingThread(RTTHREAD aThread, void *pvUser);
+    private:
+        RTTHREAD            mThread;
+        CollectorGuestList  mGuests;
+        CollectorGuest     *mVMMStatsProvider;
+        CollectorGuestQueue mQueue;
+        CollectorGuest     *mGuestBeingCalled;
     };
 
@@ -293 +382 @@
         bool collectorBeat(uint64_t nowAt);
 
-        void enable()  { mEnabled = true; };
-        void disable() { mEnabled = false; };
+        virtual int enable()  { mEnabled = true; return S_OK; };
+        virtual int disable() { mEnabled = false; return S_OK; };
         void unregister() { mUnregistered = true; };
 
@@ -398 +487 @@
     };
 
+#ifndef VBOX_COLLECTOR_TEST_CASE
     class HostRamVmm : public BaseMetric
     {
@@ -410 +500 @@
         void preCollect(CollectorHints& hints, uint64_t iTick);
         void collect();
+        int enable();
+        int disable();
         const char *getUnit() { return "kB"; };
         ULONG getMinValue() { return 0; };
@@ -426 +518 @@
         ULONG                  mSharedCurrent;
     };
+#endif /* VBOX_COLLECTOR_TEST_CASE */
 
     class MachineCpuLoad : public BaseMetric
@@ -480 +573 @@
 
 
+#ifndef VBOX_COLLECTOR_TEST_CASE
     class GuestCpuLoad : public BaseGuestMetric
     {
@@ -490 +584 @@
         void preCollect(CollectorHints& hints, uint64_t iTick);
         void collect();
+        int enable();
+        int disable();
         const char *getUnit() { return "%"; };
         ULONG getMinValue() { return 0; };
@@ -510 +606 @@
         void preCollect(CollectorHints& hints, uint64_t iTick);
         void collect();
+        int enable();
+        int disable();
         const char *getUnit() { return "kB"; };
         ULONG getMinValue() { return 0; };
@@ -517 +615 @@
         SubMetric *mTotal, *mFree, *mBallooned, *mCache, *mPagedTotal, *mShared;
     };
+#endif /* VBOX_COLLECTOR_TEST_CASE */
 
     /* Aggregate Functions **************************************************/

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

@@ -195 +195 @@
     void samplerCallback(uint64_t iTick);
 
+    const Utf8Str& getFailedGuestName();
+
     typedef std::list<pm::Metric*> MetricList;
     typedef std::list<pm::BaseMetric*> BaseMetricList;
@@ -204 +206 @@
 
     unsigned int mMagic;
+    const Utf8Str mUnknownGuest;
 
     struct Data

trunk/src/VBox/Main/src-server/Performance.cpp

@@ -109 +109 @@
 #ifndef VBOX_COLLECTOR_TEST_CASE
 
+CollectorGuestQueue::CollectorGuestQueue()
+{
+    mEvent = NIL_RTSEMEVENT;
+    RTSemEventCreate(&mEvent);
+}
+
+CollectorGuestQueue::~CollectorGuestQueue()
+{
+    RTSemEventDestroy(mEvent);
+}
+
+void CollectorGuestQueue::push(CollectorGuestRequest* rq)
+{
+    RTCLock lock(mLockMtx);
+
+    mQueue.push(rq);
+    RTSemEventSignal(mEvent);
+}
+
+CollectorGuestRequest* CollectorGuestQueue::pop()
+{
+    int rc = VINF_SUCCESS;
+    CollectorGuestRequest* rq = NULL;
+
+    do
+    {
+        {
+            RTCLock lock(mLockMtx);
+
+            if (!mQueue.empty())
+            {
+                rq = mQueue.front();
+                mQueue.pop();
+            }
+        }
+
+        if (rq)
+            return rq;
+        else
+            rc = RTSemEventWaitNoResume(mEvent, RT_INDEFINITE_WAIT);
+    }
+    while (RT_SUCCESS(rc));
+
+    return NULL;
+}
+
+int CGRQEnable::execute()
+{
+    Assert(mCGuest);
+    return mCGuest->enableInternal(mMask);
+}
+
+void CGRQEnable::debugPrint(void *aObject, const char *aFunction, const char *aText)
+{
+    LogAleksey(("{%p} " LOG_FN_FMT ": CGRQEnable(mask=0x%x) %s\n",
+                aObject, aFunction, mMask, aText));
+}
+
+int CGRQDisable::execute()
+{
+    Assert(mCGuest);
+    return mCGuest->disableInternal(mMask);
+}
+
+void CGRQDisable::debugPrint(void *aObject, const char *aFunction, const char *aText)
+{
+    LogAleksey(("{%p} " LOG_FN_FMT ": CGRQDisable(mask=0x%x) %s\n",
+                aObject, aFunction, mMask, aText));
+}
+
+int CGRQAbort::execute()
+{
+    return E_ABORT;
+}
+
+void CGRQAbort::debugPrint(void *aObject, const char *aFunction, const char *aText)
+{
+    LogAleksey(("{%p} " LOG_FN_FMT ": CGRQAbort %s\n",
+                aObject, aFunction, aText));
+}
+
 CollectorGuest::CollectorGuest(Machine *machine, RTPROCESS process) :
     mUnregistered(false), mEnabled(false), mValid(false), mMachine(machine), mProcess(process),
@@ -153 +234 @@
 }
 
-int CollectorGuest::enable()
-{
-    mEnabled = true;
-    /* Must make sure that the machine object does not get uninitialized
-     * in the middle of enabling this collector. Causes timing-related
-     * behavior otherwise, which we don't want. In particular the
-     * GetRemoteConsole call below can hang if the VM didn't completely
-     * terminate (the VM processes stop processing events shortly before
-     * closing the session). This avoids the hang. */
-    AutoCaller autoCaller(mMachine);
-    if (FAILED(autoCaller.rc())) return autoCaller.rc();
-
+int CollectorGuest::enable(ULONG mask)
+{
+    return enqueueRequest(new CGRQEnable(mask));
+}
+
+int CollectorGuest::disable(ULONG mask)
+{
+    return enqueueRequest(new CGRQDisable(mask));
+}
+
+int CollectorGuest::enableInternal(ULONG mask)
+{
     HRESULT ret = S_OK;
 
-    ComPtr<IInternalSessionControl> directControl;
-
-    ret = mMachine->getDirectControl(&directControl);
-    if (ret != S_OK)
-        return ret;
-
-    /* get the associated console; this is a remote call (!) */
-    ret = directControl->GetRemoteConsole(mConsole.asOutParam());
-    if (ret != S_OK)
-        return ret;
-
-    ret = mConsole->COMGETTER(Guest)(mGuest.asOutParam());
-    if (ret == S_OK)
-    {
-        ret = mGuest->COMSETTER(StatisticsUpdateInterval)(1 /* 1 sec */);
-        LogAleksey(("{%p} " LOG_FN_FMT ": Set guest statistics update interval to 1 sec (%s)\n",
+    if ((mEnabled & mask) == mask)
+        return E_UNEXPECTED;
+
+    if (!mEnabled)
+    {
+        /* Must make sure that the machine object does not get uninitialized
+         * in the middle of enabling this collector. Causes timing-related
+         * behavior otherwise, which we don't want. In particular the
+         * GetRemoteConsole call below can hang if the VM didn't completely
+         * terminate (the VM processes stop processing events shortly before
+         * closing the session). This avoids the hang. */
+        AutoCaller autoCaller(mMachine);
+        if (FAILED(autoCaller.rc())) return autoCaller.rc();
+
+        mMachineName = mMachine->getName();
+
+        ComPtr<IInternalSessionControl> directControl;
+
+        ret = mMachine->getDirectControl(&directControl);
+        if (ret != S_OK)
+            return ret;
+
+        /* get the associated console; this is a remote call (!) */
+        ret = directControl->GetRemoteConsole(mConsole.asOutParam());
+        if (ret != S_OK)
+            return ret;
+
+        ret = mConsole->COMGETTER(Guest)(mGuest.asOutParam());
+        if (ret == S_OK)
+        {
+            ret = mGuest->COMSETTER(StatisticsUpdateInterval)(1 /* 1 sec */);
+            LogAleksey(("{%p} " LOG_FN_FMT ": Set guest statistics update interval to 1 sec (%s)\n",
+                        this, __PRETTY_FUNCTION__, SUCCEEDED(ret)?"success":"failed"));
+        }
+    }
+    if ((mask & GUESTSTATS_VMMRAM) == GUESTSTATS_VMMRAM)
+        enableVMMStats(true);
+    mEnabled |= mask;
+
+    return ret;
+}
+
+int CollectorGuest::disableInternal(ULONG mask)
+{
+    if (!(mEnabled & mask))
+        return E_UNEXPECTED;
+
+    if ((mask & GUESTSTATS_VMMRAM) == GUESTSTATS_VMMRAM)
+        enableVMMStats(false);
+    mEnabled &= ~mask;
+    if (!mEnabled)
+    {
+        Assert(mGuest && mConsole);
+        HRESULT ret = mGuest->COMSETTER(StatisticsUpdateInterval)(0 /* off */);
+        NOREF(ret);
+        LogAleksey(("{%p} " LOG_FN_FMT ": Set guest statistics update interval to 0 sec (%s)\n",
                     this, __PRETTY_FUNCTION__, SUCCEEDED(ret)?"success":"failed"));
-    }
-
-    return ret;
-}
-
-int CollectorGuest::disable()
-{
-    mEnabled = false;
-    Assert(mGuest && mConsole);
-    HRESULT ret = mGuest->COMSETTER(StatisticsUpdateInterval)(0 /* off */);
-    NOREF(ret);
-    LogAleksey(("{%p} " LOG_FN_FMT ": Set guest statistics update interval to 0 sec (%s)\n",
-                this, __PRETTY_FUNCTION__, SUCCEEDED(ret)?"success":"failed"));
-    invalidateStats();
+        invalidate(GUESTSTATS_ALL);
+    }
 
     return S_OK;
+}
+
+int CollectorGuest::enqueueRequest(CollectorGuestRequest *aRequest)
+{
+    if (mManager)
+    {
+        aRequest->setGuest(this);
+        return mManager->enqueueRequest(aRequest);
+    }
+
+    LogAleksey(("{%p} " LOG_FN_FMT ": Attempted enqueue guest request when mManager is null\n",
+                this, __PRETTY_FUNCTION__));
+    return E_POINTER;
 }
 
@@ -235 +357 @@
 }
 
-void CollectorGuestManager::preCollect(CollectorHints& hints, uint64_t /* iTick */)
-{
-    /*
-     * Since we are running without a lock the value of mVMMStatsProvider
-     * can change at any moment. In the worst case we won't collect any data.
-     */
-    CollectorGuestList::iterator it;
-
-    LogAleksey(("{%p} " LOG_FN_FMT ": provider=%p ramvmm=%s\n",
-                this, __PRETTY_FUNCTION__, mVMMStatsProvider, hints.isHostRamVmmCollected()?"y":"n"));
-    for (it = mGuests.begin(); it != mGuests.end(); it++)
-    {
-        LogAleksey(("{%p} " LOG_FN_FMT ": it=%p pid=%d gueststats=%s...\n",
-                    this, __PRETTY_FUNCTION__, *it, (*it)->getProcess(),
-                    hints.isGuestStatsCollected((*it)->getProcess())?"y":"n"));
-        if ((*it)->isUnregistered())
-            continue;
-        if (  (hints.isHostRamVmmCollected() && *it == mVMMStatsProvider)
-            || hints.isGuestStatsCollected((*it)->getProcess()))
-        {
-            /* Guest stats collection needs to be enabled */
-            if ((*it)->isEnabled())
-            {
-                /* Already enabled, collect the data */
-                /*
-                 * Actually the data will be pushed by Guest object, so
-                 * we don't need to do anything here.
-                 */
-                //(*it)->updateStats();
-            }
-            else
-            {
-                (*it)->invalidateStats();
-                (*it)->enable();
-                (*it)->enableVMMStats(*it == mVMMStatsProvider);
-            }
-        }
-        else
-        {
-            /* Guest stats collection needs to be disabled */
-            if ((*it)->isEnabled())
-                (*it)->disable();
-        }
+CollectorGuestManager::CollectorGuestManager()
+  : mVMMStatsProvider(NULL), mGuestBeingCalled(NULL)
+{
+    int rc = RTThreadCreate(&mThread, CollectorGuestManager::requestProcessingThread,
+                            this, 0, RTTHREADTYPE_MAIN_WORKER, RTTHREADFLAGS_WAITABLE,
+                            "CGMgr");
+    LogAleksey(("{%p} " LOG_FN_FMT ": RTThreadCreate returned %u (mThread=%p)\n",
+                this, __PRETTY_FUNCTION__, rc));
+}
+
+CollectorGuestManager::~CollectorGuestManager()
+{
+    Assert(mGuests.size() == 0);
+    int rcThread = 0;
+    int rc = enqueueRequest(new CGRQAbort());
+    if (SUCCEEDED(rc))
+    {
+        /* We wait only if we were able to put the abort request to a queue */
+        LogAleksey(("{%p} " LOG_FN_FMT ": Waiting for CGM request processing thread to stop...\n",
+                    this, __PRETTY_FUNCTION__));
+        rc = RTThreadWait(mThread, 1000 /* 1 sec */, &rcThread);
+        LogAleksey(("{%p} " LOG_FN_FMT ": RTThreadWait returned %u (thread exit code: %u)\n",
+                    this, __PRETTY_FUNCTION__, rc, rcThread));
     }
 }
@@ -283 +385 @@
 void CollectorGuestManager::registerGuest(CollectorGuest* pGuest)
 {
+    pGuest->setManager(this);
     mGuests.push_back(pGuest);
     /*
@@ -296 +399 @@
 void CollectorGuestManager::unregisterGuest(CollectorGuest* pGuest)
 {
+    int rc = S_OK;
+
     LogAleksey(("{%p} " LOG_FN_FMT ": About to unregister guest=%p provider=%p\n",
                 this, __PRETTY_FUNCTION__, pGuest, mVMMStatsProvider));
@@ -317 +422 @@
                 /* Found the guest already collecting stats, elect it */
                 mVMMStatsProvider = *it;
-                mVMMStatsProvider->enableVMMStats(true);
+                rc = mVMMStatsProvider->enqueueRequest(new CGRQEnable(GUESTSTATS_VMMRAM));
+                if (FAILED(rc))
+                {
+                    /* This is not a good candidate -- try to find another */
+                    mVMMStatsProvider = NULL;
+                    continue;
+                }
                 break;
             }
-            else if (!mVMMStatsProvider)
+        }
+        if (!mVMMStatsProvider)
+        {
+            /* If nobody collects stats, take the first registered */
+            for (it = mGuests.begin(); it != mGuests.end(); it++)
             {
-                /* If nobody collects stats, take the first registered */
+                /* Skip unregistered as they are about to be destroyed */
+                if ((*it)->isUnregistered())
+                    continue;
+
                 mVMMStatsProvider = *it;
-                /*
-                 * No need to notify the guest at this point as it will be
-                 * done in CollectorGuestManager::preCollect when it gets
-                 * enabled.
-                 */
+                //mVMMStatsProvider->enable(GUESTSTATS_VMMRAM);
+                rc = mVMMStatsProvider->enqueueRequest(new CGRQEnable(GUESTSTATS_VMMRAM));
+                if (SUCCEEDED(rc))
+                    break;
+                /* This was not a good candidate -- try to find another */
+                mVMMStatsProvider = NULL;
             }
         }
@@ -351 +470 @@
             ++it;
 }
+
+int CollectorGuestManager::enqueueRequest(CollectorGuestRequest *aRequest)
+{
+#ifdef DEBUG
+    aRequest->debugPrint(this, __PRETTY_FUNCTION__, "added to CGM queue");
+#endif /* DEBUG */
+    /*
+     * It is very unlikely that we will get high frequency calls to configure
+     * guest metrics collection, so we rely on this fact to detect blocked
+     * guests. If the guest has not finished processing the previous request
+     * we consider it blocked.
+     */
+    if (aRequest->getGuest() && aRequest->getGuest() == mGuestBeingCalled)
+    {
+        /* Request execution got stalled for this guest -- report an error */
+        return E_FAIL;
+    }
+    mQueue.push(aRequest);
+    return S_OK;
+}
+
+/* static */
+DECLCALLBACK(int) CollectorGuestManager::requestProcessingThread(RTTHREAD /* aThread */, void *pvUser)
+{
+    CollectorGuestRequest *pReq;
+    CollectorGuestManager *mgr = static_cast<CollectorGuestManager*>(pvUser);
+
+    HRESULT rc = S_OK;
+
+    LogAleksey(("{%p} " LOG_FN_FMT ": Starting request processing loop...p\n",
+                mgr, __PRETTY_FUNCTION__));
+    while ((pReq = mgr->mQueue.pop()) != NULL)
+    {
+#ifdef DEBUG
+        pReq->debugPrint(mgr, __PRETTY_FUNCTION__, "is being executed...");
+#endif /* DEBUG */
+        mgr->mGuestBeingCalled = pReq->getGuest();
+        rc = pReq->execute();
+        mgr->mGuestBeingCalled = NULL;
+        delete pReq;
+        if (rc == E_ABORT)
+            break;
+        if (FAILED(rc))
+            LogAleksey(("{%p} " LOG_FN_FMT ": request::execute returned %u\n",
+                        mgr, __PRETTY_FUNCTION__, rc));
+    }
+    LogAleksey(("{%p} " LOG_FN_FMT ": Exiting request processing loop... rc=%u\n",
+                        mgr, __PRETTY_FUNCTION__, rc));
+
+    return VINF_SUCCESS;
+}
+
 
 #endif /* !VBOX_COLLECTOR_TEST_CASE */
@@ -472 +643 @@
 }
 
+#ifndef VBOX_COLLECTOR_TEST_CASE
 void HostRamVmm::init(ULONG period, ULONG length)
 {
@@ -482 +654 @@
 }
 
+int HostRamVmm::enable()
+{
+    int rc = S_OK;
+    CollectorGuest *provider = mCollectorGuestManager->getVMMStatsProvider();
+    if (provider)
+        rc = provider->enable(GUESTSTATS_VMMRAM);
+    BaseMetric::enable();
+    return rc;
+}
+
+int HostRamVmm::disable()
+{
+    int rc = S_OK;
+    BaseMetric::disable();
+    CollectorGuest *provider = mCollectorGuestManager->getVMMStatsProvider();
+    if (provider)
+        rc = provider->disable(GUESTSTATS_VMMRAM);
+    return rc;
+}
+
 void HostRamVmm::preCollect(CollectorHints& hints, uint64_t /* iTick */)
 {
@@ -502 +694 @@
             mBalloonedCurrent = provider->getBalloonedVMM();
             mSharedCurrent    = provider->getSharedVMM();
-        }
+            provider->invalidate(GUESTSTATS_VMMRAM);
+        }
+        /*
+         * Note that if there are no new values from the provider we will use
+         * the ones most recently provided instead of zeros, which is probably
+         * a desirable behavior.
+         */
     }
     else
@@ -519 +717 @@
     mSharedVMM->put(mSharedCurrent);
 }
+#endif /* !VBOX_COLLECTOR_TEST_CASE */
 
 
@@ -592 +791 @@
 
 
+#ifndef VBOX_COLLECTOR_TEST_CASE
 void GuestCpuLoad::init(ULONG period, ULONG length)
 {
@@ -614 +814 @@
         mKernel->put((ULONG)(PM_CPU_LOAD_MULTIPLIER * mCGuest->getCpuKernel()) / 100);
         mIdle->put((ULONG)(PM_CPU_LOAD_MULTIPLIER * mCGuest->getCpuIdle()) / 100);
-    }
+        mCGuest->invalidate(GUESTSTATS_CPULOAD);
+    }
+}
+
+int GuestCpuLoad::enable()
+{
+    int rc = mCGuest->enable(GUESTSTATS_CPULOAD);
+    BaseMetric::enable();
+    return rc;
+}
+
+int GuestCpuLoad::disable()
+{
+    BaseMetric::disable();
+    return mCGuest->disable(GUESTSTATS_CPULOAD);
 }
 
@@ -630 +844 @@
 }
 
-void GuestRamUsage::preCollect(CollectorHints& hints,  uint64_t /* iTick */)
-{
-    hints.collectGuestStats(mCGuest->getProcess());
-}
-
 void GuestRamUsage::collect()
 {
@@ -645 +854 @@
         mCache->put(mCGuest->getMemCache());
         mPagedTotal->put(mCGuest->getPageTotal());
-    }
-}
+        mCGuest->invalidate(GUESTSTATS_RAMUSAGE);
+    }
+}
+
+int GuestRamUsage::enable()
+{
+    int rc = mCGuest->enable(GUESTSTATS_RAMUSAGE);
+    BaseMetric::enable();
+    return rc;
+}
+
+int GuestRamUsage::disable()
+{
+    BaseMetric::disable();
+    return mCGuest->disable(GUESTSTATS_RAMUSAGE);
+}
+
+void GuestRamUsage::preCollect(CollectorHints& hints,  uint64_t /* iTick */)
+{
+    hints.collectGuestStats(mCGuest->getProcess());
+}
+#endif /* !VBOX_COLLECTOR_TEST_CASE */
 
 void CircularBuffer::init(ULONG ulLength)

trunk/src/VBox/Main/src-server/PerformanceImpl.cpp

@@ -139 +139 @@
 ////////////////////////////////////////////////////////////////////////////////
 
-PerformanceCollector::PerformanceCollector() : mMagic(0) {}
+PerformanceCollector::PerformanceCollector()
+  : mMagic(0), mUnknownGuest("unknown guest")
+{
+}
 
 PerformanceCollector::~PerformanceCollector() {}
@@ -299 +302 @@
 }
 
+const Utf8Str& PerformanceCollector::getFailedGuestName()
+{
+    pm::CollectorGuest *pGuest = m.gm->getBlockedGuest();
+    if (pGuest)
+        return pGuest->getVMName();
+    return mUnknownGuest;
+}
+
 STDMETHODIMP PerformanceCollector::GetMetrics(ComSafeArrayIn(IN_BSTR, metricNames),
                                               ComSafeArrayIn(IUnknown *, objects),
@@ -367 +378 @@
                 LogFlow (("PerformanceCollector::SetupMetrics() disabling %s\n",
                           (*it)->getName()));
-                (*it)->disable();
+                rc = (*it)->disable();
+                if (FAILED(rc))
+                    break;
             }
             else
@@ -373 +386 @@
                 LogFlow (("PerformanceCollector::SetupMetrics() enabling %s\n",
                           (*it)->getName()));
-                (*it)->enable();
+                rc = (*it)->enable();
+                if (FAILED(rc))
+                    break;
             }
             filteredMetrics.push_back(*it);
@@ -386 +401 @@
 
     LogFlowThisFuncLeave();
+
+    if (FAILED(rc))
+        return setError(E_FAIL, "Failed to setup metrics for '%s'",
+                        getFailedGuestName().c_str());
     return rc;
 }
@@ -409 +428 @@
         if (filter.match((*it)->getObject(), (*it)->getName()))
         {
-            (*it)->enable();
+            rc = (*it)->enable();
+            if (FAILED(rc))
+                break;
             filteredMetrics.push_back(*it);
         }
@@ -421 +442 @@
 
     LogFlowThisFuncLeave();
+
+    if (FAILED(rc))
+        return setError(E_FAIL, "Failed to enable metrics for '%s'",
+                        getFailedGuestName().c_str());
     return rc;
 }
@@ -444 +469 @@
         if (filter.match((*it)->getObject(), (*it)->getName()))
         {
-            (*it)->disable();
+            rc = (*it)->disable();
+            if (FAILED(rc))
+                break;
             filteredMetrics.push_back(*it);
         }
@@ -456 +483 @@
 
     LogFlowThisFuncLeave();
+
+    if (FAILED(rc))
+        return setError(E_FAIL, "Failed to disable metrics for '%s'",
+                        getFailedGuestName().c_str());
     return rc;
 }
@@ -694 +725 @@
     /* Let know the platform specific code what is being collected */
     m.hal->preCollect(hints, iTick);
+#if 0
+    /* Guest stats are now pushed by guests themselves */
     /* Collect the data in bulk from all hinted guests */
     m.gm->preCollect(hints, iTick);
+#endif
 
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);