Changeset 3001 in vbox

Timestamp:

Jun 3, 2007 7:29:42 PM (18 years ago)

Author:

vboxsync

Message:

Main/Frontends: Next step to support asynchronous USB device flow.

Location:

trunk/src/VBox

Files:

• Frontends/VBoxSDL/VBoxSDL.cpp (modified) (1 diff)
• Frontends/VirtualBox/src/VBoxConsoleView.cpp (modified) (1 diff)
• Main/ConsoleImpl.cpp (modified) (19 diffs)
• Main/HostImpl.cpp (modified) (20 diffs)
• Main/HostUSBDeviceImpl.cpp (modified) (18 diffs)
• Main/MachineImpl.cpp (modified) (7 diffs)
• Main/SessionImpl.cpp (modified) (3 diffs)
• Main/USBControllerImpl.cpp (modified) (2 diffs)
• Main/USBProxyService.cpp (modified) (20 diffs)
• Main/idl/VirtualBox.xidl (modified) (7 diffs)
• Main/include/ConsoleImpl.h (modified) (4 diffs)
• Main/include/DisplayImpl.h (modified) (1 diff)
• Main/include/HostImpl.h (modified) (2 diffs)
• Main/include/HostUSBDeviceImpl.h (modified) (3 diffs)
• Main/include/MachineImpl.h (modified) (2 diffs)
• Main/include/SessionImpl.h (modified) (1 diff)

trunk/src/VBox/Frontends/VBoxSDL/VBoxSDL.cpp

@@ -486 +486 @@
         gfGuestCapsLockPressed   = fCapsLock;
         gfGuestScrollLockPressed = fScrollLock;
+        return S_OK;
+    }
+
+    STDMETHOD(OnUSBDeviceStateChange)(IUSBDevice *device, BOOL attached,
+                                      IVirtualBoxErrorInfo *message)
+    {
         return S_OK;
     }

trunk/src/VBox/Frontends/VirtualBox/src/VBoxConsoleView.cpp

@@ -383 +383 @@
                                  new ModifierKeyChangeEvent (fNumLock, fCapsLock,
                                                              fScrollLock));
+        return S_OK;
+    }
+
+    STDMETHOD(OnUSBDeviceStateChange)(IUSBDevice *device, BOOL attached,
+                                      IVirtualBoxErrorInfo *error)
+    {
+        Q_UNUSED (device);
+        Q_UNUSED (attached);
+        Q_UNUSED (error);
+
+        /// @todo update menu entries
+
         return S_OK;
     }

trunk/src/VBox/Main/ConsoleImpl.cpp

@@ -1946 +1946 @@
             tr ("The virtual machine does not have a USB controller"));
 
-    PVUSBIRHCONFIG pRhConfig = (PVUSBIRHCONFIG) pBase->
-        pfnQueryInterface (pBase, PDMINTERFACE_VUSB_RH_CONFIG);
-    ComAssertRet (pRhConfig, E_FAIL);
-
-    /// @todo (dmik) REMOTE_USB
-    //  when remote USB devices are ready, first search for a device with the
-    //  given UUID in mRemoteUSBDevices. If found, request a capture from
-    //  a remote client. If not found, search it on the local host as done below
-
-    /*
-     *  Try attach the given host USB device (a proper errror message should
-     *  be returned in case of error).
-     */
-    ComPtr <IUSBDevice> hostDevice;
-    HRESULT hrc = mControl->CaptureUSBDevice (aId, hostDevice.asOutParam());
-    CheckComRCReturnRC (hrc);
-
-    return attachUSBDevice (hostDevice, true /* aManual */, pRhConfig);
+    /* Request the device capture */
+    HRESULT rc = mControl->CaptureUSBDevice (aId);
+    CheckComRCReturnRC (rc);
+
+    return rc;
 }
 
@@ -1991 +1978 @@
     if (!device)
         return setError (E_INVALIDARG,
-            tr ("Cannot detach the USB device (UUID: %s) as it is not attached here."),
-            Guid (aId).toString().raw());
-
-    /* protect mpVM */
-    AutoVMCaller autoVMCaller (this);
-    CheckComRCReturnRC (autoVMCaller.rc());
-
-    PPDMIBASE pBase = NULL;
-    int vrc = PDMR3QueryLun (mpVM, "usb-ohci", 0, 0, &pBase);
-
-    /* if the device is attached, then there must be a USB controller */
-    ComAssertRCRet (vrc, E_FAIL);
-
-    PVUSBIRHCONFIG pRhConfig = (PVUSBIRHCONFIG) pBase->
-        pfnQueryInterface (pBase, PDMINTERFACE_VUSB_RH_CONFIG);
-    ComAssertRet (pRhConfig, E_FAIL);
-
-    Guid Uuid(aId);
-
-    LogFlowThisFunc (("Detaching USB proxy device {%Vuuid}...\n", Uuid.raw()));
-
-    /* leave the lock before a VMR3* call (EMT will call us back)! */
-    alock.leave();
-
-    PVMREQ pReq = NULL;
-    vrc = VMR3ReqCall (mpVM, &pReq, RT_INDEFINITE_WAIT,
-                       (PFNRT) usbDetachCallback, 5,
-                       this, &it, true /* aManual */, pRhConfig, Uuid.raw());
-    if (VBOX_SUCCESS (vrc))
-        vrc = pReq->iStatus;
-    VMR3ReqFree (pReq);
-
-    HRESULT hrc = S_OK;
-
-    if (VBOX_SUCCESS (vrc))
-        device.queryInterfaceTo (aDevice);
-    else
-        hrc = setError (E_FAIL,
-            tr ("Error detaching the USB device.  (Failed to destroy the USB proxy device: %Vrc)"), vrc);
-
-    return hrc;
+            tr ("USB device with UUID {%Vuuid} is not attached to this machine"),
+            Guid (aId).raw());
+
+    /* Request the device release */
+    HRESULT rc = mControl->ReleaseUSBDevice (aId);
+    CheckComRCReturnRC (rc);
+
+    return rc;
 }
 
@@ -3221 +3175 @@
  *  @note Locks this object for writing.
  */
-HRESULT Console::onUSBDeviceAttach (IUSBDevice *aDevice)
-{
-    LogFlowThisFunc (("aDevice=%p\n", aDevice));
+HRESULT Console::onUSBDeviceAttach (IUSBDevice *aDevice, IVirtualBoxErrorInfo *aError)
+{
+    LogFlowThisFunc (("aDevice=%p aError=%p\n", aDevice, aError));
 
     AutoCaller autoCaller (this);
@@ -3236 +3190 @@
                           mMachineState));
         return E_FAIL;
+    }
+
+    if (aError != NULL)
+    {
+        /* notify callback about an error */
+        onUSBDeviceStateChange (aDevice, true /* aAttached */, aError);
+        return S_OK;
     }
 
@@ -3255 +3216 @@
     ComAssertRet (pRhConfig, E_FAIL);
 
-    return attachUSBDevice (aDevice, false /* aManual */, pRhConfig);
+    /// @todo notify listeners of IConsoleCallback in case of error
+    return attachUSBDevice (aDevice, pRhConfig);
 }
 
@@ -3264 +3226 @@
  *  @note Locks this object for writing.
  */
-HRESULT Console::onUSBDeviceDetach (INPTR GUIDPARAM aId)
+HRESULT Console::onUSBDeviceDetach (INPTR GUIDPARAM aId,
+                                    IVirtualBoxErrorInfo *aError)
 {
     Guid Uuid (aId);
-    LogFlowThisFunc (("aId={%Vuuid}\n", Uuid.raw()));
+    LogFlowThisFunc (("aId={%Vuuid} aError=%p\n", Uuid.raw(), aError));
 
     AutoCaller autoCaller (this);
@@ -3302 +3265 @@
     }
 
+    if (aError != NULL)
+    {
+        /* notify callback about an error */
+        onUSBDeviceStateChange (device, false /* aAttached */, aError);
+        return S_OK;
+    }
+
     /* protect mpVM */
     AutoVMCaller autoVMCaller (this);
@@ -3324 +3294 @@
     vrc = VMR3ReqCall (mpVM, &pReq, RT_INDEFINITE_WAIT,
                        (PFNRT) usbDetachCallback, 5,
-                       this, &it, false /* aManual */, pRhConfig, Uuid.raw());
+                       this, &it, pRhConfig, Uuid.raw());
     if (VBOX_SUCCESS (vrc))
         vrc = pReq->iStatus;
@@ -3331 +3301 @@
     AssertRC (vrc);
 
+    /// @todo notify listeners of IConsoleCallback in case of error
     return VBOX_SUCCESS (vrc) ? S_OK : E_FAIL;
 }
@@ -3520 +3491 @@
     while (it != mCallbacks.end())
         (*it++)->OnKeyboardLedsChange(fNumLock, fCapsLock, fScrollLock);
+}
+
+/**
+ *  @note Locks this object for reading.
+ */
+void Console::onUSBDeviceStateChange (IUSBDevice *aDevice, bool aAttached,
+                                      IVirtualBoxErrorInfo *aError)
+{
+    AutoCaller autoCaller (this);
+    AssertComRCReturnVoid (autoCaller.rc());
+
+    AutoReaderLock alock (this);
+
+    CallbackList::iterator it = mCallbacks.begin();
+    while (it != mCallbacks.end())
+        (*it++)->OnUSBDeviceStateChange (aDevice, aAttached, aError);
 }
 
@@ -4188 +4175 @@
  *  mUSBDevices collection.
  *
- *  If \a aManual is true and a failure occures, the given device
- *  will be returned back to the USB proxy manager.
- *
  *  @param aHostDevice  device to attach
- *  @param aManual      true if device is being manually attached
  *
  *  @note Locks this object for writing.
  *  @note Synchronously calls EMT.
  */
-HRESULT Console::attachUSBDevice (IUSBDevice *aHostDevice, bool aManual,
-                                  PVUSBIRHCONFIG aConfig)
+HRESULT Console::attachUSBDevice (IUSBDevice *aHostDevice, PVUSBIRHCONFIG aConfig)
 {
     AssertReturn (aHostDevice && aConfig, E_FAIL);
@@ -4262 +4244 @@
         LogWarningThisFunc (("Failed to create proxy device for '%s' {%Vuuid} (%Vrc)\n",
                              Address.raw(), Uuid.ptr(), vrc));
-
-        if (aManual)
-        {
-            /*
-             *  Neither SessionMachine::ReleaseUSBDevice() nor Host::releaseUSBDevice()
-             *  should call the Console back, so keep the lock to provide atomicity
-             *  (to protect Host reapplying USB filters)
-             */
-            hrc = mControl->ReleaseUSBDevice (Uuid);
-            AssertComRC (hrc);
-        }
 
         switch (vrc)
@@ -4348 +4319 @@
         that->mUSBDevices.push_back (device);
         LogFlowFunc (("Attached device {%Vuuid}\n", device->id().raw()));
+
+        /* notify callbacks */
+        that->onUSBDeviceStateChange (device, true /* aAttached */, NULL);
     }
 
