[Spice-devel] [spice-gtk v1 2/2] usb-redirection: use usb backend for usb redirection
Christophe Fergeau
cfergeau at redhat.com
Tue Sep 25 14:29:22 UTC 2018
On Mon, Sep 24, 2018 at 11:43:55AM +0300, Yuri Benditovich wrote:
> Replace all the communication with libusb and usbredirhost
> by usb backend API.
>
> Signed-off-by: Yuri Benditovich <yuri.benditovich at daynix.com>
> ---
> src/Makefile.am | 2 +
> src/channel-usbredir-priv.h | 9 +-
> src/channel-usbredir.c | 271 +++++++++-------------------
> src/meson.build | 1 +
> src/usb-device-manager-priv.h | 5 +-
> src/usb-device-manager.c | 407 ++++++++++++++++--------------------------
> src/usb-device-manager.h | 29 ++-
> src/usbutil.c | 36 ----
> src/usbutil.h | 2 -
> src/win-usb-dev.c | 59 +++---
> 10 files changed, 301 insertions(+), 520 deletions(-)
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 4dd657d..610dbba 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -249,6 +249,8 @@ libspice_client_glib_2_0_la_SOURCES = \
> spice-uri-priv.h \
> usb-device-manager.c \
> usb-device-manager-priv.h \
> + usb-backend.h \
> + usb-backend-common.c \
> usbutil.c \
> usbutil.h \
> $(USB_ACL_HELPER_SRCS) \
> diff --git a/src/channel-usbredir-priv.h b/src/channel-usbredir-priv.h
> index 17e9716..fee95f7 100644
> --- a/src/channel-usbredir-priv.h
> +++ b/src/channel-usbredir-priv.h
> @@ -21,9 +21,8 @@
> #ifndef __SPICE_CLIENT_USBREDIR_CHANNEL_PRIV_H__
> #define __SPICE_CLIENT_USBREDIR_CHANNEL_PRIV_H__
>
> -#include <libusb.h>
> -#include <usbredirfilter.h>
> #include "spice-client.h"
> +#include "usb-backend.h"
>
> G_BEGIN_DECLS
>
> @@ -31,7 +30,7 @@ G_BEGIN_DECLS
> context should not be destroyed before the last device has been
> disconnected */
> void spice_usbredir_channel_set_context(SpiceUsbredirChannel *channel,
> - libusb_context *context);
> + SpiceUsbBackend *context);
Args were aligned before, dunno if we want to preserve this or not.
>
> void spice_usbredir_channel_disconnect_device_async(SpiceUsbredirChannel *channel,
> GCancellable *cancellable,
> @@ -46,7 +45,7 @@ gboolean spice_usbredir_channel_disconnect_device_finish(SpiceUsbredirChannel *c
> (through spice_channel_connect()), before calling this. */
> void spice_usbredir_channel_connect_device_async(
> SpiceUsbredirChannel *channel,
> - libusb_device *device,
> + SpiceUsbBackendDevice *device,
> SpiceUsbDevice *spice_device,
> GCancellable *cancellable,
> GAsyncReadyCallback callback,
> @@ -58,7 +57,7 @@ gboolean spice_usbredir_channel_connect_device_finish(
>
> void spice_usbredir_channel_disconnect_device(SpiceUsbredirChannel *channel);
>
> -libusb_device *spice_usbredir_channel_get_device(SpiceUsbredirChannel *channel);
> +SpiceUsbBackendDevice *spice_usbredir_channel_get_device(SpiceUsbredirChannel *channel);
>
> void spice_usbredir_channel_lock(SpiceUsbredirChannel *channel);
>
> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
> index 1d9c380..67ba88a 100644
> --- a/src/channel-usbredir.c
> +++ b/src/channel-usbredir.c
> @@ -23,7 +23,6 @@
>
> #ifdef USE_USBREDIR
> #include <glib/gi18n-lib.h>
> -#include <usbredirhost.h>
> #ifdef USE_LZ4
> #include <lz4.h>
> #endif
> @@ -66,15 +65,12 @@ enum SpiceUsbredirChannelState {
> };
>
> struct _SpiceUsbredirChannelPrivate {
> - libusb_device *device;
> + SpiceUsbBackendDevice *device;
> SpiceUsbDevice *spice_device;
> - libusb_context *context;
> - struct usbredirhost *host;
> + SpiceUsbBackend *context;
> + SpiceUsbBackendChannel *host;
> /* To catch usbredirhost error messages and report them as a GError */
> GError **catch_error;
> - /* Data passed from channel handle msg to the usbredirhost read cb */
> - const uint8_t *read_buf;
> - int read_buf_size;
> enum SpiceUsbredirChannelState state;
> #ifdef USE_POLKIT
> GTask *task;
> @@ -90,18 +86,10 @@ static void spice_usbredir_channel_dispose(GObject *obj);
> static void spice_usbredir_channel_finalize(GObject *obj);
> static void usbredir_handle_msg(SpiceChannel *channel, SpiceMsgIn *in);
>
> -static void usbredir_log(void *user_data, int level, const char *msg);
> -static int usbredir_read_callback(void *user_data, uint8_t *data, int count);
> +static void usbredir_log(void *user_data, const char *msg, gboolean error);
> static int usbredir_write_callback(void *user_data, uint8_t *data, int count);
> -static void usbredir_write_flush_callback(void *user_data);
> -#if USBREDIR_VERSION >= 0x000701
> -static uint64_t usbredir_buffered_output_size_callback(void *user_data);
> -#endif
> -
> -static void *usbredir_alloc_lock(void);
> -static void usbredir_lock_lock(void *user_data);
> -static void usbredir_unlock_lock(void *user_data);
> -static void usbredir_free_lock(void *user_data);
> +static gboolean usbredir_is_channel_ready(void *user_data);
> +static uint64_t usbredir_get_queue_size(void *user_data);
>
> #else
> struct _SpiceUsbredirChannelPrivate {
> @@ -128,7 +116,7 @@ static void _channel_reset_finish(SpiceUsbredirChannel *channel)
>
> spice_usbredir_channel_lock(channel);
>
> - usbredirhost_close(priv->host);
> + spice_usb_backend_channel_finalize(priv->host);
> priv->host = NULL;
>
> /* Call set_context to re-create the host */
> @@ -228,7 +216,7 @@ static void spice_usbredir_channel_finalize(GObject *obj)
> SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(obj);
>
> if (channel->priv->host)
> - usbredirhost_close(channel->priv->host);
> + spice_usb_backend_channel_finalize(channel->priv->host);
> #ifdef USE_USBREDIR
> g_mutex_clear(&channel->priv->device_connect_mutex);
> #endif
> @@ -252,33 +240,24 @@ static void channel_set_handlers(SpiceChannelClass *klass)
> /* private api */
>
> G_GNUC_INTERNAL
> -void spice_usbredir_channel_set_context(SpiceUsbredirChannel *channel,
> - libusb_context *context)
> +void spice_usbredir_channel_set_context(
> + SpiceUsbredirChannel *channel,
> + SpiceUsbBackend *context)
Indentation
> {
> SpiceUsbredirChannelPrivate *priv = channel->priv;
> + SpiceUsbBackendChannelInitData init_data;
> + init_data.user_data = channel;
> + init_data.get_queue_size = usbredir_get_queue_size;
> + init_data.is_channel_ready = usbredir_is_channel_ready;
> + init_data.log = usbredir_log;
> + init_data.write_callback = usbredir_write_callback;
> + init_data.debug = spice_util_get_debug();
>
> g_return_if_fail(priv->host == NULL);
>
> priv->context = context;
> - priv->host = usbredirhost_open_full(
> - context, NULL,
> - usbredir_log,
> - usbredir_read_callback,
> - usbredir_write_callback,
> - usbredir_write_flush_callback,
> - usbredir_alloc_lock,
> - usbredir_lock_lock,
> - usbredir_unlock_lock,
> - usbredir_free_lock,
> - channel, PACKAGE_STRING,
> - spice_util_get_debug() ? usbredirparser_debug : usbredirparser_warning,
> - usbredirhost_fl_write_cb_owns_buffer);
> - if (!priv->host)
> - g_error("Out of memory allocating usbredirhost");
> + priv->host = spice_usb_backend_channel_initialize(context, &init_data);
priv->host could be NULL, but we lost the g_error().
>
> -#if USBREDIR_VERSION >= 0x000701
> - usbredirhost_set_buffered_output_size_cb(priv->host, usbredir_buffered_output_size_callback);
> -#endif
> #ifdef USE_LZ4
> spice_channel_set_capability(channel, SPICE_SPICEVMC_CAP_DATA_COMPRESS_LZ4);
> #endif
> @@ -289,9 +268,8 @@ static gboolean spice_usbredir_channel_open_device(
> {
> SpiceUsbredirChannelPrivate *priv = channel->priv;
> SpiceSession *session;
> - libusb_device_handle *handle = NULL;
> - int rc, status;
> SpiceUsbDeviceManager *manager;
> + const char *msg = NULL;
>
> g_return_val_if_fail(priv->state == STATE_DISCONNECTED
> #ifdef USE_POLKIT
> @@ -299,29 +277,28 @@ static gboolean spice_usbredir_channel_open_device(
> #endif
> , FALSE);
>
> - rc = libusb_open(priv->device, &handle);
> - if (rc != 0) {
> - g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> - "Could not open usb device: %s [%i]",
> - spice_usbutil_libusb_strerror(rc), rc);
> - return FALSE;
> - }
> -
> priv->catch_error = err;
> - status = usbredirhost_set_device(priv->host, handle);
> - priv->catch_error = NULL;
> - if (status != usb_redir_success) {
> - g_return_val_if_fail(err == NULL || *err != NULL, FALSE);
> + if (!spice_usb_backend_channel_attach(priv->host, priv->device, &msg)) {
> + priv->catch_error = NULL;
> + if (*err == NULL) {
> + if (!msg) {
> + msg = "Exact error not reported";
> + }
> + g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> + "Error attaching device: %s", msg);
> + }
> return FALSE;
> }
> + priv->catch_error = NULL;
>
> session = spice_channel_get_session(SPICE_CHANNEL(channel));
> manager = spice_usb_device_manager_get(session, NULL);
> g_return_val_if_fail(manager != NULL, FALSE);
>
> priv->usb_device_manager = g_object_ref(manager);
> - if (!spice_usb_device_manager_start_event_listening(priv->usb_device_manager, err)) {
> - usbredirhost_set_device(priv->host, NULL);
> + if (spice_usb_backend_device_need_thread(priv->device) &&
I would introduce this later on when it starts to become needed.
> + !spice_usb_device_manager_start_event_listening(priv->usb_device_manager, err)) {
> + spice_usb_backend_channel_detach(priv->host);
> return FALSE;
> }
>
> @@ -352,8 +329,7 @@ static void spice_usbredir_channel_open_acl_cb(
> spice_usbredir_channel_open_device(channel, &err);
> }
> if (err) {
> - libusb_unref_device(priv->device);
> - priv->device = NULL;
> + g_clear_pointer(&priv->device, spice_usb_backend_device_release);
> g_boxed_free(spice_usb_device_get_type(), priv->spice_device);
> priv->spice_device = NULL;
> priv->state = STATE_DISCONNECTED;
> @@ -370,7 +346,6 @@ static void spice_usbredir_channel_open_acl_cb(
> }
> #endif
>
> -#ifndef USE_POLKIT
> static void
> _open_device_async_cb(GTask *task,
> gpointer object,
> @@ -384,8 +359,7 @@ _open_device_async_cb(GTask *task,
> spice_usbredir_channel_lock(channel);
>
> if (!spice_usbredir_channel_open_device(channel, &err)) {
> - libusb_unref_device(priv->device);
> - priv->device = NULL;
> + g_clear_pointer(&priv->device, spice_usb_backend_device_release);
> g_boxed_free(spice_usb_device_get_type(), priv->spice_device);
> priv->spice_device = NULL;
> }
> @@ -398,18 +372,20 @@ _open_device_async_cb(GTask *task,
> g_task_return_boolean(task, TRUE);
> }
> }
> -#endif
>
> G_GNUC_INTERNAL
> void spice_usbredir_channel_connect_device_async(
> SpiceUsbredirChannel *channel,
> - libusb_device *device,
> + SpiceUsbBackendDevice *device,
> SpiceUsbDevice *spice_device,
> GCancellable *cancellable,
> GAsyncReadyCallback callback,
> gpointer user_data)
> {
> SpiceUsbredirChannelPrivate *priv = channel->priv;
> +#ifdef USE_POLKIT
> + const UsbDeviceInformation *info = spice_usb_backend_device_get_info(device);
> +#endif
> GTask *task;
>
> g_return_if_fail(SPICE_IS_USBREDIR_CHANNEL(channel));
> @@ -436,25 +412,28 @@ void spice_usbredir_channel_connect_device_async(
> goto done;
> }
>
> - priv->device = libusb_ref_device(device);
> + spice_usb_backend_device_acquire(device);
> + priv->device = device;
> priv->spice_device = g_boxed_copy(spice_usb_device_get_type(),
> spice_device);
> #ifdef USE_POLKIT
> - priv->task = task;
> - priv->state = STATE_WAITING_FOR_ACL_HELPER;
> - priv->acl_helper = spice_usb_acl_helper_new();
> - g_object_set(spice_channel_get_session(SPICE_CHANNEL(channel)),
> - "inhibit-keyboard-grab", TRUE, NULL);
> - spice_usb_acl_helper_open_acl_async(priv->acl_helper,
> - libusb_get_bus_number(device),
> - libusb_get_device_address(device),
> - cancellable,
> - spice_usbredir_channel_open_acl_cb,
> - channel);
> - return;
> -#else
> - g_task_run_in_thread(task, _open_device_async_cb);
> + // avoid calling ACL helper for emulated CD devices
> + if (info->max_luns == 0) {
This does not belong in this commit.
> + priv->task = task;
> + priv->state = STATE_WAITING_FOR_ACL_HELPER;
> + priv->acl_helper = spice_usb_acl_helper_new();
> + g_object_set(spice_channel_get_session(SPICE_CHANNEL(channel)),
> + "inhibit-keyboard-grab", TRUE, NULL);
> + spice_usb_acl_helper_open_acl_async(priv->acl_helper,
> + info->bus,
> + info->address,
> + cancellable,
> + spice_usbredir_channel_open_acl_cb,
> + channel);
> + return;
> + }
> #endif
> + g_task_run_in_thread(task, _open_device_async_cb);
>
> done:
> g_object_unref(task);
> @@ -501,13 +480,14 @@ void spice_usbredir_channel_disconnect_device(SpiceUsbredirChannel *channel)
> * libusb_handle_events call in the thread.
> */
> g_warn_if_fail(priv->usb_device_manager != NULL);
> - spice_usb_device_manager_stop_event_listening(priv->usb_device_manager);
> + if (spice_usb_backend_device_need_thread(priv->device)) {
> + spice_usb_device_manager_stop_event_listening(priv->usb_device_manager);
> + }
> g_clear_object(&priv->usb_device_manager);
>
> /* This also closes the libusb handle we passed from open_device */
> - usbredirhost_set_device(priv->host, NULL);
> - libusb_unref_device(priv->device);
> - priv->device = NULL;
> + spice_usb_backend_channel_detach(priv->host);
> + g_clear_pointer(&priv->device, spice_usb_backend_device_release);
> g_boxed_free(spice_usb_device_get_type(), priv->spice_device);
> priv->spice_device = NULL;
> priv->state = STATE_DISCONNECTED;
> @@ -558,7 +538,7 @@ spice_usbredir_channel_get_spice_usb_device(SpiceUsbredirChannel *channel)
> #endif
>
> G_GNUC_INTERNAL
> -libusb_device *spice_usbredir_channel_get_device(SpiceUsbredirChannel *channel)
> +SpiceUsbBackendDevice *spice_usbredir_channel_get_device(SpiceUsbredirChannel *channel)
> {
> return channel->priv->device;
> }
> @@ -573,85 +553,40 @@ void spice_usbredir_channel_get_guest_filter(
>
> g_return_if_fail(priv->host != NULL);
>
> - usbredirhost_get_guest_filter(priv->host, rules_ret, rules_count_ret);
> + spice_usb_backend_channel_get_guest_filter(priv->host, rules_ret, rules_count_ret);
> }
>
> /* ------------------------------------------------------------------ */
> /* callbacks (any context) */
>
> -#if USBREDIR_VERSION >= 0x000701
> -static uint64_t usbredir_buffered_output_size_callback(void *user_data)
> +static uint64_t usbredir_get_queue_size(void *user_data)
> {
> g_return_val_if_fail(SPICE_IS_USBREDIR_CHANNEL(user_data), 0);
> return spice_channel_get_queue_size(SPICE_CHANNEL(user_data));
> }
> -#endif
>
> -/* Note that this function must be re-entrant safe, as it can get called
> - from both the main thread as well as from the usb event handling thread */
> -static void usbredir_write_flush_callback(void *user_data)
> +static gboolean usbredir_is_channel_ready(void *user_data)
> {
> SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(user_data);
> SpiceUsbredirChannelPrivate *priv = channel->priv;
> -
> - if (spice_channel_get_state(SPICE_CHANNEL(channel)) !=
> - SPICE_CHANNEL_STATE_READY)
> - return;
> -
> + if (spice_channel_get_state(SPICE_CHANNEL(channel)) != SPICE_CHANNEL_STATE_READY)
> + return FALSE;
> if (!priv->host)
> - return;
> -
> - usbredirhost_write_guest_data(priv->host);
> -}
> -
> -static void usbredir_log(void *user_data, int level, const char *msg)
> -{
> - SpiceUsbredirChannel *channel = user_data;
> - SpiceUsbredirChannelPrivate *priv = channel->priv;
> -
> - if (priv->catch_error && level == usbredirparser_error) {
> - CHANNEL_DEBUG(channel, "%s", msg);
> - /* Remove "usbredirhost: " prefix from usbredirhost messages */
> - if (strncmp(msg, "usbredirhost: ", 14) == 0)
> - g_set_error_literal(priv->catch_error, SPICE_CLIENT_ERROR,
> - SPICE_CLIENT_ERROR_FAILED, msg + 14);
> - else
> - g_set_error_literal(priv->catch_error, SPICE_CLIENT_ERROR,
> - SPICE_CLIENT_ERROR_FAILED, msg);
> - return;
> - }
> + return FALSE;
>
> - switch (level) {
> - case usbredirparser_error:
> - g_critical("%s", msg);
> - break;
> - case usbredirparser_warning:
> - g_warning("%s", msg);
> - break;
> - default:
> - CHANNEL_DEBUG(channel, "%s", msg);
> - }
> + return TRUE;
> }
>
> -static int usbredir_read_callback(void *user_data, uint8_t *data, int count)
> +static void usbredir_log(void *user_data, const char *msg, gboolean error)
> {
> SpiceUsbredirChannel *channel = user_data;
> SpiceUsbredirChannelPrivate *priv = channel->priv;
>
> - count = MIN(priv->read_buf_size, count);
> -
> - if (count != 0) {
> - memcpy(data, priv->read_buf, count);
> + if (priv->catch_error && error) {
> + g_set_error_literal(priv->catch_error, SPICE_CLIENT_ERROR,
> + SPICE_CLIENT_ERROR_FAILED, msg);
> + priv->catch_error = NULL;
It's odd to set priv->catch_error and set it to NULL right after setting
it.
> }
> -
> - priv->read_buf_size -= count;
> - if (priv->read_buf_size) {
> - priv->read_buf += count;
> - } else {
> - priv->read_buf = NULL;
> - }
> -
> - return count;
> }
>
> static void usbredir_free_write_cb_data(uint8_t *data, void *user_data)
> @@ -659,7 +594,7 @@ static void usbredir_free_write_cb_data(uint8_t *data, void *user_data)
> SpiceUsbredirChannel *channel = user_data;
> SpiceUsbredirChannelPrivate *priv = channel->priv;
>
> - usbredirhost_free_write_buffer(priv->host, data);
> + spice_usb_backend_return_write_data(priv->host, data);
> }
>
> #ifdef USE_LZ4
> @@ -731,7 +666,7 @@ static int usbredir_write_callback(void *user_data, uint8_t *data, int count)
>
> #ifdef USE_LZ4
> if (try_write_compress_LZ4(channel, data, count)) {
> - usbredirhost_free_write_buffer(channel->priv->host, data);
> + spice_usb_backend_return_write_data(channel->priv->host, data);
> return count;
> }
> #endif
> @@ -744,15 +679,6 @@ static int usbredir_write_callback(void *user_data, uint8_t *data, int count)
> return count;
> }
>
> -static void *usbredir_alloc_lock(void) {
> - GMutex *mutex;
> -
> - mutex = g_new0(GMutex, 1);
> - g_mutex_init(mutex);
> -
> - return mutex;
> -}
> -
> G_GNUC_INTERNAL
> void spice_usbredir_channel_lock(SpiceUsbredirChannel *channel)
> {
> @@ -765,25 +691,6 @@ void spice_usbredir_channel_unlock(SpiceUsbredirChannel *channel)
> g_mutex_unlock(&channel->priv->device_connect_mutex);
> }
>
> -static void usbredir_lock_lock(void *user_data) {
> - GMutex *mutex = user_data;
> -
> - g_mutex_lock(mutex);
> -}
> -
> -static void usbredir_unlock_lock(void *user_data) {
> - GMutex *mutex = user_data;
> -
> - g_mutex_unlock(mutex);
> -}
> -
> -static void usbredir_free_lock(void *user_data) {
> - GMutex *mutex = user_data;
> -
> - g_mutex_clear(mutex);
> - g_free(mutex);
> -}
> -
> /* --------------------------------------------------------------------- */
>
> typedef struct device_error_data {
> @@ -819,10 +726,14 @@ static void spice_usbredir_channel_up(SpiceChannel *c)
> {
> SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(c);
> SpiceUsbredirChannelPrivate *priv = channel->priv;
> + SpiceSession *session = spice_channel_get_session(c);
> + SpiceUsbDeviceManager *manager = spice_usb_device_manager_get(session, NULL);
>
> g_return_if_fail(priv->host != NULL);
> /* Flush any pending writes */
> - usbredirhost_write_guest_data(priv->host);
> + spice_usb_backend_channel_up(priv->host);
> + /* Check which existing device can be redirected right now */
> + spice_usb_device_manager_check_redir_on_connect(manager, c);
> }
>
> static int try_handle_compressed_msg(SpiceMsgCompressedData *compressed_data_msg,
> @@ -872,26 +783,20 @@ static void usbredir_handle_msg(SpiceChannel *c, SpiceMsgIn *in)
>
> g_return_if_fail(priv->host != NULL);
>
> - /* No recursion allowed! */
> - g_return_if_fail(priv->read_buf == NULL);
> -
> if (spice_msg_in_type(in) == SPICE_MSG_SPICEVMC_COMPRESSED_DATA) {
> SpiceMsgCompressedData *compressed_data_msg = spice_msg_in_parsed(in);
> if (try_handle_compressed_msg(compressed_data_msg, &buf, &size)) {
> - priv->read_buf_size = size;
> - priv->read_buf = buf;
> + /* uncompressed ok*/
> } else {
> - r = usbredirhost_read_parse_error;
> + r = USB_REDIR_ERROR_READ_PARSE;
> }
> } else { /* Regular SPICE_MSG_SPICEVMC_DATA msg */
> buf = spice_msg_in_raw(in, &size);
> - priv->read_buf_size = size;
> - priv->read_buf = buf;
> }
>
> spice_usbredir_channel_lock(channel);
> if (r == 0)
> - r = usbredirhost_read_guest_data(priv->host);
> + r = spice_usb_backend_provide_read_data(priv->host, buf, size);
> if (r != 0) {
> SpiceUsbDevice *spice_device = priv->spice_device;
> device_error_data err_data;
> @@ -905,16 +810,16 @@ static void usbredir_handle_msg(SpiceChannel *c, SpiceMsgIn *in)
>
> desc = spice_usb_device_get_description(spice_device, NULL);
> switch (r) {
> - case usbredirhost_read_parse_error:
> + case USB_REDIR_ERROR_READ_PARSE:
> err = g_error_new(SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> _("usbredir protocol parse error for %s"), desc);
> break;
> - case usbredirhost_read_device_rejected:
> + case USB_REDIR_ERROR_DEV_REJECTED:
> err = g_error_new(SPICE_CLIENT_ERROR,
> SPICE_CLIENT_ERROR_USB_DEVICE_REJECTED,
> _("%s rejected by host"), desc);
> break;
> - case usbredirhost_read_device_lost:
> + case USB_REDIR_ERROR_DEV_LOST:
> err = g_error_new(SPICE_CLIENT_ERROR,
> SPICE_CLIENT_ERROR_USB_DEVICE_LOST,
> _("%s disconnected (fatal IO error)"), desc);
> diff --git a/src/meson.build b/src/meson.build
> index 8c9199e..2870102 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -78,6 +78,7 @@ spice_client_glib_introspection_sources = [
> 'spice-session.c',
> 'spice-util.c',
> 'usb-device-manager.c',
> + 'usb-backend-common.c',
I think you also need usb-backend.h in this file.
> ]
>
> spice_client_glib_sources = [
> diff --git a/src/usb-device-manager-priv.h b/src/usb-device-manager-priv.h
> index 83884d7..53149fb 100644
> --- a/src/usb-device-manager-priv.h
> +++ b/src/usb-device-manager-priv.h
> @@ -32,9 +32,12 @@ void spice_usb_device_manager_stop_event_listening(
> SpiceUsbDeviceManager *manager);
>
> #ifdef USE_USBREDIR
> -#include <libusb.h>
> +#include "usb-backend.h"
> void spice_usb_device_manager_device_error(
> SpiceUsbDeviceManager *manager, SpiceUsbDevice *device, GError *err);
> +void spice_usb_device_manager_check_redir_on_connect(
> + SpiceUsbDeviceManager *manager, SpiceChannel *channel);
> +
>
> guint8 spice_usb_device_get_busnum(const SpiceUsbDevice *device);
> guint8 spice_usb_device_get_devaddr(const SpiceUsbDevice *device);
> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> index 50fb491..2b6c049 100644
> --- a/src/usb-device-manager.c
> +++ b/src/usb-device-manager.c
> @@ -24,10 +24,11 @@
> #include <glib-object.h>
>
> #ifdef USE_USBREDIR
> +
> #include <errno.h>
> -#include <libusb.h>
>
> #ifdef G_OS_WIN32
> +#include <windows.h>
> #include "usbdk_api.h"
> #endif
>
> @@ -41,8 +42,8 @@
> #endif
>
> #include "channel-usbredir-priv.h"
> -#include "usbredirhost.h"
> #include "usbutil.h"
> +
> #endif
>
> #include "spice-session-priv.h"
> @@ -102,7 +103,7 @@ struct _SpiceUsbDeviceManagerPrivate {
> gchar *auto_connect_filter;
> gchar *redirect_on_connect;
> #ifdef USE_USBREDIR
> - libusb_context *context;
> + SpiceUsbBackend *context;
> int event_listeners;
> GThread *event_thread;
> gint event_thread_run;
> @@ -112,10 +113,9 @@ struct _SpiceUsbDeviceManagerPrivate {
> int redirect_on_connect_rules_count;
> #ifdef USE_GUDEV
> GUdevClient *udev;
> - libusb_device **coldplug_list; /* Avoid needless reprobing during init */
> + SpiceUsbBackendDevice **coldplug_list; /* Avoid needless reprobing during init */
> #else
> gboolean redirecting; /* Handled by GUdevClient in the gudev case */
> - libusb_hotplug_callback_handle hp_handle;
> #endif
> #ifdef G_OS_WIN32
> usbdk_api_wrapper *usbdk_api;
> @@ -139,6 +139,7 @@ enum {
>
> #ifdef USE_USBREDIR
>
> +// this is the structure behind SpiceUsbDevice
Maybe worth renaming that struct rather than just adding a comment?
> typedef struct _SpiceUsbDeviceInfo {
> guint8 busnum;
> guint8 devaddr;
> @@ -148,7 +149,7 @@ typedef struct _SpiceUsbDeviceInfo {
> #ifdef G_OS_WIN32
> guint8 state;
> #else
> - libusb_device *libdev;
> + SpiceUsbBackendDevice *bdev;
> #endif
> gint ref;
> } SpiceUsbDeviceInfo;
> @@ -166,15 +167,13 @@ static void spice_usb_device_manager_uevent_cb(GUdevClient *client,
> static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager *self,
> GUdevDevice *udev);
> #else
> -static int spice_usb_device_manager_hotplug_cb(libusb_context *ctx,
> - libusb_device *device,
> - libusb_hotplug_event event,
> - void *data);
> +static void spice_usb_device_manager_hotplug_cb(
> + void *data,
> + SpiceUsbBackendDevice *bdev,
> + gboolean added);
Indentation
> #endif
> -static void spice_usb_device_manager_check_redir_on_connect(
> - SpiceUsbDeviceManager *self, SpiceChannel *channel);
>
> -static SpiceUsbDeviceInfo *spice_usb_device_new(libusb_device *libdev);
> +static SpiceUsbDeviceInfo *spice_usb_device_new(SpiceUsbBackendDevice *bdev);
> static SpiceUsbDevice *spice_usb_device_ref(SpiceUsbDevice *device);
> static void spice_usb_device_unref(SpiceUsbDevice *device);
>
> @@ -183,11 +182,11 @@ static void _usbdk_hider_update(SpiceUsbDeviceManager *manager);
> static void _usbdk_hider_clear(SpiceUsbDeviceManager *manager);
> #endif
>
> -static gboolean spice_usb_manager_device_equal_libdev(SpiceUsbDeviceManager *manager,
> +static gboolean spice_usb_manager_device_equal_bdev(SpiceUsbDeviceManager *manager,
> SpiceUsbDevice *device,
> - libusb_device *libdev);
> -static libusb_device *
> -spice_usb_device_manager_device_to_libdev(SpiceUsbDeviceManager *self,
> + SpiceUsbBackendDevice *bdev);
> +static SpiceUsbBackendDevice*
> +spice_usb_device_manager_device_to_bdev(SpiceUsbDeviceManager *self,
> SpiceUsbDevice *device);
Indentation seems a bit off here too.
>
> static void
> @@ -288,27 +287,15 @@ static gboolean spice_usb_device_manager_initable_init(GInitable *initable,
> SpiceUsbDeviceManagerPrivate *priv = self->priv;
> GList *list;
> GList *it;
> - int rc;
> #ifdef USE_GUDEV
> const gchar *const subsystems[] = {"usb", NULL};
> #endif
>
> - /* Initialize libusb */
> - rc = libusb_init(&priv->context);
> - if (rc < 0) {
> - const char *desc = spice_usbutil_libusb_strerror(rc);
> - g_warning("Error initializing USB support: %s [%i]", desc, rc);
> - g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> - "Error initializing USB support: %s [%i]", desc, rc);
> + /* Initialize spice backend */
> + priv->context = spice_usb_backend_initialize();
> + if (!priv->context) {
This was returning a GError before.
> return FALSE;
> }
> -
> -#ifdef G_OS_WIN32
> -#if LIBUSB_API_VERSION >= 0x01000106
> - libusb_set_option(priv->context, LIBUSB_OPTION_USE_USBDK);
> -#endif
> -#endif
> -
> /* Start listening for usb devices plug / unplug */
> #ifdef USE_GUDEV
> priv->udev = g_udev_client_new(subsystems);
> @@ -319,26 +306,20 @@ static gboolean spice_usb_device_manager_initable_init(GInitable *initable,
> g_signal_connect(G_OBJECT(priv->udev), "uevent",
> G_CALLBACK(spice_usb_device_manager_uevent_cb), self);
> /* Do coldplug (detection of already connected devices) */
> - libusb_get_device_list(priv->context, &priv->coldplug_list);
> + priv->coldplug_list = spice_usb_backend_get_device_list(priv->context);
> list = g_udev_client_query_by_subsystem(priv->udev, "usb");
> for (it = g_list_first(list); it; it = g_list_next(it)) {
> spice_usb_device_manager_add_udev(self, it->data);
> g_object_unref(it->data);
> }
> g_list_free(list);
> - libusb_free_device_list(priv->coldplug_list, 1);
> + spice_usb_backend_free_device_list(priv->coldplug_list);
> priv->coldplug_list = NULL;
> #else
> - rc = libusb_hotplug_register_callback(priv->context,
> - LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED | LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT,
> - LIBUSB_HOTPLUG_ENUMERATE, LIBUSB_HOTPLUG_MATCH_ANY,
> - LIBUSB_HOTPLUG_MATCH_ANY, LIBUSB_HOTPLUG_MATCH_ANY,
> - spice_usb_device_manager_hotplug_cb, self, &priv->hp_handle);
> - if (rc < 0) {
> - const char *desc = spice_usbutil_libusb_strerror(rc);
> - g_warning("Error initializing USB hotplug support: %s [%i]", desc, rc);
> + if (!spice_usb_backend_handle_hotplug(priv->context,
> + self, spice_usb_device_manager_hotplug_cb)) {
> g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> - "Error initializing USB hotplug support: %s [%i]", desc, rc);
> + "Error initializing USB hotplug support");
Indentation
> return FALSE;
> }
> spice_usb_device_manager_start_event_listening(self, NULL);
> @@ -369,20 +350,18 @@ static void spice_usb_device_manager_dispose(GObject *gobject)
> SpiceUsbDeviceManagerPrivate *priv = self->priv;
>
> #ifdef USE_LIBUSB_HOTPLUG
> - if (priv->hp_handle) {
> - spice_usb_device_manager_stop_event_listening(self);
> - if (g_atomic_int_get(&priv->event_thread_run)) {
> - /* Force termination of the event thread even if there were some
> - * mismatched spice_usb_device_manager_{start,stop}_event_listening
> - * calls. Otherwise, the usb event thread will be leaked, and will
> - * try to use the libusb context we destroy in finalize(), which would
> - * cause a crash */
> - g_warn_if_reached();
> - g_atomic_int_set(&priv->event_thread_run, FALSE);
> - }
> - /* This also wakes up the libusb_handle_events() in the event_thread */
> - libusb_hotplug_deregister_callback(priv->context, priv->hp_handle);
> - priv->hp_handle = 0;
> + spice_usb_device_manager_stop_event_listening(self);
> + if (g_atomic_int_get(&priv->event_thread_run)) {
> + /* Force termination of the event thread even if there were some
> + * mismatched spice_usb_device_manager_{start,stop}_event_listening
> + * calls. Otherwise, the usb event thread will be leaked, and will
> + * try to use the libusb context we destroy in finalize(), which would
> + * cause a crash */
> + g_warn_if_reached();
> + g_atomic_int_set(&priv->event_thread_run, FALSE);
> +
> + /* This also wakes up the libusb_handle_events() in the event_thread */
> + spice_usb_backend_handle_hotplug(priv->context, NULL, NULL);
> }
> #endif
> if (priv->event_thread) {
> @@ -411,8 +390,9 @@ static void spice_usb_device_manager_finalize(GObject *gobject)
> g_clear_object(&priv->udev);
> #endif
> g_return_if_fail(priv->event_thread == NULL);
> - if (priv->context)
> - libusb_exit(priv->context);
> + if (priv->context) {
> + spice_usb_backend_finalize(priv->context);
> + }
> free(priv->auto_conn_filter_rules);
> free(priv->redirect_on_connect_rules);
> #ifdef G_OS_WIN32
> @@ -737,7 +717,7 @@ static void spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas
> #ifdef USE_USBREDIR
>
> /* ------------------------------------------------------------------ */
> -/* gudev / libusb Helper functions */
> +/* gudev / backend Helper functions */
>
> #ifdef USE_GUDEV
> static gboolean spice_usb_device_manager_get_udev_bus_n_address(
> @@ -761,40 +741,16 @@ static gboolean spice_usb_device_manager_get_udev_bus_n_address(
> }
> #endif
>
> -static gboolean spice_usb_device_manager_get_device_descriptor(
> - libusb_device *libdev,
> - struct libusb_device_descriptor *desc)
> -{
> - int errcode;
> - const gchar *errstr;
> -
> - g_return_val_if_fail(libdev != NULL, FALSE);
> - g_return_val_if_fail(desc != NULL, FALSE);
> -
> - errcode = libusb_get_device_descriptor(libdev, desc);
> - if (errcode < 0) {
> - int bus, addr;
> -
> - bus = libusb_get_bus_number(libdev);
> - addr = libusb_get_device_address(libdev);
> - errstr = spice_usbutil_libusb_strerror(errcode);
> - g_warning("cannot get device descriptor for (%p) %d.%d -- %s(%d)",
> - libdev, bus, addr, errstr, errcode);
> - return FALSE;
> - }
> - return TRUE;
> -}
> -
> #endif // USE_USBREDIR
>
> /**
> * spice_usb_device_get_libusb_device:
> - * @device: #SpiceUsbDevice to get the descriptor information of
> + * @device: #SpiceUsbDevice to get the libusb device of (if exists)
> *
> * Finds the %libusb_device associated with the @device.
> *
> - * Returns: (transfer none): the %libusb_device associated to %SpiceUsbDevice.
> - *
> + * Returns: (transfer none): the %libusb_device associated to %SpiceUsbDevice
> + * or NULL (if the device does not have associated libusb device)
This is an exported function, and if we start returning NULL in some
cases, this is going to break applications using this API :(
> * Since: 0.27
> **/
> gconstpointer
> @@ -806,34 +762,13 @@ spice_usb_device_get_libusb_device(const SpiceUsbDevice *device G_GNUC_UNUSED)
>
> g_return_val_if_fail(info != NULL, FALSE);
>
> - return info->libdev;
> + return spice_usb_backend_device_get_libdev(info->bdev);
> #endif
> #endif
> return NULL;
> }
>
> #ifdef USE_USBREDIR
> -static gboolean spice_usb_device_manager_get_libdev_vid_pid(
> - libusb_device *libdev, int *vid, int *pid)
> -{
> - struct libusb_device_descriptor desc;
> -
> - g_return_val_if_fail(libdev != NULL, FALSE);
> - g_return_val_if_fail(vid != NULL, FALSE);
> - g_return_val_if_fail(pid != NULL, FALSE);
> -
> - *vid = *pid = 0;
> -
> - if (!spice_usb_device_manager_get_device_descriptor(libdev, &desc)) {
> - return FALSE;
> - }
> - *vid = desc.idVendor;
> - *pid = desc.idProduct;
> -
> - return TRUE;
> -}
> -
> -/* ------------------------------------------------------------------ */
> /* callbacks */
>
> static void channel_new(SpiceSession *session, SpiceChannel *channel,
> @@ -849,10 +784,8 @@ static void channel_new(SpiceSession *session, SpiceChannel *channel,
> spice_channel_connect(channel);
> g_ptr_array_add(self->priv->channels, channel);
>
> - spice_usb_device_manager_check_redir_on_connect(self, channel);
> -
Not clear why thisis removed?
> /*
> - * add a reference to ourself, to make sure the libusb context is
> + * add a reference to ourself, to make sure the backend device context is
> * alive as long as the channel is.
> * TODO: moving to gusb could help here too.
> */
> @@ -889,6 +822,7 @@ static void spice_usb_device_manager_auto_connect_cb(GObject *gobject,
> g_signal_emit(self, signals[AUTO_CONNECT_FAILED], 0, device, err);
> g_error_free(err);
> }
> +
> spice_usb_device_unref(device);
Extra blank space
> }
>
> @@ -902,12 +836,12 @@ spice_usb_device_manager_device_match(SpiceUsbDeviceManager *self, SpiceUsbDevic
>
> #ifdef USE_GUDEV
> static gboolean
> -spice_usb_device_manager_libdev_match(SpiceUsbDeviceManager *self, libusb_device *libdev,
> +spice_usb_device_manager_bdev_match(SpiceUsbDeviceManager *self, SpiceUsbBackendDevice *dev,
> const int bus, const int address)
> {
> + const UsbDeviceInformation* info = spice_usb_backend_device_get_info(dev);
> /* match functions for Linux/UsbDk -- match by bus.addr */
> - return (libusb_get_bus_number(libdev) == bus &&
> - libusb_get_device_address(libdev) == address);
> + return (info->bus == bus && info->address == address);
> }
> #endif
>
> @@ -929,36 +863,36 @@ spice_usb_device_manager_find_device(SpiceUsbDeviceManager *self,
> return device;
> }
>
> -static void spice_usb_device_manager_add_dev(SpiceUsbDeviceManager *self,
> - libusb_device *libdev)
> +static void spice_usb_device_manager_add_dev(
> + SpiceUsbDeviceManager *self,
> + SpiceUsbBackendDevice *bdev)
Indentation
> {
> SpiceUsbDeviceManagerPrivate *priv = self->priv;
> - struct libusb_device_descriptor desc;
> SpiceUsbDevice *device;
> -
> - if (!spice_usb_device_manager_get_device_descriptor(libdev, &desc))
> - return;
> + const UsbDeviceInformation* info = spice_usb_backend_device_get_info(bdev);
> + // try redirecting shared CD on creation, if filter allows
> + gboolean always_redirect = info->max_luns != 0;
Does not belong here
>
> /* Skip hubs */
> - if (desc.bDeviceClass == LIBUSB_CLASS_HUB)
> + if (spice_usb_backend_device_is_hub(bdev))
> return;
>
> - device = (SpiceUsbDevice*)spice_usb_device_new(libdev);
> + device = (SpiceUsbDevice*)spice_usb_device_new(bdev);
> if (!device)
> return;
>
> g_ptr_array_add(priv->devices, device);
>
> - if (priv->auto_connect) {
> + if (priv->auto_connect || always_redirect) {
> gboolean can_redirect, auto_ok;
>
> can_redirect = spice_usb_device_manager_can_redirect_device(
> self, device, NULL);
>
> - auto_ok = usbredirhost_check_device_filter(
> - priv->auto_conn_filter_rules,
> - priv->auto_conn_filter_rules_count,
> - libdev, 0) == 0;
> + auto_ok = spice_usb_backend_device_check_filter(
> + bdev,
> + priv->auto_conn_filter_rules,
> + priv->auto_conn_filter_rules_count) == 0;
>
> if (can_redirect && auto_ok)
> spice_usb_device_manager_connect_device_async(self,
> @@ -1005,7 +939,7 @@ static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager *self,
> GUdevDevice *udev)
> {
> SpiceUsbDeviceManagerPrivate *priv = self->priv;
> - libusb_device *libdev = NULL, **dev_list = NULL;
> + SpiceUsbBackendDevice *devarrived = NULL, **dev_list = NULL;
devarrived is a bit odd, why not bdev as in the other methods?
> SpiceUsbDevice *device;
> const gchar *devtype;
> int i, bus, address;
> @@ -1033,23 +967,23 @@ static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager *self,
> if (priv->coldplug_list)
> dev_list = priv->coldplug_list;
> else
> - libusb_get_device_list(priv->context, &dev_list);
> + dev_list = spice_usb_backend_get_device_list(priv->context);
>
> for (i = 0; dev_list && dev_list[i]; i++) {
> - if (spice_usb_device_manager_libdev_match(self, dev_list[i], bus, address)) {
> - libdev = dev_list[i];
> + if (spice_usb_device_manager_bdev_match(self, dev_list[i], bus, address)) {
> + devarrived = dev_list[i];
> break;
> }
> }
>
> - if (libdev)
> - spice_usb_device_manager_add_dev(self, libdev);
> + if (devarrived)
> + spice_usb_device_manager_add_dev(self, devarrived);
> else
> g_warning("Could not find USB device to add " DEV_ID_FMT,
> (guint) bus, (guint) address);
>
> if (!priv->coldplug_list)
> - libusb_free_device_list(dev_list, 1);
> + spice_usb_backend_free_device_list(dev_list);
> }
>
> static void spice_usb_device_manager_remove_udev(SpiceUsbDeviceManager *self,
> @@ -1078,8 +1012,8 @@ static void spice_usb_device_manager_uevent_cb(GUdevClient *client,
> #else
> struct hotplug_idle_cb_args {
> SpiceUsbDeviceManager *self;
> - libusb_device *device;
> - libusb_hotplug_event event;
> + SpiceUsbBackendDevice *device;
> + gboolean device_added;
> };
>
> static gboolean spice_usb_device_manager_hotplug_idle_cb(gpointer user_data)
> @@ -1087,36 +1021,34 @@ static gboolean spice_usb_device_manager_hotplug_idle_cb(gpointer user_data)
> struct hotplug_idle_cb_args *args = user_data;
> SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(args->self);
>
> - switch (args->event) {
> - case LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED:
> + if (args->device_added) {
> spice_usb_device_manager_add_dev(self, args->device);
> - break;
> - case LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT:
> + } else {
> + const UsbDeviceInformation *info = spice_usb_backend_device_get_info(args->device);
> spice_usb_device_manager_remove_dev(self,
> - libusb_get_bus_number(args->device),
> - libusb_get_device_address(args->device));
> - break;
> + info->bus,
> + info->address);
Indentation
> }
> - libusb_unref_device(args->device);
> + spice_usb_backend_device_release(args->device);
> g_object_unref(self);
> g_free(args);
> return FALSE;
> }
>
> /* Can be called from both the main-thread as well as the event_thread */
> -static int spice_usb_device_manager_hotplug_cb(libusb_context *ctx,
> - libusb_device *device,
> - libusb_hotplug_event event,
> - void *user_data)
> +static void spice_usb_device_manager_hotplug_cb(
> + void *user_data,
> + SpiceUsbBackendDevice *bdev,
> + gboolean added)
Indentation
> {
> SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(user_data);
> struct hotplug_idle_cb_args *args = g_malloc0(sizeof(*args));
>
> args->self = g_object_ref(self);
> - args->device = libusb_ref_device(device);
> - args->event = event;
> + spice_usb_backend_device_acquire(bdev);
> + args->device_added = added;
> + args->device = bdev;
> g_idle_add(spice_usb_device_manager_hotplug_idle_cb, args);
> - return 0;
> }
> #endif // USE_USBREDIR
>
> @@ -1143,13 +1075,9 @@ static gpointer spice_usb_device_manager_usb_ev_thread(gpointer user_data)
> {
> SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(user_data);
> SpiceUsbDeviceManagerPrivate *priv = self->priv;
> - int rc;
>
> while (g_atomic_int_get(&priv->event_thread_run)) {
> - rc = libusb_handle_events(priv->context);
> - if (rc && rc != LIBUSB_ERROR_INTERRUPTED) {
> - const char *desc = spice_usbutil_libusb_strerror(rc);
> - g_warning("Error handling USB events: %s [%i]", desc, rc);
> + if (!spice_usb_backend_handle_events(priv->context)) {
> break;
> }
> }
> @@ -1194,13 +1122,13 @@ void spice_usb_device_manager_stop_event_listening(
> g_atomic_int_set(&priv->event_thread_run, FALSE);
> }
>
> -static void spice_usb_device_manager_check_redir_on_connect(
> +void spice_usb_device_manager_check_redir_on_connect(
Belongs in a different commit.
> SpiceUsbDeviceManager *self, SpiceChannel *channel)
> {
> SpiceUsbDeviceManagerPrivate *priv = self->priv;
> GTask *task;
> SpiceUsbDevice *device;
> - libusb_device *libdev;
> + SpiceUsbBackendDevice *dev;
> guint i;
>
> if (priv->redirect_on_connect == NULL)
> @@ -1212,15 +1140,15 @@ static void spice_usb_device_manager_check_redir_on_connect(
> if (spice_usb_device_manager_is_device_connected(self, device))
> continue;
>
> - libdev = spice_usb_device_manager_device_to_libdev(self, device);
> + dev = spice_usb_device_manager_device_to_bdev(self, device);
> #ifdef G_OS_WIN32
> - if (libdev == NULL)
> + if (dev == NULL)
> continue;
> #endif
> - if (usbredirhost_check_device_filter(
> - priv->redirect_on_connect_rules,
> - priv->redirect_on_connect_rules_count,
> - libdev, 0) == 0) {
> + if (spice_usb_backend_device_check_filter(
> + dev,
> + priv->redirect_on_connect_rules,
> + priv->redirect_on_connect_rules_count) == 0) {
> /* Note: re-uses spice_usb_device_manager_connect_device_async's
> completion handling code! */
> task = g_task_new(self,
> @@ -1230,14 +1158,14 @@ static void spice_usb_device_manager_check_redir_on_connect(
>
> spice_usbredir_channel_connect_device_async(
> SPICE_USBREDIR_CHANNEL(channel),
> - libdev, device, NULL,
> + dev, device, NULL,
> spice_usb_device_manager_channel_connect_cb,
> task);
> - libusb_unref_device(libdev);
> + spice_usb_backend_device_release(dev);
> return; /* We've taken the channel! */
> }
>
> - libusb_unref_device(libdev);
> + spice_usb_backend_device_release(dev);
> }
> }
>
> @@ -1261,8 +1189,8 @@ static SpiceUsbredirChannel *spice_usb_device_manager_get_channel_for_dev(
> for (i = 0; i < priv->channels->len; i++) {
> SpiceUsbredirChannel *channel = g_ptr_array_index(priv->channels, i);
> spice_usbredir_channel_lock(channel);
> - libusb_device *libdev = spice_usbredir_channel_get_device(channel);
> - if (spice_usb_manager_device_equal_libdev(manager, device, libdev)) {
> + SpiceUsbBackendDevice *dev = spice_usbredir_channel_get_device(channel);
> + if (spice_usb_manager_device_equal_bdev(manager, device, dev)) {
> spice_usbredir_channel_unlock(channel);
> return channel;
> }
> @@ -1319,13 +1247,13 @@ GPtrArray* spice_usb_device_manager_get_devices_with_filter(
> SpiceUsbDevice *device = g_ptr_array_index(priv->devices, i);
>
> if (rules) {
> - libusb_device *libdev =
> - spice_usb_device_manager_device_to_libdev(self, device);
> + SpiceUsbBackendDevice *bdev =
> + spice_usb_device_manager_device_to_bdev(self, device);
> #ifdef G_OS_WIN32
> - if (libdev == NULL)
> + if (bdev == NULL)
> continue;
> #endif
> - if (usbredirhost_check_device_filter(rules, count, libdev, 0) != 0)
> + if (spice_usb_backend_device_check_filter(bdev, rules, count) != 0)
> continue;
> }
> g_ptr_array_add(devices_copy, spice_usb_device_ref(device));
> @@ -1399,7 +1327,7 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
> task = g_task_new(self, cancellable, callback, user_data);
>
> SpiceUsbDeviceManagerPrivate *priv = self->priv;
> - libusb_device *libdev;
> + SpiceUsbBackendDevice *bdev;
> guint i;
>
> if (spice_usb_device_manager_is_device_connected(self, device)) {
> @@ -1415,9 +1343,9 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
> if (spice_usbredir_channel_get_device(channel))
> continue; /* Skip already used channels */
>
> - libdev = spice_usb_device_manager_device_to_libdev(self, device);
> + bdev = spice_usb_device_manager_device_to_bdev(self, device);
> #ifdef G_OS_WIN32
> - if (libdev == NULL) {
> + if (bdev == NULL) {
> /* Most likely, the device was plugged out at driver installation
> * time, and its remove-device event was ignored.
> * So remove the device now
> @@ -1435,12 +1363,12 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
> }
> #endif
> spice_usbredir_channel_connect_device_async(channel,
> - libdev,
> + bdev,
> device,
> cancellable,
> spice_usb_device_manager_channel_connect_cb,
> task);
> - libusb_unref_device(libdev);
> + spice_usb_backend_device_release(bdev);
> return;
> }
>
> @@ -1702,20 +1630,20 @@ spice_usb_device_manager_can_redirect_device(SpiceUsbDeviceManager *self,
>
> if (guest_filter_rules) {
> gboolean filter_ok;
> - libusb_device *libdev;
> + SpiceUsbBackendDevice *bdev;
>
> - libdev = spice_usb_device_manager_device_to_libdev(self, device);
> + bdev = spice_usb_device_manager_device_to_bdev(self, device);
> #ifdef G_OS_WIN32
> - if (libdev == NULL) {
> + if (bdev == NULL) {
> g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> _("Some USB devices were not found"));
> return FALSE;
> }
> #endif
> - filter_ok = (usbredirhost_check_device_filter(
> - guest_filter_rules, guest_filter_rules_count,
> - libdev, 0) == 0);
> - libusb_unref_device(libdev);
> + filter_ok = (spice_usb_backend_device_check_filter(
> + bdev,
> + guest_filter_rules, guest_filter_rules_count) == 0);
> + spice_usb_backend_device_release(bdev);
> if (!filter_ok) {
> g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> _("Some USB devices are blocked by host policy"));
> @@ -1789,7 +1717,7 @@ gchar *spice_usb_device_get_description(SpiceUsbDevice *device, const gchar *for
> }
>
> spice_usb_util_get_device_strings(bus, address, vid, pid,
> - &manufacturer, &product);
> + &manufacturer, &product);
Indentation
>
> if (!format)
> format = _("%s %s %s at %d-%d");
> @@ -1806,64 +1734,30 @@ gchar *spice_usb_device_get_description(SpiceUsbDevice *device, const gchar *for
> #endif
> }
>
> -
> -
> #ifdef USE_USBREDIR
> -static gboolean probe_isochronous_endpoint(libusb_device *libdev)
> -{
> - struct libusb_config_descriptor *conf_desc;
> - gboolean isoc_found = FALSE;
> - gint i, j, k;
> -
> - g_return_val_if_fail(libdev != NULL, FALSE);
> -
> - if (libusb_get_active_config_descriptor(libdev, &conf_desc) != 0) {
> - g_return_val_if_reached(FALSE);
> - }
> -
> - for (i = 0; !isoc_found && i < conf_desc->bNumInterfaces; i++) {
> - for (j = 0; !isoc_found && j < conf_desc->interface[i].num_altsetting; j++) {
> - for (k = 0; !isoc_found && k < conf_desc->interface[i].altsetting[j].bNumEndpoints;k++) {
> - gint attributes = conf_desc->interface[i].altsetting[j].endpoint[k].bmAttributes;
> - gint type = attributes & LIBUSB_TRANSFER_TYPE_MASK;
> - if (type == LIBUSB_TRANSFER_TYPE_ISOCHRONOUS)
> - isoc_found = TRUE;
> - }
> - }
> - }
> -
> - libusb_free_config_descriptor(conf_desc);
> - return isoc_found;
> -}
>
> /*
> * SpiceUsbDeviceInfo
> */
> -static SpiceUsbDeviceInfo *spice_usb_device_new(libusb_device *libdev)
> +static SpiceUsbDeviceInfo *spice_usb_device_new(SpiceUsbBackendDevice *bdev)
> {
> SpiceUsbDeviceInfo *info;
> - int vid, pid;
> - guint8 bus, addr;
> -
> - g_return_val_if_fail(libdev != NULL, NULL);
> -
> - bus = libusb_get_bus_number(libdev);
> - addr = libusb_get_device_address(libdev);
> + const UsbDeviceInformation *devinfo;
>
> - if (!spice_usb_device_manager_get_libdev_vid_pid(libdev, &vid, &pid)) {
> - return NULL;
> - }
> + g_return_val_if_fail(bdev != NULL, NULL);
> + devinfo = spice_usb_backend_device_get_info(bdev);
>
> info = g_new0(SpiceUsbDeviceInfo, 1);
>
> - info->busnum = bus;
> - info->devaddr = addr;
> - info->vid = vid;
> - info->pid = pid;
> + info->busnum = devinfo->bus;
> + info->devaddr = devinfo->address;
> + info->vid = devinfo->vid;
> + info->pid = devinfo->pid;
> info->ref = 1;
> - info->isochronous = probe_isochronous_endpoint(libdev);
> + info->isochronous = devinfo->isochronous;
> #ifndef G_OS_WIN32
> - info->libdev = libusb_ref_device(libdev);
> + info->bdev = bdev;
> + spice_usb_backend_device_acquire(bdev);
> #endif
>
> return info;
> @@ -2001,49 +1895,53 @@ static void spice_usb_device_unref(SpiceUsbDevice *device)
> ref_count_is_0 = g_atomic_int_dec_and_test(&info->ref);
> if (ref_count_is_0) {
> #ifndef G_OS_WIN32
> - libusb_unref_device(info->libdev);
> + if (info->bdev) {
> + spice_usb_backend_device_release(info->bdev);
> + }
> #endif
> + info->vid = info->pid = 0;
> + SPICE_DEBUG("%s: deleting %p", __FUNCTION__, info);
> g_free(info);
> }
> }
>
> #ifndef G_OS_WIN32 /* Linux -- directly compare libdev */
> static gboolean
> -spice_usb_manager_device_equal_libdev(SpiceUsbDeviceManager *manager,
> +spice_usb_manager_device_equal_bdev(SpiceUsbDeviceManager *manager,
> SpiceUsbDevice *device,
> - libusb_device *libdev)
> + SpiceUsbBackendDevice *bdev)
Indentation might need some little fixing here.
> {
> SpiceUsbDeviceInfo *info = (SpiceUsbDeviceInfo *)device;
>
> - if ((device == NULL) || (libdev == NULL))
> + if ((device == NULL) || (bdev == NULL))
> return FALSE;
>
> - return info->libdev == libdev;
> + return spice_usb_backend_devices_same(info->bdev, bdev);
> }
> #else /* Windows -- compare vid:pid of device and libdev */
> static gboolean
> -spice_usb_manager_device_equal_libdev(SpiceUsbDeviceManager *manager,
> - SpiceUsbDevice *device,
> - libusb_device *libdev)
> +spice_usb_manager_device_equal_bdev(SpiceUsbDeviceManager *manager,
> + SpiceUsbDevice *device,
> + SpiceUsbBackendDevice *bdev)
> {
> int busnum, devaddr;
>
> - if ((device == NULL) || (libdev == NULL))
> + if ((device == NULL) || (bdev == NULL))
> return FALSE;
>
> busnum = spice_usb_device_get_busnum(device);
> devaddr = spice_usb_device_get_devaddr(device);
> - return spice_usb_device_manager_libdev_match(manager, libdev,
> + return spice_usb_device_manager_bdev_match(manager, bdev,
> busnum, devaddr);
Indentation
> }
> #endif
>
> /*
> - * Caller must libusb_unref_device the libusb_device returned by this function.
> - * Returns a libusb_device, or NULL upon failure
> + * Caller must spice_usb_backend_device_release the SpiceUsbBackendDevice returned by this function.
> + * Returns a SpiceUsbBackendDevice, or NULL upon failure
> */
> -static libusb_device *
> -spice_usb_device_manager_device_to_libdev(SpiceUsbDeviceManager *self,
> +static SpiceUsbBackendDevice *
> +spice_usb_device_manager_device_to_bdev(SpiceUsbDeviceManager *self,
> SpiceUsbDevice *device)
Indentation
> {
> #ifdef G_OS_WIN32
> @@ -2054,7 +1952,7 @@ spice_usb_device_manager_device_to_libdev(SpiceUsbDeviceManager *self,
> * driver swap we do under windows invalidates the cached libdev.
> */
>
> - libusb_device *d, **devlist;
> + SpiceUsbBackendDevice *d, **devlist;
> int i;
>
> g_return_val_if_fail(SPICE_IS_USB_DEVICE_MANAGER(self), NULL);
> @@ -2062,26 +1960,29 @@ spice_usb_device_manager_device_to_libdev(SpiceUsbDeviceManager *self,
> g_return_val_if_fail(self->priv != NULL, NULL);
> g_return_val_if_fail(self->priv->context != NULL, NULL);
>
> - libusb_get_device_list(self->priv->context, &devlist);
> + devlist = spice_usb_backend_get_device_list(self->priv->context);
> if (!devlist)
> return NULL;
>
> for (i = 0; (d = devlist[i]) != NULL; i++) {
> - if (spice_usb_manager_device_equal_libdev(self, device, d)) {
> - libusb_ref_device(d);
> + if (spice_usb_manager_device_equal_bdev(self, device, d)) {
> + spice_usb_backend_device_acquire(d);
> break;
> }
> }
>
> - libusb_free_device_list(devlist, 1);
> + spice_usb_backend_free_device_list(devlist);
>
> return d;
>
> #else
> /* Simply return a ref to the cached libdev */
> SpiceUsbDeviceInfo *info = (SpiceUsbDeviceInfo *)device;
> -
> - return libusb_ref_device(info->libdev);
> + if (info->bdev) {
> + spice_usb_backend_device_acquire(info->bdev);
> + }
Is it possible for info->bdev to be NULL here?
> + return info->bdev;
> #endif
> }
> +
> #endif /* USE_USBREDIR */
> diff --git a/src/usb-device-manager.h b/src/usb-device-manager.h
> index 773208f..682fcf2 100644
> --- a/src/usb-device-manager.h
> +++ b/src/usb-device-manager.h
> @@ -71,6 +71,7 @@ struct _SpiceUsbDeviceManager
> * @device_removed: Signal class handler for the #SpiceUsbDeviceManager::device-removed signal.
> * @auto_connect_failed: Signal class handler for the #SpiceUsbDeviceManager::auto-connect-failed signal.
> * @device_error: Signal class handler for the #SpiceUsbDeviceManager::device_error signal.
> + * @device_changed: Signal class handler for the #SpiceUsbDeviceManager::device_changed signal.
This signal does not belong in this commit.
> *
> * Class structure for #SpiceUsbDeviceManager.
> */
> @@ -87,18 +88,44 @@ struct _SpiceUsbDeviceManagerClass
> SpiceUsbDevice *device, GError *error);
> void (*device_error) (SpiceUsbDeviceManager *manager,
> SpiceUsbDevice *device, GError *error);
> + void (*device_changed) (SpiceUsbDeviceManager *manager,
> + SpiceUsbDevice *device);
> +
> /*< private >*/
> /*
> * If adding fields to this struct, remove corresponding
> * amount of padding to avoid changing overall struct size
> */
> - gchar _spice_reserved[SPICE_RESERVED_PADDING];
> + gchar _spice_reserved[SPICE_RESERVED_PADDING - 1 * sizeof(void *)];
> };
>
> GType spice_usb_device_get_type(void);
> GType spice_usb_device_manager_get_type(void);
>
> gchar *spice_usb_device_get_description(SpiceUsbDevice *device, const gchar *format);
> +
> +/**
> +* SpiceUsbDeviceDescription:
> +* @bus: USB bus number.
> +* @address: address on the bus.
> +* @vendor_id: vendor ID value from device descriptor.
> +* @product_id: product ID value from device descriptor.
> +* @vendor: vendor name (new string allocated on return)
> +* @product: vendor name (new string allocated on return)
> +*
> +* Structure containing description of Spice USB device.
> +*/
> +typedef struct _SpiceUsbDeviceDescription
> +{
> + guint16 bus;
> + guint16 address;
> + guint16 vendor_id;
> + guint16 product_id;
> + // (OUT) allocated strings for vendor and product
> + gchar *vendor;
> + gchar *product;
> +} SpiceUsbDeviceDescription;
This mostly duplicates _SpiceUsbDeviceInfo which is already exposed as
SpiceUsbDevice, I suspect there's going to be a better way to expose
this info..
> +
> gconstpointer spice_usb_device_get_libusb_device(const SpiceUsbDevice *device);
>
> SpiceUsbDeviceManager *spice_usb_device_manager_get(SpiceSession *session,
> diff --git a/src/usbutil.c b/src/usbutil.c
> index e96ab11..5052ef3 100644
> --- a/src/usbutil.c
> +++ b/src/usbutil.c
> @@ -58,42 +58,6 @@ static GMutex usbids_load_mutex;
> static int usbids_vendor_count = 0; /* < 0: failed, 0: empty, > 0: loaded */
> static usb_vendor_info *usbids_vendor_info = NULL;
>
> -G_GNUC_INTERNAL
> -const char *spice_usbutil_libusb_strerror(enum libusb_error error_code)
> -{
> - switch (error_code) {
> - case LIBUSB_SUCCESS:
> - return "Success";
> - case LIBUSB_ERROR_IO:
> - return "Input/output error";
> - case LIBUSB_ERROR_INVALID_PARAM:
> - return "Invalid parameter";
> - case LIBUSB_ERROR_ACCESS:
> - return "Access denied (insufficient permissions)";
> - case LIBUSB_ERROR_NO_DEVICE:
> - return "No such device (it may have been disconnected)";
> - case LIBUSB_ERROR_NOT_FOUND:
> - return "Entity not found";
> - case LIBUSB_ERROR_BUSY:
> - return "Resource busy";
> - case LIBUSB_ERROR_TIMEOUT:
> - return "Operation timed out";
> - case LIBUSB_ERROR_OVERFLOW:
> - return "Overflow";
> - case LIBUSB_ERROR_PIPE:
> - return "Pipe error";
> - case LIBUSB_ERROR_INTERRUPTED:
> - return "System call interrupted (perhaps due to signal)";
> - case LIBUSB_ERROR_NO_MEM:
> - return "Insufficient memory";
> - case LIBUSB_ERROR_NOT_SUPPORTED:
> - return "Operation not supported or unimplemented on this platform";
> - case LIBUSB_ERROR_OTHER:
> - return "Other error";
> - }
> - return "Unknown error";
> -}
> -
> #ifdef __linux__
> /* <Sigh> libusb does not allow getting the manufacturer and product strings
> without opening the device, so grab them directly from sysfs */
> diff --git a/src/usbutil.h b/src/usbutil.h
> index de5e92a..d18d688 100644
> --- a/src/usbutil.h
> +++ b/src/usbutil.h
> @@ -24,11 +24,9 @@
> #include <glib.h>
>
> #ifdef USE_USBREDIR
> -#include <libusb.h>
>
> G_BEGIN_DECLS
>
> -const char *spice_usbutil_libusb_strerror(enum libusb_error error_code);
> void spice_usb_util_get_device_strings(int bus, int address,
> int vendor_id, int product_id,
> gchar **manufacturer, gchar **product);
> diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
> index 9a130a3..500ee15 100644
> --- a/src/win-usb-dev.c
> +++ b/src/win-usb-dev.c
> @@ -23,11 +23,13 @@
> #include "config.h"
>
> #include <windows.h>
> -#include <libusb.h>
> #include "win-usb-dev.h"
> #include "spice-marshal.h"
> #include "spice-util.h"
> #include "usbutil.h"
> +#include "usb-backend.h"
> +
> +#define USB_CLASS_HUB 9
>
> enum {
> PROP_0,
> @@ -35,7 +37,7 @@ enum {
> };
>
> struct _GUdevClientPrivate {
> - libusb_context *ctx;
> + SpiceUsbBackend *ctx;
> GList *udev_list;
> HWND hwnd;
> gboolean redirecting;
> @@ -85,7 +87,7 @@ static GUdevClient *singleton = NULL;
>
> static GUdevDevice *g_udev_device_new(GUdevDeviceInfo *udevinfo);
> static LRESULT CALLBACK wnd_proc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam);
> -static gboolean get_usb_dev_info(libusb_device *dev, GUdevDeviceInfo *udevinfo);
> +static gboolean get_usb_dev_info(SpiceUsbBackendDevice *dev, GUdevDeviceInfo *udevinfo);
>
> //uncomment to debug gudev device lists.
> //#define DEBUG_GUDEV_DEVICE_LISTS
> @@ -122,8 +124,7 @@ static ssize_t
> g_udev_client_list_devices(GUdevClient *self, GList **devs,
> GError **err, const gchar *name)
> {
> - gssize rc;
> - libusb_device **lusb_list, **dev;
> + SpiceUsbBackendDevice **lusb_list, **dev;
> GUdevClientPrivate *priv;
> GUdevDeviceInfo *udevinfo;
> GUdevDevice *udevice;
> @@ -136,13 +137,8 @@ g_udev_client_list_devices(GUdevClient *self, GList **devs,
>
> g_return_val_if_fail(self->priv->ctx != NULL, -3);
>
> - rc = libusb_get_device_list(priv->ctx, &lusb_list);
> - if (rc < 0) {
> - const char *errstr = spice_usbutil_libusb_strerror(rc);
> - g_warning("%s: libusb_get_device_list failed - %s", name, errstr);
> - g_set_error(err, G_UDEV_CLIENT_ERROR, G_UDEV_CLIENT_LIBUSB_FAILED,
> - "%s: Error getting device list from libusb: %s [%"G_GSSIZE_FORMAT"]",
> - name, errstr, rc);
> + lusb_list = spice_usb_backend_get_device_list(priv->ctx);
> + if (!lusb_list) {
> return -4;
> }
>
> @@ -158,7 +154,7 @@ g_udev_client_list_devices(GUdevClient *self, GList **devs,
> *devs = g_list_prepend(*devs, udevice);
> n++;
> }
> - libusb_free_device_list(lusb_list, 1);
> + spice_usb_backend_free_device_list(lusb_list);
>
> return n;
> }
> @@ -180,7 +176,6 @@ g_udev_client_initable_init(GInitable *initable, GCancellable *cancellable,
> GUdevClient *self;
> GUdevClientPrivate *priv;
> WNDCLASS wcls;
> - int rc;
>
> g_return_val_if_fail(G_UDEV_IS_CLIENT(initable), FALSE);
> g_return_val_if_fail(cancellable == NULL, FALSE);
> @@ -188,19 +183,10 @@ g_udev_client_initable_init(GInitable *initable, GCancellable *cancellable,
> self = G_UDEV_CLIENT(initable);
> priv = self->priv;
>
> - rc = libusb_init(&priv->ctx);
> - if (rc < 0) {
> - const char *errstr = spice_usbutil_libusb_strerror(rc);
> - g_warning("Error initializing USB support: %s [%i]", errstr, rc);
> - g_set_error(err, G_UDEV_CLIENT_ERROR, G_UDEV_CLIENT_LIBUSB_FAILED,
> - "Error initializing USB support: %s [%i]", errstr, rc);
> + priv->ctx = spice_usb_backend_initialize();
> + if (!priv->ctx) {
We lost the error reporting here.
Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180925/781bd7b4/attachment-0001.sig>
More information about the Spice-devel
mailing list