Changeset 36991 in vbox for trunk/src/VBox/Main

Timestamp:

May 6, 2011 7:16:50 PM (14 years ago)

Author:

vboxsync

Message:

Main: Initial support for disk hotplugging, work in progress

Location:

trunk/src/VBox/Main

Files:

• idl/VirtualBox.xidl (modified) (5 diffs)
• include/ConsoleImpl.h (modified) (3 diffs)
• include/MachineImpl.h (modified) (3 diffs)
• include/SessionImpl.h (modified) (1 diff)
• src-client/ConsoleImpl.cpp (modified) (4 diffs)
• src-client/ConsoleImpl2.cpp (modified) (4 diffs)
• src-client/SessionImpl.cpp (modified) (1 diff)
• src-server/MachineImpl.cpp (modified) (6 diffs)

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

@@ -13549 +13549 @@
   <interface
     name="IInternalSessionControl" extends="$unknown"
-    uuid="a2fbf834-149d-41da-ae52-0dc3b0f032b3"
+    uuid="3a975b65-27e7-42fa-9176-d097d7bd78d4"
     internal="yes"
     wsmap="suppress"
@@ -13715 +13715 @@
       <param name="mediumAttachment" type="IMediumAttachment" dir="in"/>
       <param name="force" type="boolean" dir="in"/>
+    </method>
+
+    <method name="onStorageDeviceChange">
+      <desc>
+        Triggered when attached storage devices of the
+        associated virtual machine have changed.
+
+        <result name="VBOX_E_INVALID_VM_STATE">
+          Session state prevents operation.
+        </result>
+        <result name="VBOX_E_INVALID_OBJECT_STATE">
+          Session type prevents operation.
+        </result>
+
+      </desc>
+
+      <param name="mediumAttachment" type="IMediumAttachment" dir="in"/>
+      <param name="remove" type="boolean" dir="in">
+        <desc>TRUE if the device is removed, FALSE if it was added.</desc>
+      </param>
     </method>
 
@@ -15437 +15457 @@
   <enum
     name="VBoxEventType"
-    uuid="e71c487f-755e-46e9-a476-dd6a5d134597"
+    uuid="cce48db6-8561-479d-8d46-1358bab45d4e"
     >
 
@@ -15690 +15710 @@
       </desc>
     </const>
+    <const name="OnStorageDeviceChanged" value="71">
+      <desc>
+        See <link to="IStorageDeviceChangedEvent">IStorageDeviceChangedEvent</link>.
+      </desc>
+    </const>
 
     <!-- Last event marker -->
-    <const name="Last" value="71">
+    <const name="Last" value="72">
       <desc>
         Must be last event, used for iterations and structures relying on numerical event values.
@@ -17088 +17113 @@
   </interface>
 
+  <interface
+    name="IStorageDeviceChangedEvent" extends="IEvent"
+    uuid="8a5c2dce-e341-49d4-afce-c95979f7d70c"
+    wsmap="managed" autogen="VBoxEvent" id="OnStorageDeviceChanged"
+    >
+    <desc>
+      Notification when a
+      <link to="IMachine::mediumAttachments">storage device</link>
+      is attached or removed.
+    </desc>
+    <attribute name="storageDevice" type="IMediumAttachment" readonly="yes">
+      <desc>
+        Storage device that is subject to change.
+      </desc>
+    </attribute>
+    <attribute name="removed" type="boolean" readonly="yes">
+      <desc>
+        Flag whether the device was removed or added to the VM.
+      </desc>
+    </attribute>
+  </interface>
+
   <module name="VBoxSVC" context="LocalServer">
     <class name="VirtualBox" uuid="B1A7A4F2-47B9-4A1E-82B2-07CCD5323C3F"

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

@@ -191 +191 @@
     HRESULT onUSBDeviceDetach(IN_BSTR aId, IVirtualBoxErrorInfo *aError);
     HRESULT onBandwidthGroupChange(IBandwidthGroup *aBandwidthGroup);
+    HRESULT onStorageDeviceChange(IMediumAttachment *aMediumAttachment, BOOL aRemove);
     HRESULT getGuestProperty(IN_BSTR aKey, BSTR *aValue, LONG64 *aTimestamp, BSTR *aFlags);
     HRESULT setGuestProperty(IN_BSTR aKey, IN_BSTR aValue, IN_BSTR aFlags);
