Changeset 33848 in vbox for trunk/src/VBox

Timestamp:

Nov 8, 2010 2:29:13 PM (14 years ago)

Author:

vboxsync

Message:

Main/Console+Machine: move progress object for Console::SaveState from the VM process to VBoxSVC

Location:

trunk/src/VBox/Main

Files:

• ConsoleImpl.cpp (modified) (19 diffs)
• MachineImpl.cpp (modified) (11 diffs)
• idl/VirtualBox.xidl (modified) (3 diffs)
• include/MachineImpl.h (modified) (4 diffs)

trunk/src/VBox/Main/ConsoleImpl.cpp

@@ -240 +240 @@
 struct VMSaveTask : public VMProgressTask
 {
-    VMSaveTask(Console *aConsole, Progress *aProgress)
+    VMSaveTask(Console *aConsole, Progress *aProgress, const ComPtr<IProgress> &aServerProgress)
         : VMProgressTask(aConsole, aProgress, true /* aUsesVMPtr */),
-          mLastMachineState(MachineState_Null)
+          mLastMachineState(MachineState_Null),
+          mServerProgress(aServerProgress)
     {}
 
@@ -2318 +2319 @@
 
     HRESULT rc = S_OK;
-
-    /* create a progress object to track operation completion */
-    ComObjPtr<Progress> progress;
-    progress.createObject();
-    progress->init(static_cast<IConsole *>(this),
-                   Bstr(tr("Saving the execution state of the virtual machine")).raw(),
-                   FALSE /* aCancelable */);
-
     bool fBeganSavingState = false;
     bool fTaskCreationFailed = false;
@@ -2331 +2324 @@
     do
     {
+        ComPtr<IProgress> pProgress;
+        Bstr stateFilePath;
+
+        /*
+         * request a saved state file path from the server
+         * (this will set the machine state to Saving on the server to block
+         * others from accessing this machine)
+         */
+        rc = mControl->BeginSavingState(pProgress.asOutParam(),
+                                        stateFilePath.asOutParam());
+        if (FAILED(rc)) break;
+
+        fBeganSavingState = true;
+
+        /* sync the state with the server */
+        setMachineStateLocally(MachineState_Saving);
+
         /* create a task object early to ensure mpVM protection is successful */
-        std::auto_ptr <VMSaveTask> task(new VMSaveTask(this, progress));
+        std::auto_ptr <VMSaveTask> task(new VMSaveTask(this, NULL, pProgress));
         rc = task->rc();
         /*
@@ -2345 +2355 @@
             break;
         }
-
-        Bstr stateFilePath;
-
-        /*
-         * request a saved state file path from the server
-         * (this will set the machine state to Saving on the server to block
-         * others from accessing this machine)
-         */
-        rc = mControl->BeginSavingState(progress, stateFilePath.asOutParam());
-        if (FAILED(rc)) break;
-
-        fBeganSavingState = true;
-
-        /* sync the state with the server */
-        setMachineStateLocally(MachineState_Saving);
 
         /* ensure the directory for the saved state file exists */
@@ -2396 +2391 @@
 
         /* return the progress to the caller */
-        progress.queryInterfaceTo(aProgress);
+        pProgress.queryInterfaceTo(aProgress);
     }
     while (0);
@@ -2412 +2407 @@
              * before calling mControl->BeginSavingState().
              */
-            mControl->EndSavingState(FALSE);
+            mControl->EndSavingState(eik.getResultCode(), eik.getText().raw());
         }
 
@@ -3505 +3500 @@
  * @note Locks this object for writing.
  */