@@ -4367 +4341 @@
 DECLCALLBACK(int)
 Console::usbDetachCallback (Console *that, USBDeviceList::iterator *aIt,
-                            bool aManual, PVUSBIRHCONFIG aConfig, PCRTUUID aUuid)
+                            PVUSBIRHCONFIG aConfig, PCRTUUID aUuid)
 {
     LogFlowFuncEnter();
@@ -4401 +4375 @@
         LogFlowFunc (("Detached device {%Vuuid}\n", (**aIt)->id().raw()));
 
-        /// @todo (dmik) REMOTE_USB
-        //  if the device is remote, notify a remote client that we have
-        //  detached the device
-
-        /* If it's a manual detach, give it back to the USB Proxy */
-        if (aManual)
-        {
-            /*
-             *  Neither SessionMachine::ReleaseUSBDevice() nor Host::releaseUSBDevice()
-             *  should call the Console back, so keep the lock to provide atomicity
-             *  (to protect Host reapplying USB filters)
-             */
-            LogFlowFunc (("Giving it back it to USB proxy...\n"));
-            HRESULT hrc = that->mControl->ReleaseUSBDevice (Guid (*aUuid));
-            AssertComRC (hrc);
-            vrc = SUCCEEDED (hrc) ? VINF_SUCCESS : VERR_GENERAL_FAILURE;
-        }
+        /* notify callbacks */
+        that->onUSBDeviceStateChange (**aIt, false /* aAttached */, NULL);
     }
 
