Index: /trunk/include/VBox/HostServices/GuestPropertySvc.h =================================================================== --- /trunk/include/VBox/HostServices/GuestPropertySvc.h (revision 24702) +++ /trunk/include/VBox/HostServices/GuestPropertySvc.h (revision 24703) @@ -245,5 +245,12 @@ * The parameter format matches that of ENUM_PROPS. */ - ENUM_PROPS_HOST = 6 + ENUM_PROPS_HOST = 6, + + /** + * Flush notifications. + * Takes one 32-bit unsigned integer parameter that gives the number of + * milliseconds to wait for the worker thread to get the work done. + */ + FLUSH_NOTIFICATIONS_HOST }; Index: /trunk/src/VBox/HostServices/GuestProperties/service.cpp =================================================================== --- /trunk/src/VBox/HostServices/GuestProperties/service.cpp (revision 24702) +++ /trunk/src/VBox/HostServices/GuestProperties/service.cpp (revision 24703) @@ -21,5 +21,6 @@ */ -/** +/** @page pg_svc_guest_properties Guest Property HGCM Service + * * This HGCM service allows the guest to set and query values in a property * store on the host. The service proxies the guest requests to the service @@ -39,21 +40,21 @@ */ -#define LOG_GROUP LOG_GROUP_HGCM - /******************************************************************************* * Header Files * *******************************************************************************/ +#define LOG_GROUP LOG_GROUP_HGCM #include #include +#include +#include +#include +#include #include -#include +#include +#include #include -#include -#include +#include #include -#include -#include -#include #include /* for auto_ptr */ @@ -163,8 +164,10 @@ /** Queue of outstanding property change notifications */ RTREQQUEUE *mReqQueue; + /** Request that we've left pending in a call to flushNotifications. */ + PRTREQ mPendingDummyReq; /** Thread for processing the request queue */ RTTHREAD mReqThread; /** Tell the thread that it should exit */ - bool mfExitThread; + bool volatile mfExitThread; /** Callback function supplied by the host for notification of updates * to properties */ @@ -222,6 +225,9 @@ public: explicit Service(PVBOXHGCMSVCHELPERS pHelpers) - : mpHelpers(pHelpers), mfExitThread(false), mpfnHostCallback(NULL), - mpvHostData(NULL) + : mpHelpers(pHelpers) + , mPendingDummyReq(NULL) + , mfExitThread(false) + , mpfnHostCallback(NULL) + , mpvHostData(NULL) { int rc = RTReqCreateQueue(&mReqQueue); @@ -321,4 +327,5 @@ int delProperty(uint32_t cParms, VBOXHGCMSVCPARM paParms[], bool isGuest); int enumProps(uint32_t cParms, VBOXHGCMSVCPARM paParms[]); + int flushNotifications(uint32_t cMsTimeout); int getNotification(VBOXHGCMCALLHANDLE callHandle, uint32_t cParms, VBOXHGCMSVCPARM paParms[]); @@ -350,4 +357,5 @@ * @copydoc FNRTTHREAD */ +/* static */ DECLCALLBACK(int) Service::reqThreadFn(RTTHREAD ThreadSelf, void *pvUser) { @@ -819,4 +827,44 @@ rc = VERR_BUFFER_OVERFLOW; } + return rc; +} + +/** + * Flushes the notifications. + * + * @returns iprt status value + * @param cMsTimeout The timeout in milliseconds. + * @thread HGCM + */ +int Service::flushNotifications(uint32_t cMsTimeout) +{ + LogFlowThisFunc(("cMsTimeout=%RU32\n", cMsTimeout)); + int rc; + +#ifndef VBOX_GUEST_PROP_TEST_NOTHREAD + /* + * Wait for the thread to finish processing all current requests. + */ + if (!mPendingDummyReq && !RTReqIsBusy(mReqQueue)) + rc = VINF_SUCCESS; + else + { + if (!mPendingDummyReq) + rc = RTReqCallEx(mReqQueue, &mPendingDummyReq, 0 /*cMillies*/, RTREQFLAGS_VOID, (PFNRT)reqVoid, 0); + else + rc = VERR_TIMEOUT; + if (rc == VERR_TIMEOUT) + rc = RTReqWait(mPendingDummyReq, cMsTimeout); + if (RT_SUCCESS(rc)) + { + RTReqFree(mPendingDummyReq); + mPendingDummyReq = NULL; + } + } +#else + NOREF(cMsTimeout); + rc = VINF_SUCCESS; +#endif /* VBOX_GUEST_PROP_TEST_NOTHREAD not defined */ + return rc; } @@ -1095,7 +1143,7 @@ */ /* static */ -int Service::reqNotify(PFNHGCMSVCEXT pfnCallback, void *pvData, - char *pszName, char *pszValue, uint32_t u32TimeHigh, - uint32_t u32TimeLow, char *pszFlags) +DECLCALLBACK(int) Service::reqNotify(PFNHGCMSVCEXT pfnCallback, void *pvData, + char *pszName, char *pszValue, uint32_t u32TimeHigh, + uint32_t u32TimeLow, char *pszFlags) { LogFlowFunc (("pfnCallback=%p, pvData=%p, pszName=%p, pszValue=%p, u32TimeHigh=%u, u32TimeLow=%u, pszFlags=%p\n", pfnCallback, pvData, pszName, pszValue, u32TimeHigh, u32TimeLow, pszFlags)); @@ -1247,4 +1295,18 @@ break; + /* The host wishes to flush all pending notification */ + case FLUSH_NOTIFICATIONS_HOST: + LogFlowFunc(("FLUSH_NOTIFICATIONS_HOST\n")); + if (cParms == 1) + { + uint32_t cMsTimeout; + rc = paParms[0].getUInt32(&cMsTimeout); + if (RT_SUCCESS(rc)) + rc = flushNotifications(cMsTimeout); + } + else + rc = VERR_INVALID_PARAMETER; + break; + default: rc = VERR_NOT_SUPPORTED; @@ -1264,10 +1326,19 @@ { int rc = VINF_SUCCESS; - unsigned count = 0; - - mfExitThread = true; + + ASMAtomicWriteBool(&mfExitThread, true); + #ifndef VBOX_GUEST_PROP_TEST_NOTHREAD + /* + * Send a dummy request to the thread so it is forced out of the loop and + * notice that the exit flag is set. Give up waiting after 5 mins. + * We call flushNotifications first to try clean up any pending request. + */ + flushNotifications(120*1000); + rc = RTReqCallEx(mReqQueue, NULL, 0, RTREQFLAGS_NO_WAIT, (PFNRT)reqVoid, 0); if (RT_SUCCESS(rc)) + { + unsigned count = 0; do { @@ -1276,4 +1347,5 @@ Assert(RT_SUCCESS(rc) || ((VERR_TIMEOUT == rc) && (count != 5))); } while ((VERR_TIMEOUT == rc) && (count < 300)); + } #endif /* VBOX_GUEST_PROP_TEST_NOTHREAD not defined */ if (RT_SUCCESS(rc)) Index: /trunk/src/VBox/Main/ConsoleImpl.cpp =================================================================== --- /trunk/src/VBox/Main/ConsoleImpl.cpp (revision 24702) +++ /trunk/src/VBox/Main/ConsoleImpl.cpp (revision 24703) @@ -105,4 +105,5 @@ #include // for auto_ptr #include +#include @@ -1309,4 +1310,87 @@ } +/** + * Helper that is used by powerDown to move the guest properties to VBoxSVC. + * + * @param fSaving Whether we're saving a machine state and should + * therefore save transient properties as well. + * + * @returns COM status code. + * + * @remarks This is called without holding the console lock. + */ +HRESULT Console::doMoveGuestPropertiesOnPowerOff(bool fSaving) +{ + /* + * First, flush any pending notifications. + */ + VBOXHGCMSVCPARM parm[1]; + parm[0].setUInt32(20*1000/*ms*/); + int vrc = mVMMDev->hgcmHostCall("VBoxGuestPropSvc", guestProp::FLUSH_NOTIFICATIONS_HOST, 1, &parm[0]); + if (RT_FAILURE(vrc)) + LogRelFunc(("Flushing notifications failed with rc=%Rrc\n", vrc)); + + /* + * Enumerate the properties and + */ + HRESULT hrc; + com::SafeArray namesOut; + com::SafeArray valuesOut; + com::SafeArray timestampsOut; + com::SafeArray flagsOut; + try + { + Bstr pattern(""); + hrc = doEnumerateGuestProperties(pattern, ComSafeArrayAsOutParam(namesOut), + ComSafeArrayAsOutParam(valuesOut), + ComSafeArrayAsOutParam(timestampsOut), + ComSafeArrayAsOutParam(flagsOut)); + if (SUCCEEDED(hrc)) + { + std::vector names; + std::vector values; + std::vector timestamps; + std::vector flags; + for (size_t i = 0; i < namesOut.size(); ++i) + { + uint32_t fFlags = guestProp::NILFLAG; + vrc = guestProp::validateFlags(Utf8Str(flagsOut[i]).raw(), &fFlags); AssertRC(vrc); + if ( fSaving + || !(fFlags & guestProp::TRANSIENT)) + { + names.push_back(namesOut[i]); + values.push_back(valuesOut[i]); + timestamps.push_back(timestampsOut[i]); + flags.push_back(flagsOut[i]); + } + } + com::SafeArray namesIn(names); + com::SafeArray valuesIn(values); + com::SafeArray timestampsIn(timestamps); + com::SafeArray flagsIn(flags); + if ( namesIn.isNull() + || valuesIn.isNull() + || timestampsIn.isNull() + || flagsIn.isNull() + ) + throw std::bad_alloc(); + /* PushGuestProperties() calls DiscardSettings(), which calls us back */ + mControl->PushGuestProperties(ComSafeArrayAsInParam(namesIn), + ComSafeArrayAsInParam(valuesIn), + ComSafeArrayAsInParam(timestampsIn), + ComSafeArrayAsInParam(flagsIn)); + } + } + catch (...) + { + hrc = Console::handleUnexpectedExceptions(RT_SRC_POS); + } + if (FAILED(hrc)) + LogRelFunc(("Failed with hrc=%Rhrc\n", hrc)); + return hrc; +} + + + #endif /* VBOX_WITH_GUEST_PROPS */ @@ -2704,4 +2788,31 @@ ///////////////////////////////////////////////////////////////////////////// +/** + * @copydoc VirtualBox::handleUnexpectedExceptions + */ +/* static */ +HRESULT Console::handleUnexpectedExceptions(RT_SRC_POS_DECL) +{ + try + { + /* re-throw the current exception */ + throw; + } + catch (const std::exception &err) + { + return setError(E_FAIL, tr("Unexpected exception: %s [%s]\n%s[%d] (%s)"), + err.what(), typeid(err).name(), + pszFile, iLine, pszFunction); + } + catch (...) + { + return setError(E_FAIL, tr("Unknown exception\n%s[%d] (%s)"), + pszFile, iLine, pszFunction); + } + + /* should not get here */ + AssertFailed(); + return E_FAIL; +} /* static */ @@ -2750,4 +2861,7 @@ } } + +// private methods +///////////////////////////////////////////////////////////////////////////// /** @@ -4876,5 +4990,5 @@ || mMachineState == MachineState_Restoring || mMachineState == MachineState_TeleportingPausedVM - || mMachineState == MachineState_TeleportingIn /** @todo Teleportation ???*/ + || mMachineState == MachineState_TeleportingIn , ("Invalid machine state: %s\n", Global::stringifyMachineState(mMachineState))); @@ -4894,9 +5008,13 @@ mVMPoweredOff = true; - /* go to Stopping state if not already there. Note that we don't go from - * Saving/Restoring to Stopping because vmstateChangeCallback() needs it to - * set the state to Saved on VMSTATE_TERMINATED. In terms of protecting from - * inappropriate operations while leaving the lock below, Saving or - * Restoring should be fine too. Ditto for Teleporting* -> Teleported. */ + /* + * Go to Stopping state if not already there. + * + * Note that we don't go from Saving/Restoring to Stopping because + * vmstateChangeCallback() needs it to set the state to Saved on + * VMSTATE_TERMINATED. In terms of protecting from inappropriate operations + * while leaving the lock below, Saving or Restoring should be fine too. + * Ditto for TeleportingPausedVM -> Teleported. + */ if ( mMachineState != MachineState_Saving && mMachineState != MachineState_Restoring @@ -4931,93 +5049,4 @@ aProgress->SetCurrentOperationProgress(99 * (++ step) / StepCount ); -#ifdef VBOX_WITH_HGCM - -# ifdef VBOX_WITH_GUEST_PROPS /** @todo r=bird: This may be premature, the VM may still be running at this point! */ - - /* Save all guest property store entries to the machine XML file */ - com::SafeArray namesOut; - com::SafeArray valuesOut; - com::SafeArray timestampsOut; - com::SafeArray flagsOut; - Bstr pattern(""); - if (pattern.isNull()) /** @todo r=bird: What is pattern actually used for? And, again, what's is the out-of-memory policy in main? */ - rc = E_OUTOFMEMORY; - else - rc = doEnumerateGuestProperties(Bstr(""), ComSafeArrayAsOutParam(namesOut), - ComSafeArrayAsOutParam(valuesOut), - ComSafeArrayAsOutParam(timestampsOut), - ComSafeArrayAsOutParam(flagsOut)); - if (SUCCEEDED(rc)) - { - try - { - std::vector names; - std::vector values; - std::vector timestamps; - std::vector flags; - for (unsigned i = 0; i < namesOut.size(); ++i) - { - uint32_t fFlags; - guestProp::validateFlags(Utf8Str(flagsOut[i]).raw(), &fFlags); - if ( !(fFlags & guestProp::TRANSIENT) - || mMachineState == MachineState_Saving - || mMachineState == MachineState_LiveSnapshotting - ) - { - names.push_back(namesOut[i]); - values.push_back(valuesOut[i]); - timestamps.push_back(timestampsOut[i]); - flags.push_back(flagsOut[i]); - } - } - com::SafeArray namesIn(names); - com::SafeArray valuesIn(values); - com::SafeArray timestampsIn(timestamps); - com::SafeArray flagsIn(flags); - if ( namesIn.isNull() - || valuesIn.isNull() - || timestampsIn.isNull() - || flagsIn.isNull() - ) - throw std::bad_alloc(); - /* PushGuestProperties() calls DiscardSettings(), which calls us back */ - alock.leave(); - mControl->PushGuestProperties(ComSafeArrayAsInParam(namesIn), - ComSafeArrayAsInParam(valuesIn), - ComSafeArrayAsInParam(timestampsIn), - ComSafeArrayAsInParam(flagsIn)); - alock.enter(); - } - catch (std::bad_alloc) - { - rc = E_OUTOFMEMORY; - } - } - - /* advance percent count */ - if (aProgress) - aProgress->SetCurrentOperationProgress(99 * (++ step) / StepCount ); - -# endif /* VBOX_WITH_GUEST_PROPS defined */ - - /* Shutdown HGCM services before stopping the guest, because they might - * need a cleanup. */ - if (mVMMDev) - { - LogFlowThisFunc(("Shutdown HGCM...\n")); - - /* Leave the lock since EMT will call us back as addVMCaller() */ - alock.leave(); - - mVMMDev->hgcmShutdown(); - - alock.enter(); - } - - /* advance percent count */ - if (aProgress) - aProgress->SetCurrentOperationProgress(99 * (++ step) / StepCount ); - -#endif /* VBOX_WITH_HGCM */ /* ---------------------------------------------------------------------- @@ -5052,24 +5081,24 @@ vrc = VINF_SUCCESS; - /* Power off the VM if not already done that */ + /* + * Power off the VM if not already done that. + * Leave the lock since EMT will call vmstateChangeCallback. + * + * Note that VMR3PowerOff() may fail here (invalid VMSTATE) if the + * VM-(guest-)initiated power off happened in parallel a ms before this + * call. So far, we let this error pop up on the user's side. + */ if (!mVMPoweredOff) { LogFlowThisFunc(("Powering off the VM...\n")); - - /* Leave the lock since EMT will call us back on VMR3PowerOff() */ alock.leave(); - vrc = VMR3PowerOff(mpVM); - - /* Note that VMR3PowerOff() may fail here (invalid VMSTATE) if the - * VM-(guest-)initiated power off happened in parallel a ms before this - * call. So far, we let this error pop up on the user's side. */ - alock.enter(); - } else { - /* reset the flag for further re-use */ + /** @todo r=bird: Doesn't make sense. Please remove after 3.1 has been branched + * off. */ + /* reset the flag for future re-use */ mVMPoweredOff = false; } @@ -5079,4 +5108,42 @@ aProgress->SetCurrentOperationProgress(99 * (++ step) / StepCount ); +#ifdef VBOX_WITH_HGCM +# ifdef VBOX_WITH_GUEST_PROPS + /* + * Save all guest property store entries to the machine XML file + * and hand controll over to VBoxSVC. Ignoring failure for now. + */ + LogFlowThisFunc(("Moving Guest Properties to XML/VBoxSVC...\n")); + bool fIsSaving = mMachineState == MachineState_Saving + || mMachineState == MachineState_LiveSnapshotting; + alock.leave(); + doMoveGuestPropertiesOnPowerOff(fIsSaving); + alock.enter(); + + /* advance percent count */ + if (aProgress) + aProgress->SetCurrentOperationProgress(99 * (++ step) / StepCount ); + +# endif /* VBOX_WITH_GUEST_PROPS defined */ + + /* Shutdown HGCM services before destroying the VM. */ + if (mVMMDev) + { + LogFlowThisFunc(("Shutdown HGCM...\n")); + + /* Leave the lock since EMT will call us back as addVMCaller() */ + alock.leave(); + + mVMMDev->hgcmShutdown(); + + alock.enter(); + } + + /* advance percent count */ + if (aProgress) + aProgress->SetCurrentOperationProgress(99 * (++ step) / StepCount ); + +#endif /* VBOX_WITH_HGCM */ + LogFlowThisFunc(("Ready for VM destruction.\n")); @@ -5085,5 +5152,5 @@ if (RT_SUCCESS(vrc) || autoCaller.state() == InUninit) { - /* If the machine has an USB comtroller, release all USB devices + /* If the machine has an USB controller, release all USB devices * (symmetric to the code in captureUSBDevices()) */ bool fHasUSBController = false; Index: /trunk/src/VBox/Main/include/ConsoleImpl.h =================================================================== --- /trunk/src/VBox/Main/include/ConsoleImpl.h (revision 24702) +++ /trunk/src/VBox/Main/include/ConsoleImpl.h (revision 24703) @@ -223,4 +223,6 @@ } + static HRESULT handleUnexpectedExceptions(RT_SRC_POS_DECL); + static const char *convertControllerTypeToDev(StorageControllerType_T enmCtrlType); static HRESULT convertBusPortDeviceToLun(StorageBus_T enmBus, LONG port, LONG device, unsigned &uLun); @@ -505,11 +507,11 @@ #ifdef VBOX_WITH_GUEST_PROPS - static DECLCALLBACK(int) doGuestPropNotification (void *pvExtension, uint32_t, - void *pvParms, uint32_t cbParms); - HRESULT doEnumerateGuestProperties (CBSTR aPatterns, - ComSafeArrayOut(BSTR, aNames), - ComSafeArrayOut(BSTR, aValues), - ComSafeArrayOut(ULONG64, aTimestamps), - ComSafeArrayOut(BSTR, aFlags)); + static DECLCALLBACK(int) doGuestPropNotification(void *pvExtension, uint32_t, void *pvParms, uint32_t cbParms); + HRESULT doMoveGuestPropertiesOnPowerOff(bool fSaving); + HRESULT doEnumerateGuestProperties(CBSTR aPatterns, + ComSafeArrayOut(BSTR, aNames), + ComSafeArrayOut(BSTR, aValues), + ComSafeArrayOut(ULONG64, aTimestamps), + ComSafeArrayOut(BSTR, aFlags)); bool enabledGuestPropertiesVRDP (void);