Index: /trunk/src/VBox/Main/include/ConsoleImpl.h =================================================================== --- /trunk/src/VBox/Main/include/ConsoleImpl.h (revision 50722) +++ /trunk/src/VBox/Main/include/ConsoleImpl.h (revision 50723) @@ -5,5 +5,5 @@ /* - * Copyright (C) 2005-2013 Oracle Corporation + * Copyright (C) 2005-2014 Oracle Corporation * * This file is part of VirtualBox Open Source Edition (OSE), as @@ -570,4 +570,7 @@ HRESULT removeSharedFolder(const Utf8Str &strName); + int suspendBeforeConfigChange(PUVM pUVM, AutoWriteLock *pAlock, bool *pfResume); + void resumeAfterConfigChange(PUVM pUVM); + static DECLCALLBACK(int) configConstructor(PUVM pUVM, PVM pVM, void *pvConsole); int configConstructorInner(PUVM pUVM, PVM pVM, AutoWriteLock *pAlock); @@ -597,5 +600,6 @@ bool fHotplug, PUVM pUVM, - DeviceType_T *paLedDevType); + DeviceType_T *paLedDevType, + PCFGMNODE *ppLunL0); int configMedium(PCFGMNODE pLunL0, bool fPassthrough, @@ -611,5 +615,5 @@ MachineState_T aMachineState, HRESULT *phrc); - static DECLCALLBACK(int) reconfigureMediumAttachment(Console *pConsole, + static DECLCALLBACK(int) reconfigureMediumAttachment(Console *pThis, PUVM pUVM, const char *pcszDevice, Index: /trunk/src/VBox/Main/src-client/ConsoleImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/ConsoleImpl.cpp (revision 50722) +++ /trunk/src/VBox/Main/src-client/ConsoleImpl.cpp (revision 50723) @@ -5,5 +5,5 @@ /* - * Copyright (C) 2005-2013 Oracle Corporation + * Copyright (C) 2005-2014 Oracle Corporation * * This file is part of VirtualBox Open Source Edition (OSE), as @@ -2412,5 +2412,5 @@ PVMREQ pReq; vrc = VMR3ReqCallU(pUVM, 0, &pReq, 0 /* no wait! */, VMREQFLAGS_VBOX_STATUS, - (PFNRT)Console::unplugCpu, 3, + (PFNRT)unplugCpu, 3, this, pUVM, (VMCPUID)aCpu); if (vrc == VERR_TIMEOUT || RT_SUCCESS(vrc)) @@ -2518,5 +2518,5 @@ PVMREQ pReq; int vrc = VMR3ReqCallU(pUVM, 0, &pReq, 0 /* no wait! */, VMREQFLAGS_VBOX_STATUS, - (PFNRT)Console::plugCpu, 3, + (PFNRT)plugCpu, 3, this, pUVM, aCpu); @@ -3534,4 +3534,69 @@ // private methods ///////////////////////////////////////////////////////////////////////////// + +/** + * Suspend the VM before we do any medium or network attachment change. + * + * @param pUVM Safe VM handle. + * @param pAlock The automatic lock instance. This is for when we have + * to leave it in order to avoid deadlocks. + * @param pfSuspend where to store the information if we need to resume + * afterwards. + */ +int Console::suspendBeforeConfigChange(PUVM pUVM, AutoWriteLock *pAlock, bool *pfResume) +{ + *pfResume = false; + VMSTATE enmVMState = VMR3GetStateU(pUVM); + switch (enmVMState) + { + case VMSTATE_RESETTING: + case VMSTATE_RUNNING: + { + LogFlowFunc(("Suspending the VM...\n")); + /* disable the callback to prevent Console-level state change */ + mVMStateChangeCallbackDisabled = true; + if (pAlock) + pAlock->release(); + int rc = VMR3Suspend(pUVM, VMSUSPENDREASON_RECONFIG); + if (pAlock) + pAlock->acquire(); + mVMStateChangeCallbackDisabled = false; + AssertRCReturn(rc, rc); + *pfResume = true; + break; + } + case VMSTATE_SUSPENDED: + break; + default: + return VERR_INVALID_STATE; + } + + return VINF_SUCCESS; +} + +/** + * Resume the VM after we did any medium or network attachment change. + * This is the counterpart to Console::suspendBeforeConfigChange(). + * + * @param pUVM Safe VM handle. + */ +void Console::resumeAfterConfigChange(PUVM pUVM) +{ + LogFlowFunc(("Resuming the VM...\n")); + /* disable the callback to prevent Console-level state change */ + mVMStateChangeCallbackDisabled = true; + int rc = VMR3Resume(pUVM, VMRESUMEREASON_RECONFIG); + mVMStateChangeCallbackDisabled = false; + AssertRC(rc); + if (RT_FAILURE(rc)) + { + VMSTATE enmVMState = VMR3GetStateU(pUVM); + if (enmVMState == VMSTATE_SUSPENDED) + { + /* too bad, we failed. try to sync the console state with the VMM state */ + vmstateChangeCallback(pUVM, VMSTATE_SUSPENDED, enmVMState, this); + } + } +} /** @@ -3603,4 +3668,13 @@ /* + * Suspend the VM first. The VM must not be running since it might have + * pending I/O to the drive which is being changed. + */ + bool fResume = false; + int vrc = suspendBeforeConfigChange(pUVM, &alock, &fResume); + if (RT_FAILURE(vrc)) + return vrc; + + /* * Call worker in EMT, that's faster and safer than doing everything * using VMR3ReqCall. Note that we separate VMR3ReqCall from VMR3ReqWait @@ -3608,19 +3682,7 @@ */ PVMREQ pReq; - int vrc = VMR3ReqCallU(pUVM, - VMCPUID_ANY, - &pReq, - 0 /* no wait! */, - VMREQFLAGS_VBOX_STATUS, - (PFNRT)Console::changeRemovableMedium, - 8, - this, - pUVM, - pszDevice, - uInstance, - enmBus, - fUseHostIOCache, - aMediumAttachment, - fForce); + vrc = VMR3ReqCallU(pUVM, VMCPUID_ANY, &pReq, 0 /* no wait! */, VMREQFLAGS_VBOX_STATUS, + (PFNRT)changeRemovableMedium, 8, + this, pUVM, pszDevice, uInstance, enmBus, fUseHostIOCache, aMediumAttachment, fForce); /* release the lock before waiting for a result (EMT will call us back!) */ @@ -3635,4 +3697,7 @@ } VMR3ReqFree(pReq); + + if (fResume) + resumeAfterConfigChange(pUVM); if (RT_SUCCESS(vrc)) @@ -3671,6 +3736,7 @@ * * @thread EMT + * @note The VM must not be running since it might have pending I/O to the drive which is being changed. */ -DECLCALLBACK(int) Console::changeRemovableMedium(Console *pConsole, +DECLCALLBACK(int) Console::changeRemovableMedium(Console *pThis, PUVM pUVM, const char *pcszDevice, @@ -3681,53 +3747,17 @@ bool fForce) { - LogFlowFunc(("pConsole=%p uInstance=%u pszDevice=%p:{%s} enmBus=%u, aMediumAtt=%p, fForce=%d\n", - pConsole, uInstance, pcszDevice, pcszDevice, enmBus, aMediumAtt, fForce)); - - AssertReturn(pConsole, VERR_INVALID_PARAMETER); - - AutoCaller autoCaller(pConsole); + LogFlowFunc(("pThis=%p uInstance=%u pszDevice=%p:{%s} enmBus=%u, aMediumAtt=%p, fForce=%d\n", + pThis, uInstance, pcszDevice, pcszDevice, enmBus, aMediumAtt, fForce)); + + AssertReturn(pThis, VERR_INVALID_PARAMETER); + + AutoCaller autoCaller(pThis); AssertComRCReturn(autoCaller.rc(), VERR_ACCESS_DENIED); /* - * Suspend the VM first. - * - * The VM must not be running since it might have pending I/O to - * the drive which is being changed. + * Check the VM for correct state. */ - bool fResume; VMSTATE enmVMState = VMR3GetStateU(pUVM); - switch (enmVMState) - { - case VMSTATE_RESETTING: - case VMSTATE_RUNNING: - { - LogFlowFunc(("Suspending the VM...\n")); - /* disable the callback to prevent Console-level state change */ - pConsole->mVMStateChangeCallbackDisabled = true; - int rc = VMR3Suspend(pUVM, VMSUSPENDREASON_RECONFIG); - pConsole->mVMStateChangeCallbackDisabled = false; - AssertRCReturn(rc, rc); - fResume = true; - break; - } - - case VMSTATE_SUSPENDED: - case VMSTATE_CREATED: - case VMSTATE_OFF: - fResume = false; - break; - - case VMSTATE_RUNNING_LS: - case VMSTATE_RUNNING_FT: - return setErrorInternal(VBOX_E_INVALID_VM_STATE, - COM_IIDOF(IConsole), - getStaticComponentName(), - (enmVMState == VMSTATE_RUNNING_LS) ? Utf8Str(tr("Cannot change drive during live migration")) : Utf8Str(tr("Cannot change drive during fault tolerant syncing")), - false /*aWarning*/, - true /*aLogIt*/); - - default: - AssertMsgFailedReturn(("enmVMState=%d\n", enmVMState), VERR_ACCESS_DENIED); - } + AssertReturn(enmVMState == VMSTATE_SUSPENDED, VERR_INVALID_STATE); /* Determine the base path for the device instance. */ @@ -3736,55 +3766,28 @@ AssertReturn(pCtlInst, VERR_INTERNAL_ERROR); - int rc = VINF_SUCCESS; - int rcRet = VINF_SUCCESS; - - rcRet = pConsole->configMediumAttachment(pCtlInst, - pcszDevice, - uInstance, - enmBus, - fUseHostIOCache, - false /* fSetupMerge */, - false /* fBuiltinIOCache */, - 0 /* uMergeSource */, - 0 /* uMergeTarget */, - aMediumAtt, - pConsole->mMachineState, - NULL /* phrc */, - true /* fAttachDetach */, - fForce /* fForceUnmount */, - false /* fHotplug */, - pUVM, - NULL /* paLedDevType */); - /** @todo this dumps everything attached to this device instance, which - * is more than necessary. Dumping the changed LUN would be enough. */ - CFGMR3Dump(pCtlInst); - - /* - * Resume the VM if necessary. - */ - if (fResume) - { - LogFlowFunc(("Resuming the VM...\n")); - /* disable the callback to prevent Console-level state change */ - pConsole->mVMStateChangeCallbackDisabled = true; - rc = VMR3Resume(pUVM, VMRESUMEREASON_RECONFIG); - pConsole->mVMStateChangeCallbackDisabled = false; - AssertRC(rc); - if (RT_FAILURE(rc)) - { - /* too bad, we failed. try to sync the console state with the VMM state */ - vmstateChangeCallback(pUVM, VMSTATE_SUSPENDED, enmVMState, pConsole); - } - /// @todo (r=dmik) if we failed with drive mount, then the VMR3Resume - // error (if any) will be hidden from the caller. For proper reporting - // of such multiple errors to the caller we need to enhance the - // IVirtualBoxError interface. For now, give the first error the higher - // priority. - if (RT_SUCCESS(rcRet)) - rcRet = rc; - } - - LogFlowFunc(("Returning %Rrc\n", rcRet)); - return rcRet; + PCFGMNODE pLunL0 = NULL; + int rc = pThis->configMediumAttachment(pCtlInst, + pcszDevice, + uInstance, + enmBus, + fUseHostIOCache, + false /* fSetupMerge */, + false /* fBuiltinIOCache */, + 0 /* uMergeSource */, + 0 /* uMergeTarget */, + aMediumAtt, + pThis->mMachineState, + NULL /* phrc */, + true /* fAttachDetach */, + fForce /* fForceUnmount */, + false /* fHotplug */, + pUVM, + NULL /* paLedDevType */, + &pLunL0); + /* Dump the changed LUN if possible, dump the complete device otherwise */ + CFGMR3Dump(pLunL0 ? pLunL0 : pCtlInst); + + LogFlowFunc(("Returning %Rrc\n", rc)); + return rc; } @@ -3858,4 +3861,13 @@ /* + * Suspend the VM first. The VM must not be running since it might have + * pending I/O to the drive which is being changed. + */ + bool fResume = false; + int vrc = suspendBeforeConfigChange(pUVM, &alock, &fResume); + if (RT_FAILURE(vrc)) + return vrc; + + /* * Call worker in EMT, that's faster and safer than doing everything * using VMR3ReqCall. Note that we separate VMR3ReqCall from VMR3ReqWait @@ -3863,19 +3875,7 @@ */ PVMREQ pReq; - int vrc = VMR3ReqCallU(pUVM, - VMCPUID_ANY, - &pReq, - 0 /* no wait! */, - VMREQFLAGS_VBOX_STATUS, - (PFNRT)Console::attachStorageDevice, - 8, - this, - pUVM, - pszDevice, - uInstance, - enmBus, - fUseHostIOCache, - aMediumAttachment, - fSilent); + vrc = VMR3ReqCallU(pUVM, VMCPUID_ANY, &pReq, 0 /* no wait! */, VMREQFLAGS_VBOX_STATUS, + (PFNRT)attachStorageDevice, 8, + this, pUVM, pszDevice, uInstance, enmBus, fUseHostIOCache, aMediumAttachment, fSilent); /* release the lock before waiting for a result (EMT will call us back!) */ @@ -3890,4 +3890,7 @@ } VMR3ReqFree(pReq); + + if (fResume) + resumeAfterConfigChange(pUVM); if (RT_SUCCESS(vrc)) @@ -3920,6 +3923,7 @@ * * @thread EMT + * @note The VM must not be running since it might have pending I/O to the drive which is being changed. */ -DECLCALLBACK(int) Console::attachStorageDevice(Console *pConsole, +DECLCALLBACK(int) Console::attachStorageDevice(Console *pThis, PUVM pUVM, const char *pcszDevice, @@ -3930,53 +3934,17 @@ bool fSilent) { - LogFlowFunc(("pConsole=%p uInstance=%u pszDevice=%p:{%s} enmBus=%u, aMediumAtt=%p\n", - pConsole, uInstance, pcszDevice, pcszDevice, enmBus, aMediumAtt)); - - AssertReturn(pConsole, VERR_INVALID_PARAMETER); - - AutoCaller autoCaller(pConsole); + LogFlowFunc(("pThis=%p uInstance=%u pszDevice=%p:{%s} enmBus=%u, aMediumAtt=%p\n", + pThis, uInstance, pcszDevice, pcszDevice, enmBus, aMediumAtt)); + + AssertReturn(pThis, VERR_INVALID_PARAMETER); + + AutoCaller autoCaller(pThis); AssertComRCReturn(autoCaller.rc(), VERR_ACCESS_DENIED); /* - * Suspend the VM first. - * - * The VM must not be running since it might have pending I/O to - * the drive which is being changed. + * Check the VM for correct state. */ - bool fResume; VMSTATE enmVMState = VMR3GetStateU(pUVM); - switch (enmVMState) - { - case VMSTATE_RESETTING: - case VMSTATE_RUNNING: - { - LogFlowFunc(("Suspending the VM...\n")); - /* disable the callback to prevent Console-level state change */ - pConsole->mVMStateChangeCallbackDisabled = true; - int rc = VMR3Suspend(pUVM, VMSUSPENDREASON_RECONFIG); - pConsole->mVMStateChangeCallbackDisabled = false; - AssertRCReturn(rc, rc); - fResume = true; - break; - } - - case VMSTATE_SUSPENDED: - case VMSTATE_CREATED: - case VMSTATE_OFF: - fResume = false; - break; - - case VMSTATE_RUNNING_LS: - case VMSTATE_RUNNING_FT: - return setErrorInternal(VBOX_E_INVALID_VM_STATE, - COM_IIDOF(IConsole), - getStaticComponentName(), - (enmVMState == VMSTATE_RUNNING_LS) ? Utf8Str(tr("Cannot change drive during live migration")) : Utf8Str(tr("Cannot change drive during fault tolerant syncing")), - false /*aWarning*/, - true /*aLogIt*/); - - default: - AssertMsgFailedReturn(("enmVMState=%d\n", enmVMState), VERR_ACCESS_DENIED); - } + AssertReturn(enmVMState == VMSTATE_SUSPENDED, VERR_INVALID_STATE); /* @@ -3993,57 +3961,29 @@ AssertReturn(pCtlInst, VERR_INTERNAL_ERROR); - int rc = VINF_SUCCESS; - int rcRet = VINF_SUCCESS; - - rcRet = pConsole->configMediumAttachment(pCtlInst, - pcszDevice, - uInstance, - enmBus, - fUseHostIOCache, - false /* fSetupMerge */, - false /* fBuiltinIOCache */, - 0 /* uMergeSource */, - 0 /* uMergeTarget */, - aMediumAtt, - pConsole->mMachineState, - NULL /* phrc */, - true /* fAttachDetach */, - false /* fForceUnmount */, - !fSilent /* fHotplug */, - pUVM, - NULL /* paLedDevType */); - /** @todo this dumps everything attached to this device instance, which - * is more than necessary. Dumping the changed LUN would be enough. */ + PCFGMNODE pLunL0 = NULL; + int rc = pThis->configMediumAttachment(pCtlInst, + pcszDevice, + uInstance, + enmBus, + fUseHostIOCache, + false /* fSetupMerge */, + false /* fBuiltinIOCache */, + 0 /* uMergeSource */, + 0 /* uMergeTarget */, + aMediumAtt, + pThis->mMachineState, + NULL /* phrc */, + true /* fAttachDetach */, + false /* fForceUnmount */, + !fSilent /* fHotplug */, + pUVM, + NULL /* paLedDevType */, + &pLunL0); + /* Dump the changed LUN if possible, dump the complete device otherwise */ if (enmBus != StorageBus_USB) - CFGMR3Dump(pCtlInst); - - /* - * Resume the VM if necessary. - */ - if (fResume) - { - LogFlowFunc(("Resuming the VM...\n")); - /* disable the callback to prevent Console-level state change */ - pConsole->mVMStateChangeCallbackDisabled = true; - rc = VMR3Resume(pUVM, VMRESUMEREASON_RECONFIG); - pConsole->mVMStateChangeCallbackDisabled = false; - AssertRC(rc); - if (RT_FAILURE(rc)) - { - /* too bad, we failed. try to sync the console state with the VMM state */ - vmstateChangeCallback(pUVM, VMSTATE_SUSPENDED, enmVMState, pConsole); - } - /** @todo if we failed with drive mount, then the VMR3Resume - * error (if any) will be hidden from the caller. For proper reporting - * of such multiple errors to the caller we need to enhance the - * IVirtualBoxError interface. For now, give the first error the higher - * priority. - */ - if (RT_SUCCESS(rcRet)) - rcRet = rc; - } - - LogFlowFunc(("Returning %Rrc\n", rcRet)); - return rcRet; + CFGMR3Dump(pLunL0 ? pLunL0 : pCtlInst); + + LogFlowFunc(("Returning %Rrc\n", rc)); + return rc; } @@ -4113,4 +4053,13 @@ /* + * Suspend the VM first. The VM must not be running since it might have + * pending I/O to the drive which is being changed. + */ + bool fResume = false; + int vrc = suspendBeforeConfigChange(pUVM, &alock, &fResume); + if (RT_FAILURE(vrc)) + return vrc; + + /* * Call worker in EMT, that's faster and safer than doing everything * using VMR3ReqCall. Note that we separate VMR3ReqCall from VMR3ReqWait @@ -4118,18 +4067,7 @@ */ PVMREQ pReq; - int vrc = VMR3ReqCallU(pUVM, - VMCPUID_ANY, - &pReq, - 0 /* no wait! */, - VMREQFLAGS_VBOX_STATUS, - (PFNRT)Console::detachStorageDevice, - 7, - this, - pUVM, - pszDevice, - uInstance, - enmBus, - aMediumAttachment, - fSilent); + vrc = VMR3ReqCallU(pUVM, VMCPUID_ANY, &pReq, 0 /* no wait! */, VMREQFLAGS_VBOX_STATUS, + (PFNRT)detachStorageDevice, 7, + this, pUVM, pszDevice, uInstance, enmBus, aMediumAttachment, fSilent); /* release the lock before waiting for a result (EMT will call us back!) */ @@ -4144,4 +4082,7 @@ } VMR3ReqFree(pReq); + + if (fResume) + resumeAfterConfigChange(pUVM); if (RT_SUCCESS(vrc)) @@ -4173,6 +4114,7 @@ * * @thread EMT + * @note The VM must not be running since it might have pending I/O to the drive which is being changed. */ -DECLCALLBACK(int) Console::detachStorageDevice(Console *pConsole, +DECLCALLBACK(int) Console::detachStorageDevice(Console *pThis, PUVM pUVM, const char *pcszDevice, @@ -4182,53 +4124,17 @@ bool fSilent) { - LogFlowFunc(("pConsole=%p uInstance=%u pszDevice=%p:{%s} enmBus=%u, pMediumAtt=%p\n", - pConsole, uInstance, pcszDevice, pcszDevice, enmBus, pMediumAtt)); - - AssertReturn(pConsole, VERR_INVALID_PARAMETER); - - AutoCaller autoCaller(pConsole); + LogFlowFunc(("pThis=%p uInstance=%u pszDevice=%p:{%s} enmBus=%u, pMediumAtt=%p\n", + pThis, uInstance, pcszDevice, pcszDevice, enmBus, pMediumAtt)); + + AssertReturn(pThis, VERR_INVALID_PARAMETER); + + AutoCaller autoCaller(pThis); AssertComRCReturn(autoCaller.rc(), VERR_ACCESS_DENIED); /* - * Suspend the VM first. - * - * The VM must not be running since it might have pending I/O to - * the drive which is being changed. + * Check the VM for correct state. */ - bool fResume; VMSTATE enmVMState = VMR3GetStateU(pUVM); - switch (enmVMState) - { - case VMSTATE_RESETTING: - case VMSTATE_RUNNING: - { - LogFlowFunc(("Suspending the VM...\n")); - /* disable the callback to prevent Console-level state change */ - pConsole->mVMStateChangeCallbackDisabled = true; - int rc = VMR3Suspend(pUVM, VMSUSPENDREASON_RECONFIG); - pConsole->mVMStateChangeCallbackDisabled = false; - AssertRCReturn(rc, rc); - fResume = true; - break; - } - - case VMSTATE_SUSPENDED: - case VMSTATE_CREATED: - case VMSTATE_OFF: - fResume = false; - break; - - case VMSTATE_RUNNING_LS: - case VMSTATE_RUNNING_FT: - return setErrorInternal(VBOX_E_INVALID_VM_STATE, - COM_IIDOF(IConsole), - getStaticComponentName(), - (enmVMState == VMSTATE_RUNNING_LS) ? Utf8Str(tr("Cannot change drive during live migration")) : Utf8Str(tr("Cannot change drive during fault tolerant syncing")), - false /*aWarning*/, - true /*aLogIt*/); - - default: - AssertMsgFailedReturn(("enmVMState=%d\n", enmVMState), VERR_ACCESS_DENIED); - } + AssertReturn(enmVMState == VMSTATE_SUSPENDED, VERR_INVALID_STATE); /* Determine the base path for the device instance. */ @@ -4274,5 +4180,5 @@ Utf8Str devicePath = Utf8StrFmt("%s/%u/LUN#%u", pcszDevice, uInstance, uLUN); - pConsole->mapMediumAttachments.erase(devicePath); + pThis->mapMediumAttachments.erase(devicePath); } @@ -4286,5 +4192,5 @@ /* Find the correct USB device in the list. */ USBStorageDeviceList::iterator it; - for (it = pConsole->mUSBStorageDevices.begin(); it != pConsole->mUSBStorageDevices.end(); it++) + for (it = pThis->mUSBStorageDevices.begin(); it != pThis->mUSBStorageDevices.end(); it++) { if (it->iPort == lPort) @@ -4292,34 +4198,8 @@ } - AssertReturn(it != pConsole->mUSBStorageDevices.end(), VERR_INTERNAL_ERROR); + AssertReturn(it != pThis->mUSBStorageDevices.end(), VERR_INTERNAL_ERROR); rc = PDMR3UsbDetachDevice(pUVM, &it->mUuid); AssertRCReturn(rc, rc); - pConsole->mUSBStorageDevices.erase(it); - } - - /* - * Resume the VM if necessary. - */ - if (fResume) - { - LogFlowFunc(("Resuming the VM...\n")); - /* disable the callback to prevent Console-level state change */ - pConsole->mVMStateChangeCallbackDisabled = true; - rc = VMR3Resume(pUVM, VMRESUMEREASON_RECONFIG); - pConsole->mVMStateChangeCallbackDisabled = false; - AssertRC(rc); - if (RT_FAILURE(rc)) - { - /* too bad, we failed. try to sync the console state with the VMM state */ - vmstateChangeCallback(pUVM, VMSTATE_SUSPENDED, enmVMState, pConsole); - } - /** @todo: if we failed with drive mount, then the VMR3Resume - * error (if any) will be hidden from the caller. For proper reporting - * of such multiple errors to the caller we need to enhance the - * IVirtualBoxError interface. For now, give the first error the higher - * priority. - */ - if (RT_SUCCESS(rcRet)) - rcRet = rc; + pThis->mUSBStorageDevices.erase(it); } @@ -4563,4 +4443,12 @@ /* + * Suspend the VM first. + */ + bool fResume = false; + int vrc = suspendBeforeConfigChange(pUVM, NULL, &fResume); + if (RT_FAILURE(vrc)) + return vrc; + + /* * Call worker in EMT, that's faster and safer than doing everything * using VM3ReqCall. Note that we separate VMR3ReqCall from VMR3ReqWait @@ -4568,7 +4456,7 @@ */ PVMREQ pReq; - int vrc = VMR3ReqCallU(pUVM, 0 /*idDstCpu*/, &pReq, 0 /* no wait! */, VMREQFLAGS_VBOX_STATUS, - (PFNRT)Console::changeNetworkAttachment, 6, - this, pUVM, pszDevice, uInstance, uLun, aNetworkAdapter); + vrc = VMR3ReqCallU(pUVM, 0 /*idDstCpu*/, &pReq, 0 /* no wait! */, VMREQFLAGS_VBOX_STATUS, + (PFNRT)changeNetworkAttachment, 6, + this, pUVM, pszDevice, uInstance, uLun, aNetworkAdapter); if (vrc == VERR_TIMEOUT || RT_SUCCESS(vrc)) @@ -4580,4 +4468,7 @@ } VMR3ReqFree(pReq); + + if (fResume) + resumeAfterConfigChange(pUVM); if (RT_SUCCESS(vrc)) @@ -4607,4 +4498,5 @@ * @thread EMT * @note Locks the Console object for writing. + * @note The VM must not be running. */ DECLCALLBACK(int) Console::changeNetworkAttachment(Console *pThis, @@ -4642,38 +4534,8 @@ /* - * Suspend the VM first. - * - * The VM must not be running since it might have pending I/O to - * the drive which is being changed. + * Check the VM for correct state. */ - bool fResume; VMSTATE enmVMState = VMR3GetStateU(pUVM); - switch (enmVMState) - { - case VMSTATE_RESETTING: - case VMSTATE_RUNNING: - { - LogFlowFunc(("Suspending the VM...\n")); - /* disable the callback to prevent Console-level state change */ - pThis->mVMStateChangeCallbackDisabled = true; - int rc = VMR3Suspend(pUVM, VMSUSPENDREASON_RECONFIG); - pThis->mVMStateChangeCallbackDisabled = false; - AssertRCReturn(rc, rc); - fResume = true; - break; - } - - case VMSTATE_SUSPENDED: - case VMSTATE_CREATED: - case VMSTATE_OFF: - fResume = false; - break; - - default: - AssertLogRelMsgFailedReturn(("enmVMState=%d\n", enmVMState), VERR_ACCESS_DENIED); - } - - int rc = VINF_SUCCESS; - int rcRet = VINF_SUCCESS; + AssertReturn(enmVMState == VMSTATE_SUSPENDED, VERR_INVALID_STATE); PCFGMNODE pCfg = NULL; /* /Devices/Dev/.../Config/ */ @@ -4682,34 +4544,9 @@ AssertRelease(pInst); - rcRet = pThis->configNetwork(pszDevice, uInstance, uLun, aNetworkAdapter, pCfg, pLunL0, pInst, - true /*fAttachDetach*/, false /*fIgnoreConnectFailure*/); - - /* - * Resume the VM if necessary. - */ - if (fResume) - { - LogFlowFunc(("Resuming the VM...\n")); - /* disable the callback to prevent Console-level state change */ - pThis->mVMStateChangeCallbackDisabled = true; - rc = VMR3Resume(pUVM, VMRESUMEREASON_RECONFIG); - pThis->mVMStateChangeCallbackDisabled = false; - AssertRC(rc); - if (RT_FAILURE(rc)) - { - /* too bad, we failed. try to sync the console state with the VMM state */ - vmstateChangeCallback(pUVM, VMSTATE_SUSPENDED, enmVMState, pThis); - } - /// @todo (r=dmik) if we failed with drive mount, then the VMR3Resume - // error (if any) will be hidden from the caller. For proper reporting - // of such multiple errors to the caller we need to enhance the - // IVirtualBoxError interface. For now, give the first error the higher - // priority. - if (RT_SUCCESS(rcRet)) - rcRet = rc; - } - - LogFlowFunc(("Returning %Rrc\n", rcRet)); - return rcRet; + int rc = pThis->configNetwork(pszDevice, uInstance, uLun, aNetworkAdapter, pCfg, pLunL0, pInst, + true /*fAttachDetach*/, false /*fIgnoreConnectFailure*/); + + LogFlowFunc(("Returning %Rrc\n", rc)); + return rc; } @@ -5760,21 +5597,9 @@ } - vrc = VMR3ReqCallWaitU(ptrVM.rawUVM(), - VMCPUID_ANY, - (PFNRT)reconfigureMediumAttachment, - 13, - this, - ptrVM.rawUVM(), - pcszDevice, - uInstance, - enmBus, - fUseHostIOCache, - fBuiltinIOCache, - true /* fSetupMerge */, - aSourceIdx, - aTargetIdx, - aMediumAttachment, - mMachineState, - &rc); + vrc = VMR3ReqCallWaitU(ptrVM.rawUVM(), VMCPUID_ANY, + (PFNRT)reconfigureMediumAttachment, 13, + this, ptrVM.rawUVM(), pcszDevice, uInstance, enmBus, fUseHostIOCache, + fBuiltinIOCache, true /* fSetupMerge */, aSourceIdx, aTargetIdx, + aMediumAttachment, mMachineState, &rc); /* error handling is after resuming the VM */ @@ -5834,21 +5659,9 @@ rc = mControl->FinishOnlineMergeMedium(); - vrc = VMR3ReqCallWaitU(ptrVM.rawUVM(), - VMCPUID_ANY, - (PFNRT)reconfigureMediumAttachment, - 13, - this, - ptrVM.rawUVM(), - pcszDevice, - uInstance, - enmBus, - fUseHostIOCache, - fBuiltinIOCache, - false /* fSetupMerge */, - 0 /* uMergeSource */, - 0 /* uMergeTarget */, - aMediumAttachment, - mMachineState, - &rc); + vrc = VMR3ReqCallWaitU(ptrVM.rawUVM(), VMCPUID_ANY, + (PFNRT)reconfigureMediumAttachment, 13, + this, ptrVM.rawUVM(), pcszDevice, uInstance, enmBus, fUseHostIOCache, + fBuiltinIOCache, false /* fSetupMerge */, 0 /* uMergeSource */, + 0 /* uMergeTarget */, aMediumAttachment, mMachineState, &rc); /* error handling is after resuming the VM */ @@ -8293,5 +8106,4 @@ this, ptrVM.rawUVM(), aHostDevice, uuid.raw(), fRemote, Address.c_str(), pvRemoteBackend, portVersion, aMaskedIfs); - if (RT_SUCCESS(vrc)) { @@ -9518,5 +9330,5 @@ * Reconfigures a medium attachment (part of taking or deleting an online snapshot). * - * @param pConsole Reference to the console object. + * @param pThis Reference to the console object. * @param pUVM The VM handle. * @param lInstance The instance of the controller. @@ -9532,5 +9344,5 @@ */ /* static */ -DECLCALLBACK(int) Console::reconfigureMediumAttachment(Console *pConsole, +DECLCALLBACK(int) Console::reconfigureMediumAttachment(Console *pThis, PUVM pUVM, const char *pcszDevice, @@ -9548,9 +9360,7 @@ LogFlowFunc(("pUVM=%p aMediumAtt=%p phrc=%p\n", pUVM, aMediumAtt, phrc)); - int rc; HRESULT hrc; Bstr bstr; *phrc = S_OK; -#define RC_CHECK() do { if (RT_FAILURE(rc)) { AssertMsgFailed(("rc=%Rrc\n", rc)); return rc; } } while (0) #define H() do { if (FAILED(hrc)) { AssertMsgFailed(("hrc=%Rhrc (%#x)\n", hrc, hrc)); *phrc = hrc; return VERR_GENERAL_FAILURE; } } while (0) @@ -9573,27 +9383,31 @@ /* Update the device instance configuration. */ - rc = pConsole->configMediumAttachment(pCtlInst, - pcszDevice, - uInstance, - enmBus, - fUseHostIOCache, - fBuiltinIOCache, - fSetupMerge, - uMergeSource, - uMergeTarget, - aMediumAtt, - aMachineState, - phrc, - true /* fAttachDetach */, - false /* fForceUnmount */, - false /* fHotplug */, - pUVM, - NULL /* paLedDevType */); - /** @todo this dumps everything attached to this device instance, which - * is more than necessary. Dumping the changed LUN would be enough. */ - CFGMR3Dump(pCtlInst); - RC_CHECK(); - -#undef RC_CHECK + PCFGMNODE pLunL0 = NULL; + int rc = pThis->configMediumAttachment(pCtlInst, + pcszDevice, + uInstance, + enmBus, + fUseHostIOCache, + fBuiltinIOCache, + fSetupMerge, + uMergeSource, + uMergeTarget, + aMediumAtt, + aMachineState, + phrc, + true /* fAttachDetach */, + false /* fForceUnmount */, + false /* fHotplug */, + pUVM, + NULL /* paLedDevType */, + &pLunL0); + /* Dump the changed LUN if possible, dump the complete device otherwise */ + CFGMR3Dump(pLunL0 ? pLunL0 : pCtlInst); + if (RT_FAILURE(rc)) + { + AssertMsgFailed(("rc=%Rrc\n", rc)); + return rc; + } + #undef H @@ -9777,21 +9591,9 @@ * isn't going to need the Console lock. */ - vrc = VMR3ReqCallWaitU(ptrVM.rawUVM(), - VMCPUID_ANY, - (PFNRT)reconfigureMediumAttachment, - 13, - that, - ptrVM.rawUVM(), - pcszDevice, - lInstance, - enmBus, - fUseHostIOCache, - fBuiltinIOCache, - false /* fSetupMerge */, - 0 /* uMergeSource */, - 0 /* uMergeTarget */, - atts[i], - that->mMachineState, - &rc); + vrc = VMR3ReqCallWaitU(ptrVM.rawUVM(), VMCPUID_ANY, + (PFNRT)reconfigureMediumAttachment, 13, + that, ptrVM.rawUVM(), pcszDevice, lInstance, enmBus, fUseHostIOCache, + fBuiltinIOCache, false /* fSetupMerge */, 0 /* uMergeSource */, + 0 /* uMergeTarget */, atts[i], that->mMachineState, &rc); if (RT_FAILURE(vrc)) throw setErrorStatic(E_FAIL, Console::tr("%Rrc"), vrc); Index: /trunk/src/VBox/Main/src-client/ConsoleImpl2.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/ConsoleImpl2.cpp (revision 50722) +++ /trunk/src/VBox/Main/src-client/ConsoleImpl2.cpp (revision 50723) @@ -2194,5 +2194,6 @@ false /* fHotplug */, pUVM, - paLedDevType); + paLedDevType, + NULL /* ppLunL0 */); if (RT_FAILURE(rc)) return rc; @@ -3496,5 +3497,6 @@ bool fHotplug, PUVM pUVM, - DeviceType_T *paLedDevType) + DeviceType_T *paLedDevType, + PCFGMNODE *ppLunL0) { // InsertConfig* throws @@ -3610,4 +3612,6 @@ InsertConfigNode(pCtlInst, Utf8StrFmt("LUN#%u", uLUN).c_str(), &pLunL0); + if (ppLunL0) + *ppLunL0 = pLunL0; PCFGMNODE pCfg = CFGMR3GetChild(pCtlInst, "Config");