@@ -505 +506 @@
                                bool fAttachDetach,
                                bool fForceUnmount,
+                               bool fHotplug,
                                PVM pVM,
                                DeviceType_T *paLedDevType);
@@ -571 +573 @@
 #endif
 
+    static DECLCALLBACK(int) attachStorageDevice(Console *pThis,
+                                                 PVM pVM,
+                                                 const char *pcszDevice,
+                                                 unsigned uInstance,
+                                                 StorageBus_T enmBus,
+                                                 bool fUseHostIOCache,
+                                                 IMediumAttachment *aMediumAtt);
+    static DECLCALLBACK(int) detachStorageDevice(Console *pThis,
+                                                 PVM pVM,
+                                                 const char *pcszDevice,
+                                                 unsigned uInstance,
+                                                 StorageBus_T enmBus,
+                                                 IMediumAttachment *aMediumAtt);
+    HRESULT doStorageDeviceAttach(IMediumAttachment *aMediumAttachment, PVM pVM);
+    HRESULT doStorageDeviceDetach(IMediumAttachment *aMediumAttachment, PVM pVM);
+
     static DECLCALLBACK(int)    fntTakeSnapshotWorker(RTTHREAD Thread, void *pvUser);
 

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

@@ -638 +638 @@
     virtual HRESULT onSharedFolderChange() { return S_OK; }
     virtual HRESULT onBandwidthGroupChange(IBandwidthGroup * /* aBandwidthGroup */) { return S_OK; }
+    virtual HRESULT onStorageDeviceChange(IMediumAttachment * /* mediumAttachment */, BOOL /* remove */) { return S_OK; }
 
     HRESULT saveRegistryEntry(settings::MachineRegistryEntry &data);
@@ -833 +834 @@
     void commit();
     void copyFrom(Machine *aThat);
+    bool isControllerHotplugCapable(StorageControllerType_T enmCtrlType);
 
     struct DeleteTask;
@@ -1005 +1007 @@
     HRESULT onSharedFolderChange();
     HRESULT onBandwidthGroupChange(IBandwidthGroup *aBandwidthGroup);
+    HRESULT onStorageDeviceChange(IMediumAttachment *aMediumAttachment, BOOL aRemove);
 
     bool hasMatchingUSBFilter(const ComObjPtr<HostUSBDevice> &aDevice, ULONG *aMaskedIfs);

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

@@ -99 +99 @@
     STDMETHOD(OnShowWindow)(BOOL aCheck, BOOL *aCanShow, LONG64 *aWinId);
     STDMETHOD(OnBandwidthGroupChange)(IBandwidthGroup *aBandwidthGroup);
+    STDMETHOD(OnStorageDeviceChange)(IMediumAttachment *aMediumAttachment, BOOL aRemove);
     STDMETHOD(AccessGuestProperty)(IN_BSTR aName, IN_BSTR aValue, IN_BSTR aFlags,
                                    BOOL aIsSetter, BSTR *aRetValue, LONG64 *aRetTimestamp, BSTR *aRetFlags);

trunk/src/VBox/Main/src-client/ConsoleImpl.cpp

@@ -3556 +3556 @@
                                              true /* fAttachDetach */,
                                              fForce /* fForceUnmount */,
+                                             false  /* fHotplug */,
                                              pVM,
                                              NULL /* paLedDevType */);
@@ -3591 +3592 @@
 }
 
