Changeset 28887 in vbox

Timestamp:

Apr 29, 2010 11:19:17 AM (15 years ago)

Author:

vboxsync

Message:

Guest Control/Main: Faster (no polling on guest side), free data on shutdown.

Location:

trunk

Files:

• include/VBox/HostServices/GuestControlSvc.h (modified) (1 diff)
• src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3LibGuestCtrl.cpp (modified) (2 diffs)
• src/VBox/Additions/common/VBoxService/VBoxService.cpp (modified) (2 diffs)
• src/VBox/Additions/common/VBoxService/VBoxServiceControl.cpp (modified) (3 diffs)
• src/VBox/HostServices/GuestControl/service.cpp (modified) (12 diffs)
• src/VBox/Main/GuestImpl.cpp (modified) (4 diffs)

trunk/include/VBox/HostServices/GuestControlSvc.h

@@ -196 +196 @@
 typedef struct _VBoxGuestCtrlHGCMMsgType
 {
-    VBoxGuestHGCMCallInfoTimed hdr;
+    VBoxGuestHGCMCallInfo hdr;
 
     /**

trunk/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3LibGuestCtrl.cpp

@@ -110 +110 @@
     VBoxGuestCtrlHGCMMsgType Msg;
 
-    Msg.hdr.u32Timeout = u32Timeout;
-    Msg.hdr.fInterruptible = true;
-
-    Msg.hdr.info.result = VERR_WRONG_ORDER;
-    Msg.hdr.info.u32ClientID = u32ClientId;
-    Msg.hdr.info.u32Function = GUEST_GET_HOST_MSG; /* Tell the host we want our next command. */
-    Msg.hdr.info.cParms = 2;                       /* Just peek for the next message! */
+    Msg.hdr.result = VERR_WRONG_ORDER;
+    Msg.hdr.u32ClientID = u32ClientId;
+    Msg.hdr.u32Function = GUEST_GET_HOST_MSG; /* Tell the host we want our next command. */
+    Msg.hdr.cParms = 2;                       /* Just peek for the next message! */
 
     VbglHGCMParmUInt32Set(&Msg.msg, 0);
     VbglHGCMParmUInt32Set(&Msg.num_parms, 0);
 
-    int rc = vbglR3DoIOCtl(VBOXGUEST_IOCTL_HGCM_CALL_TIMED(sizeof(Msg)), &Msg, sizeof(Msg));
+    int rc = vbglR3DoIOCtl(VBOXGUEST_IOCTL_HGCM_CALL(sizeof(Msg)), &Msg, sizeof(Msg));
     if (RT_SUCCESS(rc))
     {
@@ -128 +125 @@
             rc = VbglHGCMParmUInt32Get(&Msg.num_parms, puNumParms);
             if (RT_SUCCESS(rc))
-                rc = Msg.hdr.info.result;
+                rc = Msg.hdr.result;
                 /* Ok, so now we know what message type and how much parameters there are. */
     }

trunk/src/VBox/Additions/common/VBoxService/VBoxService.cpp

@@ -298 +298 @@
 {
     int rc;
-
     /*
      * Initialize the services.
@@ -314 +313 @@
             }
         }
-
     /*
      * Start the service(s).

trunk/src/VBox/Additions/common/VBoxService/VBoxServiceControl.cpp

@@ -239 +239 @@
         VBoxServiceVerbose(4, "Control: Waiting for host msg ...\n");
         rc = VbglR3GuestCtrlGetHostMsg(g_GuestControlSvcClientID, &uMsg, &uNumParms, 1000 /* 1s timeout */);
-        if (rc == VERR_TOO_MUCH_DATA)
-        {
-            VBoxServiceVerbose(3, "Control: Message requires %ld parameters, but only 2 supplied -- retrying request ...\n", uNumParms);
-            rc = VINF_SUCCESS;
-        }
-        else if (rc == VERR_TIMEOUT)
-        {
-            VBoxServiceVerbose(3, "Control: Wait timed out, waiting for next round ...\n");
-            RTThreadSleep(100);
-        }
+        if (RT_FAILURE(rc))
+        {
+            if (rc == VERR_TOO_MUCH_DATA)
+            {
+                VBoxServiceVerbose(3, "Control: Message requires %ld parameters, but only 2 supplied -- retrying request ...\n", uNumParms);
+                rc = VINF_SUCCESS; /* Try to get "real" message in next block below. */
+            }
+            else
+                VBoxServiceVerbose(3, "Control: Getting host message failed with %Rrc\n", rc); /* VERR_GEN_IO_FAILURE seems to be normal if ran  into timeout. */
+        }
+
         if (RT_SUCCESS(rc))
         {
@@ -272 +273 @@
         }
 
-        /*
-         * Block for a while.
-         *
-         * The event semaphore takes care of ignoring interruptions and it
-         * allows us to implement service wakeup later.
-         */
+        /* Do we need to shutdown? */
         if (*pfShutdown)
         {
@@ -283 +279 @@
             break;
         }
-        int rc2 = RTSemEventMultiWait(g_hControlEvent, g_ControlInterval);
-        if (*pfShutdown)
-        {
-            rc = 0;
-            break;
-        }
-        if (rc2 != VERR_TIMEOUT && RT_FAILURE(rc2))
-        {
-            VBoxServiceError("Control: RTSemEventMultiWait failed; rc2=%Rrc\n", rc2);
-            rc = rc2;
-            break;
-        }
+
+        /* Let's sleep for a bit and let others run ... */
+        RTThreadYield();
     }
 