@@ -6206 +6165 @@
 
 /**
- *  Captures and attaches USB devices to a newly created VM.
+ *  Captures USB devices that match filters of the VM.
+ *  Called at VM startup.
  *
  *  @param   pVM     The VM handle.
@@ -6219 +6179 @@
     ComAssertRet (isLockedOnCurrentThread(), E_FAIL);
 
-    /*
-     *  If the machine has an USB controller, capture devices and attach
-     *  them to it.
-     */
+    /* If the machine has an USB controller, ask the USB proxy service to
+     * capture devices */
     PPDMIBASE pBase;
     int vrc = PDMR3QueryLun (pVM, "usb-ohci", 0, 0, &pBase);
     if (VBOX_SUCCESS (vrc))
     {
-        PVUSBIRHCONFIG pRhConfig = (PVUSBIRHCONFIG) pBase->
-            pfnQueryInterface (pBase, PDMINTERFACE_VUSB_RH_CONFIG);
-        ComAssertRet (pRhConfig, E_FAIL);
-
-        /*
-         *  Get the list of USB devices that should be captured and attached to
-         *  the newly created machine.
-         */
-        ComPtr <IUSBDeviceCollection> coll;
-        HRESULT hrc = mControl->AutoCaptureUSBDevices (coll.asOutParam());
+        HRESULT hrc = mControl->AutoCaptureUSBDevices();
         ComAssertComRCRetRC (hrc);
-
-        /*
-         *  Enumerate the devices and attach them.
-         *  Failing to attach an device is currently ignored and the device
-         *  released.
-         */
-        ComPtr <IUSBDeviceEnumerator> en;
-        hrc = coll->Enumerate (en.asOutParam());
-        ComAssertComRCRetRC (hrc);
-
-        BOOL hasMore = FALSE;
-        while (SUCCEEDED (en->HasMore (&hasMore)) && hasMore)
-        {
-            ComPtr <IUSBDevice> hostDevice;
-            hrc = en->GetNext (hostDevice.asOutParam());
-            ComAssertComRCRetRC (hrc);
-            ComAssertRet (!hostDevice.isNull(), E_FAIL);
-
-            hrc = attachUSBDevice (hostDevice, true /* aManual */, pRhConfig);
-
-            /// @todo (r=dmik) warning reporting subsystem
-        }
     }
     else if (   vrc == VERR_PDM_DEVICE_NOT_FOUND
@@ -6391 +6318 @@
             if (fMatched)
             {
-                hrc = onUSBDeviceAttach(device);
+                hrc = onUSBDeviceAttach (device, NULL);
 
                 /// @todo (r=dmik) warning reporting subsystem
@@ -6457 +6384 @@
             Guid uuid;
             device->COMGETTER (Id) (uuid.asOutParam());
-            onUSBDeviceDetach (uuid);
+            onUSBDeviceDetach (uuid, NULL);
         }
 

trunk/src/VBox/Main/HostImpl.cpp

@@ -110 +110 @@
 HRESULT Host::init (VirtualBox *parent)
 {
-    LogFlowMember(("Host::init(): isReady=%d\n", isReady()));
+    LogFlowThisFunc (("isReady=%d\n", isReady()));
 
     ComAssertRet (parent, E_INVALIDARG);
@@ -142 +142 @@
 void Host::uninit()
 {
-    LogFlowMember(("Host::uninit(): isReady=%d\n", isReady()));
+    LogFlowThisFunc (("isReady=%d\n", isReady()));
 
     AssertReturn (isReady(), (void) 0);
 
-    // uninit all USB device filters still referenced by clients
+    /* wait for USB proxy service to terminate before we uninit all USB
+     * devices */
+    LogFlowThisFunc (("Stopping USB proxy service...\n"));
+    delete mUSBProxyService;
+    LogFlowThisFunc (("Done stopping USB proxy service.\n"));
+    mUSBProxyService = NULL;
+
+    /* uninit all USB device filters still referenced by clients */
     uninitDependentChildren();
-
-    delete mUSBProxyService;
-    mUSBProxyService = NULL;
 
     mUSBDeviceFilters.clear();
@@ -1202 +1206 @@
 
 /**
- *  Marks the given host USB device as captured by the given machine and returns
- *  it as IUSBDevice to the caller.
+ *  Requests the USB proxy service to capture the given host USB device.
+ *
+ *  When the request is completed,
+ *  IInternalSessionControl::onUSBDeviceAttach() will be called on the given
+ *  machine object.
  *
  *  Called by Console from the VM process (throug IInternalMachineControl).
  *  Must return extended error info in case of errors.
  */
-HRESULT Host::captureUSBDevice (SessionMachine *aMachine, INPTR GUIDPARAM aId,
-                                IUSBDevice **aHostDevice)
+HRESULT Host::captureUSBDevice (SessionMachine *aMachine, INPTR GUIDPARAM aId)
 {
     ComAssertRet (aMachine, E_INVALIDARG);
-    ComAssertRet (aHostDevice, E_POINTER);
 
     AutoLock lock (this);
@@ -1230 +1235 @@
     if (!device)
         return setError (E_INVALIDARG,
-            tr ("USB device with UUID {%s} is not currently attached to the host"),
-            id.toString().raw());
+            tr ("USB device with UUID {%Vuuid} is not currently attached to the host"),
+            id.raw());
 
     AutoLock devLock (device);
+
+    if (device->isStatePending())
+        return setError (E_INVALIDARG,
+            tr ("USB device '%s' with UUID {%Vuuid} is busy (waiting for a pending "
+                "state change). Please try later"),
+            device->name().raw(), id.raw());
 
     if (device->state() == USBDeviceState_USBDeviceNotSupported)
         return setError (E_INVALIDARG,
-            tr ("USB device '%s' with UUID {%s} cannot be accessed by guest "
+            tr ("USB device '%s' with UUID {%Vuuid} cannot be accessed by guest "
                 "computers"),
-            device->name().raw(), id.toString().raw());
+            device->name().raw(), id.raw());
 
     if (device->state() == USBDeviceState_USBDeviceUnavailable)
         return setError (E_INVALIDARG,
-            tr ("USB device '%s' with UUID {%s} is being exclusively used by the "
+            tr ("USB device '%s' with UUID {%Vuuid} is being exclusively used by the "
                 "host computer"),
-            device->name().raw(), id.toString().raw());
+            device->name().raw(), id.raw());
 
     if (device->state() == USBDeviceState_USBDeviceCaptured)
         return setError (E_INVALIDARG,
-            tr ("USB device '%s' with UUID {%s} is already captured by the virtual "
+            tr ("USB device '%s' with UUID {%Vuuid} is already captured by the virtual "
                 "machine '%ls'"),
-            device->name().raw(), id.toString().raw(),
+            device->name().raw(), id.raw(),
             aMachine->userData()->mName.raw());
 
-    // try capture the device
-    bool ok = device->setCaptured (aMachine);
-    if (!ok)
-    {
-        if (device->state() == USBDeviceState_USBDeviceBusy)
-            return setError (E_FAIL,
-                tr ("USB device with UUID {%s} is being accessed by the host "
-                    "computer and cannot be attached to the virtual machine."
-                    "Please try later"),
-                id.toString().raw());
-        else
-            ComAssertRet (ok, E_FAIL);
-    }
-
-    // return the device to the caller as IUSBDevice
-    device.queryInterfaceTo (aHostDevice);
+    /* try to capture the device */
+    device->requestCapture (aMachine);
+
     return S_OK;
 }
@@ -1277 +1275 @@
  *  machine the device is currently marked as captured by.
  *
+ *  When the request is completed,
+ *  IInternalSessionControl::onUSBDeviceDetach() will be called on the given
+ *  machine object.
+ *
  *  Called by Console from the VM process (throug IInternalMachineControl).
  */
@@ -1299 +1301 @@
     AutoLock devLock (device);
 
+    LogFlowThisFunc (("id={%Vuuid} state=%d isStatePending=%RTbool pendingState=%d\n",
+                      device->id().raw(), device->state(), device->isStatePending(),
+                      device->pendingState()));
+
+    if (device->isStatePending())
+        return setError (E_INVALIDARG,
+            tr ("USB device '%s' with UUID {%Vuuid} is busy (waiting for a pending "
+                "state change). Please try later"),
+                         device->name().raw(), device->id().raw());
+
     ComAssertRet (device->machine() == aMachine, E_FAIL);
 
-    /* reset the device and apply filters */
-    int vrc = device->reset();
-    ComAssertRCRet (vrc, E_FAIL);
-
-    HRESULT rc = applyAllUSBFilters (device, aMachine);
+    /* re-apply filters on the device before giving it back to the host */
+    device->setHeld();
+    HRESULT rc = applyAllUSBFilters (device);
     ComAssertComRC (rc);
 
@@ -1312 +1322 @@
 
 /**
- *  Runs the filters of the given machine against all currently available USB
- *  devices, marks those that match as captured and returns them as the colection
- *  of IUSBDevice instances.
+ *  Asks the USB proxy service to capture all currently available USB devices
+ *  that match filters of the given machine.
+ *
+ *  When the request is completed,
+ *  IInternalSessionControl::onUSBDeviceDetach() will be called on the given
+ *  machine object per every captured USB device.
  *
  *  Called by Console from the VM process (through IInternalMachineControl)
@@ -1322 +1335 @@
  *  to the new locking scheme).
  */
-HRESULT Host::autoCaptureUSBDevices (SessionMachine *aMachine,
-                                     IUSBDeviceCollection **aHostDevices)
+HRESULT Host::autoCaptureUSBDevices (SessionMachine *aMachine)
 {
     LogFlowThisFunc (("aMachine=%p\n", aMachine));
@@ -1330 +1342 @@
     CHECK_READY();
 
-    std::list <ComPtr <IUSBDevice> > list;
-
-    USBDeviceList::iterator it = mUSBDevices.begin();
-    while (it != mUSBDevices.end())
+    for (USBDeviceList::iterator it = mUSBDevices.begin();
+         it != mUSBDevices.end();
+         ++ it)
     {
         ComObjPtr <HostUSBDevice> device = *it;
@@ -1339 +1350 @@
         AutoLock devLock (device);
 
-/// @todo remove
-#if 0
-        if (device->isIgnored())
+        /* skip pending devices */
+        if (device->isStatePending())
             continue;
-#endif
 
         if (device->state() == USBDeviceState_USBDeviceBusy ||
@@ -1350 +1359 @@
         {
             applyMachineUSBFilters (aMachine, device);
-
-            if (device->state() == USBDeviceState_USBDeviceCaptured)
-            {
-                /* put the device to the return list */
-                ComPtr <IUSBDevice> d;
-                device.queryInterfaceTo (d.asOutParam());
-                Assert (!d.isNull());
-                list.push_back (d);
-            }
-        }
-        ++ it;
-    }
-
-    ComObjPtr <IfaceUSBDeviceCollection> coll;
-    coll.createObject();
-    coll->init (list);
-    coll.queryInterfaceTo (aHostDevices);
+        }
+    }
 
     return S_OK;
@@ -1396 +1390 @@
         if (device->machine() == aMachine)
         {
-            /* reset the device and apply filters */
-            device->reset();
-            HRESULT rc = applyAllUSBFilters (device, aMachine);
-            ComAssertComRC (rc);
+            if (!device->isStatePending())
+            {
+                Assert (device->state() == USBDeviceState_USBDeviceCaptured);
+
+                /* re-apply filters on the device before giving it back to the
+                 * host */
+                device->setHeld();
+                HRESULT rc = applyAllUSBFilters (device);
+                AssertComRC (rc);
+            }
+            else
+            {
+                device->cancelPendingState();
+            }
         }
         ++ it;
@@ -1833 +1837 @@
 
 /**
- *  Runs through all global filters to determine the state of the given
- *  USB device, then unless ignored, runs trhough filters of all running machines
- *  (excluding the given one) to automatically capture the device when there is a
- *  match (the state of the device will be set to USBDeviceCaptured if so, and
- *  the machine's process will be informed about the auto-capture).
- *
- *  @param aDevice  USB device to set state for
- *  @param aMachine the machine whose filters are to be excluded (can be NULL)
- *
- *  @note the method must be called from under this object's lock
- */
-HRESULT Host::applyAllUSBFilters (ComObjPtr <HostUSBDevice> &aDevice,
-                                  SessionMachine *aMachine /* = NULL  */)
+ *  Applies all (golbal and VM) filters to the given USB device. The device
+ *  must be either a newly attached device or a device released by a VM.
+ *
+ *  This method will request the USB proxy service to release the device (give
+ *  it back to the host) if none of the global or VM filters want to capture
+ *  the device.
+ *
+ *  @param aDevice  USB device to apply filters to.
+ *
+ *  @note the method must be called from under this object's write lock and
+ *  from the aDevice's write lock.
+ */
+HRESULT Host::applyAllUSBFilters (ComObjPtr <HostUSBDevice> &aDevice)
 {
     LogFlowThisFunc (("\n"));
 
+    /// @todo must check for read lock, it's enough here
     AssertReturn (isLockedOnCurrentThread(), E_FAIL);
 
-    AutoLock devLock (aDevice);
+    AssertReturn (aDevice->isLockedOnCurrentThread(), E_FAIL);
+
+    AssertReturn (aDevice->state() != USBDeviceState_USBDeviceCaptured, E_FAIL);
+
+    AssertReturn (aDevice->isStatePending() == false, E_FAIL);
 
     /* ignore unsupported devices */
@@ -1863 +1872 @@
     mParent->getOpenedMachines (machines);
 
+    /// @todo it may be better to take a copy of filters to iterate and leave
+    /// the host lock before calling HostUSBDevice:requestCapture() (which
+    /// calls the VM process).
+
     /* apply global filters */
     USBDeviceFilterList::const_iterator it = mUSBDeviceFilters.begin();
-    while (it != mUSBDeviceFilters.end())
+    for (; it != mUSBDeviceFilters.end(); ++ it)
     {
         AutoLock filterLock (*it);
@@ -1872 +1885 @@
         {
             if (data.mAction == USBDeviceFilterAction_USBDeviceFilterIgnore)
+            {
+                /* request to give the device back to the host*/
+                aDevice->requestRelease();
+                /* nothing to do any more */
                 return S_OK;
+            }
             if (data.mAction == USBDeviceFilterAction_USBDeviceFilterHold)
                 break;
         }
-        ++ it;
     }
 
     /* apply machine filters */
-    for (size_t i = 0; i < machines.size(); i++)
-    {
-        if (aMachine && machines [i] == aMachine)
+    size_t i = 0;
+    for (; i < machines.size(); ++ i)
+    {
+        /* skip the machine the device was just detached from */
+        if (aDevice->machine() && machines [i] == aDevice->machine())
             continue;
 
-        applyMachineUSBFilters (machines [i], aDevice);
-
-        if (aDevice->state() == USBDeviceState_USBDeviceCaptured)
-        {
-            /* inform the VM process about the auto-capture */
-            ComPtr <IUSBDevice> d;
-            aDevice.queryInterfaceTo (d.asOutParam());
-
-            /* the VM process will query the object, so leave the lock */
-            devLock.leave();
-
-            HRESULT rc = machines [i]->onUSBDeviceAttach (d);
-            if (SUCCEEDED(rc))
-                return rc;
-
-            devLock.enter();
-
-            /* the machine rejected it, continue applying filters. */
-            aDevice->reset();
-        }
-    }
-
-    /* no machine filters were matched.
-     * if no global filters were matched as well, release the device
-     * to make it available on the host */
-    if (    it == mUSBDeviceFilters.end()
-        &&  aDevice->state() == USBDeviceState_USBDeviceHeld)
-        aDevice->setHostDriven();
-/** @todo dmik, what about USBDeviceFilterHold??? */
+        if (applyMachineUSBFilters (machines [i], aDevice))
+            break;
+    }
+
+    if (i == machines.size())
+    {
+        /* no matched machine filters, check what to do */
+        if (it == mUSBDeviceFilters.end())
+        {
+            /* no any filter matched at all */
+            /* request to give the device back to the host */
+            aDevice->requestRelease();
+        }
+        else
+        {
+            /* there was a global Hold filter */
+            aDevice->requestHold();
+        }
+    }
+
+    return S_OK;
+}
+
 /**
- *  bird, everything's ok from what I see. if USBDeviceFilterHold filter was
- *  matched, setHostDevice() will not (and should not) be called
- *  (because it != mUSBDeviceFilters.end() in that case).
- */
-
-    return S_OK;
-}
-
-/**
- *  Runs through filters of the given machine in order to automatically capture
- *  the given USB device when there is a match (the state of the device will
- *  be set to USBDeviceCaptured if so, but the machine's process will *NOT* be)
- *  informed about the auto-capture).
- *
- *  @param aMachine the machine whose filters are to be run
- *  @param aDevice  USB device, a candidate for auto-attaching
+ *  Runs through filters of the given machine and asks the USB proxy service
+ *  to capture the given USB device when there is a match.
+ *
+ *  @param aMachine Machine whose filters are to be run.
+ *  @param aDevice  USB device, a candidate for auto-capturing.
+ *  @return         @c true if there was a match and @c false otherwise.
+ *
+ *  @note the method must be called from under this object's write lock and
+ *  from the aDevice's write lock.
  *
  *  @note Locks aMachine for reading.
  */
-void Host::applyMachineUSBFilters (SessionMachine *aMachine,
+bool Host::applyMachineUSBFilters (SessionMachine *aMachine,
                                    ComObjPtr <HostUSBDevice> &aDevice)
 {
     LogFlowThisFunc (("\n"));
 
-    AssertReturnVoid (isLockedOnCurrentThread());
-    AssertReturnVoid (aDevice->isLockedOnCurrentThread());
-
-    AssertReturnVoid (aMachine);
-    AssertReturnVoid (aDevice->state() != USBDeviceState_USBDeviceUnavailable);
-
-    /* We're going to use aMachine which is not our child/parent, add a caller */
-    AutoCaller autoCaller (aMachine);
-    if (!autoCaller.isOk())
-    {
-        /* silently return, the machine might be not running any more */
-        return;
-    }
-
-    /* enter the machine's lock because we want to access its USB controller */
-    AutoReaderLock machineLock (aMachine);
-
-    if (aMachine->usbController()->hasMatchingFilter (aDevice))
-    {
-        /* try capture the device */
-        bool ok = aDevice->setCaptured (aMachine);
-
-        /*
-         *  false is valid only when the state remains USBDeviceBusy meaning that
-         *  the device is currently busy (being accessed by the host)
-         */
-        Assert (ok || aDevice->state() == USBDeviceState_USBDeviceBusy);
-        NOREF (ok);
-    }
+    AssertReturn (aMachine, false);
+
+    /// @todo must check for read lock, it's enough here
+    AssertReturn (isLockedOnCurrentThread(), false);
+
+    AssertReturn (aDevice->isLockedOnCurrentThread(), false);
+
+    AssertReturn (aDevice->state() != USBDeviceState_USBDeviceNotSupported, false);
+    AssertReturn (aDevice->state() != USBDeviceState_USBDeviceUnavailable, false);
+
+    AssertReturn (aDevice->isStatePending() == false, false);
+
+    bool hasMatch = false;
+
+    {
+        /* We're going to use aMachine which is not our child/parent, add a
+         * caller */
+        AutoCaller autoCaller (aMachine);
+        if (!autoCaller.isOk())
+        {
+            /* silently return, the machine might be not running any more */
+            return false;
+        }
+
+        /* enter the machine's lock because we want to access its USB controller */
+        AutoReaderLock machineLock (aMachine);
+        hasMatch = aMachine->usbController()->hasMatchingFilter (aDevice);
+    }
+
+    if (hasMatch)
+    {
+        /* try to capture the device */
+        return aDevice->requestCapture (aMachine);
+    }
+
+    return hasMatch;
 }
 
@@ -1982 +1995 @@
     AssertReturnVoid (aDevice);
 
-    /// @todo (dmik) check locks
-    AutoLock alock (this);
-
-    AutoLock devLock (aDevice);
+    AssertReturnVoid (isLockedOnCurrentThread());
+    AssertReturnVoid (aDevice->isLockedOnCurrentThread());
+
+    LogFlowThisFunc (("id={%Vuuid} state=%d isStatePending=%RTbool pendingState=%d\n",
+                      aDevice->id().raw(), aDevice->state(), aDevice->isStatePending(),
+                      aDevice->pendingState()));
+
+    Assert (aDevice->isStatePending() == false);
 
     /* add to the collecion */
@@ -1997 +2014 @@
 
 /**
- *  Called by USB proxy service (?) when the device is physically detached
+ *  Called by USB proxy service when the device is physically detached
  *  from the host.
  *
@@ -2008 +2025 @@
     AssertReturnVoid (aDevice);
 
-    /// @todo (dmik) check locks
-    AutoLock alock (this);
-
-    AutoLock devLock (aDevice);
+    AssertReturnVoid (isLockedOnCurrentThread());
+    AssertReturnVoid (aDevice->isLockedOnCurrentThread());
+
+    LogFlowThisFunc (("id={%Vuuid} state=%d isStatePending=%RTbool pendingState=%d\n",
+                      aDevice->id().raw(), aDevice->state(), aDevice->isStatePending(),
+                      aDevice->pendingState()));
 
     Guid id = aDevice->id();
@@ -2032 +2051 @@
     mUSBDevices.erase (it);
 
-    if (device->machine())
-    {
-        /* the device is captured by a machine, instruct it to release */
-
-        devLock.leave();
-        alock.leave();
-
-        HRESULT rc = device->machine()->onUSBDeviceDetach (device->id());
-        AssertComRC (rc);
-    }
+    /* reset all data */
+    device->reset();
 }
 
 /**
- *  Called by USB proxy service  when the state of the host-driven device
- *  has changed because of non proxy interaction.
+ *  Called by USB proxy service when the state of the device has changed
+ *  either because of the state change request or because of some external
+ *  interaction.
  *
  *  @param  aDevice     The device in question.
@@ -2054 +2066 @@
     LogFlowThisFunc (("aDevice=%p\n", aDevice));
 
-    /// @todo (dmik) check locks
-    AutoLock alock (this);
-
-    /** @todo dmik, is there anything we should do here? For instance if the device now is available? */
+    AssertReturnVoid (aDevice);
+
+    AssertReturnVoid (isLockedOnCurrentThread());
+    AssertReturnVoid (aDevice->isLockedOnCurrentThread());
+
+    LogFlowThisFunc (("id={%Vuuid} state=%d isStatePending=%RTbool pendingState=%d\n",
+                      aDevice->id().raw(), aDevice->state(), aDevice->isStatePending(),
+                      aDevice->pendingState()));
+
+    if (aDevice->isStatePending())
+    {
+        /* it was a state change request */
+        aDevice->handlePendingStateChange();
+    }
+    else
+    {
+        /* some external state change */
+
+        /// @todo re-run all USB filters probably
+        AssertFailed();
+    }
 }
 