+
+/**
+ * Attach a new storage device to the VM.
+ *
+ * @param aMediumAttachment The medium attachmentwhich is added.
+ * @param pVM               Safe VM handle.
+ *
+ * @note Locks this object for writing.
+ */
+HRESULT Console::doStorageDeviceAttach(IMediumAttachment *aMediumAttachment, PVM pVM)
+{
+    AutoCaller autoCaller(this);
+    AssertComRCReturnRC(autoCaller.rc());
+
+    /* We will need to release the write lock before calling EMT */
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+    HRESULT rc = S_OK;
+    const char *pszDevice = NULL;
+
+    SafeIfaceArray<IStorageController> ctrls;
+    rc = mMachine->COMGETTER(StorageControllers)(ComSafeArrayAsOutParam(ctrls));
+    AssertComRC(rc);
+    IMedium *pMedium;
+    rc = aMediumAttachment->COMGETTER(Medium)(&pMedium);
+    AssertComRC(rc);
+    Bstr mediumLocation;
+    if (pMedium)
+    {
+        rc = pMedium->COMGETTER(Location)(mediumLocation.asOutParam());
+        AssertComRC(rc);
+    }
+
+    Bstr attCtrlName;
+    rc = aMediumAttachment->COMGETTER(Controller)(attCtrlName.asOutParam());
+    AssertComRC(rc);
+    ComPtr<IStorageController> pStorageController;
+    for (size_t i = 0; i < ctrls.size(); ++i)
+    {
+        Bstr ctrlName;
+        rc = ctrls[i]->COMGETTER(Name)(ctrlName.asOutParam());
+        AssertComRC(rc);
+        if (attCtrlName == ctrlName)
+        {
+            pStorageController = ctrls[i];
+            break;
+        }
+    }
+    if (pStorageController.isNull())
+        return setError(E_FAIL,
+                        tr("Could not find storage controller '%ls'"), attCtrlName.raw());
+
+    StorageControllerType_T enmCtrlType;
+    rc = pStorageController->COMGETTER(ControllerType)(&enmCtrlType);
+    AssertComRC(rc);
+    pszDevice = convertControllerTypeToDev(enmCtrlType);
+
+    StorageBus_T enmBus;
+    rc = pStorageController->COMGETTER(Bus)(&enmBus);
+    AssertComRC(rc);
+    ULONG uInstance;
+    rc = pStorageController->COMGETTER(Instance)(&uInstance);
+    AssertComRC(rc);
+    BOOL fUseHostIOCache;
+    rc = pStorageController->COMGETTER(UseHostIOCache)(&fUseHostIOCache);
+    AssertComRC(rc);
+
+    /*
+     * Call worker in EMT, that's faster and safer than doing everything
+     * using VMR3ReqCall. Note that we separate VMR3ReqCall from VMR3ReqWait
+     * here to make requests from under the lock in order to serialize them.
+     */
+    PVMREQ pReq;
+    int vrc = VMR3ReqCall(pVM,
+                          VMCPUID_ANY,
+                          &pReq,
+                          0 /* no wait! */,
+                          VMREQFLAGS_VBOX_STATUS,
+                          (PFNRT)Console::attachStorageDevice,
+                          7,
+                          this,
+                          pVM,
+                          pszDevice,
+                          uInstance,
+                          enmBus,
+                          fUseHostIOCache,
+                          aMediumAttachment);
+
+    /* leave the lock before waiting for a result (EMT will call us back!) */
+    alock.leave();
+
+    if (vrc == VERR_TIMEOUT || RT_SUCCESS(vrc))
+    {
+        vrc = VMR3ReqWait(pReq, RT_INDEFINITE_WAIT);
+        AssertRC(vrc);
+        if (RT_SUCCESS(vrc))
+            vrc = pReq->iStatus;
+    }
+    VMR3ReqFree(pReq);
+
+    if (RT_SUCCESS(vrc))
+    {
+        LogFlowThisFunc(("Returns S_OK\n"));
+        return S_OK;
+    }
+
+    if (!pMedium)
+        return setError(E_FAIL,
+                        tr("Could not mount the media/drive '%ls' (%Rrc)"),
+                        mediumLocation.raw(), vrc);
+
+    return setError(E_FAIL,
+                    tr("Could not unmount the currently mounted media/drive (%Rrc)"),
+                    vrc);
+}
+
+
+/**
+ * Performs the storage attach operation in EMT.
+ *
+ * @returns VBox status code.
+ *
+ * @param   pThis           Pointer to the Console object.
+ * @param   pVM             The VM handle.
+ * @param   pcszDevice      The PDM device name.
+ * @param   uInstance       The PDM device instance.
+ *
+ * @thread  EMT
+ */
+DECLCALLBACK(int) Console::attachStorageDevice(Console *pConsole,
+                                               PVM pVM,
+                                               const char *pcszDevice,
+                                               unsigned uInstance,
+                                               StorageBus_T enmBus,
+                                               bool fUseHostIOCache,
+                                               IMediumAttachment *aMediumAtt)
+{
+    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);
+    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.
+     */
+    bool fResume;
+    VMSTATE enmVMState = VMR3GetState(pVM);
+    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(pVM);
+            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);
+    }
+
+    /* Determine the base path for the device instance. */
+    PCFGMNODE pCtlInst;
+    pCtlInst = CFGMR3GetChildF(CFGMR3GetRoot(pVM), "Devices/%s/%u/", pcszDevice, uInstance);
+    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 */,
+                                             true   /* fHotplug */,
+                                             pVM,
+                                             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(pVM);
+        pConsole->mVMStateChangeCallbackDisabled = false;
+        AssertRC(rc);
+        if (RT_FAILURE(rc))
+        {
+            /* too bad, we failed. try to sync the console state with the VMM state */
+            vmstateChangeCallback(pVM, 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;
+}
+
+/**
+ * Attach a new storage device to the VM.
+ *
+ * @param aMediumAttachment The medium attachmentwhich is added.
+ * @param pVM               Safe VM handle.
+ *
+ * @note Locks this object for writing.
+ */
+HRESULT Console::doStorageDeviceDetach(IMediumAttachment *aMediumAttachment, PVM pVM)
+{
+    AutoCaller autoCaller(this);
+    AssertComRCReturnRC(autoCaller.rc());
+
+    /* We will need to release the write lock before calling EMT */
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+    HRESULT rc = S_OK;
+    const char *pszDevice = NULL;
+
+    SafeIfaceArray<IStorageController> ctrls;
+    rc = mMachine->COMGETTER(StorageControllers)(ComSafeArrayAsOutParam(ctrls));
+    AssertComRC(rc);
+    IMedium *pMedium;
+    rc = aMediumAttachment->COMGETTER(Medium)(&pMedium);
+    AssertComRC(rc);
+    Bstr mediumLocation;
+    if (pMedium)
+    {
+        rc = pMedium->COMGETTER(Location)(mediumLocation.asOutParam());
+        AssertComRC(rc);
+    }
+
+    Bstr attCtrlName;
+    rc = aMediumAttachment->COMGETTER(Controller)(attCtrlName.asOutParam());
+    AssertComRC(rc);
+    ComPtr<IStorageController> pStorageController;
+    for (size_t i = 0; i < ctrls.size(); ++i)
+    {
+        Bstr ctrlName;
+        rc = ctrls[i]->COMGETTER(Name)(ctrlName.asOutParam());
+        AssertComRC(rc);
+        if (attCtrlName == ctrlName)
+        {
+            pStorageController = ctrls[i];
+            break;
+        }
+    }
+    if (pStorageController.isNull())
+        return setError(E_FAIL,
+                        tr("Could not find storage controller '%ls'"), attCtrlName.raw());
+
+    StorageControllerType_T enmCtrlType;
+    rc = pStorageController->COMGETTER(ControllerType)(&enmCtrlType);
+    AssertComRC(rc);
+    pszDevice = convertControllerTypeToDev(enmCtrlType);
+
+    StorageBus_T enmBus;
+    rc = pStorageController->COMGETTER(Bus)(&enmBus);
+    AssertComRC(rc);
+    ULONG uInstance;
+    rc = pStorageController->COMGETTER(Instance)(&uInstance);
+    AssertComRC(rc);
+
+    /*
+     * Call worker in EMT, that's faster and safer than doing everything
+     * using VMR3ReqCall. Note that we separate VMR3ReqCall from VMR3ReqWait
+     * here to make requests from under the lock in order to serialize them.
+     */
+    PVMREQ pReq;
+    int vrc = VMR3ReqCall(pVM,
+                          VMCPUID_ANY,
+                          &pReq,
+                          0 /* no wait! */,
+                          VMREQFLAGS_VBOX_STATUS,
+                          (PFNRT)Console::detachStorageDevice,
+                          6,
+                          this,
+                          pVM,
+                          pszDevice,
+                          uInstance,
+                          enmBus,
+                          aMediumAttachment);
+
+    /* leave the lock before waiting for a result (EMT will call us back!) */
+    alock.leave();
+
+    if (vrc == VERR_TIMEOUT || RT_SUCCESS(vrc))
+    {
+        vrc = VMR3ReqWait(pReq, RT_INDEFINITE_WAIT);
+        AssertRC(vrc);
+        if (RT_SUCCESS(vrc))
+            vrc = pReq->iStatus;
+    }
+    VMR3ReqFree(pReq);
+
+    if (RT_SUCCESS(vrc))
+    {
+        LogFlowThisFunc(("Returns S_OK\n"));
+        return S_OK;
+    }
+
+    if (!pMedium)
+        return setError(E_FAIL,
+                        tr("Could not mount the media/drive '%ls' (%Rrc)"),
+                        mediumLocation.raw(), vrc);
+
+    return setError(E_FAIL,
+                    tr("Could not unmount the currently mounted media/drive (%Rrc)"),
+                    vrc);
+}
+
+/**
+ * Performs the storage detach operation in EMT.
+ *
+ * @returns VBox status code.
+ *
+ * @param   pThis           Pointer to the Console object.
+ * @param   pVM             The VM handle.
+ * @param   pcszDevice      The PDM device name.
+ * @param   uInstance       The PDM device instance.
+ *
+ * @thread  EMT
+ */
+DECLCALLBACK(int) Console::detachStorageDevice(Console *pConsole,
+                                               PVM pVM,
+                                               const char *pcszDevice,
+                                               unsigned uInstance,
+                                               StorageBus_T enmBus,
+                                               IMediumAttachment *pMediumAtt)
+{
+    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);
+    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.
+     */
+    bool fResume;
+    VMSTATE enmVMState = VMR3GetState(pVM);
+    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(pVM);
+            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);
+    }
+
+    /* Determine the base path for the device instance. */
+    PCFGMNODE pCtlInst;
+    pCtlInst = CFGMR3GetChildF(CFGMR3GetRoot(pVM), "Devices/%s/%u/", pcszDevice, uInstance);
+    AssertReturn(pCtlInst, VERR_INTERNAL_ERROR);
+
+#define H()         AssertMsgReturn(!FAILED(hrc), ("hrc=%Rhrc\n", hrc), VERR_GENERAL_FAILURE)
+
+    HRESULT hrc;
+    int rc = VINF_SUCCESS;
+    int rcRet = VINF_SUCCESS;
+    unsigned uLUN;
+    LONG lDev;
+    LONG lPort;
+    DeviceType_T lType;
+    PCFGMNODE pLunL0 = NULL;
+    PCFGMNODE pCfg = NULL;
+
+    hrc = pMediumAtt->COMGETTER(Device)(&lDev);                             H();
+    hrc = pMediumAtt->COMGETTER(Port)(&lPort);                              H();
+    hrc = pMediumAtt->COMGETTER(Type)(&lType);                              H();
+    hrc = Console::convertBusPortDeviceToLun(enmBus, lPort, lDev, uLUN);    H();
+
+#undef H
+
+    /* First check if the LUN really exists. */
+    pLunL0 = CFGMR3GetChildF(pCtlInst, "LUN#%u", uLUN);
+    if (pLunL0)
+    {
+            rc = PDMR3DeviceDetach(pVM, pcszDevice, uInstance, uLUN, 0);
+            if (rc == VERR_PDM_NO_DRIVER_ATTACHED_TO_LUN)
+                rc = VINF_SUCCESS;
+            AssertRCReturn(rc, rc);
+            CFGMR3RemoveNode(pLunL0);
+    }
+    else
+        AssertFailedReturn(VERR_INTERNAL_ERROR);
+
+    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(pVM);
+        pConsole->mVMStateChangeCallbackDisabled = false;
+        AssertRC(rc);
+        if (RT_FAILURE(rc))
+        {
+            /* too bad, we failed. try to sync the console state with the VMM state */
+            vmstateChangeCallback(pVM, 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;
+}
 
 /**
@@ -4513 +5008 @@
 
 /**
+ * Called by IInternalSessionControl::OnStorageDeviceChange().
+ *
+ * @note Locks this object for writing.
+ */
+HRESULT Console::onStorageDeviceChange(IMediumAttachment *aMediumAttachment, BOOL aRemove)
+{
+    LogFlowThisFunc(("\n"));
+
+    AutoCaller autoCaller(this);
+    AssertComRCReturnRC(autoCaller.rc());
+
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+    HRESULT rc = S_OK;
+
+    /* don't trigger medium change if the VM isn't running */
+    SafeVMPtrQuiet ptrVM(this);
+    if (ptrVM.isOk())
+    {
+        if (aRemove)
+            rc = doStorageDeviceDetach(aMediumAttachment, ptrVM);
+        else
+            rc = doStorageDeviceAttach(aMediumAttachment, ptrVM);
+        ptrVM.release();
+    }
+
+    /* notify console callbacks on success */
+    if (SUCCEEDED(rc))
+        fireStorageDeviceChangedEvent(mEventSource, aMediumAttachment, aRemove);
+
+    LogFlowThisFunc(("Leaving rc=%#x\n", rc));
+    return rc;
+}
+
+/**
  * @note Temporarily locks this object for writing.
  */
@@ -8332 +8862 @@
                                           true /* fAttachDetach */,
                                           false /* fForceUnmount */,
+                                          false /* fHotplug */,
                                           pVM,
                                           NULL /* paLedDevType */);

