Changeset 75853 in vbox for trunk/src/VBox/Additions

Timestamp:

Nov 30, 2018 7:26:42 PM (6 years ago)

Author:

vboxsync

Message:

GuestControl,HGCM,VBoxService: Save/restore related optimizations and changes. bugref:9313

Location:

trunk/src/VBox/Additions/common

Files:

• VBoxGuest/lib/VBoxGuestR3LibGuestCtrl.cpp (modified) (6 diffs)
• VBoxService/VBoxServiceControl.cpp (modified) (5 diffs)
• VBoxService/VBoxServiceControlSession.cpp (modified) (1 diff)

trunk/src/VBox/Additions/common/VBoxGuest/lib/VBoxGuestR3LibGuestCtrl.cpp

@@ -307 +307 @@
 
 
-
 /**
  * Peeks at the next host message, waiting for one to turn up.
@@ -314 +313 @@
  * @retval  VERR_INTERRUPTED if interrupted.  Does the necessary cleanup, so
  *          caller just have to repeat this call.
+ * @retval  VERR_VM_RESTORED if the VM has been restored (idRestoreCheck).
  *
  * @param   idClient        The client ID returned by VbglR3GuestCtrlConnect().
@@ -319 +319 @@
  * @param   pcParameters    Where to store the number  of parameters which will
  *                          be received in a second call to the host.
- */
-VBGLR3DECL(int) VbglR3GuestCtrlMsgPeekWait(uint32_t idClient, uint32_t *pidMsg, uint32_t *pcParameters)
+ * @param   pidRestoreCheck Pointer to the VbglR3GetSessionId() variable to use
+ *                          for the VM restore check.  Optional.
+ *
+ * @note    Restore check is only performed optimally with a 6.0 host.
+ */
+VBGLR3DECL(int) VbglR3GuestCtrlMsgPeekWait(uint32_t idClient, uint32_t *pidMsg, uint32_t *pcParameters, uint64_t *pidRestoreCheck)
 {
     AssertPtrReturn(pidMsg, VERR_INVALID_POINTER);
@@ -328 +332 @@
     if (vbglR3GuestCtrlSupportsPeekGetCancel(idClient))
     {
-        HGCMMsgCmdWaitFor Msg;
-        VBGL_HGCM_HDR_INIT(&Msg.hdr, idClient, GUEST_MSG_PEEK_WAIT, 2);
-        VbglHGCMParmUInt32Set(&Msg.msg, 0);
-        VbglHGCMParmUInt32Set(&Msg.num_parms, 0);
-        rc = VbglR3HGCMCall(&Msg.hdr, sizeof(Msg));
+        struct
+        {
+            VBGLIOCHGCMCALL Hdr;
+            HGCMFunctionParameter idMsg;       /* Doubles as restore check on input. */
+            HGCMFunctionParameter cParameters;
+        } Msg;
+        VBGL_HGCM_HDR_INIT(&Msg.Hdr, idClient, GUEST_MSG_PEEK_WAIT, 2);
+        VbglHGCMParmUInt64Set(&Msg.idMsg, pidRestoreCheck ? *pidRestoreCheck : 0);
+        VbglHGCMParmUInt32Set(&Msg.cParameters, 0);
+        rc = VbglR3HGCMCall(&Msg.Hdr, sizeof(Msg));
         LogRel(("VbglR3GuestCtrlMsgPeekWait -> %Rrc\n", rc));
         if (RT_SUCCESS(rc))
         {
-            AssertMsgReturn(   Msg.msg.type       == VMMDevHGCMParmType_32bit
-                            && Msg.num_parms.type == VMMDevHGCMParmType_32bit,
-                            ("msg.type=%d num_parms.type=%d\n", Msg.msg.type, Msg.num_parms.type),
+            AssertMsgReturn(   Msg.idMsg.type       == VMMDevHGCMParmType_64bit
+                            && Msg.cParameters.type == VMMDevHGCMParmType_32bit,
+                            ("msg.type=%d num_parms.type=%d\n", Msg.idMsg.type, Msg.cParameters.type),
                             VERR_INTERNAL_ERROR_3);
 
-            *pidMsg       = Msg.msg.u.value32;
-            *pcParameters = Msg.num_parms.u.value32;
+            *pidMsg       = (uint32_t)Msg.idMsg.u.value64;
+            *pcParameters = Msg.cParameters.u.value32;
             return rc;
         }
@@ -351 +360 @@
         if (rc == VERR_INTERRUPTED)
         {
-            VBGL_HGCM_HDR_INIT(&Msg.hdr, idClient, GUEST_MSG_CANCEL, 0);
-            int rc2 = VbglR3HGCMCall(&Msg.hdr, sizeof(Msg.hdr));
+            VBGL_HGCM_HDR_INIT(&Msg.Hdr, idClient, GUEST_MSG_CANCEL, 0);
+            int rc2 = VbglR3HGCMCall(&Msg.Hdr, sizeof(Msg.Hdr));
             AssertRC(rc2);
         }
+
+        /*
+         * If restored, update pidRestoreCheck.
+         */
+        if (rc == VERR_VM_RESTORED && pidRestoreCheck)
+            *pidRestoreCheck = Msg.idMsg.u.value64;
 
         *pidMsg       = UINT32_MAX - 1;
@@ -363 +378 @@
     /*
      * Fallback if host < v6.0.
+     *
+     * Note! The restore check isn't perfect. Would require checking afterwards
+     *       and stash the result if we were restored during the call.  Too much
+     *       hazzle for a downgrade scenario.
      */
+    if (pidRestoreCheck)
+    {
+        uint64_t idRestoreCur = *pidRestoreCheck;
+        rc = VbglR3GetSessionId(&idRestoreCur);
+        if (RT_SUCCESS(rc) && idRestoreCur != *pidRestoreCheck)
+        {
+            *pidRestoreCheck = idRestoreCur;
+            return VERR_VM_RESTORED;
+        }
+    }
+
     rc = vbglR3GuestCtrlMsgWaitFor(idClient, pidMsg, pcParameters);
     if (rc == VERR_TOO_MUCH_DATA)

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

@@ -251 +251 @@
     AssertReturn(pvScratchBuf, VERR_NO_MEMORY);
 
-    VBGLR3GUESTCTRLCMDCTX ctxHost = { g_idControlSvcClient, 0 /*idContext*/, 2 /*uProtocol*/, 0  /*cParms*/ };
-
     int rc = VINF_SUCCESS;      /* (shut up compiler warnings) */
     int cRetrievalFailed = 0;   /* Number of failed message retrievals in a row. */
@@ -258 +256 @@
     {
         VGSvcVerbose(3, "GstCtrl: Waiting for host msg ...\n");
-        uint32_t idMsg  = 0;
-        uint32_t cParms = 0;
-        rc = VbglR3GuestCtrlMsgPeekWait(g_idControlSvcClient, &idMsg, &cParms);
+        VBGLR3GUESTCTRLCMDCTX ctxHost = { g_idControlSvcClient, 0 /*idContext*/, 2 /*uProtocol*/, 0  /*cParms*/ };
+        uint32_t              idMsg   = 0;
+        rc = VbglR3GuestCtrlMsgPeekWait(g_idControlSvcClient, &idMsg, &ctxHost.uNumParms, &g_idControlSession);
         if (RT_SUCCESS(rc))
         {
             cRetrievalFailed = 0; /* Reset failed retrieval count. */
-            VGSvcVerbose(4, "idMsg=%RU32 (%s) (%RU32 parms) retrieved\n", idMsg, GstCtrlHostFnName((eHostFn)idMsg), cParms);
-
-            /*
-             * Close all open guest sessions if the VM was restored (all context IDs,
-             * sessions, etc. are now invalid).
-             *
-             * Note! This will suck performance wise if we get a lot of messages thru here.
-             *       What about the service returning a HOST_MSG_RESTORED message?
-             */
-            uint64_t idNewSession = g_idControlSession;
-            int rc2 = VbglR3GetSessionId(&idNewSession);
-            if (   RT_SUCCESS(rc2)
-                && idNewSession != g_idControlSession)
-            {
-                VGSvcVerbose(1, "The VM session ID changed (i.e. restored).\n");
-                g_idControlSession = idNewSession;
-                rc2 = VGSvcGstCtrlSessionClose(&g_Session);
-                AssertRC(rc2);
-            }
+            VGSvcVerbose(4, "idMsg=%RU32 (%s) (%RU32 parms) retrieved\n",
+                         idMsg, GstCtrlHostFnName((eHostFn)idMsg), ctxHost.uNumParms);
 
             /*
              * Handle the host message.
              */
-            ctxHost.uNumParms = cParms;
             switch (idMsg)
             {
@@ -308 +288 @@
                         rc = VbglR3GuestCtrlMsgSkip(g_idControlSvcClient, VERR_NOT_SUPPORTED, idMsg);
                         VGSvcVerbose(1, "Skipped unexpected message idMsg=%RU32 (%s), cParms=%RU32 (rc=%Rrc)\n",
-                                     idMsg, GstCtrlHostFnName((eHostFn)idMsg), cParms, rc);
+                                     idMsg, GstCtrlHostFnName((eHostFn)idMsg), ctxHost.uNumParms, rc);
                     }
                     else
                     {
                         rc = VbglR3GuestCtrlMsgSkipOld(g_idControlSvcClient);
-                        VGSvcVerbose(3, "Skipped idMsg=%RU32, cParms=%RU32, rc=%Rrc\n", idMsg, cParms, rc);
+                        VGSvcVerbose(3, "Skipped idMsg=%RU32, cParms=%RU32, rc=%Rrc\n", idMsg, ctxHost.uNumParms, rc);
                     }
                     break;
@@ -325 +305 @@
             RTThreadYield();
         }
+        /*
+         * Handle restore notification from host.  All the context IDs (sessions,
+         * files, proceses, etc) are invalidated by a VM restore and must be closed.
+         */
+        else if (rc == VERR_VM_RESTORED)
+        {
+            VGSvcVerbose(1, "The VM session ID changed (i.e. restored).\n");
+            int rc2 = VGSvcGstCtrlSessionClose(&g_Session);
+            AssertRC(rc2);
+        }
         else
         {
@@ -332 +322 @@
 
             /* Check for VM session change. */
+            /** @todo  We don't need to check the host here.  */
             uint64_t idNewSession = g_idControlSession;
             int rc2 = VbglR3GetSessionId(&idNewSession);

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

@@ -1579 +1579 @@
                     VGSvcVerbose(3, "Waiting for host msg ...\n");
                     uint32_t uMsg = 0;
-                    rc = VbglR3GuestCtrlMsgPeekWait(idClient, &uMsg, &CtxHost.uNumParms);
+                    rc = VbglR3GuestCtrlMsgPeekWait(idClient, &uMsg, &CtxHost.uNumParms, NULL);
                     if (RT_SUCCESS(rc))
                     {