Changeset 3566 in vbox for trunk/src/VBox/Main/HostUSBDeviceImpl.cpp

Timestamp:

Jul 11, 2007 11:15:44 PM (17 years ago)

Author:

vboxsync

Message:

Async USB detach (for darwin) with async operation timeout (not enabled on windows). Drop the USBProxyService::reset method. (Hope I didn't break anything, it's only tested on Darwin...)

File:

• trunk/src/VBox/Main/HostUSBDeviceImpl.cpp (modified) (23 diffs)

trunk/src/VBox/Main/HostUSBDeviceImpl.cpp

@@ -20 +20 @@
  */
 
+#include <iprt/types.h> /* for UINT64_C */
+
 #include "HostUSBDeviceImpl.h"
 #include "MachineImpl.h"
@@ -108 +110 @@
 
     mPendingState = mState;
+    mPendingStateEx = kNothingPending;
 
     /* Other data members */
@@ -381 +384 @@
 
         mIsStatePending = true;
+        mPendingSince = 0;
 
         /* the VM process will query the object, so leave the lock */
@@ -401 +405 @@
 
         mIsStatePending = false;
+        mPendingStateEx = kNothingPending;
 
         if (SUCCEEDED (rc))
@@ -414 +419 @@
     mIsStatePending = true;
     mPendingState = USBDeviceState_USBDeviceCaptured;
+    mPendingStateEx = kNothingPending;
+    mPendingSince = RTTimeNanoTS();
     mMachine = aMachine;
 
@@ -448 +455 @@
     mIsStatePending = true;
     mPendingState = USBDeviceState_USBDeviceAvailable;
+    mPendingStateEx = kNothingPending;
+    mPendingSince = RTTimeNanoTS();
 
     mUSBProxyService->releaseDevice (this);
@@ -482 +491 @@
     mIsStatePending = true;
     mPendingState = USBDeviceState_USBDeviceHeld;
+    mPendingStateEx = kNothingPending;
+    mPendingSince = RTTimeNanoTS();
 
     mUSBProxyService->captureDevice (this);
@@ -513 +524 @@
  *  @note Must be called from under the object write lock.
  */
-void HostUSBDevice::reset()
+void HostUSBDevice::onDetachedPhys()
 {
     LogFlowThisFunc (("\n"));
@@ -524 +535 @@
 
         mIsStatePending = true;
+        mPendingSince = 0;
 
         /* the VM process will query the object, so leave the lock */
@@ -547 +559 @@
         alock.enter();
 
-        /* Reset all fields. Tthe object should have been
+        /* Reset all fields. The object should have been
          * uninitialized after this method returns, so it doesn't really
          * matter what state we put it in. */
         mIsStatePending = false;
         mState = mPendingState = USBDeviceState_USBDeviceNotSupported;
+        mPendingStateEx = kNothingPending;
         mMachine.setNull();
     }
@@ -569 +582 @@
 
     AssertReturnVoid (mIsStatePending == true);
-    AssertReturnVoid (mState != USBDeviceState_USBDeviceCaptured);
+    AssertReturnVoid (mState != USBDeviceState_USBDeviceCaptured || mPendingStateEx != kNothingPending);
 
     bool wasCapture = false;
@@ -576 +589 @@
     Bstr errorText;
 
-    switch (mPendingState)
-    {
-        case USBDeviceState_USBDeviceCaptured:
-        {
-            if (mState == USBDeviceState_USBDeviceHeld)
+    switch (mPendingStateEx)
+    {
+        case kNothingPending:
+            switch (mPendingState)
             {
-                if (!mMachine.isNull())
-                    wasCapture = true;
-                else
+                case USBDeviceState_USBDeviceCaptured:
                 {
-                    /* it is a canceled capture request. Give the device back
-                     * to the host. */
-                    mPendingState = USBDeviceState_USBDeviceAvailable;
-                    mUSBProxyService->releaseDevice (this);
+                    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:
+                {
+                    Assert (mMachine.isNull());
+        
+                    if (mState == USBDeviceState_USBDeviceHeld)
+                    {
+                        /* couldn't release the device (give it back to the host).
+                         * there is nobody to report an error to (the machine has
+                         * already been deassociated because VMM has already detached
+                         * the device before requesting a release). */
+                    }
+                    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)
+                    {
+                        /* All right, the device is now held (due to some global
+                         * filter). */
+                        break;
+                    }
+                    else
+                    {
+                        /* couldn't capture the device requested by the global
+                         * filter, there is nobody to report an error to. */
+                    }
+                    break;
+                }
+                default:
+                    AssertFailed();
             }
-            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:
-        {
-            Assert (mMachine.isNull());
-
-            if (mState == USBDeviceState_USBDeviceHeld)
-            {
-                /* couldn't release the device (give it back to the host).
-                 * there is nobody to report an error to (the machine has
-                 * already been deassociated because VMM has already detached
-                 * the device before requesting a release). */
-            }
-            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)
-            {
-                /* All right, the device is now held (due to some global
-                 * filter). */
-                break;
-            }
-            else
-            {
-                /* couldn't capture the device requested by the global
-                 * filter, there is nobody to report an error to. */
-            }
-            break;
-        }
+            break;
+
+        /*
+         * The device has reappeared, the caller (Host) will (maybe) reapply filters,
+         * since we don't quite know we set the machine to NULL.
+         */
+        case kDetachingPendingAttachFilters:
+            mMachine.setNull();
+            break;
+
+        /*
+         * The device has reappeared while the detach operation is still in 
+         * progress, just clear the pending operation and leave the machine as is.
+         */
+        case kDetachingPendingAttach:
+            break;
+
+        case kDetachingPendingDetach:
+        case kDetachingPendingDetachFilters:
         default:
-            AssertFailed();
+            AssertMsgFailed(("%d\n", mPendingStateEx));
+            return;
     }
 
@@ -690 +729 @@
             mIsStatePending = false;
             mState = mPendingState = USBDeviceState_USBDeviceCaptured;
+            mPendingStateEx = kNothingPending;
             return;
         }