trunk/src/VBox/HostServices/GuestControl/service.cpp

@@ -55 +55 @@
     VBOXGUESTCTRPARAMBUFFER parmBuf;
 };
-/** The host cmd list type */
-typedef std::list <HostCmd> HostCmdList;
+/** The host cmd list + iterator type */
+typedef std::list< HostCmd > HostCmdList;
+typedef std::list< HostCmd >::iterator HostCmdListIter;
+typedef std::list< HostCmd >::const_iterator HostCmdListIterConst;
 
 /**
@@ -65 +67 @@
     /** The call handle */
     VBOXHGCMCALLHANDLE mHandle;
-    /** The function that was requested */
-    uint32_t mFunction;
     /** The call parameters */
     VBOXHGCMSVCPARM *mParms;
     /** Number of parameters */
     uint32_t mNumParms;
-    /** The default return value, used for passing warnings */
-    int mRc;
 
     /** The standard constructor */
-    GuestCall() : mFunction(0) {}
+    GuestCall() : mHandle(0), mParms(NULL), mNumParms(0) {}
     /** The normal contructor */
-    GuestCall(VBOXHGCMCALLHANDLE aHandle, uint32_t aFunction,
-              VBOXHGCMSVCPARM aParms[], int cParms, int aRc)
-              : mHandle(aHandle), mFunction(aFunction), mParms(aParms),
-                mNumParms(cParms), mRc(aRc) {}
+    GuestCall(VBOXHGCMCALLHANDLE aHandle, VBOXHGCMSVCPARM aParms[], int cParms)
+              : mHandle(aHandle), mParms(aParms), mNumParms(cParms) {}
 };
 /** The guest call list type */
@@ -110 +106 @@
     void *mpvHostData;
     /** The deferred calls list */
-    CallList mGuestWaiters;
+    CallList mClientList;
     /** The host command list */
     HostCmdList mHostCmds;
@@ -238 +234 @@
     int clientConnect(uint32_t u32ClientID, void *pvClient);
     int clientDisconnect(uint32_t u32ClientID, void *pvClient);
-    int processHostMsg(VBOXHGCMCALLHANDLE callHandle, uint32_t cParms, VBOXHGCMSVCPARM paParms[]);
-    int notifyGuest(GuestCall *pCall, uint32_t eFunction, uint32_t cParms, VBOXHGCMSVCPARM paParms[]);
+    int sendHostCmdToGuest(HostCmd *pCmd, VBOXHGCMCALLHANDLE callHandle, uint32_t cParms, VBOXHGCMSVCPARM paParms[]);
+    int retrieveNextHostCmd(VBOXHGCMCALLHANDLE callHandle, uint32_t cParms, VBOXHGCMSVCPARM paParms[]);
     int notifyHost(uint32_t eFunction, uint32_t cParms, VBOXHGCMSVCPARM paParms[]);
-    int processCmd(uint32_t eFunction, uint32_t cParms, VBOXHGCMSVCPARM paParms[]);
+    int processHostCmd(uint32_t eFunction, uint32_t cParms, VBOXHGCMSVCPARM paParms[]);
     void call(VBOXHGCMCALLHANDLE callHandle, uint32_t u32ClientID,
               void *pvClient, uint32_t eFunction, uint32_t cParms,
@@ -399 +395 @@
 }
 
+int Service::sendHostCmdToGuest(HostCmd *pCmd, VBOXHGCMCALLHANDLE callHandle, uint32_t cParms, VBOXHGCMSVCPARM paParms[])
+{
+    AssertPtr(pCmd);
+    int rc;
+  
+    /* Sufficient parameter space? */
+    if (pCmd->parmBuf.uParmCount > cParms)
+    {
+        paParms[0].setUInt32(pCmd->parmBuf.uMsg);       /* Message ID */
+        paParms[1].setUInt32(pCmd->parmBuf.uParmCount); /* Required parameters for message */
+        
+        /*
+        * So this call apparently failed because the guest wanted to peek
+        * how much parameters it has to supply in order to successfully retrieve
+        * this command. Let's tell him so!
+        */
+        rc = VERR_TOO_MUCH_DATA;
+    }
+    else
+    {
+        rc = paramBufferAssign(&pCmd->parmBuf, cParms, paParms);
+    }
+    return rc;
+}
+
 /*
  * Either fills in parameters from a pending host command into our guest context or
  * defer the guest call until we have something from the host.
  */