trunk/src/VBox/Main/src-client/ConsoleImpl2.cpp

@@ -1651 +1651 @@
                                             false /* fAttachDetach */,
                                             false /* fForceUnmount */,
+                                            false /* fHotplug */,
                                             pVM,
                                             paLedDevType);
@@ -2813 +2814 @@
                                     bool fAttachDetach,
                                     bool fForceUnmount,
+                                    bool fHotplug,
                                     PVM pVM,
                                     DeviceType_T *paLedDevType)
@@ -2870 +2872 @@
                 }
 
-                rc = PDMR3DeviceDetach(pVM, pcszDevice, uInstance, uLUN, PDM_TACH_FLAGS_NOT_HOT_PLUG);
+                rc = PDMR3DeviceDetach(pVM, pcszDevice, uInstance, uLUN, fHotplug ? 0 : PDM_TACH_FLAGS_NOT_HOT_PLUG);
                 if (rc == VERR_PDM_NO_DRIVER_ATTACHED_TO_LUN)
                     rc = VINF_SUCCESS;
@@ -3109 +3111 @@
             /* Attach the new driver. */
             rc = PDMR3DeviceAttach(pVM, pcszDevice, uInstance, uLUN,
-                                PDM_TACH_FLAGS_NOT_HOT_PLUG, NULL /*ppBase*/);
+                                fHotplug ? 0 : PDM_TACH_FLAGS_NOT_HOT_PLUG, NULL /*ppBase*/);
             AssertRCReturn(rc, rc);
 