@@ -700 +740 @@
     mIsStatePending = false;
     mPendingState = mState;
-}
-
-/**
- *  Cancels pending state change due to machine termination.
+    mPendingStateEx = kNothingPending;
+}
+
+/**
+ *  Cancels pending state change due to machine termination or timeout.
  *
  *  @note Must be called from under the object write lock.
- */
-void HostUSBDevice::cancelPendingState()
+ *  @param  aTimeout        Whether this is a timeout or not.
+ */
+void HostUSBDevice::cancelPendingState(bool aTimeout /*= false*/)
 {
     LogFlowThisFunc (("\n"));
@@ -714 +756 @@
 
     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 */
+    AssertReturnVoid (aTimeout || !mMachine.isNull());
+
+    switch (mPendingStateEx)
+    {
+        case kNothingPending:
+            switch (mPendingState)
+            {
+                case USBDeviceState_USBDeviceCaptured:
+                    /* reset mMachine to deassociate it from the filter and tell
+                     * handlePendingStateChange() what to do */
+                    mMachine.setNull();
+                    if (!aTimeout)
+                        break;
+                case USBDeviceState_USBDeviceAvailable:
+                case USBDeviceState_USBDeviceHeld:
+                    if (aTimeout)
+                    {
+                        mPendingStateEx = kNothingPending;
+                        mIsStatePending = false;
+                        break;
+                    }
+                    /* fall thru */
+                default:
+                    AssertFailed();
+            }
+            break;
+
+        case kDetachingPendingDetach:
+        case kDetachingPendingDetachFilters:
+        case kDetachingPendingAttach:
+        case kDetachingPendingAttachFilters:
             mMachine.setNull();
-            break;
-        }
+            mPendingStateEx = kNothingPending;
+            mIsStatePending = false;
+            break;
+
         default:
-            AssertFailed();
+            AssertMsgFailed(("%d\n", mPendingStateEx));
+            break;
     }
 }
@@ -848 +916 @@
     AssertReturn (isLockedOnCurrentThread(), -1);
 
-#ifdef __WIN__
     return compare (mUsb, aDev2, !isStatePending());
-#else
-    /* Since we fake the requests anyway, there is no need to unnecessarily
-       expose ourselves to trouble the non-strict compare may cause on
-       release/capture/unplug/plug/similar-devices. */
-    return compare (mUsb, aDev2, true /* strict */);
-#endif
 }
 
@@ -882 +943 @@
                             bool aIsStrict /* = true */)
 {
+    /* The non-strict checks tries as best as it can to distiguish between 
+       different physical devices of the same product. Unfortunately this
+       isn't always possible and we might end up a bit confused in rare cases... */
+
     int iDiff = aDev1->idVendor - aDev2->idVendor;
     if (iDiff)
@@ -890 +955 @@
         return iDiff;
 
+    iDiff = aDev1->bcdDevice - aDev2->bcdDevice;
+    if (iDiff)
+        return iDiff;
+
+    if (aDev1->u64SerialHash != aDev2->u64SerialHash)
+        return aDev1->u64SerialHash < aDev2->u64SerialHash ? -1 : 1;
+
     if (!aIsStrict)
         return 0;
 
-    /* The rest is considered as a strict check. */
-
+    /* The rest is considered as a strict check since it includes bits that
+       may vary on logical reconnects (or whatever you wish to call it). */
     return strcmp (aDev1->pszAddress, aDev2->pszAddress);
 }