-HRESULT Console::onNATRedirectRuleChange(INetworkAdapter *aNetworkAdapter, BOOL aNatRuleRemove, IN_BSTR aRuleName, 
+HRESULT Console::onNATRedirectRuleChange(INetworkAdapter *aNetworkAdapter, BOOL aNatRuleRemove, IN_BSTR aRuleName,
                                  NATProtocol_T aProto, IN_BSTR aHostIp, LONG aHostPort, IN_BSTR aGuestIp, LONG aGuestPort)
 {
@@ -3554 +3549 @@
         vrc = aNetworkAdapter->COMGETTER(AttachmentType)(&attachmentType);
 
-        if (   RT_FAILURE(vrc) 
+        if (   RT_FAILURE(vrc)
             || attachmentType != NetworkAttachmentType_NAT)
         {
@@ -3560 +3555 @@
             goto done;
         }
-        
+
         /* look down for PDMINETWORKNATCONFIG interface */
         while (pBase)
@@ -3572 +3567 @@
             goto done;
         bool fUdp = (aProto == NATProtocol_UDP);
-        vrc = pNetNatCfg->pfnRedirectRuleCommand(pNetNatCfg, aNatRuleRemove, Utf8Str(aRuleName).c_str(), fUdp, 
-                                                 Utf8Str(aHostIp).c_str(), aHostPort, Utf8Str(aGuestIp).c_str(), 
+        vrc = pNetNatCfg->pfnRedirectRuleCommand(pNetNatCfg, aNatRuleRemove, Utf8Str(aRuleName).c_str(), fUdp,
+                                                 Utf8Str(aHostIp).c_str(), aHostPort, Utf8Str(aGuestIp).c_str(),
                                                  aGuestPort);
         if (RT_FAILURE(vrc))
@@ -6368 +6363 @@
                     break;
                 case MachineState_Saving:
-                    /* successfully saved (note that the machine is already in
-                     * the Saved state on the server due to EndSavingState()
-                     * called from saveStateThread(), so only change the local
-                     * state) */
-                    that->setMachineStateLocally(MachineState_Saved);
+                    /* successfully saved */
+                    that->setMachineState(MachineState_Saved);
                     break;
                 case MachineState_Starting:
@@ -7069 +7061 @@
  * @param   pVM         The VM handle.
  * @param   uPercent    Completion percentage (0-100).
- * @param   pvUser      Pointer to the VMProgressTask structure.
+ * @param   pvUser      Pointer to an IProgress instance.
  * @return  VINF_SUCCESS.
  */
@@ -7075 +7067 @@
 DECLCALLBACK(int) Console::stateProgressCallback(PVM pVM, unsigned uPercent, void *pvUser)
 {
-    VMProgressTask *task = static_cast<VMProgressTask *>(pvUser);
-    AssertReturn(task, VERR_INVALID_PARAMETER);
+    IProgress *pProgress = static_cast<IProgress *>(pvUser);
 
     /* update the progress object */
-    if (task->mProgress)
-        task->mProgress->SetCurrentOperationProgress(uPercent);
+    if (pProgress)
+        pProgress->SetCurrentOperationProgress(uPercent);
 
     return VINF_SUCCESS;
@@ -7613 +7604 @@
                                            task->mSavedStateFile.c_str(),
                                            Console::stateProgressCallback,
-                                           static_cast<VMProgressTask*>(task.get()));
+                                           static_cast<IProgress *>(task->mProgress));
 
                     if (RT_SUCCESS(vrc))
@@ -8005 +7996 @@
                                true /*fContinueAfterwards*/,
                                Console::stateProgressCallback,
-                               (void*)pTask,
+                               static_cast<IProgress *>(pTask->mProgress),
                                &fSuspenededBySave);
             alock.enter();
@@ -8251 +8242 @@
 
     Assert(task->mSavedStateFile.length());
-    Assert(!task->mProgress.isNull());
+    Assert(task->mProgress.isNull());
+    Assert(!task->mServerProgress.isNull());
 
     const ComObjPtr<Console> &that = task->mConsole;
@@ -8264 +8256 @@
                        false, /*fContinueAfterwards*/
                        Console::stateProgressCallback,
-                       static_cast<VMProgressTask*>(task.get()),
+                       static_cast<IProgress *>(task->mServerProgress),
                        &fSuspenededBySave);
     if (RT_FAILURE(vrc))
@@ -8277 +8269 @@
     AutoWriteLock thatLock(that COMMA_LOCKVAL_SRC_POS);
 
-    /*
-     * finalize the requested save state procedure.
-     * In case of success, the server will set the machine state to Saved;
-     * in case of failure it will reset the it to the state it had right
-     * before calling mControl->BeginSavingState().
-     */
-    that->mControl->EndSavingState(SUCCEEDED(rc));
-
     /* synchronize the state with the server */
-    if (!FAILED(rc))
+    if (SUCCEEDED(rc))
     {
         /*
@@ -8295 +8279 @@
          */
         task->releaseVMCaller();
-
         rc = that->powerDown();
     }
 
-    /* notify the progress object about operation completion */
-    if (SUCCEEDED(rc))
-        task->mProgress->notifyComplete(S_OK);
-    else
-    {
-        if (errMsg.length())
-            task->mProgress->notifyComplete(rc,
-                                            COM_IIDOF(IConsole),
-                                            Console::getStaticComponentName(),
-                                            errMsg.c_str());
-        else
-            task->mProgress->notifyComplete(rc);
-    }
+    /*
+     * Finalize the requested save state procedure. In case of failure it will
+     * reset the machine state to the state it had right before calling
+     * mControl->BeginSavingState(). This must be the last thing because it
+     * will set the progress to completed, and that means that the frontend
+     * can immediately uninit the associated console object.
+     */
+    that->mControl->EndSavingState(rc, Bstr(errMsg).raw());
 
     LogFlowFuncLeave();

trunk/src/VBox/Main/MachineImpl.cpp

@@ -3216 +3216 @@
     CheckComArgNotNull(aSession);
     CheckComArgStrNotEmptyOrNull(aType);
-    CheckComArgOutSafeArrayPointerValid(aProgress);
+    CheckComArgOutPointerValid(aProgress);
 
     AutoCaller autoCaller(this);
@@ -10035 +10035 @@
     {
         LogWarningThisFunc(("canceling failed save state request!\n"));
-        endSavingState(FALSE /* aSuccess  */);
+        endSavingState(E_FAIL, tr("Machine terminated with pending save state!"));
     }
     else if (!mSnapshotData.mSnapshot.isNull())
@@ -10528 +10528 @@
  *  @note Locks this object for writing.
  */
-STDMETHODIMP SessionMachine::BeginSavingState(IProgress *aProgress, BSTR *aStateFilePath)
+STDMETHODIMP SessionMachine::BeginSavingState(IProgress **aProgress, BSTR *aStateFilePath)
 {
     LogFlowThisFuncEnter();
 
-    AssertReturn(aProgress, E_INVALIDARG);
-    AssertReturn(aStateFilePath, E_POINTER);
+    CheckComArgOutPointerValid(aProgress);
+    CheckComArgOutPointerValid(aStateFilePath);
 
     AutoCaller autoCaller(this);
@@ -10542 +10542 @@
     AssertReturn(    mData->mMachineState == MachineState_Paused
                   && mSnapshotData.mLastState == MachineState_Null
-                  && mSnapshotData.mProgressId.isEmpty()
                   && mSnapshotData.mStateFilePath.isEmpty(),
                  E_FAIL);
 
-    /* memorize the progress ID and add it to the global collection */
-    Bstr progressId;
-    HRESULT rc = aProgress->COMGETTER(Id)(progressId.asOutParam());
-    AssertComRCReturn(rc, rc);
-    rc = mParent->addProgress(aProgress);
-    AssertComRCReturn(rc, rc);
+    /* create a progress object to track operation completion */
+    ComObjPtr<Progress> pProgress;
+    pProgress.createObject();
+    pProgress->init(getVirtualBox(),
+                    static_cast<IMachine *>(this) /* aInitiator */,
+                    Bstr(tr("Saving the execution state of the virtual machine")).raw(),
+                    FALSE /* aCancelable */);
 
     Bstr stateFilePath;
@@ -10564 +10564 @@
     /* fill in the snapshot data */
     mSnapshotData.mLastState = mData->mMachineState;
-    mSnapshotData.mProgressId = Guid(progressId);
     mSnapshotData.mStateFilePath = stateFilePath;
+    mSnapshotData.mProgress = pProgress;
 
     /* set the state to Saving (this is expected by Console::SaveState()) */
@@ -10571 +10571 @@
 
     stateFilePath.cloneTo(aStateFilePath);
+    pProgress.queryInterfaceTo(aProgress);
 
     return S_OK;
@@ -10578 +10579 @@
  *  @note Locks mParent + this object for writing.
  */
-STDMETHODIMP SessionMachine::EndSavingState(BOOL aSuccess)
+STDMETHODIMP SessionMachine::EndSavingState(LONG iResult, IN_BSTR aErrMsg)
 {
     LogFlowThisFunc(("\n"));
@@ -10588 +10589 @@
     AutoMultiWriteLock2 alock(mParent, this COMMA_LOCKVAL_SRC_POS);
 
-    AssertReturn(    mData->mMachineState == MachineState_Saving
+    AssertReturn(    (   (SUCCEEDED(iResult) && mData->mMachineState == MachineState_Saved)
+                      || (FAILED(iResult) && mData->mMachineState == MachineState_Saving))
                   && mSnapshotData.mLastState != MachineState_Null
-                  && !mSnapshotData.mProgressId.isEmpty()
                   && !mSnapshotData.mStateFilePath.isEmpty(),
                  E_FAIL);
 
     /*
-     *  on success, set the state to Saved;
-     *  on failure, set the state to the state we had when BeginSavingState() was
-     *  called (this is expected by Console::SaveState() and
-     *  Console::saveStateThread())
+     * On failure, set the state to the state we had when BeginSavingState()
+     * was called (this is expected by Console::SaveState() and the associated
+     * task). On success the VM process already changed the state to
+     * MachineState_Saved, so no need to do anything.
      */
-    if (aSuccess)
-        setMachineState(MachineState_Saved);
-    else
+    if (FAILED(iResult))
         setMachineState(mSnapshotData.mLastState);
 
-    return endSavingState(aSuccess);
+    return endSavingState(iResult, aErrMsg);
 }
 
@@ -11259 +11258 @@
  *  @note Must be called from under this object's lock.
  *
- *  @param aSuccess TRUE if the snapshot has been taken successfully
+ *  @param aRc      S_OK if the snapshot has been taken successfully
+ *  @param aErrMsg  human readable error message for failure
  *
  *  @note Locks mParent + this objects for writing.
  */
-HRESULT SessionMachine::endSavingState(BOOL aSuccess)
+HRESULT SessionMachine::endSavingState(HRESULT aRc, const Utf8Str &aErrMsg)
 {
     LogFlowThisFuncEnter();
@@ -11274 +11274 @@
     HRESULT rc = S_OK;
 
-    if (aSuccess)
+    if (SUCCEEDED(aRc))
     {
         mSSData->mStateFilePath = mSnapshotData.mStateFilePath;
@@ -11289 +11289 @@
     }
 
-    /* remove the completed progress object */
-    mParent->removeProgress(mSnapshotData.mProgressId.ref());
+    /* notify the progress object about operation completion */
+    Assert(mSnapshotData.mProgress);
+    if (SUCCEEDED(aRc))
+        mSnapshotData.mProgress->notifyComplete(S_OK);
+    else
+    {
+        if (aErrMsg.length())
+            mSnapshotData.mProgress->notifyComplete(aRc,
+                                                    COM_IIDOF(ISession),
+                                                    getComponentName(),
+                                                    aErrMsg.c_str());
+        else
+            mSnapshotData.mProgress->notifyComplete(aRc);
+    }
 
     /* clear out the temporary saved state data */
     mSnapshotData.mLastState = MachineState_Null;
-    mSnapshotData.mProgressId.clear();
     mSnapshotData.mStateFilePath.setNull();
+    mSnapshotData.mProgress.setNull();
 
     LogFlowThisFuncLeave();

trunk/src/VBox/Main/idl/VirtualBox.xidl

@@ -2945 +2945 @@
   <interface
      name="IInternalMachineControl" extends="$unknown"
-     uuid="e2da8b1a-2ad1-490e-b29e-c33a144791b6"
+     uuid="476126af-e223-4490-a8a0-b1f1575be013"
      internal="yes"
      wsmap="suppress"
@@ -3087 +3087 @@
         save the current state and stop the VM execution.
       </desc>
-      <param name="progress" type="IProgress" dir="in">
-        <desc>
-          Progress object created by the VM process to wait until
+      <param name="progress" type="IProgress" dir="out">
+        <desc>
+          Progress object created by VBoxSVC to wait until
           the state is saved.
         </desc>
@@ -3115 +3115 @@
       </desc>
 
-      <param name="success" type="boolean" dir="in">
-        <desc>@c true to indicate success and @c false otherwise.
+      <param name="result" type="long" dir="in">
+        <desc>@c S_OK to indicate success.
+        </desc>
+      </param>
+      <param name="errMsg" type="wstring" dir="in">
+        <desc>@c human readable error message in case of failure.
         </desc>
       </param>

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

@@ -21 +21 @@
 #include "VirtualBoxBase.h"
 #include "SnapshotImpl.h"
+#include "ProgressImpl.h"
 #include "VRDEServerImpl.h"
 #include "MediumAttachmentImpl.h"
@@ -918 +919 @@
     STDMETHOD(DetachAllUSBDevices)(BOOL aDone);
     STDMETHOD(OnSessionEnd)(ISession *aSession, IProgress **aProgress);
-    STDMETHOD(BeginSavingState)(IProgress *aProgress, BSTR *aStateFilePath);
-    STDMETHOD(EndSavingState)(BOOL aSuccess);
+    STDMETHOD(BeginSavingState)(IProgress **aProgress, BSTR *aStateFilePath);
+    STDMETHOD(EndSavingState)(LONG aResult, IN_BSTR aErrMsg);
     STDMETHOD(AdoptSavedState)(IN_BSTR aSavedStateFile);
     STDMETHOD(BeginTakingSnapshot)(IConsole *aInitiator,
@@ -987 +988 @@
 
         // used when saving state
-        Guid mProgressId;
         Utf8Str mStateFilePath;
+        ComObjPtr<Progress> mProgress;
     };
 
@@ -1005 +1006 @@
     void uninit(Uninit::Reason aReason);
 
-    HRESULT endSavingState(BOOL aSuccess);
+    HRESULT endSavingState(HRESULT aRC, const Utf8Str &aErrMsg);
 
     void deleteSnapshotHandler(DeleteSnapshotTask &aTask);