trunk/src/VBox/Main/src-client/SessionImpl.cpp

@@ -725 +725 @@
 
     return mConsole->onBandwidthGroupChange(aBandwidthGroup);
+}
+
+STDMETHODIMP Session::OnStorageDeviceChange(IMediumAttachment *aMediumAttachment, BOOL aRemove)
+{
+    LogFlowThisFunc(("\n"));
+
+    AutoCaller autoCaller(this);
+    AssertComRCReturn(autoCaller.rc(), autoCaller.rc());
+
+    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+    AssertReturn(mState == SessionState_Locked, VBOX_E_INVALID_VM_STATE);
+    AssertReturn(mType == SessionType_WriteLock, VBOX_E_INVALID_OBJECT_STATE);
+
+    return mConsole->onStorageDeviceChange(aMediumAttachment, aRemove);
 }
 

trunk/src/VBox/Main/src-server/MachineImpl.cpp

@@ -3404 +3404 @@
     AssertReturn(mData->mMachineState != MachineState_Saved, E_FAIL);
 
-    if (Global::IsOnlineOrTransient(mData->mMachineState))
-        return setError(VBOX_E_INVALID_VM_STATE,
-                        tr("Invalid machine state: %s"),
-                        Global::stringifyMachineState(mData->mMachineState));
-
     /* Check for an existing controller. */
     ComObjPtr<StorageController> ctl;
     rc = getStorageControllerByName(aControllerName, ctl, true /* aSetError */);
     if (FAILED(rc)) return rc;