trunk/src/VBox/Main/HostUSBDeviceImpl.cpp

@@ -21 +21 @@
 
 #include "HostUSBDeviceImpl.h"
+#include "MachineImpl.h"
+#include "VirtualBoxErrorInfoImpl.h"
 #include "USBProxyService.h"
+
 #include "Logging.h"
 
@@ -96 +99 @@
             break;
         case USBDEVICESTATE_USED_BY_GUEST:
-            mState = USBDeviceState_USBDeviceCaptured;
+            /* @todo USBDEVICESTATE_USED_BY_GUEST seems not to be used
+             * anywhere in the proxy code; it's quite logical because the
+             * proxy doesn't know anything about guest VMs. */
+            AssertFailedReturn (E_FAIL);
             break;
     }
@@ -104 +110 @@
     /* Other data members */
     mIsStatePending = false;
-/// @todo remove
-#if 0
-    mIgnored = false;
-#endif
     mUSBProxyService = aUSBProxyService;
     mUsb = aUsb;
@@ -339 +341 @@
 }
 
-/// @todo remove
-#if 0
-/** 
- * Sets the ignored flag and returns the device to the host.
- *
- * @note Locks this object for writing.
- */
-void HostUSBDevice::setIgnored()
-{
-    AutoCaller autoCaller (this);
-    AssertComRCReturnVoid (autoCaller.rc());
-
-    AutoLock alock (this);
-
-    AssertReturnVoid (!mIgnored);
-
-    mIgnored = true;
-
-    setHostDriven();
-}
-#endif
-
-/** 
- * @note Locks this object for writing.
- */
-bool HostUSBDevice::setCaptured (SessionMachine *aMachine)
-{
+/**
+ *  Requests the USB proxy service to capture the device and sets the pending
+ *  state to Captured.
+ *
+ *  If the state change may be performed immediately (for example, Hold ->
+ *  Captured), then the machine is informed before this method returns.
+ *
+ *  @param aMachine     Machine that will capture this device on success.
+ *  @return             @c false if the device could be immediately captured
+ *                      but the VM process refused to grab it;
+ *                      @c true otherwise.
+ *
+ *  @note Must be called from under the object write lock.
+ *
+ *  @note May lock the given machine object for reading.
+ */
+bool HostUSBDevice::requestCapture (SessionMachine *aMachine)
+{
+    LogFlowThisFunc (("\n"));
+
     AssertReturn (aMachine, false);
 
-    AutoCaller autoCaller (this);
-    AssertComRCReturn (autoCaller.rc(), false);
-
-    AutoLock alock (this);
+    AssertReturn (isLockedOnCurrentThread(), false);
+
+    AssertReturn (mIsStatePending == false, false);
 
     AssertReturn (
@@ -379 +373 @@
         false);
 
-    mState = USBDeviceState_USBDeviceCaptured;
+    if (mState == USBDeviceState_USBDeviceHeld)
+    {
+        /* can perform immediate capture, inform the VM process */
+
+        ComPtr <IUSBDevice> d = this;
+
+        mIsStatePending = true;
+
+        /* the VM process will query the object, so leave the lock */
+        AutoLock alock (this);
+        alock.leave();
+
+        LogFlowThisFunc (("Calling machine->onUSBDeviceAttach()...\n"));
+
+        HRESULT rc = aMachine->onUSBDeviceAttach (d, NULL);
+
+        LogFlowThisFunc (("Done machine->onUSBDeviceAttach()=%08X\n", rc));
+
+        alock.enter();
+
+        mIsStatePending = false;
+
+        if (SUCCEEDED (rc))
+        {
+            mState = mPendingState = USBDeviceState_USBDeviceCaptured;
+            mMachine = aMachine;
+            return true;
+        }
+
+        return false;
+    }
+
+    mIsStatePending = true;
+    mPendingState = USBDeviceState_USBDeviceCaptured;
     mMachine = aMachine;
 
@@ -388 +415 @@
 
 /**
- * Returns the device back to the host
- *
- * @returns VBox status code.
- *
- * @note Locks this object for writing.
- */
-int HostUSBDevice::setHostDriven()
-{
-    AutoCaller autoCaller (this);
-    AssertComRCReturn (autoCaller.rc(), VERR_INVALID_PARAMETER);
-
-    AutoLock alock (this);
-
-    AssertReturn (mState == USBDeviceState_USBDeviceHeld, VERR_INVALID_PARAMETER);
-
-    mState = USBDeviceState_USBDeviceAvailable;
-
-    return mUSBProxyService->releaseDevice (this);
-}
-
-/**
- * Resets the device as if it were just attached to the host
- *
- * @returns VBox status code.
- *
- * @note Locks this object for writing.
- */
-int HostUSBDevice::reset()
-{
-    AutoCaller autoCaller (this);
-    AssertComRCReturn (autoCaller.rc(), VERR_INVALID_PARAMETER);
-
-    AutoLock alock (this);
+ *  Requests the USB proxy service to release the device and sets the pending
+ *  state to Available.
+ *
+ *  If the state change may be performed immediately (for example, the current
+ *  state is Busy), this method does nothing.
+ *
+ *  @note Must be called from under the object write lock.
+ */
+void HostUSBDevice::requestRelease()
+{
+    LogFlowThisFunc (("\n"));
+
+    AssertReturnVoid (isLockedOnCurrentThread());
+
+    AssertReturnVoid (mIsStatePending == false);
+
+    AssertReturnVoid (
+        mState == USBDeviceState_USBDeviceBusy ||
+        mState == USBDeviceState_USBDeviceAvailable ||
+        mState == USBDeviceState_USBDeviceHeld);
+
+    if (mState != USBDeviceState_USBDeviceHeld)
+        return;
+
+    mIsStatePending = true;
+    mPendingState = USBDeviceState_USBDeviceAvailable;
+
+    mUSBProxyService->releaseDevice (this);
+}
+
+/**
+ *  Requests the USB proxy service to release the device, sets the pending
+ *  state to Held and removes the machine association if any. 
+ *
+ *  If the state change may be performed immediately (for example, the current
+ *  state is already Held), this method does nothing but removes the machine
+ *  association.
+ *
+ *  @note Must be called from under the object write lock.
+ */
+void HostUSBDevice::requestHold()
+{
+    LogFlowThisFunc (("\n"));
+
+    AssertReturnVoid (isLockedOnCurrentThread());
+
+    AssertReturnVoid (mIsStatePending == false);
+
+    AssertReturnVoid (
+        mState == USBDeviceState_USBDeviceBusy ||
+        mState == USBDeviceState_USBDeviceAvailable ||
+        mState == USBDeviceState_USBDeviceHeld);
+
+    mMachine.setNull();
+
+    if (mState == USBDeviceState_USBDeviceHeld)
+        return;
+
+    mIsStatePending = true;
+    mPendingState = USBDeviceState_USBDeviceHeld;
+
+    mUSBProxyService->captureDevice (this);
+}
+
+/**
+ *  Sets the device state from Captured to Held and preserves the machine
+ *  association (if any). Usually called before applying filters.
+ *
+ *  @note Must be called from under the object write lock.
+ */
+void HostUSBDevice::setHeld()
+{
+    LogFlowThisFunc (("\n"));
+
+    AssertReturnVoid (isLockedOnCurrentThread());
+
+    AssertReturnVoid (mState == USBDeviceState_USBDeviceCaptured);
+    AssertReturnVoid (mPendingState == USBDeviceState_USBDeviceCaptured);
+    AssertReturnVoid (mIsStatePending == false);
 
     mState = USBDeviceState_USBDeviceHeld;
-    mMachine.setNull();
-/// @todo remove
-#if 0
-    mIgnored = false;
-#endif
-
-    /** @todo this operation might fail and cause the device to the reattached with a different address and all that. */
-    return mUSBProxyService->resetDevice (this);
-}
-
-/// @todo remove
-#if 0
-/**
- *  Sets the state of the device, as it was reported by the host.
- *  This method applicable only for devices currently controlled by the host.
- *
- *  @param  aState      new state
- *
- *  @note Locks this object for writing.
- */
-void HostUSBDevice::setHostState (USBDeviceState_T aState)
-{
-    AutoCaller autoCaller (this);
-    AssertComRCReturnVoid (autoCaller.rc());
-
-    AutoLock alock (this);
-
-    AssertReturn (
-        aState == USBDeviceState_USBDeviceUnavailable ||
-        aState == USBDeviceState_USBDeviceBusy ||
-        aState == USBDeviceState_USBDeviceAvailable,
-        (void) 0);
-
-    AssertReturn (
-        mState == USBDeviceState_USBDeviceNotSupported || /* initial state */
-        mState == USBDeviceState_USBDeviceUnavailable ||
-        mState == USBDeviceState_USBDeviceBusy ||
-        mState == USBDeviceState_USBDeviceAvailable,
-        (void) 0);
-
-    if (mState != aState)
-    {
-        mState = aState;
-    }
-}
-#endif
+}
+
+/**
+ *  Resets all device data and informs the machine (if any) about the
+ *  detachment. Must be called when this device is physically detached from
+ *  the host.
+ *
+ *  @note Must be called from under the object write lock.
+ */
+void HostUSBDevice::reset()
+{
+    LogFlowThisFunc (("\n"));
+
+    AssertReturnVoid (isLockedOnCurrentThread());
+
+    if (!mMachine.isNull() && mState == USBDeviceState_USBDeviceCaptured)
+    {
+        /* the device is captured by a machine, instruct it to release */
+
+        mIsStatePending = true;
+
+        /* the VM process will query the object, so leave the lock */
+        AutoLock alock (this);
+        alock.leave();
+
+        LogFlowThisFunc (("Calling machine->onUSBDeviceDetach()...\n"));
+
+        HRESULT rc = mMachine->onUSBDeviceDetach (mId, NULL);
+        AssertComRC (rc);
+
+        LogFlowThisFunc (("Done machine->onUSBDeviceDetach()=%08X\n", rc));
+
+        alock.enter();
+
+        mIsStatePending = false;
+        mState = mPendingState = USBDeviceState_USBDeviceNotSupported;
+    }
+}
+
+/**
+ *  Handles the finished pending state change and informs the VM process if
+ *  necessary.
+ *
+ *  @note Must be called from under the object write lock.
+ */
+void HostUSBDevice::handlePendingStateChange()
+{
+    LogFlowThisFunc (("\n"));
+
+    AssertReturnVoid (isLockedOnCurrentThread());
+
+    AssertReturnVoid (mIsStatePending == true);
+    AssertReturnVoid (mState != USBDeviceState_USBDeviceCaptured);
+
+    bool wasCapture = false;
+    bool wasRelease = false;
+
+    HRESULT requestRC = S_OK;
+    Bstr errorText;
+
+    switch (mPendingState)
+    {
+        case USBDeviceState_USBDeviceCaptured:
+        {
+            if (mState == USBDeviceState_USBDeviceHeld)
+            {
+                if (!mMachine.isNull())
+                    wasCapture = true;
+                else
+                {
+                    /* it is a canceled capture request. Give the device back
+                     * to the host. */
+                    mPendingState = USBDeviceState_USBDeviceAvailable;
+                    mUSBProxyService->releaseDevice (this);
+                }
+            }
+            else
+            {
+                /* couldn't capture the device, will report an error */
+                wasCapture = true;
+
+                Assert (!mMachine.isNull());
+                
+                /// @todo more detailed error message depending on the state?
+                //  probably need some error code/string from the USB proxy itself
+                
+                requestRC = E_FAIL;
+                errorText = Utf8StrFmt (
+                    tr ("USB device '%s' with UUID {%Vuuid} is being accessed by the host "
+                        "computer and cannot be attached to the virtual machine."
+                        "Please try later"),
+                    name().raw(), id().raw());
+            }
+            break;
+        }
+        case USBDeviceState_USBDeviceAvailable:
+        {
+            if (mState == USBDeviceState_USBDeviceHeld)
+            {
+                /* couldn't release the device */
+                wasRelease = true;
+
+                Assert (!mMachine.isNull());
+
+                /* return the captured state back */
+                mState = USBDeviceState_USBDeviceCaptured;
+
+                /// @todo more detailed error message depending on the state?
+                //  probably need some error code/string from the USB proxy itself
+                
+                requestRC = E_FAIL;
+                errorText = Utf8StrFmt (
+                    tr ("USB device '%s' with UUID {%Vuuid} is being accessed by the guest "
+                        "computer and cannot be attached from the virtual machine."
+                        "Please try later"),
+                    name().raw(), id().raw());
+            }
+            else
+            {
+                if (!mMachine.isNull())
+                    wasRelease = true;
+                else
+                {
+                    /* it is a canceled release request. Leave at the host */
+                    /// @todo we may want to re-run all filters in this case
+                }
+            }
+            break;
+        }
+        case USBDeviceState_USBDeviceHeld:
+        {
+            if (mState == USBDeviceState_USBDeviceHeld)
+            {
+                break;
+            }
+            else
+            {
+                /* couldn't capture the device requested by the global
+                 * filter, there is nobody to report an error to. */
+            }
+            break;
+        }
+        default:
+            AssertFailed();
+    }
+
+    ComObjPtr <VirtualBoxErrorInfo> error;
+    if (FAILED (requestRC))
+    {
+        LogFlowThisFunc (("Request failed, requestRC=%08X, text='%ls'\n",
+                          requestRC, errorText.raw()));
+
+        error.createObject();
+        error->init (requestRC, COM_IIDOF (IHostUSBDevice),
+                     Bstr (HostUSBDevice::getComponentName()),
+                     errorText.raw());
+    }
+
+    if (wasCapture)
+    {
+        /* inform the VM process */
+
+        ComPtr <IUSBDevice> d = this;
+
+        /* the VM process will query the object, so leave the lock */
+        AutoLock alock (this);
+        alock.leave();
+
+        LogFlowThisFunc (("Calling machine->onUSBDeviceAttach()...\n"));
+
+        HRESULT rc = mMachine->onUSBDeviceAttach (d, error);
+        /// @todo we will probably want to re-run all filters on failure
+        //  instead of asserting
+        AssertComRC (rc);
+
+        LogFlowThisFunc (("Done machine->onUSBDeviceAttach()=%08X\n", rc));
+
+        alock.enter();
+
+        if (SUCCEEDED (requestRC) && SUCCEEDED (rc))
+        {
+            mIsStatePending = false;
+            mState = mPendingState = USBDeviceState_USBDeviceCaptured;
+            return;
+        }
+    }
+    else if (wasRelease)
+    {
+        /* inform the VM process */
+
+        /* the VM process will query the object, so leave the lock */
+        AutoLock alock (this);
+        alock.leave();
+
+        LogFlowThisFunc (("Calling machine->onUSBDeviceDetach()...\n"));
+
+        HRESULT rc = mMachine->onUSBDeviceDetach (mId, error);
+
+        /* This call may expectedly fail with rc = NS_ERROR_FAILURE (on XPCOM)
+         * if the VM process requests device release right before termination
+         * and then terminates before onUSBDeviceDetach() is reached
+         * it. Therefore, we don't assert here. On MS COM, there should be
+         * something similar (with the different error code). */
+
+        LogFlowThisFunc (("Done machine->onUSBDeviceDetach()=%08X\n", rc));
+
+        alock.enter();
+
+        if (SUCCEEDED (requestRC))
+        {
+            /* deassociate from the machine */
+            mMachine.setNull();
+        }
+    }
+
+    mIsStatePending = false;
+    mPendingState = mState;
+}
+
+/**
+ *  Cancels pending state change due to machine termination.
+ *
+ *  @note Must be called from under the object write lock.
+ */
+void HostUSBDevice::cancelPendingState()
+{
+    LogFlowThisFunc (("\n"));
+
+    AssertReturnVoid (isLockedOnCurrentThread());
+
+    AssertReturnVoid (mIsStatePending == true);
+    AssertReturnVoid (!mMachine.isNull());
+
+    switch (mPendingState)
+    {
+        case USBDeviceState_USBDeviceCaptured:
+        {
+            /* reset mMachine to deassociate it from the filter and tell
+             * handlePendingStateChange() what to do */
+            mMachine.setNull();
+            break;
+        }
+        case USBDeviceState_USBDeviceAvailable:
+        {
+            /* reset mMachine to deassociate it from the filter and tell
+             * handlePendingStateChange() what to do */
+            mMachine.setNull();
+            break;
+        }
+        default:
+            AssertFailed();
+    }
+}
 
 /**
@@ -495 +775 @@
     if (!aData.mVendorId.isMatch (mUsb->idVendor))
     {
-        LogFlowMember (("HostUSBDevice::isMatch: vendor not match %04X\n",
-                        mUsb->idVendor));
+        LogFlowThisFunc (("vendor not match %04X\n",
+                          mUsb->idVendor));
         return false;
     }
     if (!aData.mProductId.isMatch (mUsb->idProduct))
     {
-        LogFlowMember (("HostUSBDevice::isMatch: product id not match %04X\n",
-                        mUsb->idProduct));
+        LogFlowThisFunc (("product id not match %04X\n",
+                          mUsb->idProduct));
         return false;
     }
     if (!aData.mRevision.isMatch (mUsb->bcdDevice))
     {
-        LogFlowMember (("HostUSBDevice::isMatch: rev not match %04X\n",
-                        mUsb->bcdDevice));
+        LogFlowThisFunc (("rev not match %04X\n",
+                          mUsb->bcdDevice));
         return false;
     }
@@ -564 +844 @@
         return false;
 
-    LogFlowMember (("HostUSBDevice::isMatch: returns true\n"));
+    LogFlowThisFunc (("returns true\n"));
     return true;
 }
@@ -570 +850 @@
 
 /**
- * Compares this device with a USBDEVICE and decides which comes first.
- *
- * @returns < 0 if this should come before pDev2.
- * @returns   0 if this and pDev2 are equal.
- * @returns > 0 if this should come after pDev2.
- *
- * @param   pDev2   Device 2.
+ *  Compares this device with a USBDEVICE and decides which comes first.
+ *
+ *  @return < 0 if this should come before pDev2.
+ *  @return   0 if this and pDev2 are equal.
+ *  @return > 0 if this should come after pDev2.
+ *
+ *  @param   pDev2   Device 2.
+ *
+ *  @note Must be called from under the object write lock.
  */
 int HostUSBDevice::compare (PCUSBDEVICE pDev2)
 {
+    AssertReturn (isLockedOnCurrentThread(), -1);
+
     return compare (mUsb, pDev2);
 }
@@ -585 +869 @@
 
 /**
- * Compares two USB devices and decides which comes first.
- *
- * @returns < 0 if pDev1 should come before pDev2.
- * @returns   0 if pDev1 and pDev2 are equal.
- * @returns > 0 if pDev1 should come after pDev2.
- *
- * @param   pDev1   Device 1.
- * @param   pDev2   Device 2.
+ *  Compares two USB devices and decides which comes first.
+ *
+ *  @return < 0 if pDev1 should come before pDev2.
+ *  @return   0 if pDev1 and pDev2 are equal.
+ *  @return > 0 if pDev1 should come after pDev2.
+ *
+ *  @param   pDev1   Device 1.
+ *  @param   pDev2   Device 2.
  */
 /*static*/ int HostUSBDevice::compare (PCUSBDEVICE pDev1, PCUSBDEVICE pDev2)
@@ -609 +893 @@
 
 /**
- * Updates the state of the device.
- *
- * @return true if the state has actually changed.
- * @return false if the state didn't change, or the change might have been caused by VBox.
- *
- * @param   aDev    The current device state as seen by the proxy backend.
- *
- * @note Locks this object for writing.
+ *  Updates the state of the device.
+ *
+ *  If this method returns @c true, Host::onUSBDeviceStateChanged() will be
+ *  called to process the state change (complete the state change request,
+ *  inform the VM process etc.).
+ *
+ *  If this method returns @c false, it is assumed that the given state change
+ *  is "minor": it doesn't require any further action other than update the
+ *  mState field with the actual state value.
+ *
+ *  @param   aDev    The current device state as seen by the proxy backend.
+ *
+ *  @note Locks this object for writing.
  */
 bool HostUSBDevice::updateState (PCUSBDEVICE aDev)
 {
+    LogFlowThisFunc (("\n"));
+
+    AssertReturn (isLockedOnCurrentThread(), false);
+
     AutoCaller autoCaller (this);
     AssertComRCReturn (autoCaller.rc(), false);
 
     AutoLock alock (this);
+
+    bool isImportant = false;
 
     /*
@@ -629 +924 @@
      * doesn't necessarily know everything that's going on. So, rather
      * be overly careful than changing the state once when we shouldn't!
+     *
+     * In particular, we treat changing between three states Unavailable, Busy
+     * and Available as non-important (because they all mean that the device
+     * is owned by the host) and return false in this case. We may want to
+     * change it later and, e.g. re-run all USB filters when the device goes from
+     * from Busy to Available).
      */
+
     switch (aDev->enmState)
     {
@@ -642 +944 @@
             {
                 case USBDeviceState_USBDeviceUnavailable:
-                /* the proxy may confuse following states with unavailable */
-                case USBDeviceState_USBDeviceHeld:
-                case USBDeviceState_USBDeviceCaptured:
                     return false;
+                /* the following state changes don't require any action for now */
+                case USBDeviceState_USBDeviceBusy:
+                case USBDeviceState_USBDeviceAvailable:
+                    isImportant = false;
                 default:
-                    LogFlowMember ((" HostUSBDevice::updateState: %d -> %d\n",
-                                    mState, USBDeviceState_USBDeviceUnavailable));
-                    mState = USBDeviceState_USBDeviceUnavailable;
-                    return true;
+                    isImportant = true;
             }
-            break;
+            LogFlowThisFunc (("%d -> %d\n",
+                              mState, USBDeviceState_USBDeviceUnavailable));
+            mState = USBDeviceState_USBDeviceUnavailable;
+            return isImportant;
 
         case USBDEVICESTATE_USED_BY_HOST_CAPTURABLE:
@@ -658 +961 @@
             {
                 case USBDeviceState_USBDeviceBusy:
-                /* the proxy may confuse following states with capturable */
-                case USBDeviceState_USBDeviceHeld:
-                case USBDeviceState_USBDeviceCaptured:
                     return false;
+                /* the following state changes don't require any action for now */
+                case USBDeviceState_USBDeviceUnavailable:
+                case USBDeviceState_USBDeviceAvailable:
+                    isImportant = false;
                 default:
-                    LogFlowMember ((" HostUSBDevice::updateState: %d -> %d\n",
-                                    mState, USBDeviceState_USBDeviceBusy));
-                    mState = USBDeviceState_USBDeviceBusy;
-                    return true;
+                    isImportant = true;
             }
-            break;
+            LogFlowThisFunc (("%d -> %d\n",
+                              mState, USBDeviceState_USBDeviceBusy));
+            mState = USBDeviceState_USBDeviceBusy;
+            return isImportant;
 
         case USBDEVICESTATE_UNUSED:
@@ -674 +978 @@
             {
                 case USBDeviceState_USBDeviceAvailable:
-                /* the proxy may confuse following state(s) with available */
-                case USBDeviceState_USBDeviceHeld:
-                case USBDeviceState_USBDeviceCaptured:
                     return false;
+                /* the following state changes don't require any action for now */
+                case USBDeviceState_USBDeviceUnavailable:
+                case USBDeviceState_USBDeviceBusy:
+                    isImportant = false;
                 default:
-                    LogFlowMember ((" HostUSBDevice::updateState: %d -> %d\n",
-                                    mState, USBDeviceState_USBDeviceAvailable));
-                    mState = USBDeviceState_USBDeviceAvailable;
-                    return true;
+                    isImportant = true;
             }
-            break;
+            LogFlowThisFunc (("%d -> %d\n",
+                              mState, USBDeviceState_USBDeviceAvailable));
+            mState = USBDeviceState_USBDeviceAvailable;
+            return isImportant;
 
         case USBDEVICESTATE_HELD_BY_PROXY:
@@ -690 +995 @@
             {
                 case USBDeviceState_USBDeviceHeld:
-                /* the proxy may confuse following state(s) with held */
-                case USBDeviceState_USBDeviceAvailable:
-                case USBDeviceState_USBDeviceBusy:
-                case USBDeviceState_USBDeviceCaptured:
                     return false;
                 default:
-                    LogFlowMember ((" HostUSBDevice::updateState: %d -> %d\n",
-                                    mState, USBDeviceState_USBDeviceHeld));
+                    LogFlowThisFunc (("%d -> %d\n",
+                                      mState, USBDeviceState_USBDeviceHeld));
                     mState = USBDeviceState_USBDeviceHeld;
                     return true;
@@ -704 +1005 @@
 
         case USBDEVICESTATE_USED_BY_GUEST:
+            /* @todo USBDEVICESTATE_USED_BY_GUEST seems not to be used
+             * anywhere in the proxy code; it's quite logical because the
+             * proxy doesn't know anything about guest VMs. */
+            AssertFailed();
+#if 0
             switch (mState)
             {
@@ -713 +1019 @@
                     return false;
                 default:
-                    LogFlowMember ((" HostUSBDevice::updateState: %d -> %d\n",
-                                    mState, USBDeviceState_USBDeviceHeld));
+                    LogFlowThisFunc (("%d -> %d\n",
+                                      mState, USBDeviceState_USBDeviceHeld));
                     mState = USBDeviceState_USBDeviceHeld;
                     return true;
             }
+#endif
             break;
     }

trunk/src/VBox/Main/MachineImpl.cpp

@@ -7709 +7709 @@
  *  @note Locks the same as Host::captureUSBDevice() does.
  */
-STDMETHODIMP SessionMachine::CaptureUSBDevice (INPTR GUIDPARAM aId,
-                                               IUSBDevice **aHostDevice)
+STDMETHODIMP SessionMachine::CaptureUSBDevice (INPTR GUIDPARAM aId)
 {
     LogFlowThisFunc (("\n"));
 
-    if (!aHostDevice)
-        return E_POINTER;
-
     AutoCaller autoCaller (this);
     AssertComRCReturn (autoCaller.rc(), autoCaller.rc());
 
     // if cautureUSBDevice() fails, it must have set extended error info
-    return mParent->host()->captureUSBDevice (this, aId, aHostDevice);
+    return mParent->host()->captureUSBDevice (this, aId);
 }
 
@@ -7745 +7741 @@
  *  @note Locks what called methods lock.
  */
-STDMETHODIMP SessionMachine::AutoCaptureUSBDevices (IUSBDeviceCollection **aHostDevices)
+STDMETHODIMP SessionMachine::AutoCaptureUSBDevices()
 {
     LogFlowThisFunc (("\n"));
@@ -7756 +7752 @@
     NOREF (rc);
 
-    return mParent->host()->autoCaptureUSBDevices (this, aHostDevices);
+    return mParent->host()->autoCaptureUSBDevices (this);
 }
 
@@ -8584 +8580 @@
  *  @note Locks this object for reading.
  */
-HRESULT SessionMachine::onUSBDeviceAttach (IUSBDevice *aDevice)
+HRESULT SessionMachine::onUSBDeviceAttach (IUSBDevice *aDevice,
+                                           IVirtualBoxErrorInfo *aError)
 {
     LogFlowThisFunc (("\n"));
@@ -8601 +8598 @@
         return S_OK;
 
-    return directControl->OnUSBDeviceAttach (aDevice);
+    return directControl->OnUSBDeviceAttach (aDevice, aError);
 }
 
@@ -8607 +8604 @@
  *  @note Locks this object for reading.
  */
-HRESULT SessionMachine::onUSBDeviceDetach (INPTR GUIDPARAM aId)
+HRESULT SessionMachine::onUSBDeviceDetach (INPTR GUIDPARAM aId,
+                                           IVirtualBoxErrorInfo *aError)
 {
     LogFlowThisFunc (("\n"));
@@ -8624 +8622 @@
         return S_OK;
 
-    return directControl->OnUSBDeviceDetach (aId);
+    return directControl->OnUSBDeviceDetach (aId, aError);
 }
 

trunk/src/VBox/Main/SessionImpl.cpp

@@ -562 +562 @@
 }
 
-STDMETHODIMP Session::OnUSBDeviceAttach (IUSBDevice *aDevice)
+STDMETHODIMP Session::OnUSBDeviceAttach (IUSBDevice *aDevice,
+                                         IVirtualBoxErrorInfo *aError)
 {
     LogFlowThisFunc (("\n"));
@@ -573 +574 @@
                   mType == SessionType_DirectSession, E_FAIL);
 
-    return mConsole->onUSBDeviceAttach (aDevice);
-}
-
-STDMETHODIMP Session::OnUSBDeviceDetach (INPTR GUIDPARAM aId)
+    return mConsole->onUSBDeviceAttach (aDevice, aError);
+}
+
+STDMETHODIMP Session::OnUSBDeviceDetach (INPTR GUIDPARAM aId,
+                                         IVirtualBoxErrorInfo *aError)
 {
     LogFlowThisFunc (("\n"));
@@ -587 +589 @@
                   mType == SessionType_DirectSession, E_FAIL);
 
-    return mConsole->onUSBDeviceDetach (aId);
+    return mConsole->onUSBDeviceDetach (aId, aError);
 }
 

