[Spice-devel] [spice-gtk v1 2/2] usb-redirection: use usb backend for usb redirection
Yuri Benditovich
yuri.benditovich at daynix.com
Thu Sep 27 08:48:13 UTC 2018
On Tue, Sep 25, 2018 at 5:29 PM Christophe Fergeau <cfergeau at redhat.com>
wrote:
> 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.
>
It's correct to set it to NULL if used once.
I'll add comment for that.
>
> > }
> > -
> > - 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 :(
>
>
This means we'll need to send commit to gnome-boxes to check returned value.
In general, when the external application (like gnome-boxes) uses spice-gtk
and
does not create devices that do not have libusb_device, it never find ones.
Are there other uses of spice-gtk except of gnome-boxes?
> > * 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?
>
I'll return it for now and send as separate commit.
In general, it looks like the proper thing is to check redirection on
connect when the channel is up,
not when it is created.
>
> > /*
> > - * 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;
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180927/05fc6f94/attachment-0001.html>
More information about the Spice-devel
mailing list