Changeset 39437 in vbox

Timestamp:

Nov 28, 2011 1:31:03 PM (13 years ago)

Author:

vboxsync

Message:

VBoxService/GuestCtrl: Adjusted guest process error reporting.

Location:

trunk/src/VBox/Additions/common/VBoxService

Files:

• VBoxServiceControl.cpp (modified) (2 diffs)
• VBoxServiceControlThread.cpp (modified) (3 diffs)

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

@@ -262 +262 @@
 #endif
 
-    if (cParms != 11)
-        return VERR_INVALID_PARAMETER;
-
-    int rc = VbglR3GuestCtrlExecGetHostCmdExec(idClient,
+    int rc;
+    bool fStartAllowed = false; /* Flag indicating whether starting a process is allowed or not. */
+    if (cParms == 11)
+    {
+        rc = VbglR3GuestCtrlExecGetHostCmdExec(idClient,
                                                cParms,
                                                &uContextID,
@@ -281 +282 @@
                                                /* Timelimit */
                                                &uTimeLimitMS);
-    if (RT_SUCCESS(rc))
-    {
-#ifdef DEBUG
-        VBoxServiceVerbose(3, "Control: Start process szCmd=%s, uFlags=%u, szArgs=%s, szEnv=%s, szUser=%s, szPW=%s, uTimeout=%u\n",
-                           szCmd, uFlags, cArgs ? szArgs : "<None>", cEnvVars ? szEnv : "<None>", szUser, szPassword, uTimeLimitMS);
-#endif
-        bool fAllowed = false;
-        rc = VBoxServiceControlStartAllowed(&fAllowed);
-        if (RT_FAILURE(rc))
-            VBoxServiceError("Control: Error determining whether process can be started or not, rc=%Rrc\n", rc);
-
-        if (   RT_SUCCESS(rc)
-            && fAllowed)
-        {
-            rc = RTCritSectEnter(&g_GuestControlThreadsCritSect);
+
+        if (RT_SUCCESS(rc))
+        {
+    #ifdef DEBUG
+            VBoxServiceVerbose(3, "Control: Start process szCmd=%s, uFlags=%u, szArgs=%s, szEnv=%s, szUser=%s, szPW=%s, uTimeout=%u\n",
+                               szCmd, uFlags, cArgs ? szArgs : "<None>", cEnvVars ? szEnv : "<None>", szUser, szPassword, uTimeLimitMS);
+    #endif
+            rc = VBoxServiceControlStartAllowed(&fStartAllowed);
+            if (RT_FAILURE(rc))
+                VBoxServiceError("Control: Error determining whether process can be started or not, rc=%Rrc\n", rc);
+
             if (RT_SUCCESS(rc))
             {
-                /** @todo Put the following params into a struct! */
-                RTLISTNODE *pThreadNode;
-                rc = VBoxServiceControlThreadStart(idClient, uContextID,
-                                                   szCmd, uFlags, szArgs, cArgs,
-                                                   szEnv, cbEnv, cEnvVars,
-                                                   szUser, szPassword, uTimeLimitMS,
-                                                   &pThreadNode);
-                if (RT_SUCCESS(rc))
+                if (fStartAllowed)
                 {
-                    /* Insert thread node into thread list. */
-                    /*rc =*/ RTListAppend(&g_GuestControlThreads, pThreadNode);
+                    rc = RTCritSectEnter(&g_GuestControlThreadsCritSect);
+                    if (RT_SUCCESS(rc))
+                    {
+                        /** @todo Put the following params into a struct! */
+                        RTLISTNODE *pThreadNode;
+                        rc = VBoxServiceControlThreadStart(idClient, uContextID,
+                                                           szCmd, uFlags, szArgs, cArgs,
+                                                           szEnv, cbEnv, cEnvVars,
+                                                           szUser, szPassword, uTimeLimitMS,
+                                                           &pThreadNode);
+                        if (RT_SUCCESS(rc))
+                        {
+                            /* Insert thread node into thread list. */
+                            /*rc =*/ RTListAppend(&g_GuestControlThreads, pThreadNode);
+                        }
+
+                        int rc2 = RTCritSectLeave(&g_GuestControlThreadsCritSect);
+                        if (RT_SUCCESS(rc))
+                            rc = rc2;
+                    }
                 }
-
-                int rc2 = RTCritSectLeave(&g_GuestControlThreadsCritSect);
-                if (RT_SUCCESS(rc))
-                    rc = rc2;
+                else
+                    rc = VERR_MAX_PROCS_REACHED; /* Maximum number of processes reached. */
             }
-/** @todo r=bird: VbglR3GuestCtrlExecReportStatus isn't called for a number
- * of error paths. Both here and in the thread.  I guess that will leave the
- * host waiting, possibly for ever, for a status report that never arrives.
- *
- * There are similar concerns with the termination status report in case the
- * loop exitted with an error status (VBoxServiceControlThreadProcLoop).
- */
-        }
-        else /* Process start is not allowed due to policy settings. */
-        {
-            /* Tell the host. */
-            rc = VbglR3GuestCtrlExecReportStatus(idClient, uContextID, 0 /* PID, invalid. */,
-                                                 PROC_STS_ERROR,
-                                                 !fAllowed ? VERR_MAX_PROCS_REACHED : rc,
-                                                 NULL /* pvData */, 0 /* cbData */);
         }
     }
     else
-        VBoxServiceError("Control: Failed to retrieve exec start command! Error: %Rrc\n", rc);
+        rc = VERR_INVALID_PARAMETER; /* Incorrect number of parameters. */
+
+    /* In case of an error we need to notify the host to not wait forever for our response. */
+    if (RT_FAILURE(rc))
+    {
+        VBoxServiceError("Control: Starting process failed with rc=%Rrc\n", rc);
+
+        int rc2 = VbglR3GuestCtrlExecReportStatus(idClient, uContextID, 0 /* PID, invalid. */,
+                                                  PROC_STS_ERROR, rc,
+                                                  NULL /* pvData */, 0 /* cbData */);
+        if (RT_FAILURE(rc2))
+        {
+            VBoxServiceError("Control: Error sending start process status to host, rc=%Rrc\n", rc2);
+            if (RT_SUCCESS(rc))
+                rc = rc2;
+        }
+    }
+
     return rc;
 }

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

@@ -429 +429 @@
 
     PVBOXSERVICECTRLREQUEST pRequest = pThread->pRequest;
-    AssertPtr(pRequest); /** @todo r=bird: Print error and return if invalid pointer! */
+    if (!pRequest)
+    {
+        VBoxServiceError("ControlThread: IPC request is invalid\n");
+        return VERR_INVALID_POINTER;
+    }
 
     switch (pRequest->enmType)
@@ -1291 +1295 @@
                                         pThread->pipeStdInW = NIL_RTPIPE;
                                 }
-                                else /* Something went wrong; report error! */
-                                {
-                                    VBoxServiceError("ControlThread: Could not start process '%s' (CID: %u)! Error: %Rrc\n",
-                                                     pThread->pszCmd, pThread->uContextID, rc);
-
-                                    rc2 = VbglR3GuestCtrlExecReportStatus(pThread->uClientID, pThread->uContextID, pThread->uPID,
-                                                                          PROC_STS_ERROR, rc,
-                                                                          NULL /* pvData */, 0 /* cbData */);
-                                    if (RT_FAILURE(rc2))
-                                        VBoxServiceError("ControlThread: Could not report process start error! Error: %Rrc (process error %Rrc)\n",
-                                                         rc2, rc);
-                                }
                             }
                             RTPollSetDestroy(hPollSet);
@@ -1328 +1320 @@
     if (pThread->uClientID)
     {
+        int rc2;
+        if (RT_FAILURE(rc))
+        {
+            rc2 = VbglR3GuestCtrlExecReportStatus(pThread->uClientID, pThread->uContextID, pThread->uPID,
+                                                  PROC_STS_ERROR, rc,
+                                                  NULL /* pvData */, 0 /* cbData */);
+            if (RT_FAILURE(rc2))
+                VBoxServiceError("ControlThread: Could not report process failure error; rc=%Rrc (process error %Rrc)\n",
+                                 rc2, rc);
+        }
+
         VBoxServiceVerbose(3, "ControlThread: [PID %u]: Cancelling pending waits (client ID=%u)\n",
                            pThread->uPID, pThread->uClientID);
-        int rc2 = VbglR3GuestCtrlCancelPendingWaits(pThread->uClientID);
+        rc2 = VbglR3GuestCtrlCancelPendingWaits(pThread->uClientID);
         if (RT_FAILURE(rc2))
         {