-int Service::processHostMsg(VBOXHGCMCALLHANDLE callHandle, uint32_t cParms, VBOXHGCMSVCPARM paParms[])
+int Service::retrieveNextHostCmd(VBOXHGCMCALLHANDLE callHandle, uint32_t cParms, VBOXHGCMSVCPARM paParms[])
 {
     int rc = VINF_SUCCESS;
 
-    if (cParms < 2)
-    {
-        LogFlowFunc(("Guest parameter buffer is too small!\n"));
-        rc = VERR_INVALID_PARAMETER;
+    /*
+     * If host command list is empty (nothing to do right now) just
+     * defer the call until we got something to do (makes the client
+     * wait, depending on the flags set).
+     */
+    if (mHostCmds.empty()) /* If command list is empty, defer ... */
+    {
+        mClientList.push_back(GuestCall(callHandle, paParms, cParms));
+        rc = VINF_HGCM_ASYNC_EXECUTE;
     }
     else
     {
         /*
-         * If host command list is empty (nothing to do right now) just
-         * defer the call until we got something to do (makes the client
-         * wait, depending on the flags set).
+         * Get the next unassigned host command in the list.
          */
-        if (mHostCmds.empty()) /* Command list is empty, defer ... */
-            rc = VINF_HGCM_ASYNC_EXECUTE;
-
-        if (rc != VINF_HGCM_ASYNC_EXECUTE)
-        {
-            /*
-             * Get the next unassigned host command in the list.
-             */
-             HostCmd curCmd = mHostCmds.front();
-             uint32_t uParmCount = curCmd.parmBuf.uParmCount;
-
-             /* Sufficient parameter space? */
-             if (uParmCount > cParms)
-             {
-                 paParms[0].setUInt32(curCmd.parmBuf.uMsg); /* Message ID */
-                 paParms[1].setUInt32(uParmCount); /* Required parameters for message */
-
-                 /*
-                  * So this call apparently failed because the guest wanted to peek
-                  * how much parameters it has to supply in order to successfully retrieve
-                  * this command. Let's tell him so!
-                  */
-                 rc = VERR_TOO_MUCH_DATA;
-             }
-             else
-             {
-                 rc = paramBufferAssign(&curCmd.parmBuf, cParms, paParms);
-                 if (RT_SUCCESS(rc))
-                 {
-                     paramBufferFree(&curCmd.parmBuf);
-                     mHostCmds.pop_front();
-                 }
-             }
-        }
-        else
-        {
-            /* Call is deferred because of reasons above. */
-            mGuestWaiters.push_back(GuestCall(callHandle, GUEST_GET_HOST_MSG,
-                                              paParms, cParms, rc));
-        }
-    }
-    return rc;
-}
-
-/*
- * Sends a command notification to the first waiting (deferred) client/guest in line in
- * order to wake up and do some work.
- */
-int Service::notifyGuest(GuestCall *pCall, uint32_t eFunction, uint32_t cParms, VBOXHGCMSVCPARM paParms[])
-{
-    AssertPtr(pCall);
-    int rc = VINF_SUCCESS;
-
-    int rc2 = processHostMsg(pCall->mHandle, pCall->mNumParms, pCall->mParms);
-    if (RT_SUCCESS(rc2))
-        rc2 = pCall->mRc;
-    AssertPtr(mpHelpers);
-    mpHelpers->pfnCallComplete(pCall->mHandle, rc2);
+         HostCmd curCmd = mHostCmds.front();
+         rc = sendHostCmdToGuest(&curCmd, callHandle, cParms, paParms);
+         if (RT_SUCCESS(rc))
+         {
+             /* Only if the guest really got and understood the message 
+              * remove it from the list. */
+             paramBufferFree(&curCmd.parmBuf);
+             mHostCmds.pop_front();
+         }
+    }
     return rc;
 }
@@ -523 +499 @@
 }
 
