Index: /trunk/src/VBox/Main/ConsoleImpl.cpp =================================================================== --- /trunk/src/VBox/Main/ConsoleImpl.cpp (revision 33847) +++ /trunk/src/VBox/Main/ConsoleImpl.cpp (revision 33848) @@ -240,7 +240,8 @@ struct VMSaveTask : public VMProgressTask { - VMSaveTask(Console *aConsole, Progress *aProgress) + VMSaveTask(Console *aConsole, Progress *aProgress, const ComPtr &aServerProgress) : VMProgressTask(aConsole, aProgress, true /* aUsesVMPtr */), - mLastMachineState(MachineState_Null) + mLastMachineState(MachineState_Null), + mServerProgress(aServerProgress) {} @@ -2318,12 +2319,4 @@ HRESULT rc = S_OK; - - /* create a progress object to track operation completion */ - ComObjPtr progress; - progress.createObject(); - progress->init(static_cast(this), - Bstr(tr("Saving the execution state of the virtual machine")).raw(), - FALSE /* aCancelable */); - bool fBeganSavingState = false; bool fTaskCreationFailed = false; @@ -2331,6 +2324,23 @@ do { + ComPtr 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 task(new VMSaveTask(this, progress)); + std::auto_ptr task(new VMSaveTask(this, NULL, pProgress)); rc = task->rc(); /* @@ -2345,19 +2355,4 @@ 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,5 +2391,5 @@ /* return the progress to the caller */ - progress.queryInterfaceTo(aProgress); + pProgress.queryInterfaceTo(aProgress); } while (0); @@ -2412,5 +2407,5 @@ * before calling mControl->BeginSavingState(). */ - mControl->EndSavingState(FALSE); + mControl->EndSavingState(eik.getResultCode(), eik.getText().raw()); } @@ -3505,5 +3500,5 @@ * @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,5 +3549,5 @@ vrc = aNetworkAdapter->COMGETTER(AttachmentType)(&attachmentType); - if ( RT_FAILURE(vrc) + if ( RT_FAILURE(vrc) || attachmentType != NetworkAttachmentType_NAT) { @@ -3560,5 +3555,5 @@ goto done; } - + /* look down for PDMINETWORKNATCONFIG interface */ while (pBase) @@ -3572,6 +3567,6 @@ 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,9 +6363,6 @@ 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,5 +7061,5 @@ * @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,10 +7067,9 @@ DECLCALLBACK(int) Console::stateProgressCallback(PVM pVM, unsigned uPercent, void *pvUser) { - VMProgressTask *task = static_cast(pvUser); - AssertReturn(task, VERR_INVALID_PARAMETER); + IProgress *pProgress = static_cast(pvUser); /* update the progress object */ - if (task->mProgress) - task->mProgress->SetCurrentOperationProgress(uPercent); + if (pProgress) + pProgress->SetCurrentOperationProgress(uPercent); return VINF_SUCCESS; @@ -7613,5 +7604,5 @@ task->mSavedStateFile.c_str(), Console::stateProgressCallback, - static_cast(task.get())); + static_cast(task->mProgress)); if (RT_SUCCESS(vrc)) @@ -8005,5 +7996,5 @@ true /*fContinueAfterwards*/, Console::stateProgressCallback, - (void*)pTask, + static_cast(pTask->mProgress), &fSuspenededBySave); alock.enter(); @@ -8251,5 +8242,6 @@ Assert(task->mSavedStateFile.length()); - Assert(!task->mProgress.isNull()); + Assert(task->mProgress.isNull()); + Assert(!task->mServerProgress.isNull()); const ComObjPtr &that = task->mConsole; @@ -8264,5 +8256,5 @@ false, /*fContinueAfterwards*/ Console::stateProgressCallback, - static_cast(task.get()), + static_cast(task->mServerProgress), &fSuspenededBySave); if (RT_FAILURE(vrc)) @@ -8277,14 +8269,6 @@ 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,21 +8279,15 @@ */ 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(); Index: /trunk/src/VBox/Main/MachineImpl.cpp =================================================================== --- /trunk/src/VBox/Main/MachineImpl.cpp (revision 33847) +++ /trunk/src/VBox/Main/MachineImpl.cpp (revision 33848) @@ -3216,5 +3216,5 @@ CheckComArgNotNull(aSession); CheckComArgStrNotEmptyOrNull(aType); - CheckComArgOutSafeArrayPointerValid(aProgress); + CheckComArgOutPointerValid(aProgress); AutoCaller autoCaller(this); @@ -10035,5 +10035,5 @@ { 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,10 +10528,10 @@ * @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,14 +10542,14 @@ 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 pProgress; + pProgress.createObject(); + pProgress->init(getVirtualBox(), + static_cast(this) /* aInitiator */, + Bstr(tr("Saving the execution state of the virtual machine")).raw(), + FALSE /* aCancelable */); Bstr stateFilePath; @@ -10564,6 +10564,6 @@ /* 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,4 +10571,5 @@ stateFilePath.cloneTo(aStateFilePath); + pProgress.queryInterfaceTo(aProgress); return S_OK; @@ -10578,5 +10579,5 @@ * @note Locks mParent + this object for writing. */ -STDMETHODIMP SessionMachine::EndSavingState(BOOL aSuccess) +STDMETHODIMP SessionMachine::EndSavingState(LONG iResult, IN_BSTR aErrMsg) { LogFlowThisFunc(("\n")); @@ -10588,22 +10589,20 @@ 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,9 +11258,10 @@ * @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,5 +11274,5 @@ HRESULT rc = S_OK; - if (aSuccess) + if (SUCCEEDED(aRc)) { mSSData->mStateFilePath = mSnapshotData.mStateFilePath; @@ -11289,11 +11289,23 @@ } - /* 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(); Index: /trunk/src/VBox/Main/idl/VirtualBox.xidl =================================================================== --- /trunk/src/VBox/Main/idl/VirtualBox.xidl (revision 33847) +++ /trunk/src/VBox/Main/idl/VirtualBox.xidl (revision 33848) @@ -2945,5 +2945,5 @@ - - - Progress object created by the VM process to wait until + + + Progress object created by VBoxSVC to wait until the state is saved. @@ -3115,6 +3115,10 @@ - - @c true to indicate success and @c false otherwise. + + @c S_OK to indicate success. + + + + @c human readable error message in case of failure. Index: /trunk/src/VBox/Main/include/MachineImpl.h =================================================================== --- /trunk/src/VBox/Main/include/MachineImpl.h (revision 33847) +++ /trunk/src/VBox/Main/include/MachineImpl.h (revision 33848) @@ -21,4 +21,5 @@ #include "VirtualBoxBase.h" #include "SnapshotImpl.h" +#include "ProgressImpl.h" #include "VRDEServerImpl.h" #include "MediumAttachmentImpl.h" @@ -918,6 +919,6 @@ 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,6 +988,6 @@ // used when saving state - Guid mProgressId; Utf8Str mStateFilePath; + ComObjPtr mProgress; }; @@ -1005,5 +1006,5 @@ void uninit(Uninit::Reason aReason); - HRESULT endSavingState(BOOL aSuccess); + HRESULT endSavingState(HRESULT aRC, const Utf8Str &aErrMsg); void deleteSnapshotHandler(DeleteSnapshotTask &aTask);