usb: clarify transfer cancellation API

Rather than having a get_canceller() function on UsbTransfer, make the
submit function return the canceller.  This makes it clear that the
transfer can't be cancelled before it is submitted.

BUG=None
TEST=None

Change-Id: Ice36c3096a1f8a5aafe93b5d5e27eb371183c19f
Signed-off-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1783599
Reviewed-by: Zach Reizner <zachr@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>

3 files changed, 20 insertions, 24 deletions

diff --git a/devices/src/usb/host_backend/utils.rs b/devices/src/usb/host_backend/utils.rs
index 7d23bd4..bcbec20 100644
--- a/devices/src/usb/host_backend/utils.rs
+++ b/devices/src/usb/host_backend/utils.rs
@@ -58,17 +58,6 @@ pub fn submit_transfer<T: UsbTransferBuffer>(
         let mut state = xhci_transfer.state().lock();
         match mem::replace(&mut *state, XhciTransferState::Cancelled) {
             XhciTransferState::Created => {
-                let canceller = usb_transfer.get_canceller();
-                // TODO(jkwang) refactor canceller to return Cancel::Ok or Cancel::Err.
-                let cancel_callback = Box::new(move || match canceller.try_cancel() {
-                    true => {
-                        usb_debug!("cancel issued to libusb backend");
-                    }
-                    false => {
-                        usb_debug!("fail to cancel");
-                    }
-                });
-                *state = XhciTransferState::Submitted { cancel_callback };
                 match device_handle.lock().submit_async_transfer(usb_transfer) {
                     Err(e) => {
                         error!("fail to submit transfer {:?}", e);
@@ -76,7 +65,19 @@ pub fn submit_transfer<T: UsbTransferBuffer>(
                         TransferStatus::NoDevice
                     }
                     // If it's submitted, we don't need to send on_transfer_complete now.
-                    Ok(_) => return Ok(()),
+                    Ok(canceller) => {
+                        // TODO(jkwang) refactor canceller to return Cancel::Ok or Cancel::Err.
+                        let cancel_callback = Box::new(move || match canceller.try_cancel() {
+                            true => {
+                                usb_debug!("cancel issued to libusb backend");
+                            }
+                            false => {
+                                usb_debug!("fail to cancel");
+                            }
+                        });
+                        *state = XhciTransferState::Submitted { cancel_callback };
+                        return Ok(());
+                    }
                 }
             }
             XhciTransferState::Cancelled => {
diff --git a/usb_util/src/device_handle.rs b/usb_util/src/device_handle.rs
index 77a91be..17b0f68 100644
--- a/usb_util/src/device_handle.rs
+++ b/usb_util/src/device_handle.rs
@@ -9,7 +9,7 @@ use crate::bindings;
 use crate::error::{Error, Result};
 use crate::libusb_context::LibUsbContextInner;
 use crate::libusb_device::LibUsbDevice;
-use crate::usb_transfer::{UsbTransfer, UsbTransferBuffer};
+use crate::usb_transfer::{TransferCanceller, UsbTransfer, UsbTransferBuffer};
 
 /// DeviceHandle wraps libusb_device_handle.
 pub struct DeviceHandle {
@@ -155,7 +155,7 @@ impl DeviceHandle {
     pub fn submit_async_transfer<T: UsbTransferBuffer>(
         &self,
         transfer: UsbTransfer<T>,
-    ) -> Result<()> {
+    ) -> Result<TransferCanceller> {
         unsafe { transfer.submit(self.handle) }
     }
 }
diff --git a/usb_util/src/usb_transfer.rs b/usb_util/src/usb_transfer.rs
index e08bc92..e3ca623 100644
--- a/usb_util/src/usb_transfer.rs
+++ b/usb_util/src/usb_transfer.rs
@@ -241,14 +241,6 @@ impl<T: UsbTransferBuffer> UsbTransfer<T> {
         UsbTransfer { inner }
     }
 
-    /// Get canceller of this transfer.
-    pub fn get_canceller(&self) -> TransferCanceller {
-        let weak_transfer = Arc::downgrade(&self.inner.transfer);
-        TransferCanceller {
-            transfer: weak_transfer,
-        }
-    }
-
     /// Set callback function for transfer completion.
     pub fn set_callback<C: 'static + Fn(UsbTransfer<T>) + Send>(&mut self, cb: C) {
         self.inner.callback = Some(Box::new(cb));
@@ -287,11 +279,14 @@ impl<T: UsbTransferBuffer> UsbTransfer<T> {
     ///
     /// Assumes libusb_device_handle is an handled opened by libusb, self.inner.transfer.ptr is
     /// initialized with correct buffer and length.
-    pub unsafe fn submit(self, handle: *mut libusb_device_handle) -> Result<()> {
+    pub unsafe fn submit(self, handle: *mut libusb_device_handle) -> Result<TransferCanceller> {
+        let weak_transfer = Arc::downgrade(&self.inner.transfer);
         let transfer = self.into_raw();
         (*transfer).dev_handle = handle;
         match Error::from(libusb_submit_transfer(transfer)) {
-            Error::Success(_e) => Ok(()),
+            Error::Success(_e) => Ok(TransferCanceller {
+                transfer: weak_transfer,
+            }),
             err => {
                 UsbTransfer::<T>::from_raw(transfer);
                 Err(err)