@@ -957 +1029 @@
      */
 
-    LogFlowThisFunc (("aDev->enmState=%d mState=%d mPendingState=%d\n",
-                      aDev->enmState, mState, mPendingState));
+    LogFlowThisFunc (("aDev->enmState=%d mState=%d mPendingState=%d mPendingStateEx=%d\n",
+                      aDev->enmState, mState, mPendingState, mPendingStateEx));
 
     switch (aDev->enmState)
@@ -966 +1038 @@
         case USBDEVICESTATE_UNSUPPORTED:
             Assert (mState == USBDeviceState_USBDeviceNotSupported);
-            return false;
+            switch (mState)
+            {
+                case USBDeviceState_USBDeviceCaptured:
+                    isImportant = mIsStatePending;
+                    break;
+            }
+            return isImportant;
 
         case USBDEVICESTATE_USED_BY_HOST:
@@ -1000 +1078 @@
                     isImportant = false;
                     break;
+                case USBDeviceState_USBDeviceCaptured:
 #ifndef __WIN__ /* Only Windows really knows whether the device is busy or captured. */
-                case USBDeviceState_USBDeviceCaptured:
                     if (!mIsStatePending)
                         return false;
+#endif
+                    /* Remain in the captured state if it's an async detach. */
+                    if (mPendingStateEx != kNothingPending)
+                    {
+                        LogFlowThisFunc (("USBDeviceCaptured - async detach completed (%d)\n", mPendingStateEx));
+                        return true;
+                    }
                     /* fall thru */
-#endif
                 default:
                     /* USBDeviceState_USBDeviceUnavailable: The device has become capturable, re-run filters. */
@@ -1065 +1149 @@
 
         case USBDEVICESTATE_USED_BY_GUEST:
-            /* @todo USBDEVICESTATE_USED_BY_GUEST seems not to be used
+            /** @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. */
@@ -1091 +1175 @@
 }
 
+/**
+ *  Checks for timeout of any pending async operation.
+ * 
+ *  The caller must write lock the object prior to calling 
+ *  this method.
+ */
+void HostUSBDevice::checkForAsyncTimeout()
+{
+    AssertReturnVoid (isLockedOnCurrentThread());
+
+#ifndef __WIN__ /* no timeouts on windows yet since I don't have all the details here... */
+    if (isStatePending() && mPendingSince)
+    {
+        uint64_t elapsedNanoseconds = RTTimeNanoTS() - mPendingSince;
+        if (elapsedNanoseconds > UINT64_C (60000000000) ) /* 60 seconds */
+        {
+            LogRel (("USB: Async operation timed out; mPendingState=%d mPendingStateEx=%d idVendor=%04x (%s) idProduct=%04x (%s) bcdDevice=%04x\n",
+                     mPendingState, mPendingStateEx, mUsb->idVendor, mUsb->pszManufacturer, mUsb->idProduct,  mUsb->pszProduct, mUsb->bcdDevice));
+
+            cancelPendingState (true);
+        }
+    }
+#endif
+}
+
+/** 
+ *  This method is called by the USB proxy and Host to work the 
+ *  logical reconnection operation.
+ * 
+ *  @param  aStage      kDeatchingPendingDetach, kDeatchingPendingDetachFilters,
+ *                      kDetachingPendingAttach or kDetachingPendingAttachFilters.
+ * 
+ *  @returns Success indicator.
+ */
+bool HostUSBDevice::setLogicalReconnect (InternalState aStage)
+{
+    AssertReturn (isLockedOnCurrentThread(), false);
+
+    switch (aStage)
+    {
+        case kDetachingPendingDetach:
+            AssertReturn (!mIsStatePending, false);
+            mPendingState = mState;
+            mIsStatePending = true;
+            mPendingSince = RTTimeNanoTS();
+            break;
+
+        case kDetachingPendingDetachFilters:
+            AssertReturn (mIsStatePending, false);
+            AssertReturn (mPendingStateEx == kDetachingPendingDetach, false);
+            break;
+
+        case kDetachingPendingAttach:
+            AssertReturn (mIsStatePending, false);
+            AssertReturn (mPendingStateEx == kDetachingPendingDetach, false);
+            break;
+
+        case kDetachingPendingAttachFilters:
+            AssertReturn (mIsStatePending, false);
+            AssertReturn (   mPendingStateEx == kDetachingPendingAttach 
+                          || mPendingStateEx == kDetachingPendingDetachFilters, false);
+            break;
+
+        default:
+            AssertFailedReturn (false);
+    }
+    mPendingStateEx = aStage;
+    return true;
+}
+