+
+    StorageControllerType_T ctrlType;
+    rc = ctl->COMGETTER(ControllerType)(&ctrlType);
+    if (FAILED(rc))
+        return setError(E_FAIL,
+                        tr("Could not get type of controller '%ls'"),
+                        aControllerName);
+
+    /* Check that the controller can do hotplugging if we detach the device while the VM is running. */
+    bool fHotplug = false;
+    if (Global::IsOnlineOrTransient(mData->mMachineState))
+        fHotplug = true;
+
+    if (fHotplug && !isControllerHotplugCapable(ctrlType))
+        return setError(VBOX_E_INVALID_VM_STATE,
+                        tr("Invalid machine state: %s"),
+                        Global::stringifyMachineState(mData->mMachineState));
 
     // check that the port and device are not out of range
@@ -3784 +3796 @@
     alock.release();
 
+    if (fHotplug)
+        rc = onStorageDeviceChange(attachment, FALSE /* aRemove */);
+
     mParent->saveRegistries(llRegistriesThatNeedSaving);
 
@@ -3809 +3824 @@
     AssertReturn(mData->mMachineState != MachineState_Saved, E_FAIL);
 
+    /* Check for an existing controller. */
+    ComObjPtr<StorageController> ctl;
+    rc = getStorageControllerByName(aControllerName, ctl, true /* aSetError */);
+    if (FAILED(rc)) return rc;
+
+    StorageControllerType_T ctrlType;
+    rc = ctl->COMGETTER(ControllerType)(&ctrlType);
+    if (FAILED(rc))
+        return setError(E_FAIL,
+                        tr("Could not get type of controller '%ls'"),
+                        aControllerName);
+
+    /* Check that the controller can do hotplugging if we detach the device while the VM is running. */
+    bool fHotplug = false;
     if (Global::IsOnlineOrTransient(mData->mMachineState))