trunk/src/VBox/Main/USBControllerImpl.cpp

@@ -1011 +1011 @@
     AutoReaderLock alock (this);
 
+    /* Disabled USB controllers cannot actually work with USB devices */
+    if (!mData->mEnabled)
+        return false;
+
     bool match = false;
 
@@ -1045 +1049 @@
 
     AutoReaderLock alock (this);
+
+    /* Disabled USB controllers cannot actually work with USB devices */
+    if (!mData->mEnabled)
+        return false;
 
     HRESULT rc = S_OK;

trunk/src/VBox/Main/USBProxyService.cpp

@@ -36 +36 @@
     : mHost (aHost), mThread (NIL_RTTHREAD), mTerminate (false), mDevices (), mLastError (VINF_SUCCESS)
 {
-    LogFlowMember (("USBProxyService::USBProxyService: aHost=%p\n", aHost));
+    LogFlowThisFunc (("aHost=%p\n", aHost));
 }
 
@@ -45 +45 @@
 USBProxyService::~USBProxyService()
 {
-    LogFlowMember (("USBProxyService::~USBProxyService: \n"));
+    LogFlowThisFunc (("\n"));
     Assert (mThread == NIL_RTTHREAD);
     mDevices.clear();
@@ -84 +84 @@
         AssertRC (rc);
         if (VBOX_SUCCESS (rc))
-            LogFlow (("USBProxyService::start: started mThread=%RTthrd\n", mThread));
+            LogFlowThisFunc (("started mThread=%RTthrd\n", mThread));
         else
         {
@@ -92 +92 @@
     }
     else
-        LogFlow (("USBProxyService::start: already running, mThread=%RTthrd\n", mThread));
+        LogFlowThisFunc (("already running, mThread=%RTthrd\n", mThread));
     return rc;
 }
