On Thu, May 28, 2015 at 01:24:04PM +0300, Kirill Moizik wrote:From: Dmitry Fleytman <dmitry@xxxxxxxxxx>
When spice run with auto redirection rule, hide rule should be applied to detach device from host stack
during device plug-in to avoid "new hardware wizard" pop up (in case there is no driver for redirected device on host)
Signed-off-by: Kirill Moizik <kirill@xxxxxxxxxx>
Signed-off-by: Dmitry Fleytman <dmitry@xxxxxxxxxx>
---
gtk/usb-device-manager.c | 102 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 98 insertions(+), 4 deletions(-)
diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c
index 841e3a4..9786f16 100644
--- a/gtk/usb-device-manager.c
+++ b/gtk/usb-device-manager.c
@@ -29,6 +29,10 @@
#include <errno.h>
#include <libusb.h>
+#ifdef USE_WINUSB
+#include "usbdk_api.h"
+#endif
+
#if defined(USE_GUDEV)
#include <gudev/gudev.h>
#elif defined(G_OS_WIN32)
@@ -124,6 +128,8 @@ struct _SpiceUsbDeviceManagerPrivate {
#endif
#ifdef USE_WINUSB
SpiceWinUsbDriver *installer;
+ usbdk_api_wrapper usbdk_api;
+ HANDLE usbdk_hider_handle;
#endif
#endif
GPtrArray *devices;
@@ -246,13 +252,19 @@ static gboolean is_usbdk_driver_installed(void)
static void spice_usb_device_manager_init(SpiceUsbDeviceManager *self)
{
+ SpiceUsbDeviceManagerPrivate *priv =
+ SPICE_USB_DEVICE_MANAGER_GET_PRIVATE(self);
+
#ifdef USE_WINUSB
use_usbdk = is_usbdk_driver_installed();
Any reason why line was added before rather than in this commit?
Yes, this line is a part of backend selection mechanism. use_usbdk variable needs to be set and used by previous patches.
We could actually merge patches 3/4/5 into one big patch for all UsbDk-related changes, but I tend to keep patches as small as possible. -#endif
- SpiceUsbDeviceManagerPrivate *priv;
+ if(use_usbdk) {
+ if (usbdk_api_load(&priv->usbdk_api) == -1) {
+ SPICE_DEBUG("Failed to load UsbDk API DLL");
missing use_usbdk = FALSE; in error cases.
Sire. Will fix.
+ }
+ }
+#endif
- priv = SPICE_USB_DEVICE_MANAGER_GET_PRIVATE(self);
self->priv = priv;
priv->channels = g_ptr_array_new();
@@ -411,6 +423,12 @@ static void spice_usb_device_manager_finalize(GObject *gobject)
g_free(priv->auto_connect_filter);
g_free(priv->redirect_on_connect);
+#ifdef USE_WINUSB
+ if (use_usbdk) {
+ usbdk_api_unload(&priv->usbdk_api);
+ }
+#endif
You probably need if(priv->usbdk_hider_handle != NULL) { priv->usbdk_api.ClearRules(priv->usbdk_hider_handle); priv->usbdk_api.CloseHiderHandle(priv->usbdk_hider_handle); }?
Yes, good idea. +
/* Chain up to the parent class */
if (G_OBJECT_CLASS(spice_usb_device_manager_parent_class)->finalize)
G_OBJECT_CLASS(spice_usb_device_manager_parent_class)->finalize(gobject);
@@ -428,7 +446,6 @@ static void spice_usb_device_manager_get_property(GObject *gobject,
{
SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(gobject);
SpiceUsbDeviceManagerPrivate *priv = self->priv;
-
Unrelated white space change
Right. Thanks.
switch (prop_id) {
case PROP_SESSION:
g_value_set_object(value, priv->session);
@@ -448,6 +465,13 @@ static void spice_usb_device_manager_get_property(GObject *gobject,
}
}
+#ifdef USE_WINUSB
+static
+void _usbdk_autoredir_enable(SpiceUsbDeviceManager *manager);
+static
+void _usbdk_autoredir_disable(SpiceUsbDeviceManager *manager);
+#endif
+
static void spice_usb_device_manager_set_property(GObject *gobject,
guint prop_id,
const GValue *value,
@@ -462,6 +486,13 @@ static void spice_usb_device_manager_set_property(GObject *gobject,
break;
case PROP_AUTO_CONNECT:
priv->auto_connect = g_value_get_boolean(value);
+#ifdef USE_WINUSB
+ if (priv->auto_connect) {
+ _usbdk_autoredir_enable(self);
+ } else {
+ _usbdk_autoredir_disable(self);
+ }
+#endif
break;
case PROP_AUTO_CONNECT_FILTER: {
const gchar *filter = g_value_get_string(value);
@@ -1906,6 +1937,69 @@ guint8 spice_usb_device_get_state(SpiceUsbDevice *device)
return info->state;
}
+
+static
+void spice_usb_device_manager_set_rules(SpiceUsbDeviceManagerPrivate *priv)
+{
+ struct usbredirfilter_rule *rules;
+ int r, count;
+
+ r = usbredirfilter_string_to_rules(priv->redirect_on_connect, ",", "|", &rules, &count);
+ if (r) {
+ SPICE_DEBUG("auto-conenct rules parsing failed with error %d", r);
+ return;
+ }
+
+ for (int i = 0; i < count; i++) {
+ USB_DK_HIDE_RULE rule;
+ rule.Hide = (uint64_t)rules[i].allow;
+ rule.Class = (uint64_t)rules[i].device_class;
+ rule.VID = (uint64_t)rules[i].vendor_id;
+ rule.PID = (uint64_t)rules[i].product_id;
+ rule.BCD = (uint64_t)rules[i].device_version_bcd;
+ if(!priv->usbdk_api.AddRule(priv->usbdk_hider_handle, &rule)) {
+ SPICE_DEBUG("UsbDk set hide rule API failed");
+ }
+ }
+
+ free(rules);
+}
+
This helper could go to usbdk_api_wrapper maybe:usbdk_api_wrapper_set_rules(wrapper, handle, priv->redirect_on_connect) ?
I’d prefer to keep UsbDk wrapper code as independent as possible. This parsing logic looks too specific. +static
+void _usbdk_autoredir_enable(SpiceUsbDeviceManager *manager)
+{
+ if(!use_usbdk)
+ return;
+
+ SpiceUsbDeviceManagerPrivate *priv = manager->priv;
+
+ if(priv->redirect_on_connect == NULL) {
+ SPICE_DEBUG("No autoredirect rules, no hider setup needed");
+ return;
+ }
+
+ priv->usbdk_hider_handle = priv->usbdk_api.CreateHandle();
+ if(priv->usbdk_hider_handle == NULL) {
+ SPICE_DEBUG("Failed to instanciate UsbDk interface");
+ return;
+ }
+
+ spice_usb_device_manager_set_rules(priv);
+}
+
+static
+void _usbdk_autoredir_disable(SpiceUsbDeviceManager *manager)
+{
+ if(!use_usbdk)
+ return;
+
+ SpiceUsbDeviceManagerPrivate *priv = manager->priv;
+ if(priv->usbdk_hider_handle != NULL) {
+ priv->usbdk_api.ClearRules(priv->usbdk_hider_handle);
+ priv->usbdk_api.CloseHiderHandle(priv->usbdk_hider_handle);
priv->usbdk_hider_handle = NULL;
Will fix. Thanks again for the review, Christophe
|