-int Service::processCmd(uint32_t eFunction, uint32_t cParms, VBOXHGCMSVCPARM paParms[])
+int Service::processHostCmd(uint32_t eFunction, uint32_t cParms, VBOXHGCMSVCPARM paParms[])
 {
     int rc = VINF_SUCCESS;
 
-    /* Some lazy guests to wake up which can process this command right now? */
-    if (!mGuestWaiters.empty())
-    {
-        HostCmd newCmd;
-        rc = paramBufferAllocate(&newCmd.parmBuf, eFunction, cParms, paParms);
-        if (RT_SUCCESS(rc))
+    HostCmd newCmd;
+    bool fProcessed = false;
+    rc = paramBufferAllocate(&newCmd.parmBuf, eFunction, cParms, paParms);
+    if (RT_SUCCESS(rc))
+    {
+        /* Can we wake up a waiting client on guest? */
+        if (!mClientList.empty())
+        {
+            GuestCall guest = mClientList.front();
+            rc = sendHostCmdToGuest(&newCmd, 
+                             guest.mHandle, guest.mNumParms, guest.mParms);
+
+            /* In any case the client did something, so wake up and remove from list. */
+            mpHelpers->pfnCallComplete(guest.mHandle, rc);
+            mClientList.pop_front();                       
+
+            /* If command was understood by client, free and remove from host commands list. */
+            if (RT_SUCCESS(rc))
+            {
+                paramBufferFree(&newCmd.parmBuf);
+                fProcessed = true;
+            }
+            else if (rc == VERR_TOO_MUCH_DATA)
+            {
+                /* If we got VERR_TOO_MUCH_DATA we buffer the host command in the next block
+                 * and return success to the host. */
+                rc = VINF_SUCCESS;
+            }
+        }
+
+        /* If not processed, buffer it ... */
+        if (!fProcessed)
         {
             mHostCmds.push_back(newCmd);
@@ -539 +541 @@
             if (mHostCmds.size() > 256) /** @todo Use a define! */
                 mHostCmds.pop_front();
-
-            GuestCall curCall = mGuestWaiters.front();
-            rc = notifyGuest(&curCall, eFunction, cParms, paParms);
-            mGuestWaiters.pop_front();
-        }
-    }
-    else /* No guest waiting, don't bother ... */
-        rc = VERR_TIMEOUT;
+        }
+    }
     return rc;
 }
@@ -573 +569 @@
             case GUEST_GET_HOST_MSG:
                 LogFlowFunc(("GUEST_GET_HOST_MSG\n"));
-                rc = processHostMsg(callHandle, cParms, paParms);
+                rc = retrieveNextHostCmd(callHandle, cParms, paParms);
                 break;
 
@@ -592 +588 @@
                 break;
         }
-        /*
-         * If current call is not deferred, call the completion function.
-         */
         if (rc != VINF_HGCM_ASYNC_EXECUTE)
         {
+            /* Tell the client that the call is complete (unblocks waiting). */
             mpHelpers->pfnCallComplete(callHandle, rc);
         }
@@ -624 +618 @@
             case HOST_EXEC_CMD:
                 LogFlowFunc(("HOST_EXEC_CMD\n"));
-                rc = processCmd(eFunction, cParms, paParms);
+                rc = processHostCmd(eFunction, cParms, paParms);
                 break;
 
@@ -634 +628 @@
             case HOST_EXEC_GET_OUTPUT:
                 LogFlowFunc(("HOST_EXEC_GET_OUTPUT\n"));
-                rc = processCmd(eFunction, cParms, paParms);
+                rc = processHostCmd(eFunction, cParms, paParms);
                 break;
 
@@ -653 +647 @@
 int Service::uninit()
 {
-    int rc = VINF_SUCCESS;
-    return rc;
+    /* Free allocated buffered host commands. */
+    HostCmdListIter it;
+    for (it = mHostCmds.begin(); it != mHostCmds.end(); it++)
+        paramBufferFree(&it->parmBuf);
+    mHostCmds.clear();
+
+    return VINF_SUCCESS;
 }
 

trunk/src/VBox/Main/GuestImpl.cpp

@@ -839 +839 @@
                         unsigned cMsWait;
                         if (aTimeoutMS == RT_INDEFINITE_WAIT)
-                            cMsWait = 1000;
+                            cMsWait = 10;
                         else
                         {
@@ -845 +845 @@
                             if (cMsElapsed >= aTimeoutMS)
                                 break; /* Timed out. */
-                            cMsWait = RT_MIN(1000, aTimeoutMS - (uint32_t)cMsElapsed);
+                            cMsWait = RT_MIN(10, aTimeoutMS - (uint32_t)cMsElapsed);
                         }
-                        RTThreadYield();
+                        RTThreadSleep(cMsWait);
                     }
                 }
@@ -1035 +1035 @@
                     unsigned cMsWait;
                     if (aTimeoutMS == RT_INDEFINITE_WAIT)
-                        cMsWait = 1000;
+                        cMsWait = 10;
                     else
                     {
@@ -1041 +1041 @@
                         if (cMsElapsed >= aTimeoutMS)
                             break; /* timed out */
-                        cMsWait = RT_MIN(1000, aTimeoutMS - (uint32_t)cMsElapsed);
-                    }
-                    RTThreadYield();
+                        cMsWait = RT_MIN(10, aTimeoutMS - (uint32_t)cMsElapsed);
+                    }
+                    RTThreadSleep(cMsWait);
                 }