@@ -117 +117 @@
         if (VBOX_SUCCESS (rc))
         {
-            LogFlowMember (("USBProxyService::stop: stopped mThread=%RTthrd\n", mThread));
+            LogFlowThisFunc (("stopped mThread=%RTthrd\n", mThread));
             mThread = NIL_RTTHREAD;
             mTerminate = false;
@@ -128 +128 @@
     }
     else
-        LogFlowMember (("USBProxyService::stop: not active\n"));
+        LogFlowThisFunc (("not active\n"));
 
     return rc;
@@ -186 +186 @@
 void USBProxyService::processChanges (void)
 {
-    LogFlowMember (("USBProxyService::processChanges: \n"));
+    LogFlowThisFunc (("\n"));
 
     /*
@@ -195 +195 @@
     {
         pDevices = sortDevices (pDevices);
+
+        /* we need to lock the host object for writing because
+         * a) the subsequent code may call Host methods that require a write
+         *    lock
+         * b) we will lock HostUSBDevice objects below and want to make sure
+         *    the lock order is always the same (Host, HostUSBDevice, as
+         *    expected by Host) to avoid cross-deadlocks */
+
+        AutoLock hostLock (mHost);
 
         /*
@@ -204 +213 @@
                ||   pDevices)
         {
+            ComObjPtr <HostUSBDevice> DevPtr;
+
+            if (It != mDevices.end())
+                DevPtr = *It;
+
+            /// @todo we want to AddCaller here to make sure the device hasn't
+            //  been uninitialized (needs support for the no-op AddCaller when
+            //  its argument is NULL)
+
+            /* Lock the device object since we will read/write it's
+             * properties. All Host callbacks also imply the object is
+             * locked. */
+            AutoLock devLock (DevPtr.isNull() ? NULL : DevPtr);
+
             /*
              * Compare.
              */
-            ComObjPtr <HostUSBDevice> DevPtr;
             int iDiff;
-            if (It == mDevices.end())
+            if (DevPtr.isNull())
                 iDiff = 1;
             else
             {
-                DevPtr = *It;
                 if (!pDevices)
                     iDiff = -1;
@@ -248 +269 @@
                           (HostUSBDevice *)NewObj, pNew, pNew->idVendor, pNew->idProduct, pNew->pszProduct, pNew->pszManufacturer));
 
