On Sun, Sep 30, 2018 at 06:13:04PM +0300, Yuri Benditovich wrote: > This layer communicates with libusb and libusbredir and > provides the API for USB redirection procedures. > All the modules of spice-gtk communicate only with usb > backend instead of calling libusb and usbredirhost directly. > This is prerequisite of further implementation of > cd-sharing via USB redirection. > > Signed-off-by: Yuri Benditovich <yuri.benditovich@xxxxxxxxxx> > --- > src/Makefile.am | 2 + > src/channel-usbredir-priv.h | 9 +- > src/channel-usbredir.c | 232 ++++-------- > src/meson.build | 1 + > src/usb-backend-common.c | 688 ++++++++++++++++++++++++++++++++++ > src/usb-backend.h | 115 ++++++ > src/usb-device-manager-priv.h | 3 +- > src/usb-device-manager.c | 398 +++++++------------- > src/usb-device-manager.h | 1 + > src/usbutil.c | 36 -- > src/usbutil.h | 2 - > src/win-usb-dev.c | 59 +-- > 12 files changed, 1044 insertions(+), 502 deletions(-) > create mode 100644 src/usb-backend-common.c > create mode 100644 src/usb-backend.h > > diff --git a/src/Makefile.am b/src/Makefile.am > index 1bb6f9b..3431459 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -253,6 +253,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..4064f36 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); > > 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 182edc4..0eba954 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_error(void *user_data, const char *msg); > 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, gboolean migrat > > spice_usbredir_channel_lock(channel); > > - usbredirhost_close(priv->host); > + spice_usb_backend_channel_finalize(priv->host); I'd call this _free rather than _finalize to avoid confusion with GObject terminology. > priv->host = NULL; > > /* Call set_context to re-create the host */ > @@ -238,7 +226,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 > @@ -262,33 +250,25 @@ 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) You could keep the initial 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.on_error = usbredir_error; > + init_data.write_callback = usbredir_write_callback; > > 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); > + priv->host = spice_usb_backend_channel_initialize(context, &init_data); spice_usb_backend_channel_new() ? > if (!priv->host) > - g_error("Out of memory allocating usbredirhost"); > + g_error("Out of memory initializing redirection support"); > > -#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 > @@ -299,8 +279,6 @@ static gboolean spice_usbredir_channel_open_device( > { > SpiceUsbredirChannelPrivate *priv = channel->priv; > SpiceSession *session; > - libusb_device_handle *handle = NULL; > - int rc, status; > SpiceUsbDeviceManager *manager; > > g_return_val_if_fail(priv->state == STATE_DISCONNECTED > @@ -309,21 +287,16 @@ 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, err)) { > + priv->catch_error = NULL; > + if (*err == NULL) { > + g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED, > + "Error attaching device: (no error information)"); You could line up the "Error .." with the opening parenthesis. > + } > return FALSE; > } > + priv->catch_error = NULL; > > session = spice_channel_get_session(SPICE_CHANNEL(channel)); > manager = spice_usb_device_manager_get(session, NULL); > @@ -331,7 +304,7 @@ static gboolean spice_usbredir_channel_open_device( > > 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); > + spice_usb_backend_channel_detach(priv->host); > return FALSE; > } > > @@ -362,8 +335,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); _unref rather than _release? > g_boxed_free(spice_usb_device_get_type(), priv->spice_device); > priv->spice_device = NULL; > priv->state = STATE_DISCONNECTED; > @@ -394,8 +366,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; > } > @@ -413,13 +384,16 @@ _open_device_async_cb(GTask *task, > 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)); > @@ -446,7 +420,8 @@ void spice_usbredir_channel_connect_device_async( > goto done; > } > > - priv->device = libusb_ref_device(device); > + spice_usb_backend_device_acquire(device); > + priv->device = device; You could mimic libusb API actually, priv->device = spice_usb_backend_device_ref(device); > priv->spice_device = g_boxed_copy(spice_usb_device_get_type(), > spice_device); > #ifdef USE_POLKIT > @@ -456,8 +431,8 @@ void spice_usbredir_channel_connect_device_async( > 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), > + info->bus, > + info->address, > cancellable, > spice_usbredir_channel_open_acl_cb, > channel); > @@ -515,9 +490,8 @@ void spice_usbredir_channel_disconnect_device(SpiceUsbredirChannel *channel) > 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; > @@ -568,7 +542,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; > } > @@ -583,85 +557,45 @@ 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_error(void *user_data, const char *msg) > { > SpiceUsbredirChannel *channel = user_data; > SpiceUsbredirChannelPrivate *priv = channel->priv; > > - count = MIN(priv->read_buf_size, count); > - > - if (count != 0) { > - memcpy(data, priv->read_buf, count); > - } > - > - priv->read_buf_size -= count; > - if (priv->read_buf_size) { > - priv->read_buf += count; > - } else { > - priv->read_buf = NULL; > + if (priv->catch_error) { > + g_set_error_literal(priv->catch_error, SPICE_CLIENT_ERROR, > + SPICE_CLIENT_ERROR_FAILED, msg); > + /* > + if catch_error was set once, it is correct to prevent > + further attempts to set it, they will overwrite already > + used GError, cause memory leaks and GLib warnings. > + */ > + priv->catch_error = NULL; > } > - > - return count; > } > > static void usbredir_free_write_cb_data(uint8_t *data, void *user_data) > @@ -669,7 +603,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 > @@ -741,7 +675,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 > @@ -754,15 +688,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) > { > @@ -775,25 +700,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 { > @@ -832,7 +738,7 @@ static void spice_usbredir_channel_up(SpiceChannel *c) > > g_return_if_fail(priv->host != NULL); > /* Flush any pending writes */ > - usbredirhost_write_guest_data(priv->host); > + spice_usb_backend_channel_up(priv->host); > } > > static int try_handle_compressed_msg(SpiceMsgCompressedData *compressed_data_msg, > @@ -882,26 +788,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; > @@ -915,16 +815,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 dcf4dcc..62a4c51 100644 > --- a/src/meson.build > +++ b/src/meson.build > @@ -80,6 +80,7 @@ spice_client_glib_introspection_sources = [ > 'spice-session.c', > 'spice-util.c', > 'usb-device-manager.c', > + 'usb-backend-common.c', This should go in spice_client_glib_sources together with usb-backend.h imo as suggested here https://lists.freedesktop.org/archives/spice-devel/2018-September/045818.html > ] > > spice_client_glib_sources = [ > diff --git a/src/usb-backend-common.c b/src/usb-backend-common.c > new file mode 100644 > index 0000000..6f61193 > --- /dev/null > +++ b/src/usb-backend-common.c > @@ -0,0 +1,688 @@ > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */ > +/* > + Copyright (C) 2012-2018 Red Hat, Inc. > + > + Red Hat Authors: > + Yuri Benditovich<ybendito@xxxxxxxxxx> > + Hans de Goede <hdegoede@xxxxxxxxxx> > + > + This library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + This library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with this library; if not, see <http://www.gnu.org/licenses/>. > +*/ > + > +#include "config.h" > + > +#ifdef USE_USBREDIR > + > +#include <glib-object.h> > +#include <inttypes.h> > +#include <gio/gio.h> > +#include <errno.h> > +#include <libusb.h> > +#include <string.h> > +#include <fcntl.h> > +#include "usbredirhost.h" > +#include "usbredirparser.h" > +#include "spice-util.h" > +#include "usb-backend.h" > +#if defined(G_OS_WIN32) > +#include <windows.h> > +#include "win-usb-dev.h" > +#else > +#include <sys/stat.h> > +#include <sys/ioctl.h> > +#include <linux/fs.h> > +#endif > + > +//#define LOUD_DEBUG SPICE_DEBUG > +#define LOUD_DEBUG(x, ...) > + > +struct _SpiceUsbBackendDevice > +{ > + union > + { > + libusb_device *libusb_device; In _SpiceUsbDeviceManagerPrivate, you replaced #ifndef G_OS_WIN32 libusb_device *libdev; #endif with #ifndef G_OS_WIN32 SpiceUsbBackendDevice *bdev; #endif The #ifdef is there because of this comment in spice_usb_device_manager_device_to_bdev: /* * On win32 we need to do this the hard and slow way, by asking libusb to * re-enumerate all devices and then finding a matching device. * We cannot cache the libusb_device like we do under Linux since the * driver swap we do under windows invalidates the cached libdev. */ After your patch, spice_usb_device_manager_device_to_bdev is no longer at the right level of indirection imo, it looks up the 'bdev' when needed on Windows in usb-device-manager.c, but my understanding of that comment is that any libusb call within SpiceUsbBackendDevice should not use a cached libusb_device? > + void *msc; > + } d; > + gboolean is_libusb; > + gint ref_count; > + SpiceUsbBackendChannel *attached_to; You don't need 'msc' just yet, nor 'is_libusb', and I'm not sure about 'attached_to' > + UsbDeviceInformation device_info; > +}; > + > +struct _SpiceUsbBackend > +{ > + libusb_context *libusb_context; > + usb_hot_plug_callback hotplug_callback; "usb_hotplug_callback" > + void *hotplug_user_data; > + libusb_hotplug_callback_handle hotplug_handle; > +}; > + > +struct _SpiceUsbBackendChannel > +{ > + struct usbredirhost *usbredirhost; > + uint8_t *read_buf; > + int read_buf_size; > + struct usbredirfilter_rule *rules; > + int rules_count; > + SpiceUsbBackendDevice *attached; I don't think this is used. > + SpiceUsbBackendChannelInitData channel_data; I don't think 'channel_data' is a much more descriptive than the previously used 'data'. SpiceUsbBackendChannelInitData is a bunch of vfuncs/callbacks, so maybe use one of these words in the naming? > +}; > + > +// it's unclear why we use this procedure instead of libusb_error_name, > +// which by definition supports any error that libusb returns Why not add a commit before this one switching libusb_error_name() then? > +static 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"; > +} > + > +// lock functions for usbredirhost and usbredirparser > +static void *usbredir_alloc_lock(void) { > + GMutex *mutex; > + > + mutex = g_new0(GMutex, 1); > + g_mutex_init(mutex); > + > + return mutex; > +} > + > +static void usbredir_free_lock(void *user_data) { > + GMutex *mutex = user_data; > + > + g_mutex_clear(mutex); > + g_free(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 uint8_t is_libusb_isochronous(libusb_device *libdev) > +{ > + struct libusb_config_descriptor *conf_desc; > + uint8_t isoc_found = FALSE; > + gint i, j, k; > + > + g_return_val_if_fail(libdev != NULL, 0); > + > + if (libusb_get_active_config_descriptor(libdev, &conf_desc) != 0) { > + g_return_val_if_reached(0); > + } > + > + 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; > +} > + > +static gboolean fill_usb_info(SpiceUsbBackendDevice *bdev) > +{ > + UsbDeviceInformation *info = &bdev->device_info; > + > + if (bdev->is_libusb) > + { > + struct libusb_device_descriptor desc; > + libusb_device *libdev = bdev->d.libusb_device; > + int res = libusb_get_device_descriptor(libdev, &desc); > + info->bus = libusb_get_bus_number(libdev); > + info->address = libusb_get_device_address(libdev); > + if (res < 0) { > + g_warning("cannot get device descriptor for (%p) %d.%d", > + libdev, info->bus, info->address); > + return FALSE; > + } > + info->vid = desc.idVendor; > + info->pid = desc.idProduct; > + info->class = desc.bDeviceClass; > + info->subclass = desc.bDeviceSubClass; > + info->protocol = desc.bDeviceProtocol; > + info->isochronous = is_libusb_isochronous(libdev); > + } > + return TRUE; > +} > + > +static SpiceUsbBackendDevice *allocate_backend_device(libusb_device *libdev) > +{ > + SpiceUsbBackendDevice *dev = g_new0(SpiceUsbBackendDevice, 1); > + dev->is_libusb = 1; > + dev->ref_count = 1; > + dev->d.libusb_device = libdev; > + if (!fill_usb_info(dev)) { > + g_free(dev); > + dev = NULL; > + } > + return dev; > +} > + > +/* 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) > +{ > + SpiceUsbBackendChannel *ch = user_data; > + gboolean ok = ch->channel_data.is_channel_ready(ch->channel_data.user_data); > + if (ok && ch->usbredirhost) { > + SPICE_DEBUG("%s ch %p -> usbredirhost", __FUNCTION__, ch); > + usbredirhost_write_guest_data(ch->usbredirhost); > + } else { > + SPICE_DEBUG("%s ch %p (not ready)", __FUNCTION__, ch); > + } > +} > + > +SpiceUsbBackend *spice_usb_backend_initialize(GError **error) > +{ > + int rc; > + SpiceUsbBackend *be; > + SPICE_DEBUG("%s >>", __FUNCTION__); > + be = (SpiceUsbBackend *)g_new0(SpiceUsbBackend, 1); > + rc = libusb_init(&be->libusb_context); > + if (rc < 0) { > + const char *desc = spice_usbutil_libusb_strerror(rc); > + g_warning("Error initializing LIBUSB support: %s [%i]", desc, rc); > + g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED, > + "Error initializing LIBUSB support: %s [%i]", desc, rc); We usually align with the opening ( > + g_free(be); > + be = NULL; > + } else { > +#ifdef G_OS_WIN32 > +#if LIBUSB_API_VERSION >= 0x01000106 > + libusb_set_option(be->libusb_context, LIBUSB_OPTION_USE_USBDK); This could be indented one more level. > +#endif > +#endif > + } > + SPICE_DEBUG("%s <<", __FUNCTION__); > + return be; > +} > + > +gboolean spice_usb_backend_handle_events(SpiceUsbBackend *be) > +{ > + SPICE_DEBUG("%s >>", __FUNCTION__); > + gboolean ok = FALSE; > + if (be->libusb_context) { > + SPICE_DEBUG("%s >> libusb", __FUNCTION__); > + int res = libusb_handle_events(be->libusb_context); > + ok = res == 0; > + if (res && res != LIBUSB_ERROR_INTERRUPTED) { > + const char *desc = spice_usbutil_libusb_strerror(res); > + g_warning("Error handling USB events: %s [%i]", desc, res); > + ok = FALSE; You don't really need 'ok' in that method, just return FALSE; here... > + } > + } > + SPICE_DEBUG("%s << %s %d", __FUNCTION__, > + be->libusb_context ? "libusb" : "no libusb", ok); > + return ok; ... and return TRUE; there (and I'm not sure how useful the SPICE_DEBUG are going to be?) > +} > + > +static int LIBUSB_CALL hotplug_callback(libusb_context *ctx, > + libusb_device *device, > + libusb_hotplug_event event, > + void *user_data) > +{ > + SpiceUsbBackend *be = (SpiceUsbBackend *)user_data; > + if (be->hotplug_callback) { > + SpiceUsbBackendDevice *dev; > + gboolean val = event == LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED; > + dev = allocate_backend_device(device); > + if (dev) { > + SPICE_DEBUG("created dev %p, usblib dev %p", dev, device); > + libusb_ref_device(device); > + be->hotplug_callback(be->hotplug_user_data, dev, val); > + spice_usb_backend_device_release(dev); > + } > + } > + return 0; > +} > + > +gboolean spice_usb_backend_handle_hotplug(SpiceUsbBackend *be, > + void *user_data, > + usb_hot_plug_callback proc) Why not spice_usb_backend_enable_hotplug/spice_usb_backend_disable_hotplug? (and now that we have a proper SpiceUsbBackend struct wrapping the libusb stuff, I wonder if it would not make sense to just turn it into a gobject, and emit signals when we get new devices). > +{ > + int rc; > + g_return_val_if_fail(be != NULL, FALSE); > + if (!proc) { > + if (be->hotplug_handle) { > + libusb_hotplug_deregister_callback(be->libusb_context, be->hotplug_handle); > + be->hotplug_handle = 0; > + } > + be->hotplug_callback = proc; > + return TRUE; > + } > + > + be->hotplug_callback = proc; > + be->hotplug_user_data = user_data; > + rc = libusb_hotplug_register_callback(be->libusb_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, > + hotplug_callback, be, &be->hotplug_handle); > + if (rc != LIBUSB_SUCCESS) { > + const char *desc = spice_usbutil_libusb_strerror(rc); > + g_warning("Error initializing USB hotplug support: %s [%i]", desc, rc); > + be->hotplug_callback = NULL; > + return FALSE; > + } > + return TRUE; > +} > + > +void spice_usb_backend_finalize(SpiceUsbBackend *be) > +{ > + g_return_if_fail(be != NULL); > + SPICE_DEBUG("%s >>", __FUNCTION__); > + if (be->libusb_context) { > + libusb_exit(be->libusb_context); > + } > + g_free(be); > + SPICE_DEBUG("%s <<", __FUNCTION__); > +} > + > +SpiceUsbBackendDevice **spice_usb_backend_get_device_list(SpiceUsbBackend *be) > +{ > + LOUD_DEBUG("%s >>", __FUNCTION__); > + libusb_device **devlist = NULL, **dev; > + SpiceUsbBackendDevice *d, **list; > + > + int n = 0, index; > + > + if (be && be->libusb_context) { > + libusb_get_device_list(be->libusb_context, &devlist); > + } > + > + // add all the libusb device that not present in our list > + for (dev = devlist; dev && *dev; dev++) { > + n++; > + } > + > + list = g_new0(SpiceUsbBackendDevice*, n + 1); > + > + index = 0; > + > + for (dev = devlist; dev && *dev; dev++) { > + d = allocate_backend_device(*dev); > + if (!d) { > + libusb_unref_device(*dev); > + } else { > + SPICE_DEBUG("created dev %p, usblib dev %p", d, *dev); > + list[index++] = d; > + } > + } > + > + if (devlist) { > + libusb_free_device_list(devlist, 0); > + } > + > + LOUD_DEBUG("%s <<", __FUNCTION__); > + return list; > +} I still think this could be made slightly simpler with GArray/GPtrArray. > + > +gboolean spice_usb_backend_device_is_hub(SpiceUsbBackendDevice *dev) > +{ > + return dev->device_info.class == LIBUSB_CLASS_HUB; > +} > + > +const UsbDeviceInformation* spice_usb_backend_device_get_info(SpiceUsbBackendDevice *dev) > +{ > + return &dev->device_info; > +} > + > +gboolean spice_usb_backend_devices_same(SpiceUsbBackendDevice *dev1, > + SpiceUsbBackendDevice *dev2) > +{ > + if (dev1->is_libusb != dev2->is_libusb) { > + return FALSE; > + } > + if (dev1->is_libusb) { > + return dev1->d.libusb_device == dev2->d.libusb_device; > + } > + return dev1 == dev2; > +} > + > +gconstpointer spice_usb_backend_device_get_libdev(SpiceUsbBackendDevice *dev) > +{ > + if (dev->is_libusb) { > + return dev->d.libusb_device; > + } > + return NULL; > +} > + > +void spice_usb_backend_free_device_list(SpiceUsbBackendDevice **devlist) > +{ > + LOUD_DEBUG("%s >>", __FUNCTION__); > + SpiceUsbBackendDevice **dev; > + for (dev = devlist; *dev; dev++) { > + SpiceUsbBackendDevice *d = *dev; > + spice_usb_backend_device_release(d); > + } > + g_free(devlist); > + LOUD_DEBUG("%s <<", __FUNCTION__); > +} > + > +void spice_usb_backend_device_acquire(SpiceUsbBackendDevice *dev) > +{ > + LOUD_DEBUG("%s >> %p", __FUNCTION__, dev); > + g_atomic_int_inc(&dev->ref_count); > +} > + > +void spice_usb_backend_device_release(SpiceUsbBackendDevice *dev) > +{ > + LOUD_DEBUG("%s >> %p(%d)", __FUNCTION__, dev, dev->ref_count); > + if (g_atomic_int_dec_and_test(&dev->ref_count)) { > + if (dev->is_libusb) { > + libusb_unref_device(dev->d.libusb_device); > + LOUD_DEBUG("%s freeing %p (libusb %p)", __FUNCTION__, dev, dev->d.libusb_device); > + g_free(dev); > + } > + } > +} > + > +int spice_usb_backend_device_check_filter( > + SpiceUsbBackendDevice *dev, > + const struct usbredirfilter_rule *rules, > + int count) > +{ > + if (dev->is_libusb) { > + return usbredirhost_check_device_filter( > + rules, count, dev->d.libusb_device, 0); > + } > + return -1; > +} > + > +static int usbredir_read_callback(void *user_data, uint8_t *data, int count) > +{ > + SpiceUsbBackendChannel *ch = user_data; > + > + count = MIN(ch->read_buf_size, count); > + > + if (count != 0) { > + memcpy(data, ch->read_buf, count); > + } > + > + ch->read_buf_size -= count; > + if (ch->read_buf_size) { > + ch->read_buf += count; > + } > + else { > + ch->read_buf = NULL; > + } > + SPICE_DEBUG("%s ch %p, %d bytes", __FUNCTION__, ch, count); > + > + return count; > +} > + > +static const char *strip_usbredir_prefix(const char *msg) > +{ > + if (strncmp(msg, "usbredirhost: ", 14) == 0) { > + msg += 14; > + } > + return msg; > +} > + > +static void usbredir_log(void *user_data, int level, const char *msg) > +{ > + SpiceUsbBackendChannel *ch = (SpiceUsbBackendChannel *)user_data; > + const char *stripped_msg = strip_usbredir_prefix(msg); > + switch (level) { > + case usbredirparser_error: > + g_critical("%s", msg); > + ch->channel_data.on_error(ch->channel_data.user_data, stripped_msg); > + break; > + case usbredirparser_warning: > + g_warning("%s", msg); > + ch->channel_data.on_error(ch->channel_data.user_data, stripped_msg); > + break; > + default: > + break; > + } > +} > + > +static int usbredir_write_callback(void *user_data, uint8_t *data, int count) > +{ > + SpiceUsbBackendChannel *ch = user_data; > + int res; > + SPICE_DEBUG("%s ch %p, %d bytes", __FUNCTION__, ch, count); > + res = ch->channel_data.write_callback(ch->channel_data.user_data, data, count); > + return res; > +} > + > +#if USBREDIR_VERSION >= 0x000701 > +static uint64_t usbredir_buffered_output_size_callback(void *user_data) > +{ > + SpiceUsbBackendChannel *ch = user_data; > + return ch->channel_data.get_queue_size(ch->channel_data.user_data); > +} > +#endif > + > +int spice_usb_backend_provide_read_data(SpiceUsbBackendChannel *ch, uint8_t *data, int count) > +{ > + int res = 0; > + > + g_return_val_if_fail(ch->read_buf == NULL, USB_REDIR_ERROR_READ_PARSE); > + > + ch->read_buf = data; > + ch->read_buf_size = count; > + if (ch->usbredirhost) { > + res = usbredirhost_read_guest_data(ch->usbredirhost); > + } else { > + res = USB_REDIR_ERROR_IO; > + } > + switch (res) > + { > + case usbredirhost_read_io_error: > + res = USB_REDIR_ERROR_IO; > + break; > + case usbredirhost_read_parse_error: > + res = USB_REDIR_ERROR_READ_PARSE; > + break; > + case usbredirhost_read_device_rejected: > + res = USB_REDIR_ERROR_DEV_REJECTED; > + break; > + case usbredirhost_read_device_lost: > + res = USB_REDIR_ERROR_DEV_LOST; > + break; > + } > + SPICE_DEBUG("%s ch %p, %d bytes, res %d", __FUNCTION__, ch, count, res); > + > + return res; > +} > + > +void spice_usb_backend_return_write_data(SpiceUsbBackendChannel *ch, void *data) > +{ > + if (ch->usbredirhost) { > + SPICE_DEBUG("%s ch %p", __FUNCTION__, ch); > + usbredirhost_free_write_buffer(ch->usbredirhost, data); > + } else { > + SPICE_DEBUG("%s ch %p - NOBODY TO CALL", __FUNCTION__, ch); > + } > +} > + > +gboolean spice_usb_backend_channel_attach(SpiceUsbBackendChannel *ch, > + SpiceUsbBackendDevice *dev, > + GError **error) > +{ > + SPICE_DEBUG("%s >> ch %p, dev %p (was attached %p)", __FUNCTION__, ch, dev, ch->attached); > + gboolean ok = FALSE; > + if (!dev) { > + return ok; > + } > + > + if (dev->is_libusb) { > + libusb_device_handle *handle = NULL; > + int rc = libusb_open(dev->d.libusb_device, &handle); > + ok = rc == 0; > + if (ok) { > + rc = usbredirhost_set_device(ch->usbredirhost, handle); > + if (rc) { > + SPICE_DEBUG("%s ch %p, dev %p usbredirhost error %d", __FUNCTION__, ch, dev, rc); > + ok = FALSE; > + } else { > + ch->attached = dev; > + dev->attached_to = ch; > + } > + } else { > + const char *desc = spice_usbutil_libusb_strerror(rc); > + g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED, > + "Error libusb_open: %s [%i]", desc, rc); > + } > + } > + > + return ok; > +} > + > +void spice_usb_backend_channel_detach(SpiceUsbBackendChannel *ch) > +{ > + SPICE_DEBUG("%s >> ch %p, was attached %p", __FUNCTION__, ch, ch->attached); > + if (!ch->attached) { > + SPICE_DEBUG("%s: nothing to detach", __FUNCTION__); > + return; > + } > + if (ch->usbredirhost) { > + // it will call libusb_close internally > + usbredirhost_set_device(ch->usbredirhost, NULL); > + } > + SPICE_DEBUG("%s ch %p, detach done", __FUNCTION__, ch); > + ch->attached->attached_to = NULL; > + ch->attached = NULL; > +} > + > +SpiceUsbBackendChannel *spice_usb_backend_channel_initialize(SpiceUsbBackend *be, > + const SpiceUsbBackendChannelInitData *init_data) > +{ > + SpiceUsbBackendChannel *ch = g_new0(SpiceUsbBackendChannel, 1); > + SPICE_DEBUG("%s >>", __FUNCTION__); > + ch->channel_data = *init_data; > + if (be->libusb_context) { You probably can just have g_return_val_if_fail(be->libusb_context != NULL, NULL) at the very beginning of that method, and only do the allocation after that. > + ch->usbredirhost = usbredirhost_open_full( > + be->libusb_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, > + ch, PACKAGE_STRING, > + spice_util_get_debug() ? usbredirparser_debug : usbredirparser_warning, > + usbredirhost_fl_write_cb_owns_buffer); > + g_warn_if_fail(ch->usbredirhost != NULL); > + } > + if (ch->usbredirhost) { > +#if USBREDIR_VERSION >= 0x000701 > + usbredirhost_set_buffered_output_size_cb(ch->usbredirhost, usbredir_buffered_output_size_callback); > +#endif > + } else { > + g_free(ch); > + ch = NULL; > + } > + > + SPICE_DEBUG("%s << %p", __FUNCTION__, ch); > + return ch; > +} > + > +void spice_usb_backend_channel_up(SpiceUsbBackendChannel *ch) > +{ > + SPICE_DEBUG("%s %p, host %p", __FUNCTION__, ch, ch->usbredirhost); > + if (ch->usbredirhost) { > + usbredirhost_write_guest_data(ch->usbredirhost); > + } > +} > + > +void spice_usb_backend_channel_finalize(SpiceUsbBackendChannel *ch) > +{ > + SPICE_DEBUG("%s >> %p", __FUNCTION__, ch); > + if (ch->usbredirhost) { > + usbredirhost_close(ch->usbredirhost); > + } > + > + if (ch->rules) { > + g_free(ch->rules); Should be 'free(ch->rules);' as this was allocated by libusbredir. > + } > + > + g_free(ch); > + SPICE_DEBUG("%s << %p", __FUNCTION__, ch); > +} > + > +void spice_usb_backend_channel_get_guest_filter( > + SpiceUsbBackendChannel *ch, > + const struct usbredirfilter_rule **r, > + int *count) > +{ > + int i; > + *r = NULL; > + *count = 0; > + if (ch->usbredirhost) { > + usbredirhost_get_guest_filter(ch->usbredirhost, r, count); > + } > + if (*r == NULL) { > + *r = ch->rules; > + *count = ch->rules_count; > + } > + > + if (*count) { > + SPICE_DEBUG("%s ch %p: %d filters", __FUNCTION__, ch, *count); > + } > + for (i = 0; i < *count; i++) { > + const struct usbredirfilter_rule *ra = *r; > + SPICE_DEBUG("%s class %d, %X:%X", > + ra[i].allow ? "allowed" : "denied", ra[i].device_class, > + (uint32_t)ra[i].vendor_id, (uint32_t)ra[i].product_id); > + } > +} > + > +#endif // USB_REDIR > diff --git a/src/usb-backend.h b/src/usb-backend.h > new file mode 100644 > index 0000000..31c31eb > --- /dev/null > +++ b/src/usb-backend.h > @@ -0,0 +1,115 @@ > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */ > +/* > + Copyright (C) 2018 Red Hat, Inc. > + > + Red Hat Authors: > + Yuri Benditovich<ybendito@xxxxxxxxxx> > + > + This library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + This library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with this library; if not, see <http://www.gnu.org/licenses/>. > +*/ > + > +#ifndef __SPICE_USB_BACKEND_H__ > +#define __SPICE_USB_BACKEND_H__ > + > +#include <usbredirfilter.h> > +#include "usb-device-manager.h" > + > +G_BEGIN_DECLS > + > +typedef struct _SpiceUsbBackend SpiceUsbBackend; > +typedef struct _SpiceUsbBackendDevice SpiceUsbBackendDevice; > +typedef struct _SpiceUsbBackendChannel SpiceUsbBackendChannel; > + > +typedef struct UsbDeviceInformation > +{ > + uint16_t bus; > + uint16_t address; > + uint16_t vid; > + uint16_t pid; > + uint8_t class; > + uint8_t subclass; > + uint8_t protocol; > + uint8_t isochronous; > +} UsbDeviceInformation; As already mentioned in the past, please let's not duplicate very similar information with different names/different types in 2 structures with very close names (UsbDeviceInformation and SpiceUsbDeviceInfo) > + > +typedef void (*usb_channel_error_callback)(void *user_data, const char *msg); > +typedef int (*usb_channel_write_callback)(void *user_data, uint8_t *data, int count); > +typedef gboolean (*usb_channel_is_ready_callback)(void *user_data); > +typedef uint64_t (*usb_channel_get_queue_size)(void *user_data); > + > +typedef struct SpiceUsbBackendChannelInitData > +{ > + void *user_data; > + usb_channel_error_callback on_error; > + usb_channel_write_callback write_callback; > + usb_channel_is_ready_callback is_channel_ready; > + usb_channel_get_queue_size get_queue_size; I would not add typedef for these vfuncs, I think the types are only used here, so I'd directly use: void *user_data; void (*on_error)(void *user_data, const char *msg); ... > +} SpiceUsbBackendChannelInitData; > + > +typedef void(*usb_hot_plug_callback)(void *user_data, SpiceUsbBackendDevice *dev, gboolean added); usb_hotplug_callback. > + > +enum { > + USB_REDIR_ERROR_IO = -1, > + USB_REDIR_ERROR_READ_PARSE = -2, > + USB_REDIR_ERROR_DEV_REJECTED = -3, > + USB_REDIR_ERROR_DEV_LOST = -4, > +}; > + > +/* Spice USB backend API */ > +/* sets error on failure */ > +SpiceUsbBackend *spice_usb_backend_initialize(GError **error); > +void spice_usb_backend_finalize(SpiceUsbBackend *context); > + > +/* > +returns newly-allocated null-terminated list of s/list/array/ > +SpiceUsbBackendDevice pointers. > +The caller must call spice_usb_backend_free_device_list > +after it finishes list processing "processing the list" > +*/ > +SpiceUsbBackendDevice **spice_usb_backend_get_device_list(SpiceUsbBackend *backend); > +void spice_usb_backend_free_device_list(SpiceUsbBackendDevice **devlist); > +gboolean spice_usb_backend_handle_events(SpiceUsbBackend *be); > +gboolean spice_usb_backend_handle_hotplug(SpiceUsbBackend *be, > + void *user_data, usb_hot_plug_callback proc); > + > +/* Spice USB backend device API */ > +gboolean spice_usb_backend_device_is_hub(SpiceUsbBackendDevice *dev); > +void spice_usb_backend_device_acquire(SpiceUsbBackendDevice *dev); > +void spice_usb_backend_device_release(SpiceUsbBackendDevice *dev); > +gboolean spice_usb_backend_devices_same(SpiceUsbBackendDevice *dev1, SpiceUsbBackendDevice *dev2); > +gconstpointer spice_usb_backend_device_get_libdev(SpiceUsbBackendDevice *dev); > +const UsbDeviceInformation* spice_usb_backend_device_get_info(SpiceUsbBackendDevice *dev); > +/* returns 0 if the device passes the filter */ > +int spice_usb_backend_device_check_filter(SpiceUsbBackendDevice *dev, > + const struct usbredirfilter_rule *rules, int count); > + > +/* Spice USB backend channel API */ > +SpiceUsbBackendChannel *spice_usb_backend_channel_initialize(SpiceUsbBackend *context, > + const SpiceUsbBackendChannelInitData *init_data); > +void spice_usb_backend_channel_finalize(SpiceUsbBackendChannel *ch); > +/* returns 0 for success or error code */ > +int spice_usb_backend_provide_read_data(SpiceUsbBackendChannel *ch, uint8_t *data, int count); > +gboolean spice_usb_backend_channel_attach(SpiceUsbBackendChannel *ch, > + SpiceUsbBackendDevice *dev, > + GError **error); > +void spice_usb_backend_channel_detach(SpiceUsbBackendChannel *ch); > +void spice_usb_backend_channel_up(SpiceUsbBackendChannel *ch); > +void spice_usb_backend_channel_get_guest_filter(SpiceUsbBackendChannel *ch, > + const struct usbredirfilter_rule **rules, > + int *count); > +void spice_usb_backend_return_write_data(SpiceUsbBackendChannel *ch, void *data); > + > +G_END_DECLS > + > +#endif > diff --git a/src/usb-device-manager-priv.h b/src/usb-device-manager-priv.h > index 83884d7..4a1b592 100644 > --- a/src/usb-device-manager-priv.h > +++ b/src/usb-device-manager-priv.h > @@ -22,6 +22,7 @@ > #define __SPICE_USB_DEVICE_MANAGER_PRIV_H__ > > #include "usb-device-manager.h" > +#include "usb-backend.h" > > G_BEGIN_DECLS > > @@ -32,7 +33,7 @@ void spice_usb_device_manager_stop_event_listening( > SpiceUsbDeviceManager *manager); > > #ifdef USE_USBREDIR > -#include <libusb.h> > + > void spice_usb_device_manager_device_error( > SpiceUsbDeviceManager *manager, SpiceUsbDevice *device, GError *err); > > diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c > index 354038a..e7c363a 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; > @@ -148,7 +148,7 @@ typedef struct _SpiceUsbDeviceInfo { > #ifdef G_OS_WIN32 > guint8 state; > #else > - libusb_device *libdev; > + SpiceUsbBackendDevice *bdev; > #endif > gint ref; > } SpiceUsbDeviceInfo; > @@ -168,15 +168,14 @@ 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); > #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); > > @@ -185,12 +184,12 @@ 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, > - SpiceUsbDevice *device, > - libusb_device *libdev); > -static libusb_device * > -spice_usb_device_manager_device_to_libdev(SpiceUsbDeviceManager *self, > - SpiceUsbDevice *device); > +static gboolean spice_usb_manager_device_equal_bdev(SpiceUsbDeviceManager *manager, > + SpiceUsbDevice *device, > + SpiceUsbBackendDevice *bdev); > +static SpiceUsbBackendDevice* > +spice_usb_device_manager_device_to_bdev(SpiceUsbDeviceManager *self, > + SpiceUsbDevice *device); > > static void > _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self, > @@ -290,27 +289,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(err); > + if (!priv->context) { > 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); > @@ -321,26 +308,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"); You don't need to change the indentation here > return FALSE; > } > spice_usb_device_manager_start_event_listening(self, NULL); > @@ -371,21 +352,19 @@ 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 */ Indentation > + 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) { > g_warn_if_fail(g_atomic_int_get(&priv->event_thread_run) == FALSE); > @@ -413,8 +392,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 > @@ -739,7 +719,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( > @@ -763,40 +743,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) > * Since: 0.27 > **/ > gconstpointer > @@ -808,34 +764,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, > @@ -854,9 +789,8 @@ static void channel_new(SpiceSession *session, SpiceChannel *channel, > g_signal_connect(channel, "channel-event", G_CALLBACK(channel_event), self); > > spice_usb_device_manager_check_redir_on_connect(self, channel); > - > /* > - * 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. > */ > @@ -933,12 +867,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) Indentation > { > + 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 > > @@ -960,21 +894,17 @@ 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) Here I don't think you need that many spaces before *self and *bdev > { > SpiceUsbDeviceManagerPrivate *priv = self->priv; > - struct libusb_device_descriptor desc; > SpiceUsbDevice *device; > > - if (!spice_usb_device_manager_get_device_descriptor(libdev, &desc)) > - return; > - > /* 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; > > @@ -986,10 +916,10 @@ static void spice_usb_device_manager_add_dev(SpiceUsbDeviceManager *self, > 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, > @@ -1036,7 +966,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 *bdev = NULL, **dev_list = NULL; > SpiceUsbDevice *device; > const gchar *devtype; > int i, bus, address; > @@ -1064,23 +994,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)) { > + bdev = dev_list[i]; > break; > } > } > > - if (libdev) > - spice_usb_device_manager_add_dev(self, libdev); > + if (bdev) > + spice_usb_device_manager_add_dev(self, bdev); > 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, > @@ -1109,8 +1039,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) > @@ -1118,36 +1048,31 @@ 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: > - spice_usb_device_manager_remove_dev(self, > - libusb_get_bus_number(args->device), > - libusb_get_device_address(args->device)); > - break; > + } else { > + const UsbDeviceInformation *info = spice_usb_backend_device_get_info(args->device); > + spice_usb_device_manager_remove_dev(self, info->bus, info->address); > } > - 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) > { > 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 > > @@ -1174,13 +1099,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; > } > } > @@ -1231,7 +1152,7 @@ static void spice_usb_device_manager_check_redir_on_connect( > SpiceUsbDeviceManagerPrivate *priv = self->priv; > GTask *task; > SpiceUsbDevice *device; > - libusb_device *libdev; > + SpiceUsbBackendDevice *dev; > guint i; > > if (priv->redirect_on_connect == NULL) > @@ -1243,15 +1164,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, > @@ -1261,14 +1182,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); > } > } > > @@ -1292,8 +1213,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; > } > @@ -1350,13 +1271,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)); > @@ -1430,7 +1351,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)) { > @@ -1446,9 +1367,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 > @@ -1466,12 +1387,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; > } > > @@ -1733,20 +1654,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")); > @@ -1837,64 +1758,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; > + const UsbDeviceInformation *devinfo; > > - g_return_val_if_fail(libdev != NULL, NULL); > - > - bus = libusb_get_bus_number(libdev); > - addr = libusb_get_device_address(libdev); > - > - 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; > @@ -2032,50 +1919,54 @@ 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, > - SpiceUsbDevice *device, > - libusb_device *libdev) > +spice_usb_manager_device_equal_bdev(SpiceUsbDeviceManager *manager, > + SpiceUsbDevice *device, > + SpiceUsbBackendDevice *bdev) > { > 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, > - busnum, devaddr); > + return spice_usb_device_manager_bdev_match(manager, bdev, > + busnum, devaddr); > } > #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, > - SpiceUsbDevice *device) > +static SpiceUsbBackendDevice * > +spice_usb_device_manager_device_to_bdev(SpiceUsbDeviceManager *self, > + SpiceUsbDevice *device) > { > #ifdef G_OS_WIN32 > /* > @@ -2085,7 +1976,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); > @@ -2093,26 +1984,27 @@ 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); > + spice_usb_backend_device_acquire(info->bdev); > + return info->bdev; > #endif > } > + > #endif /* USE_USBREDIR */ > diff --git a/src/usb-device-manager.h b/src/usb-device-manager.h > index 773208f..4ba7b2f 100644 > --- a/src/usb-device-manager.h > +++ b/src/usb-device-manager.h > @@ -87,6 +87,7 @@ struct _SpiceUsbDeviceManagerClass > SpiceUsbDevice *device, GError *error); > void (*device_error) (SpiceUsbDeviceManager *manager, > SpiceUsbDevice *device, GError *error); > + Spurious whitespace change. > /*< private >*/ > /* > * If adding fields to this struct, remove corresponding > 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..e5b6d62 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; > } Why did you remove the g_set_error? > > @@ -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(err); > + if (!priv->ctx) { > return FALSE; > } > -#ifdef G_OS_WIN32 > -#if LIBUSB_API_VERSION >= 0x01000106 > - libusb_set_option(priv->ctx, LIBUSB_OPTION_USE_USBDK); > -#endif > -#endif > > /* get initial device list */ > if (g_udev_client_list_devices(self, &priv->udev_list, err, __FUNCTION__) < 0) { > @@ -267,7 +253,7 @@ static void g_udev_client_finalize(GObject *gobject) > > /* free libusb context initializing by libusb_init() */ > g_warn_if_fail(priv->ctx != NULL); > - libusb_exit(priv->ctx); > + spice_usb_backend_finalize(priv->ctx); > > /* Chain up to the parent class */ > if (G_OBJECT_CLASS(g_udev_client_parent_class)->finalize) > @@ -356,23 +342,18 @@ static void g_udev_client_class_init(GUdevClientClass *klass) > g_object_class_install_property(gobject_class, PROP_REDIRECTING, pspec); > } > > -static gboolean get_usb_dev_info(libusb_device *dev, GUdevDeviceInfo *udevinfo) > +static gboolean get_usb_dev_info(SpiceUsbBackendDevice *dev, GUdevDeviceInfo *udevinfo) > { > - struct libusb_device_descriptor desc; > + const UsbDeviceInformation* info = spice_usb_backend_device_get_info(dev); > > g_return_val_if_fail(dev, FALSE); > g_return_val_if_fail(udevinfo, FALSE); > > - if (libusb_get_device_descriptor(dev, &desc) < 0) { > - g_warning("cannot get device descriptor %p", dev); > - return FALSE; > - } > - > - udevinfo->bus = libusb_get_bus_number(dev); > - udevinfo->addr = libusb_get_device_address(dev); > - udevinfo->class = desc.bDeviceClass; > - udevinfo->vid = desc.idVendor; > - udevinfo->pid = desc.idProduct; > + udevinfo->bus = info->bus; > + udevinfo->addr = info->address; > + udevinfo->class = info->class; > + udevinfo->vid = info->vid; > + udevinfo->pid = info->pid; > snprintf(udevinfo->sclass, sizeof(udevinfo->sclass), "%d", udevinfo->class); > snprintf(udevinfo->sbus, sizeof(udevinfo->sbus), "%d", udevinfo->bus); > snprintf(udevinfo->saddr, sizeof(udevinfo->saddr), "%d", udevinfo->addr); > @@ -573,7 +554,7 @@ static gboolean g_udev_skip_search(GUdevDevice *udev) > #if defined(LIBUSBX_API_VERSION) && (LIBUSBX_API_VERSION >= 0x010000FF) > (udevinfo->addr == 1) || /* root hub addr for libusbx >= 1.0.13 */ > #endif > - (udevinfo->class == LIBUSB_CLASS_HUB) || /* hub*/ > + (udevinfo->class == USB_CLASS_HUB) || /* hub*/ > (udevinfo->addr == 0)); /* bad address */ > return skip; > } Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel