<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Sep 25, 2018 at 5:29 PM Christophe Fergeau <<a href="mailto:cfergeau@redhat.com">cfergeau@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Mon, Sep 24, 2018 at 11:43:55AM +0300, Yuri Benditovich wrote:<br>
> Replace all the communication with libusb and usbredirhost<br>
> by usb backend API.<br>
> <br>
> Signed-off-by: Yuri Benditovich <<a href="mailto:yuri.benditovich@daynix.com" target="_blank">yuri.benditovich@daynix.com</a>><br>
> ---<br>
> src/Makefile.am | 2 +<br>
> src/channel-usbredir-priv.h | 9 +-<br>
> src/channel-usbredir.c | 271 +++++++++-------------------<br>
> src/meson.build | 1 +<br>
> src/usb-device-manager-priv.h | 5 +-<br>
> src/usb-device-manager.c | 407 ++++++++++++++++--------------------------<br>
> src/usb-device-manager.h | 29 ++-<br>
> src/usbutil.c | 36 ----<br>
> src/usbutil.h | 2 -<br>
> src/win-usb-dev.c | 59 +++---<br>
> 10 files changed, 301 insertions(+), 520 deletions(-)<br>
> <br>
> diff --git a/src/Makefile.am b/src/Makefile.am<br>
> index 4dd657d..610dbba 100644<br>
> --- a/src/Makefile.am<br>
> +++ b/src/Makefile.am<br>
> @@ -249,6 +249,8 @@ libspice_client_glib_2_0_la_SOURCES = \<br>
> spice-uri-priv.h \<br>
> usb-device-manager.c \<br>
> usb-device-manager-priv.h \<br>
> + usb-backend.h \<br>
> + usb-backend-common.c \<br>
> usbutil.c \<br>
> usbutil.h \<br>
> $(USB_ACL_HELPER_SRCS) \<br>
> diff --git a/src/channel-usbredir-priv.h b/src/channel-usbredir-priv.h<br>
> index 17e9716..fee95f7 100644<br>
> --- a/src/channel-usbredir-priv.h<br>
> +++ b/src/channel-usbredir-priv.h<br>
> @@ -21,9 +21,8 @@<br>
> #ifndef __SPICE_CLIENT_USBREDIR_CHANNEL_PRIV_H__<br>
> #define __SPICE_CLIENT_USBREDIR_CHANNEL_PRIV_H__<br>
> <br>
> -#include <libusb.h><br>
> -#include <usbredirfilter.h><br>
> #include "spice-client.h"<br>
> +#include "usb-backend.h"<br>
> <br>
> G_BEGIN_DECLS<br>
> <br>
> @@ -31,7 +30,7 @@ G_BEGIN_DECLS<br>
> context should not be destroyed before the last device has been<br>
> disconnected */<br>
> void spice_usbredir_channel_set_context(SpiceUsbredirChannel *channel,<br>
> - libusb_context *context);<br>
> + SpiceUsbBackend *context);<br>
<br>
Args were aligned before, dunno if we want to preserve this or not.<br>
<br>
> <br>
> void spice_usbredir_channel_disconnect_device_async(SpiceUsbredirChannel *channel,<br>
> GCancellable *cancellable,<br>
> @@ -46,7 +45,7 @@ gboolean spice_usbredir_channel_disconnect_device_finish(SpiceUsbredirChannel *c<br>
> (through spice_channel_connect()), before calling this. */<br>
> void spice_usbredir_channel_connect_device_async(<br>
> SpiceUsbredirChannel *channel,<br>
> - libusb_device *device,<br>
> + SpiceUsbBackendDevice *device,<br>
> SpiceUsbDevice *spice_device,<br>
> GCancellable *cancellable,<br>
> GAsyncReadyCallback callback,<br>
> @@ -58,7 +57,7 @@ gboolean spice_usbredir_channel_connect_device_finish(<br>
> <br>
> void spice_usbredir_channel_disconnect_device(SpiceUsbredirChannel *channel);<br>
> <br>
> -libusb_device *spice_usbredir_channel_get_device(SpiceUsbredirChannel *channel);<br>
> +SpiceUsbBackendDevice *spice_usbredir_channel_get_device(SpiceUsbredirChannel *channel);<br>
> <br>
> void spice_usbredir_channel_lock(SpiceUsbredirChannel *channel);<br>
> <br>
> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c<br>
> index 1d9c380..67ba88a 100644<br>
> --- a/src/channel-usbredir.c<br>
> +++ b/src/channel-usbredir.c<br>
> @@ -23,7 +23,6 @@<br>
> <br>
> #ifdef USE_USBREDIR<br>
> #include <glib/gi18n-lib.h><br>
> -#include <usbredirhost.h><br>
> #ifdef USE_LZ4<br>
> #include <lz4.h><br>
> #endif<br>
> @@ -66,15 +65,12 @@ enum SpiceUsbredirChannelState {<br>
> };<br>
> <br>
> struct _SpiceUsbredirChannelPrivate {<br>
> - libusb_device *device;<br>
> + SpiceUsbBackendDevice *device;<br>
> SpiceUsbDevice *spice_device;<br>
> - libusb_context *context;<br>
> - struct usbredirhost *host;<br>
> + SpiceUsbBackend *context;<br>
> + SpiceUsbBackendChannel *host;<br>
> /* To catch usbredirhost error messages and report them as a GError */<br>
> GError **catch_error;<br>
> - /* Data passed from channel handle msg to the usbredirhost read cb */<br>
> - const uint8_t *read_buf;<br>
> - int read_buf_size;<br>
> enum SpiceUsbredirChannelState state;<br>
> #ifdef USE_POLKIT<br>
> GTask *task;<br>
> @@ -90,18 +86,10 @@ static void spice_usbredir_channel_dispose(GObject *obj);<br>
> static void spice_usbredir_channel_finalize(GObject *obj);<br>
> static void usbredir_handle_msg(SpiceChannel *channel, SpiceMsgIn *in);<br>
> <br>
> -static void usbredir_log(void *user_data, int level, const char *msg);<br>
> -static int usbredir_read_callback(void *user_data, uint8_t *data, int count);<br>
> +static void usbredir_log(void *user_data, const char *msg, gboolean error);<br>
> static int usbredir_write_callback(void *user_data, uint8_t *data, int count);<br>
> -static void usbredir_write_flush_callback(void *user_data);<br>
> -#if USBREDIR_VERSION >= 0x000701<br>
> -static uint64_t usbredir_buffered_output_size_callback(void *user_data);<br>
> -#endif<br>
> -<br>
> -static void *usbredir_alloc_lock(void);<br>
> -static void usbredir_lock_lock(void *user_data);<br>
> -static void usbredir_unlock_lock(void *user_data);<br>
> -static void usbredir_free_lock(void *user_data);<br>
> +static gboolean usbredir_is_channel_ready(void *user_data);<br>
> +static uint64_t usbredir_get_queue_size(void *user_data);<br>
> <br>
> #else<br>
> struct _SpiceUsbredirChannelPrivate {<br>
> @@ -128,7 +116,7 @@ static void _channel_reset_finish(SpiceUsbredirChannel *channel)<br>
> <br>
> spice_usbredir_channel_lock(channel);<br>
> <br>
> - usbredirhost_close(priv->host);<br>
> + spice_usb_backend_channel_finalize(priv->host);<br>
> priv->host = NULL;<br>
> <br>
> /* Call set_context to re-create the host */<br>
> @@ -228,7 +216,7 @@ static void spice_usbredir_channel_finalize(GObject *obj)<br>
> SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(obj);<br>
> <br>
> if (channel->priv->host)<br>
> - usbredirhost_close(channel->priv->host);<br>
> + spice_usb_backend_channel_finalize(channel->priv->host);<br>
> #ifdef USE_USBREDIR<br>
> g_mutex_clear(&channel->priv->device_connect_mutex);<br>
> #endif<br>
> @@ -252,33 +240,24 @@ static void channel_set_handlers(SpiceChannelClass *klass)<br>
> /* private api */<br>
> <br>
> G_GNUC_INTERNAL<br>
> -void spice_usbredir_channel_set_context(SpiceUsbredirChannel *channel,<br>
> - libusb_context *context)<br>
> +void spice_usbredir_channel_set_context(<br>
> + SpiceUsbredirChannel *channel,<br>
> + SpiceUsbBackend *context)<br>
<br>
Indentation<br>
<br>
> {<br>
> SpiceUsbredirChannelPrivate *priv = channel->priv;<br>
> + SpiceUsbBackendChannelInitData init_data;<br>
> + init_data.user_data = channel;<br>
> + init_data.get_queue_size = usbredir_get_queue_size;<br>
> + init_data.is_channel_ready = usbredir_is_channel_ready;<br>
> + init_data.log = usbredir_log;<br>
> + init_data.write_callback = usbredir_write_callback;<br>
> + init_data.debug = spice_util_get_debug();<br>
> <br>
> g_return_if_fail(priv->host == NULL);<br>
> <br>
> priv->context = context;<br>
> - priv->host = usbredirhost_open_full(<br>
> - context, NULL,<br>
> - usbredir_log,<br>
> - usbredir_read_callback,<br>
> - usbredir_write_callback,<br>
> - usbredir_write_flush_callback,<br>
> - usbredir_alloc_lock,<br>
> - usbredir_lock_lock,<br>
> - usbredir_unlock_lock,<br>
> - usbredir_free_lock,<br>
> - channel, PACKAGE_STRING,<br>
> - spice_util_get_debug() ? usbredirparser_debug : usbredirparser_warning,<br>
> - usbredirhost_fl_write_cb_owns_buffer);<br>
> - if (!priv->host)<br>
> - g_error("Out of memory allocating usbredirhost");<br>
> + priv->host = spice_usb_backend_channel_initialize(context, &init_data);<br>
<br>
priv->host could be NULL, but we lost the g_error().<br>
<br>
> <br>
> -#if USBREDIR_VERSION >= 0x000701<br>
> - usbredirhost_set_buffered_output_size_cb(priv->host, usbredir_buffered_output_size_callback);<br>
> -#endif<br>
> #ifdef USE_LZ4<br>
> spice_channel_set_capability(channel, SPICE_SPICEVMC_CAP_DATA_COMPRESS_LZ4);<br>
> #endif<br>
> @@ -289,9 +268,8 @@ static gboolean spice_usbredir_channel_open_device(<br>
> {<br>
> SpiceUsbredirChannelPrivate *priv = channel->priv;<br>
> SpiceSession *session;<br>
> - libusb_device_handle *handle = NULL;<br>
> - int rc, status;<br>
> SpiceUsbDeviceManager *manager;<br>
> + const char *msg = NULL;<br>
> <br>
> g_return_val_if_fail(priv->state == STATE_DISCONNECTED<br>
> #ifdef USE_POLKIT<br>
> @@ -299,29 +277,28 @@ static gboolean spice_usbredir_channel_open_device(<br>
> #endif<br>
> , FALSE);<br>
> <br>
> - rc = libusb_open(priv->device, &handle);<br>
> - if (rc != 0) {<br>
> - g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,<br>
> - "Could not open usb device: %s [%i]",<br>
> - spice_usbutil_libusb_strerror(rc), rc);<br>
> - return FALSE;<br>
> - }<br>
> -<br>
> priv->catch_error = err;<br>
> - status = usbredirhost_set_device(priv->host, handle);<br>
> - priv->catch_error = NULL;<br>
> - if (status != usb_redir_success) {<br>
> - g_return_val_if_fail(err == NULL || *err != NULL, FALSE);<br>
> + if (!spice_usb_backend_channel_attach(priv->host, priv->device, &msg)) {<br>
> + priv->catch_error = NULL;<br>
> + if (*err == NULL) {<br>
> + if (!msg) {<br>
> + msg = "Exact error not reported";<br>
> + }<br>
> + g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,<br>
> + "Error attaching device: %s", msg);<br>
> + }<br>
> return FALSE;<br>
> }<br>
> + priv->catch_error = NULL;<br>
> <br>
> session = spice_channel_get_session(SPICE_CHANNEL(channel));<br>
> manager = spice_usb_device_manager_get(session, NULL);<br>
> g_return_val_if_fail(manager != NULL, FALSE);<br>
> <br>
> priv->usb_device_manager = g_object_ref(manager);<br>
> - if (!spice_usb_device_manager_start_event_listening(priv->usb_device_manager, err)) {<br>
> - usbredirhost_set_device(priv->host, NULL);<br>
> + if (spice_usb_backend_device_need_thread(priv->device) &&<br>
<br>
I would introduce this later on when it starts to become needed.<br>
<br>
> + !spice_usb_device_manager_start_event_listening(priv->usb_device_manager, err)) {<br>
> + spice_usb_backend_channel_detach(priv->host);<br>
> return FALSE;<br>
> }<br>
> <br>
> @@ -352,8 +329,7 @@ static void spice_usbredir_channel_open_acl_cb(<br>
> spice_usbredir_channel_open_device(channel, &err);<br>
> }<br>
> if (err) {<br>
> - libusb_unref_device(priv->device);<br>
> - priv->device = NULL;<br>
> + g_clear_pointer(&priv->device, spice_usb_backend_device_release);<br>
> g_boxed_free(spice_usb_device_get_type(), priv->spice_device);<br>
> priv->spice_device = NULL;<br>
> priv->state = STATE_DISCONNECTED;<br>
> @@ -370,7 +346,6 @@ static void spice_usbredir_channel_open_acl_cb(<br>
> }<br>
> #endif<br>
> <br>
> -#ifndef USE_POLKIT<br>
> static void<br>
> _open_device_async_cb(GTask *task,<br>
> gpointer object,<br>
> @@ -384,8 +359,7 @@ _open_device_async_cb(GTask *task,<br>
> spice_usbredir_channel_lock(channel);<br>
> <br>
> if (!spice_usbredir_channel_open_device(channel, &err)) {<br>
> - libusb_unref_device(priv->device);<br>
> - priv->device = NULL;<br>
> + g_clear_pointer(&priv->device, spice_usb_backend_device_release);<br>
> g_boxed_free(spice_usb_device_get_type(), priv->spice_device);<br>
> priv->spice_device = NULL;<br>
> }<br>
> @@ -398,18 +372,20 @@ _open_device_async_cb(GTask *task,<br>
> g_task_return_boolean(task, TRUE);<br>
> }<br>
> }<br>
> -#endif<br>
> <br>
> G_GNUC_INTERNAL<br>
> void spice_usbredir_channel_connect_device_async(<br>
> SpiceUsbredirChannel *channel,<br>
> - libusb_device *device,<br>
> + SpiceUsbBackendDevice *device,<br>
> SpiceUsbDevice *spice_device,<br>
> GCancellable *cancellable,<br>
> GAsyncReadyCallback callback,<br>
> gpointer user_data)<br>
> {<br>
> SpiceUsbredirChannelPrivate *priv = channel->priv;<br>
> +#ifdef USE_POLKIT<br>
> + const UsbDeviceInformation *info = spice_usb_backend_device_get_info(device);<br>
> +#endif<br>
> GTask *task;<br>
> <br>
> g_return_if_fail(SPICE_IS_USBREDIR_CHANNEL(channel));<br>
> @@ -436,25 +412,28 @@ void spice_usbredir_channel_connect_device_async(<br>
> goto done;<br>
> }<br>
> <br>
> - priv->device = libusb_ref_device(device);<br>
> + spice_usb_backend_device_acquire(device);<br>
> + priv->device = device;<br>
> priv->spice_device = g_boxed_copy(spice_usb_device_get_type(),<br>
> spice_device);<br>
> #ifdef USE_POLKIT<br>
> - priv->task = task;<br>
> - priv->state = STATE_WAITING_FOR_ACL_HELPER;<br>
> - priv->acl_helper = spice_usb_acl_helper_new();<br>
> - g_object_set(spice_channel_get_session(SPICE_CHANNEL(channel)),<br>
> - "inhibit-keyboard-grab", TRUE, NULL);<br>
> - spice_usb_acl_helper_open_acl_async(priv->acl_helper,<br>
> - libusb_get_bus_number(device),<br>
> - libusb_get_device_address(device),<br>
> - cancellable,<br>
> - spice_usbredir_channel_open_acl_cb,<br>
> - channel);<br>
> - return;<br>
> -#else<br>
> - g_task_run_in_thread(task, _open_device_async_cb);<br>
> + // avoid calling ACL helper for emulated CD devices<br>
> + if (info->max_luns == 0) {<br>
<br>
This does not belong in this commit.<br>
<br>
> + priv->task = task;<br>
> + priv->state = STATE_WAITING_FOR_ACL_HELPER;<br>
> + priv->acl_helper = spice_usb_acl_helper_new();<br>
> + g_object_set(spice_channel_get_session(SPICE_CHANNEL(channel)),<br>
> + "inhibit-keyboard-grab", TRUE, NULL);<br>
> + spice_usb_acl_helper_open_acl_async(priv->acl_helper,<br>
> + info->bus,<br>
> + info->address,<br>
> + cancellable,<br>
> + spice_usbredir_channel_open_acl_cb,<br>
> + channel);<br>
> + return;<br>
> + }<br>
> #endif<br>
> + g_task_run_in_thread(task, _open_device_async_cb);<br>
> <br>
> done:<br>
> g_object_unref(task);<br>
> @@ -501,13 +480,14 @@ void spice_usbredir_channel_disconnect_device(SpiceUsbredirChannel *channel)<br>
> * libusb_handle_events call in the thread.<br>
> */<br>
> g_warn_if_fail(priv->usb_device_manager != NULL);<br>
> - spice_usb_device_manager_stop_event_listening(priv->usb_device_manager);<br>
> + if (spice_usb_backend_device_need_thread(priv->device)) {<br>
> + spice_usb_device_manager_stop_event_listening(priv->usb_device_manager);<br>
> + }<br>
> g_clear_object(&priv->usb_device_manager);<br>
> <br>
> /* This also closes the libusb handle we passed from open_device */<br>
> - usbredirhost_set_device(priv->host, NULL);<br>
> - libusb_unref_device(priv->device);<br>
> - priv->device = NULL;<br>
> + spice_usb_backend_channel_detach(priv->host);<br>
> + g_clear_pointer(&priv->device, spice_usb_backend_device_release);<br>
> g_boxed_free(spice_usb_device_get_type(), priv->spice_device);<br>
> priv->spice_device = NULL;<br>
> priv->state = STATE_DISCONNECTED;<br>
> @@ -558,7 +538,7 @@ spice_usbredir_channel_get_spice_usb_device(SpiceUsbredirChannel *channel)<br>
> #endif<br>
> <br>
> G_GNUC_INTERNAL<br>
> -libusb_device *spice_usbredir_channel_get_device(SpiceUsbredirChannel *channel)<br>
> +SpiceUsbBackendDevice *spice_usbredir_channel_get_device(SpiceUsbredirChannel *channel)<br>
> {<br>
> return channel->priv->device;<br>
> }<br>
> @@ -573,85 +553,40 @@ void spice_usbredir_channel_get_guest_filter(<br>
> <br>
> g_return_if_fail(priv->host != NULL);<br>
> <br>
> - usbredirhost_get_guest_filter(priv->host, rules_ret, rules_count_ret);<br>
> + spice_usb_backend_channel_get_guest_filter(priv->host, rules_ret, rules_count_ret);<br>
> }<br>
> <br>
> /* ------------------------------------------------------------------ */<br>
> /* callbacks (any context) */<br>
> <br>
> -#if USBREDIR_VERSION >= 0x000701<br>
> -static uint64_t usbredir_buffered_output_size_callback(void *user_data)<br>
> +static uint64_t usbredir_get_queue_size(void *user_data)<br>
> {<br>
> g_return_val_if_fail(SPICE_IS_USBREDIR_CHANNEL(user_data), 0);<br>
> return spice_channel_get_queue_size(SPICE_CHANNEL(user_data));<br>
> }<br>
> -#endif<br>
> <br>
> -/* Note that this function must be re-entrant safe, as it can get called<br>
> - from both the main thread as well as from the usb event handling thread */<br>
> -static void usbredir_write_flush_callback(void *user_data)<br>
> +static gboolean usbredir_is_channel_ready(void *user_data)<br>
> {<br>
> SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(user_data);<br>
> SpiceUsbredirChannelPrivate *priv = channel->priv;<br>
> -<br>
> - if (spice_channel_get_state(SPICE_CHANNEL(channel)) !=<br>
> - SPICE_CHANNEL_STATE_READY)<br>
> - return;<br>
> -<br>
> + if (spice_channel_get_state(SPICE_CHANNEL(channel)) != SPICE_CHANNEL_STATE_READY)<br>
> + return FALSE;<br>
> if (!priv->host)<br>
> - return;<br>
> -<br>
> - usbredirhost_write_guest_data(priv->host);<br>
> -}<br>
> -<br>
> -static void usbredir_log(void *user_data, int level, const char *msg)<br>
> -{<br>
> - SpiceUsbredirChannel *channel = user_data;<br>
> - SpiceUsbredirChannelPrivate *priv = channel->priv;<br>
> -<br>
> - if (priv->catch_error && level == usbredirparser_error) {<br>
> - CHANNEL_DEBUG(channel, "%s", msg);<br>
> - /* Remove "usbredirhost: " prefix from usbredirhost messages */<br>
> - if (strncmp(msg, "usbredirhost: ", 14) == 0)<br>
> - g_set_error_literal(priv->catch_error, SPICE_CLIENT_ERROR,<br>
> - SPICE_CLIENT_ERROR_FAILED, msg + 14);<br>
> - else<br>
> - g_set_error_literal(priv->catch_error, SPICE_CLIENT_ERROR,<br>
> - SPICE_CLIENT_ERROR_FAILED, msg);<br>
> - return;<br>
> - }<br>
> + return FALSE;<br>
> <br>
> - switch (level) {<br>
> - case usbredirparser_error:<br>
> - g_critical("%s", msg);<br>
> - break;<br>
> - case usbredirparser_warning:<br>
> - g_warning("%s", msg);<br>
> - break;<br>
> - default:<br>
> - CHANNEL_DEBUG(channel, "%s", msg);<br>
> - }<br>
> + return TRUE;<br>
> }<br>
> <br>
> -static int usbredir_read_callback(void *user_data, uint8_t *data, int count)<br>
> +static void usbredir_log(void *user_data, const char *msg, gboolean error)<br>
> {<br>
> SpiceUsbredirChannel *channel = user_data;<br>
> SpiceUsbredirChannelPrivate *priv = channel->priv;<br>
> <br>
> - count = MIN(priv->read_buf_size, count);<br>
> -<br>
> - if (count != 0) {<br>
> - memcpy(data, priv->read_buf, count);<br>
> + if (priv->catch_error && error) {<br>
> + g_set_error_literal(priv->catch_error, SPICE_CLIENT_ERROR,<br>
> + SPICE_CLIENT_ERROR_FAILED, msg);<br>
> + priv->catch_error = NULL;<br>
<br>
It's odd to set priv->catch_error and set it to NULL right after setting<br>
it.<br>
<br>
> }<br>
> -<br>
> - priv->read_buf_size -= count;<br>
> - if (priv->read_buf_size) {<br>
> - priv->read_buf += count;<br>
> - } else {<br>
> - priv->read_buf = NULL;<br>
> - }<br>
> -<br>
> - return count;<br>
> }<br>
> <br>
> static void usbredir_free_write_cb_data(uint8_t *data, void *user_data)<br>
> @@ -659,7 +594,7 @@ static void usbredir_free_write_cb_data(uint8_t *data, void *user_data)<br>
> SpiceUsbredirChannel *channel = user_data;<br>
> SpiceUsbredirChannelPrivate *priv = channel->priv;<br>
> <br>
> - usbredirhost_free_write_buffer(priv->host, data);<br>
> + spice_usb_backend_return_write_data(priv->host, data);<br>
> }<br>
> <br>
> #ifdef USE_LZ4<br>
> @@ -731,7 +666,7 @@ static int usbredir_write_callback(void *user_data, uint8_t *data, int count)<br>
> <br>
> #ifdef USE_LZ4<br>
> if (try_write_compress_LZ4(channel, data, count)) {<br>
> - usbredirhost_free_write_buffer(channel->priv->host, data);<br>
> + spice_usb_backend_return_write_data(channel->priv->host, data);<br>
> return count;<br>
> }<br>
> #endif<br>
> @@ -744,15 +679,6 @@ static int usbredir_write_callback(void *user_data, uint8_t *data, int count)<br>
> return count;<br>
> }<br>
> <br>
> -static void *usbredir_alloc_lock(void) {<br>
> - GMutex *mutex;<br>
> -<br>
> - mutex = g_new0(GMutex, 1);<br>
> - g_mutex_init(mutex);<br>
> -<br>
> - return mutex;<br>
> -}<br>
> -<br>
> G_GNUC_INTERNAL<br>
> void spice_usbredir_channel_lock(SpiceUsbredirChannel *channel)<br>
> {<br>
> @@ -765,25 +691,6 @@ void spice_usbredir_channel_unlock(SpiceUsbredirChannel *channel)<br>
> g_mutex_unlock(&channel->priv->device_connect_mutex);<br>
> }<br>
> <br>
> -static void usbredir_lock_lock(void *user_data) {<br>
> - GMutex *mutex = user_data;<br>
> -<br>
> - g_mutex_lock(mutex);<br>
> -}<br>
> -<br>
> -static void usbredir_unlock_lock(void *user_data) {<br>
> - GMutex *mutex = user_data;<br>
> -<br>
> - g_mutex_unlock(mutex);<br>
> -}<br>
> -<br>
> -static void usbredir_free_lock(void *user_data) {<br>
> - GMutex *mutex = user_data;<br>
> -<br>
> - g_mutex_clear(mutex);<br>
> - g_free(mutex);<br>
> -}<br>
> -<br>
> /* --------------------------------------------------------------------- */<br>
> <br>
> typedef struct device_error_data {<br>
> @@ -819,10 +726,14 @@ static void spice_usbredir_channel_up(SpiceChannel *c)<br>
> {<br>
> SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(c);<br>
> SpiceUsbredirChannelPrivate *priv = channel->priv;<br>
> + SpiceSession *session = spice_channel_get_session(c);<br>
> + SpiceUsbDeviceManager *manager = spice_usb_device_manager_get(session, NULL);<br>
> <br>
> g_return_if_fail(priv->host != NULL);<br>
> /* Flush any pending writes */<br>
> - usbredirhost_write_guest_data(priv->host);<br>
> + spice_usb_backend_channel_up(priv->host);<br>
> + /* Check which existing device can be redirected right now */<br>
> + spice_usb_device_manager_check_redir_on_connect(manager, c);<br>
> }<br>
> <br>
> static int try_handle_compressed_msg(SpiceMsgCompressedData *compressed_data_msg,<br>
> @@ -872,26 +783,20 @@ static void usbredir_handle_msg(SpiceChannel *c, SpiceMsgIn *in)<br>
> <br>
> g_return_if_fail(priv->host != NULL);<br>
> <br>
> - /* No recursion allowed! */<br>
> - g_return_if_fail(priv->read_buf == NULL);<br>
> -<br>
> if (spice_msg_in_type(in) == SPICE_MSG_SPICEVMC_COMPRESSED_DATA) {<br>
> SpiceMsgCompressedData *compressed_data_msg = spice_msg_in_parsed(in);<br>
> if (try_handle_compressed_msg(compressed_data_msg, &buf, &size)) {<br>
> - priv->read_buf_size = size;<br>
> - priv->read_buf = buf;<br>
> + /* uncompressed ok*/<br>
> } else {<br>
> - r = usbredirhost_read_parse_error;<br>
> + r = USB_REDIR_ERROR_READ_PARSE;<br>
> }<br>
> } else { /* Regular SPICE_MSG_SPICEVMC_DATA msg */<br>
> buf = spice_msg_in_raw(in, &size);<br>
> - priv->read_buf_size = size;<br>
> - priv->read_buf = buf;<br>
> }<br>
> <br>
> spice_usbredir_channel_lock(channel);<br>
> if (r == 0)<br>
> - r = usbredirhost_read_guest_data(priv->host);<br>
> + r = spice_usb_backend_provide_read_data(priv->host, buf, size);<br>
> if (r != 0) {<br>
> SpiceUsbDevice *spice_device = priv->spice_device;<br>
> device_error_data err_data;<br>
> @@ -905,16 +810,16 @@ static void usbredir_handle_msg(SpiceChannel *c, SpiceMsgIn *in)<br>
> <br>
> desc = spice_usb_device_get_description(spice_device, NULL);<br>
> switch (r) {<br>
> - case usbredirhost_read_parse_error:<br>
> + case USB_REDIR_ERROR_READ_PARSE:<br>
> err = g_error_new(SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,<br>
> _("usbredir protocol parse error for %s"), desc);<br>
> break;<br>
> - case usbredirhost_read_device_rejected:<br>
> + case USB_REDIR_ERROR_DEV_REJECTED:<br>
> err = g_error_new(SPICE_CLIENT_ERROR,<br>
> SPICE_CLIENT_ERROR_USB_DEVICE_REJECTED,<br>
> _("%s rejected by host"), desc);<br>
> break;<br>
> - case usbredirhost_read_device_lost:<br>
> + case USB_REDIR_ERROR_DEV_LOST:<br>
> err = g_error_new(SPICE_CLIENT_ERROR,<br>
> SPICE_CLIENT_ERROR_USB_DEVICE_LOST,<br>
> _("%s disconnected (fatal IO error)"), desc);<br>
> diff --git a/src/meson.build b/src/meson.build<br>
> index 8c9199e..2870102 100644<br>
> --- a/src/meson.build<br>
> +++ b/src/meson.build<br>
> @@ -78,6 +78,7 @@ spice_client_glib_introspection_sources = [<br>
> 'spice-session.c',<br>
> 'spice-util.c',<br>
> 'usb-device-manager.c',<br>
> + 'usb-backend-common.c',<br>
<br>
I think you also need usb-backend.h in this file.<br></blockquote><div><br></div><div>'backend.h' is not public. If it is included into 'introspection' it seems that this requires inline documentation for some small subset of procedures (not sure how they are selected).</div><div>What is the documenting policy for non-piblic headers?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> ]<br>
> <br>
> spice_client_glib_sources = [<br>
> diff --git a/src/usb-device-manager-priv.h b/src/usb-device-manager-priv.h<br>
> index 83884d7..53149fb 100644<br>
> --- a/src/usb-device-manager-priv.h<br>
> +++ b/src/usb-device-manager-priv.h<br>
> @@ -32,9 +32,12 @@ void spice_usb_device_manager_stop_event_listening(<br>
> SpiceUsbDeviceManager *manager);<br>
> <br>
> #ifdef USE_USBREDIR<br>
> -#include <libusb.h><br>
> +#include "usb-backend.h"<br>
> void spice_usb_device_manager_device_error(<br>
> SpiceUsbDeviceManager *manager, SpiceUsbDevice *device, GError *err);<br>
> +void spice_usb_device_manager_check_redir_on_connect(<br>
> + SpiceUsbDeviceManager *manager, SpiceChannel *channel);<br>
> +<br>
> <br>
> guint8 spice_usb_device_get_busnum(const SpiceUsbDevice *device);<br>
> guint8 spice_usb_device_get_devaddr(const SpiceUsbDevice *device);<br>
> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c<br>
> index 50fb491..2b6c049 100644<br>
> --- a/src/usb-device-manager.c<br>
> +++ b/src/usb-device-manager.c<br>
> @@ -24,10 +24,11 @@<br>
> #include <glib-object.h><br>
> <br>
> #ifdef USE_USBREDIR<br>
> +<br>
> #include <errno.h><br>
> -#include <libusb.h><br>
> <br>
> #ifdef G_OS_WIN32<br>
> +#include <windows.h><br>
> #include "usbdk_api.h"<br>
> #endif<br>
> <br>
> @@ -41,8 +42,8 @@<br>
> #endif<br>
> <br>
> #include "channel-usbredir-priv.h"<br>
> -#include "usbredirhost.h"<br>
> #include "usbutil.h"<br>
> +<br>
> #endif<br>
> <br>
> #include "spice-session-priv.h"<br>
> @@ -102,7 +103,7 @@ struct _SpiceUsbDeviceManagerPrivate {<br>
> gchar *auto_connect_filter;<br>
> gchar *redirect_on_connect;<br>
> #ifdef USE_USBREDIR<br>
> - libusb_context *context;<br>
> + SpiceUsbBackend *context;<br>
> int event_listeners;<br>
> GThread *event_thread;<br>
> gint event_thread_run;<br>
> @@ -112,10 +113,9 @@ struct _SpiceUsbDeviceManagerPrivate {<br>
> int redirect_on_connect_rules_count;<br>
> #ifdef USE_GUDEV<br>
> GUdevClient *udev;<br>
> - libusb_device **coldplug_list; /* Avoid needless reprobing during init */<br>
> + SpiceUsbBackendDevice **coldplug_list; /* Avoid needless reprobing during init */<br>
> #else<br>
> gboolean redirecting; /* Handled by GUdevClient in the gudev case */<br>
> - libusb_hotplug_callback_handle hp_handle;<br>
> #endif<br>
> #ifdef G_OS_WIN32<br>
> usbdk_api_wrapper *usbdk_api;<br>
> @@ -139,6 +139,7 @@ enum {<br>
> <br>
> #ifdef USE_USBREDIR<br>
> <br>
> +// this is the structure behind SpiceUsbDevice<br>
<br>
Maybe worth renaming that struct rather than just adding a comment?<br>
<br>
> typedef struct _SpiceUsbDeviceInfo {<br>
> guint8 busnum;<br>
> guint8 devaddr;<br>
> @@ -148,7 +149,7 @@ typedef struct _SpiceUsbDeviceInfo {<br>
> #ifdef G_OS_WIN32<br>
> guint8 state;<br>
> #else<br>
> - libusb_device *libdev;<br>
> + SpiceUsbBackendDevice *bdev;<br>
> #endif<br>
> gint ref;<br>
> } SpiceUsbDeviceInfo;<br>
> @@ -166,15 +167,13 @@ static void spice_usb_device_manager_uevent_cb(GUdevClient *client,<br>
> static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager *self,<br>
> GUdevDevice *udev);<br>
> #else<br>
> -static int spice_usb_device_manager_hotplug_cb(libusb_context *ctx,<br>
> - libusb_device *device,<br>
> - libusb_hotplug_event event,<br>
> - void *data);<br>
> +static void spice_usb_device_manager_hotplug_cb(<br>
> + void *data,<br>
> + SpiceUsbBackendDevice *bdev,<br>
> + gboolean added);<br>
<br>
Indentation<br>
<br>
> #endif<br>
> -static void spice_usb_device_manager_check_redir_on_connect(<br>
> - SpiceUsbDeviceManager *self, SpiceChannel *channel);<br>
> <br>
> -static SpiceUsbDeviceInfo *spice_usb_device_new(libusb_device *libdev);<br>
> +static SpiceUsbDeviceInfo *spice_usb_device_new(SpiceUsbBackendDevice *bdev);<br>
> static SpiceUsbDevice *spice_usb_device_ref(SpiceUsbDevice *device);<br>
> static void spice_usb_device_unref(SpiceUsbDevice *device);<br>
> <br>
> @@ -183,11 +182,11 @@ static void _usbdk_hider_update(SpiceUsbDeviceManager *manager);<br>
> static void _usbdk_hider_clear(SpiceUsbDeviceManager *manager);<br>
> #endif<br>
> <br>
> -static gboolean spice_usb_manager_device_equal_libdev(SpiceUsbDeviceManager *manager,<br>
> +static gboolean spice_usb_manager_device_equal_bdev(SpiceUsbDeviceManager *manager,<br>
> SpiceUsbDevice *device,<br>
> - libusb_device *libdev);<br>
> -static libusb_device *<br>
> -spice_usb_device_manager_device_to_libdev(SpiceUsbDeviceManager *self,<br>
> + SpiceUsbBackendDevice *bdev);<br>
> +static SpiceUsbBackendDevice*<br>
> +spice_usb_device_manager_device_to_bdev(SpiceUsbDeviceManager *self,<br>
> SpiceUsbDevice *device);<br>
<br>
Indentation seems a bit off here too.<br>
<br>
> <br>
> static void<br>
> @@ -288,27 +287,15 @@ static gboolean spice_usb_device_manager_initable_init(GInitable *initable,<br>
> SpiceUsbDeviceManagerPrivate *priv = self->priv;<br>
> GList *list;<br>
> GList *it;<br>
> - int rc;<br>
> #ifdef USE_GUDEV<br>
> const gchar *const subsystems[] = {"usb", NULL};<br>
> #endif<br>
> <br>
> - /* Initialize libusb */<br>
> - rc = libusb_init(&priv->context);<br>
> - if (rc < 0) {<br>
> - const char *desc = spice_usbutil_libusb_strerror(rc);<br>
> - g_warning("Error initializing USB support: %s [%i]", desc, rc);<br>
> - g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,<br>
> - "Error initializing USB support: %s [%i]", desc, rc);<br>
> + /* Initialize spice backend */<br>
> + priv->context = spice_usb_backend_initialize();<br>
> + if (!priv->context) {<br>
<br>
This was returning a GError before.<br>
<br>
> return FALSE;<br>
> }<br>
> -<br>
> -#ifdef G_OS_WIN32<br>
> -#if LIBUSB_API_VERSION >= 0x01000106<br>
> - libusb_set_option(priv->context, LIBUSB_OPTION_USE_USBDK);<br>
> -#endif<br>
> -#endif<br>
> -<br>
> /* Start listening for usb devices plug / unplug */<br>
> #ifdef USE_GUDEV<br>
> priv->udev = g_udev_client_new(subsystems);<br>
> @@ -319,26 +306,20 @@ static gboolean spice_usb_device_manager_initable_init(GInitable *initable,<br>
> g_signal_connect(G_OBJECT(priv->udev), "uevent",<br>
> G_CALLBACK(spice_usb_device_manager_uevent_cb), self);<br>
> /* Do coldplug (detection of already connected devices) */<br>
> - libusb_get_device_list(priv->context, &priv->coldplug_list);<br>
> + priv->coldplug_list = spice_usb_backend_get_device_list(priv->context);<br>
> list = g_udev_client_query_by_subsystem(priv->udev, "usb");<br>
> for (it = g_list_first(list); it; it = g_list_next(it)) {<br>
> spice_usb_device_manager_add_udev(self, it->data);<br>
> g_object_unref(it->data);<br>
> }<br>
> g_list_free(list);<br>
> - libusb_free_device_list(priv->coldplug_list, 1);<br>
> + spice_usb_backend_free_device_list(priv->coldplug_list);<br>
> priv->coldplug_list = NULL;<br>
> #else<br>
> - rc = libusb_hotplug_register_callback(priv->context,<br>
> - LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED | LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT,<br>
> - LIBUSB_HOTPLUG_ENUMERATE, LIBUSB_HOTPLUG_MATCH_ANY,<br>
> - LIBUSB_HOTPLUG_MATCH_ANY, LIBUSB_HOTPLUG_MATCH_ANY,<br>
> - spice_usb_device_manager_hotplug_cb, self, &priv->hp_handle);<br>
> - if (rc < 0) {<br>
> - const char *desc = spice_usbutil_libusb_strerror(rc);<br>
> - g_warning("Error initializing USB hotplug support: %s [%i]", desc, rc);<br>
> + if (!spice_usb_backend_handle_hotplug(priv->context,<br>
> + self, spice_usb_device_manager_hotplug_cb)) {<br>
> g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,<br>
> - "Error initializing USB hotplug support: %s [%i]", desc, rc);<br>
> + "Error initializing USB hotplug support");<br>
<br>
Indentation<br>
<br>
> return FALSE;<br>
> }<br>
> spice_usb_device_manager_start_event_listening(self, NULL);<br>
> @@ -369,20 +350,18 @@ static void spice_usb_device_manager_dispose(GObject *gobject)<br>
> SpiceUsbDeviceManagerPrivate *priv = self->priv;<br>
> <br>
> #ifdef USE_LIBUSB_HOTPLUG<br>
> - if (priv->hp_handle) {<br>
> - spice_usb_device_manager_stop_event_listening(self);<br>
> - if (g_atomic_int_get(&priv->event_thread_run)) {<br>
> - /* Force termination of the event thread even if there were some<br>
> - * mismatched spice_usb_device_manager_{start,stop}_event_listening<br>
> - * calls. Otherwise, the usb event thread will be leaked, and will<br>
> - * try to use the libusb context we destroy in finalize(), which would<br>
> - * cause a crash */<br>
> - g_warn_if_reached();<br>
> - g_atomic_int_set(&priv->event_thread_run, FALSE);<br>
> - }<br>
> - /* This also wakes up the libusb_handle_events() in the event_thread */<br>
> - libusb_hotplug_deregister_callback(priv->context, priv->hp_handle);<br>
> - priv->hp_handle = 0;<br>
> + spice_usb_device_manager_stop_event_listening(self);<br>
> + if (g_atomic_int_get(&priv->event_thread_run)) {<br>
> + /* Force termination of the event thread even if there were some<br>
> + * mismatched spice_usb_device_manager_{start,stop}_event_listening<br>
> + * calls. Otherwise, the usb event thread will be leaked, and will<br>
> + * try to use the libusb context we destroy in finalize(), which would<br>
> + * cause a crash */<br>
> + g_warn_if_reached();<br>
> + g_atomic_int_set(&priv->event_thread_run, FALSE);<br>
> +<br>
> + /* This also wakes up the libusb_handle_events() in the event_thread */<br>
> + spice_usb_backend_handle_hotplug(priv->context, NULL, NULL);<br>
> }<br>
> #endif<br>
> if (priv->event_thread) {<br>
> @@ -411,8 +390,9 @@ static void spice_usb_device_manager_finalize(GObject *gobject)<br>
> g_clear_object(&priv->udev);<br>
> #endif<br>
> g_return_if_fail(priv->event_thread == NULL);<br>
> - if (priv->context)<br>
> - libusb_exit(priv->context);<br>
> + if (priv->context) {<br>
> + spice_usb_backend_finalize(priv->context);<br>
> + }<br>
> free(priv->auto_conn_filter_rules);<br>
> free(priv->redirect_on_connect_rules);<br>
> #ifdef G_OS_WIN32<br>
> @@ -737,7 +717,7 @@ static void spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas<br>
> #ifdef USE_USBREDIR<br>
> <br>
> /* ------------------------------------------------------------------ */<br>
> -/* gudev / libusb Helper functions */<br>
> +/* gudev / backend Helper functions */<br>
> <br>
> #ifdef USE_GUDEV<br>
> static gboolean spice_usb_device_manager_get_udev_bus_n_address(<br>
> @@ -761,40 +741,16 @@ static gboolean spice_usb_device_manager_get_udev_bus_n_address(<br>
> }<br>
> #endif<br>
> <br>
> -static gboolean spice_usb_device_manager_get_device_descriptor(<br>
> - libusb_device *libdev,<br>
> - struct libusb_device_descriptor *desc)<br>
> -{<br>
> - int errcode;<br>
> - const gchar *errstr;<br>
> -<br>
> - g_return_val_if_fail(libdev != NULL, FALSE);<br>
> - g_return_val_if_fail(desc != NULL, FALSE);<br>
> -<br>
> - errcode = libusb_get_device_descriptor(libdev, desc);<br>
> - if (errcode < 0) {<br>
> - int bus, addr;<br>
> -<br>
> - bus = libusb_get_bus_number(libdev);<br>
> - addr = libusb_get_device_address(libdev);<br>
> - errstr = spice_usbutil_libusb_strerror(errcode);<br>
> - g_warning("cannot get device descriptor for (%p) %d.%d -- %s(%d)",<br>
> - libdev, bus, addr, errstr, errcode);<br>
> - return FALSE;<br>
> - }<br>
> - return TRUE;<br>
> -}<br>
> -<br>
> #endif // USE_USBREDIR<br>
> <br>
> /**<br>
> * spice_usb_device_get_libusb_device:<br>
> - * @device: #SpiceUsbDevice to get the descriptor information of<br>
> + * @device: #SpiceUsbDevice to get the libusb device of (if exists)<br>
> *<br>
> * Finds the %libusb_device associated with the @device.<br>
> *<br>
> - * Returns: (transfer none): the %libusb_device associated to %SpiceUsbDevice.<br>
> - *<br>
> + * Returns: (transfer none): the %libusb_device associated to %SpiceUsbDevice<br>
> + * or NULL (if the device does not have associated libusb device)<br>
<br>
This is an exported function, and if we start returning NULL in some<br>
cases, this is going to break applications using this API :(<br>
<br>
> * Since: 0.27<br>
> **/<br>
> gconstpointer<br>
> @@ -806,34 +762,13 @@ spice_usb_device_get_libusb_device(const SpiceUsbDevice *device G_GNUC_UNUSED)<br>
> <br>
> g_return_val_if_fail(info != NULL, FALSE);<br>
> <br>
> - return info->libdev;<br>
> + return spice_usb_backend_device_get_libdev(info->bdev);<br>
> #endif<br>
> #endif<br>
> return NULL;<br>
> }<br>
> <br>
> #ifdef USE_USBREDIR<br>
> -static gboolean spice_usb_device_manager_get_libdev_vid_pid(<br>
> - libusb_device *libdev, int *vid, int *pid)<br>
> -{<br>
> - struct libusb_device_descriptor desc;<br>
> -<br>
> - g_return_val_if_fail(libdev != NULL, FALSE);<br>
> - g_return_val_if_fail(vid != NULL, FALSE);<br>
> - g_return_val_if_fail(pid != NULL, FALSE);<br>
> -<br>
> - *vid = *pid = 0;<br>
> -<br>
> - if (!spice_usb_device_manager_get_device_descriptor(libdev, &desc)) {<br>
> - return FALSE;<br>
> - }<br>
> - *vid = desc.idVendor;<br>
> - *pid = desc.idProduct;<br>
> -<br>
> - return TRUE;<br>
> -}<br>
> -<br>
> -/* ------------------------------------------------------------------ */<br>
> /* callbacks */<br>
> <br>
> static void channel_new(SpiceSession *session, SpiceChannel *channel,<br>
> @@ -849,10 +784,8 @@ static void channel_new(SpiceSession *session, SpiceChannel *channel,<br>
> spice_channel_connect(channel);<br>
> g_ptr_array_add(self->priv->channels, channel);<br>
> <br>
> - spice_usb_device_manager_check_redir_on_connect(self, channel);<br>
> -<br>
<br>
Not clear why thisis removed?<br>
<br>
> /*<br>
> - * add a reference to ourself, to make sure the libusb context is<br>
> + * add a reference to ourself, to make sure the backend device context is<br>
> * alive as long as the channel is.<br>
> * TODO: moving to gusb could help here too.<br>
> */<br>
> @@ -889,6 +822,7 @@ static void spice_usb_device_manager_auto_connect_cb(GObject *gobject,<br>
> g_signal_emit(self, signals[AUTO_CONNECT_FAILED], 0, device, err);<br>
> g_error_free(err);<br>
> }<br>
> +<br>
> spice_usb_device_unref(device);<br>
<br>
Extra blank space<br>
<br>
> }<br>
> <br>
> @@ -902,12 +836,12 @@ spice_usb_device_manager_device_match(SpiceUsbDeviceManager *self, SpiceUsbDevic<br>
> <br>
> #ifdef USE_GUDEV<br>
> static gboolean<br>
> -spice_usb_device_manager_libdev_match(SpiceUsbDeviceManager *self, libusb_device *libdev,<br>
> +spice_usb_device_manager_bdev_match(SpiceUsbDeviceManager *self, SpiceUsbBackendDevice *dev,<br>
> const int bus, const int address)<br>
> {<br>
> + const UsbDeviceInformation* info = spice_usb_backend_device_get_info(dev);<br>
> /* match functions for Linux/UsbDk -- match by bus.addr */<br>
> - return (libusb_get_bus_number(libdev) == bus &&<br>
> - libusb_get_device_address(libdev) == address);<br>
> + return (info->bus == bus && info->address == address);<br>
> }<br>
> #endif<br>
> <br>
> @@ -929,36 +863,36 @@ spice_usb_device_manager_find_device(SpiceUsbDeviceManager *self,<br>
> return device;<br>
> }<br>
> <br>
> -static void spice_usb_device_manager_add_dev(SpiceUsbDeviceManager *self,<br>
> - libusb_device *libdev)<br>
> +static void spice_usb_device_manager_add_dev(<br>
> + SpiceUsbDeviceManager *self,<br>
> + SpiceUsbBackendDevice *bdev)<br>
<br>
Indentation<br>
<br>
> {<br>
> SpiceUsbDeviceManagerPrivate *priv = self->priv;<br>
> - struct libusb_device_descriptor desc;<br>
> SpiceUsbDevice *device;<br>
> -<br>
> - if (!spice_usb_device_manager_get_device_descriptor(libdev, &desc))<br>
> - return;<br>
> + const UsbDeviceInformation* info = spice_usb_backend_device_get_info(bdev);<br>
> + // try redirecting shared CD on creation, if filter allows<br>
> + gboolean always_redirect = info->max_luns != 0;<br>
<br>
Does not belong here<br>
> <br>
> /* Skip hubs */<br>
> - if (desc.bDeviceClass == LIBUSB_CLASS_HUB)<br>
> + if (spice_usb_backend_device_is_hub(bdev))<br>
> return;<br>
> <br>
> - device = (SpiceUsbDevice*)spice_usb_device_new(libdev);<br>
> + device = (SpiceUsbDevice*)spice_usb_device_new(bdev);<br>
> if (!device)<br>
> return;<br>
> <br>
> g_ptr_array_add(priv->devices, device);<br>
> <br>
> - if (priv->auto_connect) {<br>
> + if (priv->auto_connect || always_redirect) {<br>
> gboolean can_redirect, auto_ok;<br>
> <br>
> can_redirect = spice_usb_device_manager_can_redirect_device(<br>
> self, device, NULL);<br>
> <br>
> - auto_ok = usbredirhost_check_device_filter(<br>
> - priv->auto_conn_filter_rules,<br>
> - priv->auto_conn_filter_rules_count,<br>
> - libdev, 0) == 0;<br>
> + auto_ok = spice_usb_backend_device_check_filter(<br>
> + bdev,<br>
> + priv->auto_conn_filter_rules,<br>
> + priv->auto_conn_filter_rules_count) == 0;<br>
> <br>
> if (can_redirect && auto_ok)<br>
> spice_usb_device_manager_connect_device_async(self,<br>
> @@ -1005,7 +939,7 @@ static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager *self,<br>
> GUdevDevice *udev)<br>
> {<br>
> SpiceUsbDeviceManagerPrivate *priv = self->priv;<br>
> - libusb_device *libdev = NULL, **dev_list = NULL;<br>
> + SpiceUsbBackendDevice *devarrived = NULL, **dev_list = NULL;<br>
<br>
devarrived is a bit odd, why not bdev as in the other methods?<br>
<br>
> SpiceUsbDevice *device;<br>
> const gchar *devtype;<br>
> int i, bus, address;<br>
> @@ -1033,23 +967,23 @@ static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager *self,<br>
> if (priv->coldplug_list)<br>
> dev_list = priv->coldplug_list;<br>
> else<br>
> - libusb_get_device_list(priv->context, &dev_list);<br>
> + dev_list = spice_usb_backend_get_device_list(priv->context);<br>
> <br>
> for (i = 0; dev_list && dev_list[i]; i++) {<br>
> - if (spice_usb_device_manager_libdev_match(self, dev_list[i], bus, address)) {<br>
> - libdev = dev_list[i];<br>
> + if (spice_usb_device_manager_bdev_match(self, dev_list[i], bus, address)) {<br>
> + devarrived = dev_list[i];<br>
> break;<br>
> }<br>
> }<br>
> <br>
> - if (libdev)<br>
> - spice_usb_device_manager_add_dev(self, libdev);<br>
> + if (devarrived)<br>
> + spice_usb_device_manager_add_dev(self, devarrived);<br>
> else<br>
> g_warning("Could not find USB device to add " DEV_ID_FMT,<br>
> (guint) bus, (guint) address);<br>
> <br>
> if (!priv->coldplug_list)<br>
> - libusb_free_device_list(dev_list, 1);<br>
> + spice_usb_backend_free_device_list(dev_list);<br>
> }<br>
> <br>
> static void spice_usb_device_manager_remove_udev(SpiceUsbDeviceManager *self,<br>
> @@ -1078,8 +1012,8 @@ static void spice_usb_device_manager_uevent_cb(GUdevClient *client,<br>
> #else<br>
> struct hotplug_idle_cb_args {<br>
> SpiceUsbDeviceManager *self;<br>
> - libusb_device *device;<br>
> - libusb_hotplug_event event;<br>
> + SpiceUsbBackendDevice *device;<br>
> + gboolean device_added;<br>
> };<br>
> <br>
> static gboolean spice_usb_device_manager_hotplug_idle_cb(gpointer user_data)<br>
> @@ -1087,36 +1021,34 @@ static gboolean spice_usb_device_manager_hotplug_idle_cb(gpointer user_data)<br>
> struct hotplug_idle_cb_args *args = user_data;<br>
> SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(args->self);<br>
> <br>
> - switch (args->event) {<br>
> - case LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED:<br>
> + if (args->device_added) {<br>
> spice_usb_device_manager_add_dev(self, args->device);<br>
> - break;<br>
> - case LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT:<br>
> + } else {<br>
> + const UsbDeviceInformation *info = spice_usb_backend_device_get_info(args->device);<br>
> spice_usb_device_manager_remove_dev(self,<br>
> - libusb_get_bus_number(args->device),<br>
> - libusb_get_device_address(args->device));<br>
> - break;<br>
> + info->bus,<br>
> + info->address);<br>
<br>
Indentation<br>
<br>
> }<br>
> - libusb_unref_device(args->device);<br>
> + spice_usb_backend_device_release(args->device);<br>
> g_object_unref(self);<br>
> g_free(args);<br>
> return FALSE;<br>
> }<br>
> <br>
> /* Can be called from both the main-thread as well as the event_thread */<br>
> -static int spice_usb_device_manager_hotplug_cb(libusb_context *ctx,<br>
> - libusb_device *device,<br>
> - libusb_hotplug_event event,<br>
> - void *user_data)<br>
> +static void spice_usb_device_manager_hotplug_cb(<br>
> + void *user_data,<br>
> + SpiceUsbBackendDevice *bdev,<br>
> + gboolean added)<br>
<br>
Indentation<br>
<br>
> {<br>
> SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(user_data);<br>
> struct hotplug_idle_cb_args *args = g_malloc0(sizeof(*args));<br>
> <br>
> args->self = g_object_ref(self);<br>
> - args->device = libusb_ref_device(device);<br>
> - args->event = event;<br>
> + spice_usb_backend_device_acquire(bdev);<br>
> + args->device_added = added;<br>
> + args->device = bdev;<br>
> g_idle_add(spice_usb_device_manager_hotplug_idle_cb, args);<br>
> - return 0;<br>
> }<br>
> #endif // USE_USBREDIR<br>
> <br>
> @@ -1143,13 +1075,9 @@ static gpointer spice_usb_device_manager_usb_ev_thread(gpointer user_data)<br>
> {<br>
> SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(user_data);<br>
> SpiceUsbDeviceManagerPrivate *priv = self->priv;<br>
> - int rc;<br>
> <br>
> while (g_atomic_int_get(&priv->event_thread_run)) {<br>
> - rc = libusb_handle_events(priv->context);<br>
> - if (rc && rc != LIBUSB_ERROR_INTERRUPTED) {<br>
> - const char *desc = spice_usbutil_libusb_strerror(rc);<br>
> - g_warning("Error handling USB events: %s [%i]", desc, rc);<br>
> + if (!spice_usb_backend_handle_events(priv->context)) {<br>
> break;<br>
> }<br>
> }<br>
> @@ -1194,13 +1122,13 @@ void spice_usb_device_manager_stop_event_listening(<br>
> g_atomic_int_set(&priv->event_thread_run, FALSE);<br>
> }<br>
> <br>
> -static void spice_usb_device_manager_check_redir_on_connect(<br>
> +void spice_usb_device_manager_check_redir_on_connect(<br>
<br>
Belongs in a different commit.<br>
<br>
> SpiceUsbDeviceManager *self, SpiceChannel *channel)<br>
> {<br>
> SpiceUsbDeviceManagerPrivate *priv = self->priv;<br>
> GTask *task;<br>
> SpiceUsbDevice *device;<br>
> - libusb_device *libdev;<br>
> + SpiceUsbBackendDevice *dev;<br>
> guint i;<br>
> <br>
> if (priv->redirect_on_connect == NULL)<br>
> @@ -1212,15 +1140,15 @@ static void spice_usb_device_manager_check_redir_on_connect(<br>
> if (spice_usb_device_manager_is_device_connected(self, device))<br>
> continue;<br>
> <br>
> - libdev = spice_usb_device_manager_device_to_libdev(self, device);<br>
> + dev = spice_usb_device_manager_device_to_bdev(self, device);<br>
> #ifdef G_OS_WIN32<br>
> - if (libdev == NULL)<br>
> + if (dev == NULL)<br>
> continue;<br>
> #endif<br>
> - if (usbredirhost_check_device_filter(<br>
> - priv->redirect_on_connect_rules,<br>
> - priv->redirect_on_connect_rules_count,<br>
> - libdev, 0) == 0) {<br>
> + if (spice_usb_backend_device_check_filter(<br>
> + dev,<br>
> + priv->redirect_on_connect_rules,<br>
> + priv->redirect_on_connect_rules_count) == 0) {<br>
> /* Note: re-uses spice_usb_device_manager_connect_device_async's<br>
> completion handling code! */<br>
> task = g_task_new(self,<br>
> @@ -1230,14 +1158,14 @@ static void spice_usb_device_manager_check_redir_on_connect(<br>
> <br>
> spice_usbredir_channel_connect_device_async(<br>
> SPICE_USBREDIR_CHANNEL(channel),<br>
> - libdev, device, NULL,<br>
> + dev, device, NULL,<br>
> spice_usb_device_manager_channel_connect_cb,<br>
> task);<br>
> - libusb_unref_device(libdev);<br>
> + spice_usb_backend_device_release(dev);<br>
> return; /* We've taken the channel! */<br>
> }<br>
> <br>
> - libusb_unref_device(libdev);<br>
> + spice_usb_backend_device_release(dev);<br>
> }<br>
> }<br>
> <br>
> @@ -1261,8 +1189,8 @@ static SpiceUsbredirChannel *spice_usb_device_manager_get_channel_for_dev(<br>
> for (i = 0; i < priv->channels->len; i++) {<br>
> SpiceUsbredirChannel *channel = g_ptr_array_index(priv->channels, i);<br>
> spice_usbredir_channel_lock(channel);<br>
> - libusb_device *libdev = spice_usbredir_channel_get_device(channel);<br>
> - if (spice_usb_manager_device_equal_libdev(manager, device, libdev)) {<br>
> + SpiceUsbBackendDevice *dev = spice_usbredir_channel_get_device(channel);<br>
> + if (spice_usb_manager_device_equal_bdev(manager, device, dev)) {<br>
> spice_usbredir_channel_unlock(channel);<br>
> return channel;<br>
> }<br>
> @@ -1319,13 +1247,13 @@ GPtrArray* spice_usb_device_manager_get_devices_with_filter(<br>
> SpiceUsbDevice *device = g_ptr_array_index(priv->devices, i);<br>
> <br>
> if (rules) {<br>
> - libusb_device *libdev =<br>
> - spice_usb_device_manager_device_to_libdev(self, device);<br>
> + SpiceUsbBackendDevice *bdev =<br>
> + spice_usb_device_manager_device_to_bdev(self, device);<br>
> #ifdef G_OS_WIN32<br>
> - if (libdev == NULL)<br>
> + if (bdev == NULL)<br>
> continue;<br>
> #endif<br>
> - if (usbredirhost_check_device_filter(rules, count, libdev, 0) != 0)<br>
> + if (spice_usb_backend_device_check_filter(bdev, rules, count) != 0)<br>
> continue;<br>
> }<br>
> g_ptr_array_add(devices_copy, spice_usb_device_ref(device));<br>
> @@ -1399,7 +1327,7 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,<br>
> task = g_task_new(self, cancellable, callback, user_data);<br>
> <br>
> SpiceUsbDeviceManagerPrivate *priv = self->priv;<br>
> - libusb_device *libdev;<br>
> + SpiceUsbBackendDevice *bdev;<br>
> guint i;<br>
> <br>
> if (spice_usb_device_manager_is_device_connected(self, device)) {<br>
> @@ -1415,9 +1343,9 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,<br>
> if (spice_usbredir_channel_get_device(channel))<br>
> continue; /* Skip already used channels */<br>
> <br>
> - libdev = spice_usb_device_manager_device_to_libdev(self, device);<br>
> + bdev = spice_usb_device_manager_device_to_bdev(self, device);<br>
> #ifdef G_OS_WIN32<br>
> - if (libdev == NULL) {<br>
> + if (bdev == NULL) {<br>
> /* Most likely, the device was plugged out at driver installation<br>
> * time, and its remove-device event was ignored.<br>
> * So remove the device now<br>
> @@ -1435,12 +1363,12 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,<br>
> }<br>
> #endif<br>
> spice_usbredir_channel_connect_device_async(channel,<br>
> - libdev,<br>
> + bdev,<br>
> device,<br>
> cancellable,<br>
> spice_usb_device_manager_channel_connect_cb,<br>
> task);<br>
> - libusb_unref_device(libdev);<br>
> + spice_usb_backend_device_release(bdev);<br>
> return;<br>
> }<br>
> <br>
> @@ -1702,20 +1630,20 @@ spice_usb_device_manager_can_redirect_device(SpiceUsbDeviceManager *self,<br>
> <br>
> if (guest_filter_rules) {<br>
> gboolean filter_ok;<br>
> - libusb_device *libdev;<br>
> + SpiceUsbBackendDevice *bdev;<br>
> <br>
> - libdev = spice_usb_device_manager_device_to_libdev(self, device);<br>
> + bdev = spice_usb_device_manager_device_to_bdev(self, device);<br>
> #ifdef G_OS_WIN32<br>
> - if (libdev == NULL) {<br>
> + if (bdev == NULL) {<br>
> g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,<br>
> _("Some USB devices were not found"));<br>
> return FALSE;<br>
> }<br>
> #endif<br>
> - filter_ok = (usbredirhost_check_device_filter(<br>
> - guest_filter_rules, guest_filter_rules_count,<br>
> - libdev, 0) == 0);<br>
> - libusb_unref_device(libdev);<br>
> + filter_ok = (spice_usb_backend_device_check_filter(<br>
> + bdev,<br>
> + guest_filter_rules, guest_filter_rules_count) == 0);<br>
> + spice_usb_backend_device_release(bdev);<br>
> if (!filter_ok) {<br>
> g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,<br>
> _("Some USB devices are blocked by host policy"));<br>
> @@ -1789,7 +1717,7 @@ gchar *spice_usb_device_get_description(SpiceUsbDevice *device, const gchar *for<br>
> }<br>
> <br>
> spice_usb_util_get_device_strings(bus, address, vid, pid,<br>
> - &manufacturer, &product);<br>
> + &manufacturer, &product);<br>
<br>
Indentation<br>
<br>
> <br>
> if (!format)<br>
> format = _("%s %s %s at %d-%d");<br>
> @@ -1806,64 +1734,30 @@ gchar *spice_usb_device_get_description(SpiceUsbDevice *device, const gchar *for<br>
> #endif<br>
> }<br>
> <br>
> -<br>
> -<br>
> #ifdef USE_USBREDIR<br>
> -static gboolean probe_isochronous_endpoint(libusb_device *libdev)<br>
> -{<br>
> - struct libusb_config_descriptor *conf_desc;<br>
> - gboolean isoc_found = FALSE;<br>
> - gint i, j, k;<br>
> -<br>
> - g_return_val_if_fail(libdev != NULL, FALSE);<br>
> -<br>
> - if (libusb_get_active_config_descriptor(libdev, &conf_desc) != 0) {<br>
> - g_return_val_if_reached(FALSE);<br>
> - }<br>
> -<br>
> - for (i = 0; !isoc_found && i < conf_desc->bNumInterfaces; i++) {<br>
> - for (j = 0; !isoc_found && j < conf_desc->interface[i].num_altsetting; j++) {<br>
> - for (k = 0; !isoc_found && k < conf_desc->interface[i].altsetting[j].bNumEndpoints;k++) {<br>
> - gint attributes = conf_desc->interface[i].altsetting[j].endpoint[k].bmAttributes;<br>
> - gint type = attributes & LIBUSB_TRANSFER_TYPE_MASK;<br>
> - if (type == LIBUSB_TRANSFER_TYPE_ISOCHRONOUS)<br>
> - isoc_found = TRUE;<br>
> - }<br>
> - }<br>
> - }<br>
> -<br>
> - libusb_free_config_descriptor(conf_desc);<br>
> - return isoc_found;<br>
> -}<br>
> <br>
> /*<br>
> * SpiceUsbDeviceInfo<br>
> */<br>
> -static SpiceUsbDeviceInfo *spice_usb_device_new(libusb_device *libdev)<br>
> +static SpiceUsbDeviceInfo *spice_usb_device_new(SpiceUsbBackendDevice *bdev)<br>
> {<br>
> SpiceUsbDeviceInfo *info;<br>
> - int vid, pid;<br>
> - guint8 bus, addr;<br>
> -<br>
> - g_return_val_if_fail(libdev != NULL, NULL);<br>
> -<br>
> - bus = libusb_get_bus_number(libdev);<br>
> - addr = libusb_get_device_address(libdev);<br>
> + const UsbDeviceInformation *devinfo;<br>
> <br>
> - if (!spice_usb_device_manager_get_libdev_vid_pid(libdev, &vid, &pid)) {<br>
> - return NULL;<br>
> - }<br>
> + g_return_val_if_fail(bdev != NULL, NULL);<br>
> + devinfo = spice_usb_backend_device_get_info(bdev);<br>
> <br>
> info = g_new0(SpiceUsbDeviceInfo, 1);<br>
> <br>
> - info->busnum = bus;<br>
> - info->devaddr = addr;<br>
> - info->vid = vid;<br>
> - info->pid = pid;<br>
> + info->busnum = devinfo->bus;<br>
> + info->devaddr = devinfo->address;<br>
> + info->vid = devinfo->vid;<br>
> + info->pid = devinfo->pid;<br>
> info->ref = 1;<br>
> - info->isochronous = probe_isochronous_endpoint(libdev);<br>
> + info->isochronous = devinfo->isochronous;<br>
> #ifndef G_OS_WIN32<br>
> - info->libdev = libusb_ref_device(libdev);<br>
> + info->bdev = bdev;<br>
> + spice_usb_backend_device_acquire(bdev);<br>
> #endif<br>
> <br>
> return info;<br>
> @@ -2001,49 +1895,53 @@ static void spice_usb_device_unref(SpiceUsbDevice *device)<br>
> ref_count_is_0 = g_atomic_int_dec_and_test(&info->ref);<br>
> if (ref_count_is_0) {<br>
> #ifndef G_OS_WIN32<br>
> - libusb_unref_device(info->libdev);<br>
> + if (info->bdev) {<br>
> + spice_usb_backend_device_release(info->bdev);<br>
> + }<br>
> #endif<br>
> + info->vid = info->pid = 0;<br>
> + SPICE_DEBUG("%s: deleting %p", __FUNCTION__, info);<br>
> g_free(info);<br>
> }<br>
> }<br>
> <br>
> #ifndef G_OS_WIN32 /* Linux -- directly compare libdev */<br>
> static gboolean<br>
> -spice_usb_manager_device_equal_libdev(SpiceUsbDeviceManager *manager,<br>
> +spice_usb_manager_device_equal_bdev(SpiceUsbDeviceManager *manager,<br>
> SpiceUsbDevice *device,<br>
> - libusb_device *libdev)<br>
> + SpiceUsbBackendDevice *bdev)<br>
<br>
Indentation might need some little fixing here.<br>
<br>
> {<br>
> SpiceUsbDeviceInfo *info = (SpiceUsbDeviceInfo *)device;<br>
> <br>
> - if ((device == NULL) || (libdev == NULL))<br>
> + if ((device == NULL) || (bdev == NULL))<br>
> return FALSE;<br>
> <br>
> - return info->libdev == libdev;<br>
> + return spice_usb_backend_devices_same(info->bdev, bdev);<br>
> }<br>
> #else /* Windows -- compare vid:pid of device and libdev */<br>
> static gboolean<br>
> -spice_usb_manager_device_equal_libdev(SpiceUsbDeviceManager *manager,<br>
> - SpiceUsbDevice *device,<br>
> - libusb_device *libdev)<br>
> +spice_usb_manager_device_equal_bdev(SpiceUsbDeviceManager *manager,<br>
> + SpiceUsbDevice *device,<br>
> + SpiceUsbBackendDevice *bdev)<br>
> {<br>
> int busnum, devaddr;<br>
> <br>
> - if ((device == NULL) || (libdev == NULL))<br>
> + if ((device == NULL) || (bdev == NULL))<br>
> return FALSE;<br>
> <br></blockquote></div></div>