+                    /* not really necessary to lock here, but make Assert
+                     * checks happy */
+                    AutoLock newDevLock (NewObj);
+                    
                     mDevices.insert (It, NewObj);
                     mHost->onUSBDeviceAttached (NewObj);
@@ -257 +282 @@
                      */
                     /** @todo add a timeout here. */
-                    if (!DevPtr->isStatePendingUnlocked())
+                    if (!DevPtr->isStatePending())
                     {
                         It = mDevices.erase (It);
@@ -263 +288 @@
                         Log (("USBProxyService::processChanges: detached %p\n", (HostUSBDevice *)DevPtr)); /** @todo add details .*/
                     }
-                    /* else: operation pending */
+                    else
+                    {
+                        /* a state change (re-cycle) request is pending, go
+                         * to the next device */
+                        It++;
+                    }
                 }
             }
@@ -270 +300 @@
     else
     {
+        /* we need to lock the host object for writing because
+         * a) the subsequent code may call Host methods that require a write
+         *    lock
+         * b) we will lock HostUSBDevice objects below and want to make sure
+         *    the lock order is always the same (Host, HostUSBDevice, as
+         *    expected by Host) to avoid cross-deadlocks */
+
+        AutoLock hostLock (mHost);
+
         /* All devices were detached */
         HostUSBDeviceList::iterator It = this->mDevices.begin();
@@ -275 +314 @@
         {
             ComObjPtr <HostUSBDevice> DevPtr = *It;
+            
+            AutoLock devLock (DevPtr);
+
             /*
              * DevPtr was detached.
@@ -284 +326 @@
     }
 
-    LogFlowMember (("USBProxyService::processChanges: returns void\n"));
+    LogFlowThisFunc (("returns void\n"));
 }
 
@@ -291 +333 @@
 {
     USBProxyService *pThis = (USBProxyService *)pvUser;
-    LogFlow (("USBProxyService::serviceThread: pThis=%p\n", pThis));
+    LogFlowFunc (("pThis=%p\n", pThis));
     pThis->serviceThreadInit();
 
@@ -306 +348 @@
 
     pThis->serviceThreadTerm();
-    LogFlow (("USBProxyService::serviceThread: returns VINF_SUCCESS\n"));
+    LogFlowFunc (("returns VINF_SUCCESS\n"));
     return VINF_SUCCESS;
 }
@@ -378 +420 @@
 bool USBProxyService::updateDeviceStateFake (HostUSBDevice *aDevice, PUSBDEVICE aUSBDevice)
 {
-    if (aDevice->isStatePendingUnlocked())
-    {
-        switch (aDevice->pendingStateUnlocked())
-        {
-            case USBDeviceState_USBDeviceCaptured:      aUSBDevice->enmState = USBDEVICESTATE_USED_BY_GUEST; break;
+    AssertReturn (aDevice, false);
+    AssertReturn (aDevice->isLockedOnCurrentThread(), false);
+
+    if (aDevice->isStatePending())
+    {
+        switch (aDevice->pendingState())
+        {
+            /* @todo USBDEVICESTATE_USED_BY_GUEST seems not to be used anywhere in the proxy code; it's
+             * quite logical because the proxy doesn't know anything about guest VMs. We use HELD_BY_PROXY
+             * instead -- it is sufficient and is what Main expects. */
+            case USBDeviceState_USBDeviceCaptured:      aUSBDevice->enmState = USBDEVICESTATE_HELD_BY_PROXY; break;
             case USBDeviceState_USBDeviceHeld:          aUSBDevice->enmState = USBDEVICESTATE_HELD_BY_PROXY; break;
             case USBDeviceState_USBDeviceAvailable:     aUSBDevice->enmState = USBDEVICESTATE_UNUSED; break;
@@ -388 +436 @@
             case USBDeviceState_USBDeviceBusy:          aUSBDevice->enmState = USBDEVICESTATE_USED_BY_HOST_CAPTURABLE; break;
             default:
-                AssertMsgFailed(("%d\n", aDevice->pendingStateUnlocked()));
+                AssertMsgFailed(("%d\n", aDevice->pendingState()));
                 break;
         }
@@ -471 +519 @@
 bool USBProxyService::updateDeviceState (HostUSBDevice *pDevice, PUSBDEVICE pUSBDevice)
 {
+    AssertReturn (pDevice, false);
+    AssertReturn (pDevice->isLockedOnCurrentThread(), false);
+
     return pDevice->updateState (pUSBDevice);
 }

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

@@ -1600 +1600 @@
     <interface
         name="IInternalMachineControl" extends="$unknown"
-        uuid="e780b585-585d-47e2-ab53-e94dc552353c"
+        uuid="67ef427d-5f01-4791-a45e-ae5e646ed7c6"
         internal="yes"
         wsmap="suppress"
@@ -1637 +1637 @@
         <method name="captureUSBDevice">
             <desc>
-                Requests a capture of the given host USB device, and returns
-                the captured device (as IUSBDevice) to the caller.
-                <note>
-                    The server must mark the device as USBDeviceCaptured
-                    during this call.
-
-                    This method must return extended error info in case of any
-                    eroror (<link to="IConsole::detachUSBDevice()"/>) relies
-                    on this.
-                </note>
+                Requests a capture of the given host USB device.
+                When the request is completed, the VM process will
+                get a <link to="IInternalSessionControl::onUSBDeviceAttach"/>
+                notification.
             </desc>
             <param name="id" type="uuid" dir="in"/>
-            <param name="hostDevice" type="IUSBDevice" dir="return"/>
         </method>
 
         <method name="releaseUSBDevice">
             <desc>
-                Releases the given USB device.
+                Requests to release the given USB device.
+                When the request is completed, the VM process will
+                get a <link to="IInternalSessionControl::onUSBDeviceDetach"/>
+                notification.
                 <note>
                     The server must run its own filters and filters of all VMs
-                    but this one on the given device as if it were just attached
-                    to the host computer.
+                    but this one on all released devices as if they were just
+                    attached to the host computer.
                 </note>
             </desc>
@@ -1666 +1662 @@
         <method name="autoCaptureUSBDevices">
             <desc>
-                Queries the list of available devices that must be auto-captured
-                according to this VM's filters. Devices returned as IUSBDevice
-                instances.
-                <note>
-                    The server must mark all returned devices as USBDeviceCaptured
-                    during this call.
-                </note>
-            </desc>
-            <param name="hostDevices" type="IUSBDeviceCollection" dir="return"/>
+                Requests a capture all matching USB devices attached to the host.
+                When the request is completed, the VM process will
+                get a <link to="IInternalSessionControl::onUSBDeviceAttach"/>
+                notification per every captured device.
+            </desc>
         </method>
 
@@ -2647 +2639 @@
     <interface
         name="IConsoleCallback" extends="$unknown"
-        uuid="a56eff10-6db9-4985-a12a-039b604b491b"
+        uuid="ebd0c5f2-7b11-4508-803f-012099caee03"
         wsmap="suppress"
     >
@@ -2748 +2740 @@
                 find out what has changed.
             </desc>
+        </method>
+
+        <method name="onUSBDeviceStateChange">
+            <desc>
+                Notification when a USB device is attached to or detached from
+                the virtual USB controller.
+
+                This notification is sent as a result of the indirect
+                request to attach the device because it matches one of the
+                machine USB filters, or as a result of the direct request
+                issued by <link to="IConsole::attachUSBDevice"/> or
+                <link to="IConsole::detachUSBDevice"/>.
+
+                This notification is sent in case of both a succeeded and a
+                failed request completion. When the request succeeds, the @a
+                error parameter is @c null, and the given device has been
+                already added to (when @a attached is @c true) or removed from
+                (when @a attached is @c false) the collection represented by
+                <link to="IConsole::USBDevices"/>. On failure, the collection
+                doesn't change and the @a error perameter represents the error
+                message describing the failure.
+
+            </desc>
+            <param name="device" type="IUSBDevice" dir="in">
+                <desc>Device that is subject to state change.</desc>
+            </param>
+            <param name="attached" type="boolean" dir="in">
+                <desc>
+                    <tt>true</tt> if the device was attached
+                    and <tt>false</tt> otherwise.
+                </desc>
+            </param>
+            <param name="error" type="IVirtualBoxErrorInfo" dir="in">
+                <desc>
+                   <tt>null</tt> on success or an error message object on
+                   failure.
+                </desc>
+            </param>
         </method>
 
@@ -7411 +7441 @@
     <interface
         name="IInternalSessionControl" extends="$unknown"
-        uuid="9c6bec01-4135-40a0-b9d3-ab119a9c5412"
+        uuid="80a9b698-cc60-48cf-ab88-a7c2ea4013a6"
         internal="yes"
         wsmap="suppress"
@@ -7499 +7529 @@
         <method name="onUSBDeviceAttach">
             <desc>
-                Triggered when a USB device has just been attached to the host
-                computer and is to be auto-captured by the machine according
-                to its USB filters.
+                Triggered when a request to capture a USB device (as a result
+                of matched USB filters or direct call to
+                <link to="IConsole::attachUSBDevice"/>) has completed.
+                A @c null @a error object means success, otherwise it
+                describes a failure.
             </desc>
             <param name="device" type="IUSBDevice" dir="in"/>
+            <param name="error" type="IVirtualBoxErrorInfo" dir="in"/>
         </method>
 
         <method name="onUSBDeviceDetach">
             <desc>
-                Triggered when a USB device has just been detached from the host
-                computer and needs to be detached from the machine.
+                Triggered when a request to release the USB device (as a result
+                of machine termination or direct call to
+                <link to="IConsole::detachUSBDevice"/>) has completed.
+                A @c null @a error object means success, otherwise it
             </desc>
             <param name="id" type="uuid" dir="in"/>
+            <param name="error" type="IVirtualBoxErrorInfo" dir="in"/>
         </method>
 

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

@@ -174 +174 @@
     HRESULT onVRDPServerChange();
     HRESULT onUSBControllerChange();
-    HRESULT onUSBDeviceAttach(IUSBDevice *aDevice);
-    HRESULT onUSBDeviceDetach(INPTR GUIDPARAM aId);
+    HRESULT onUSBDeviceAttach (IUSBDevice *aDevice, IVirtualBoxErrorInfo *aError);
+    HRESULT onUSBDeviceDetach (INPTR GUIDPARAM aId, IVirtualBoxErrorInfo *aError);
 
     VMMDev *getVMMDev() { return mVMMDev; }
@@ -198 +198 @@
     void onAdditionsOutdated();
     void onKeyboardLedsChange (bool fNumLock, bool fCapsLock, bool fScrollLock);
+    void onUSBDeviceStateChange (IUSBDevice *aDevice, bool aAttached,
+                                 IVirtualBoxErrorInfo *aError);
     void onRuntimeError (BOOL aFatal, INPTR BSTR aErrorID, INPTR BSTR aMessage);
     HRESULT onShowWindow (BOOL aCheck, BOOL *aCanShow, ULONG64 *aWinId);
@@ -369 +371 @@
                                           const char *pszPath, bool fPassthrough);
 