+        fHotplug = true;
+
+    if (fHotplug && !isControllerHotplugCapable(ctrlType))
         return setError(VBOX_E_INVALID_VM_STATE,
                         tr("Invalid machine state: %s"),
@@ -3823 +3855 @@
                         aDevice, aControllerPort, aControllerName);
 
+    /*
+     * The VM has to detach the device before we delete any implicit diffs.
+     * If this fails we can roll back without loosing data.
+     */
+    if (fHotplug)
+    {
+        alock.leave();
+        rc = onStorageDeviceChange(pAttach, TRUE /* aRemove */);
+        alock.enter();
+    }
+    if (FAILED(rc)) return rc;
+
+    /* If we are here everything went well and we can delete the implicit now. */
     rc = detachDevice(pAttach, alock, NULL /* pSnapshot */, &llRegistriesThatNeedSaving);
 
@@ -10116 +10161 @@
     for (ULONG slot = 0; slot < RT_ELEMENTS(mParallelPorts); slot++)
         mParallelPorts[slot]->copyFrom(aThat->mParallelPorts[slot]);
+}
+
+/**
+ * Returns whether the given storage controller is hotplug capable.
+ *
+ * @returns true if the controller supports hotplugging
+ *          false otherwise.
+ * @param   enmCtrlType    The controller type to check for.
+ */
+bool Machine::isControllerHotplugCapable(StorageControllerType_T enmCtrlType)
+{
+    switch (enmCtrlType)
+    {
+        case StorageControllerType_IntelAhci:
+            return true;
+        case StorageControllerType_LsiLogic:
+        case StorageControllerType_LsiLogicSas:
+        case StorageControllerType_BusLogic:
+        case StorageControllerType_PIIX3:
+        case StorageControllerType_PIIX4:
+        case StorageControllerType_ICH6:
+        case StorageControllerType_I82078:
+        default:
+            return false;
+    }
 }
 
@@ -11836 +11906 @@
 
 /**
+ *  @note Locks this object for reading.
+ */
+HRESULT SessionMachine::onStorageDeviceChange(IMediumAttachment *aAttachment, BOOL aRemove)
+{
+    LogFlowThisFunc(("\n"));
+
+    AutoCaller autoCaller(this);
+    AssertComRCReturn(autoCaller.rc(), autoCaller.rc());
+
+    ComPtr<IInternalSessionControl> directControl;
+    {
+        AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+        directControl = mData->mSession.mDirectControl;
+    }
+
+    /* ignore notifications sent after #OnSessionEnd() is called */
+    if (!directControl)
+        return S_OK;
+
+    return directControl->OnStorageDeviceChange(aAttachment, aRemove);
+}
+
+/**
  *  Returns @c true if this machine's USB controller reports it has a matching
  *  filter for the given USB device and @c false otherwise.