-    HRESULT attachUSBDevice (IUSBDevice *aHostDevice, bool aManual,
-                             PVUSBIRHCONFIG aConfig);
+    HRESULT attachUSBDevice (IUSBDevice *aHostDevice, PVUSBIRHCONFIG aConfig);
 
     static DECLCALLBACK(int)
@@ -378 +379 @@
     static DECLCALLBACK(int)
     usbDetachCallback (Console *that, USBDeviceList::iterator *aIt,
-                       bool aManual, PVUSBIRHCONFIG aConfig, PCRTUUID aUuid);
+                       PVUSBIRHCONFIG aConfig, PCRTUUID aUuid);
 
     static DECLCALLBACK (int)

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

@@ -103 +103 @@
     }
 
+    STDMETHOD(OnUSBDeviceStateChange)(IUSBDevice *device, BOOL attached,
+                                      IVirtualBoxErrorInfo *message)
+    {
+        return S_OK;
+    }
+
     STDMETHOD(OnRuntimeError)(BOOL fatal, INPTR BSTR id, INPTR BSTR message)
     {

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

@@ -105 +105 @@
     HRESULT saveSettings (CFGNODE aGlobal);
 
-    HRESULT captureUSBDevice (SessionMachine *aMachine, INPTR GUIDPARAM aId,
-                              IUSBDevice **aHostDevice);
+    HRESULT captureUSBDevice (SessionMachine *aMachine, INPTR GUIDPARAM aId);
     HRESULT releaseUSBDevice (SessionMachine *aMachine, INPTR GUIDPARAM aId);
-    HRESULT autoCaptureUSBDevices (SessionMachine *aMachine,
-                                   IUSBDeviceCollection **aHostDevices);
+    HRESULT autoCaptureUSBDevices (SessionMachine *aMachine);
     HRESULT releaseAllUSBDevices (SessionMachine *aMachine);
 
@@ -149 +147 @@
     }
 
-    HRESULT applyAllUSBFilters (ComObjPtr <HostUSBDevice> &aDevice,
-                                SessionMachine *aMachine = NULL);
-    void applyMachineUSBFilters (SessionMachine *aMachine,
+    HRESULT applyAllUSBFilters (ComObjPtr <HostUSBDevice> &aDevice);
+
+    bool applyMachineUSBFilters (SessionMachine *aMachine,
                                  ComObjPtr <HostUSBDevice> &aDevice);
 

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

@@ -91 +91 @@
     USBDeviceState_T state() const { return mState; }
 
-    /** Same as state() except you don't need to lock any thing. */
-    USBDeviceState_T stateUnlocked() const
-    {
-        AutoReaderLock (this);
-        return state();
-    }
-
     /* @note Must be called from under the object read lock. */
     USBDeviceState_T pendingState() const { return mPendingState; }
 
-    /** Same as pendingState() except you don't need to lock any thing. */
-    USBDeviceState_T pendingStateUnlocked() const
-    {
-        AutoReaderLock (this);
-        return pendingState();
-    }
-
     /* @note Must be called from under the object read lock. */
     ComObjPtr <SessionMachine, ComWeakRef> &machine() { return mMachine; }
 
-/// @todo remove
-#if 0
-    /* @note Must be called from under the object read lock. */
-    bool isIgnored() { return mIgnored; }
-#endif
-
     /* @note Must be called from under the object read lock. */
     bool isStatePending() const { return mIsStatePending; }
 
-    /** Same as isStatePending() except you don't need to lock any thing. */
-    bool isStatePendingUnlocked() const
-    {
-        AutoReaderLock (this);
-        return isStatePending();
-    }
-
     /* @note Must be called from under the object read lock. */
     PCUSBDEVICE usbData() const { return mUsb; }
@@ -132 +105 @@
     Utf8Str name();
 
-/// @todo remove
-#if 0
-    void setIgnored();
-#endif
-
-    bool setCaptured (SessionMachine *aMachine);
-    int  setHostDriven();
-    int  reset();
-
-/// @todo remove
-#if 0
-    void setHostState (USBDeviceState_T aState);
-#endif
+    bool requestCapture (SessionMachine *aMachine);
+    void requestRelease();
+    void requestHold();
+
+    void setHeld();
+    void reset();
+
+    void handlePendingStateChange();
+    void cancelPendingState();
 
     bool isMatch (const USBDeviceFilter::Data &aData);
@@ -163 +132 @@
     ComObjPtr <SessionMachine, ComWeakRef> mMachine;
     bool mIsStatePending : 1;
-/// @todo remove
-#if 0
-    bool mIgnored : 1;
-#endif
 
     /** Pointer to the USB Proxy Service instance. */

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

@@ -727 +727 @@
     STDMETHOD(GetIPCId)(BSTR *id);
     STDMETHOD(RunUSBDeviceFilters) (IUSBDevice *aUSBDevice, BOOL *aMatched);
-    STDMETHOD(CaptureUSBDevice) (INPTR GUIDPARAM aId, IUSBDevice **aHostDevice);
+    STDMETHOD(CaptureUSBDevice) (INPTR GUIDPARAM aId);
     STDMETHOD(ReleaseUSBDevice) (INPTR GUIDPARAM aId);
-    STDMETHOD(AutoCaptureUSBDevices) (IUSBDeviceCollection **aHostDevices);
+    STDMETHOD(AutoCaptureUSBDevices)();
     STDMETHOD(ReleaseAllUSBDevices)();
     STDMETHOD(OnSessionEnd)(ISession *aSession, IProgress **aProgress);
@@ -758 +758 @@
     HRESULT onVRDPServerChange();
     HRESULT onUSBControllerChange();
-    HRESULT onUSBDeviceAttach (IUSBDevice *aDevice);
-    HRESULT onUSBDeviceDetach (INPTR GUIDPARAM aId);
+    HRESULT onUSBDeviceAttach (IUSBDevice *aDevice,
+                               IVirtualBoxErrorInfo *aError);
+    HRESULT onUSBDeviceDetach (INPTR GUIDPARAM aId,
+                               IVirtualBoxErrorInfo *aError);
 
 private:

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

@@ -100 +100 @@
     STDMETHOD(OnVRDPServerChange)();
     STDMETHOD(OnUSBControllerChange)();
-    STDMETHOD(OnUSBDeviceAttach) (IUSBDevice *aDevice);
-    STDMETHOD(OnUSBDeviceDetach) (INPTR GUIDPARAM aId);
+    STDMETHOD(OnUSBDeviceAttach) (IUSBDevice *aDevice, IVirtualBoxErrorInfo *aError);
+    STDMETHOD(OnUSBDeviceDetach) (INPTR GUIDPARAM aId, IVirtualBoxErrorInfo *aError);
     STDMETHOD(OnShowWindow) (BOOL aCheck, BOOL *aCanShow, ULONG64 *aWinId);