[Spice-devel] [PATCH 5/5] usb-device-manager: Configure UsbDk hiding rules on auto-redirection
Dmitry Fleytman
dmitry at daynix.com
Sun May 31 02:30:56 PDT 2015
> On May 28, 2015, at 18:48 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
>
> On Thu, May 28, 2015 at 01:24:04PM +0300, Kirill Moizik wrote:
>> From: Dmitry Fleytman <dmitry at daynix.com>
>>
>> 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 at daynix.com>
>> Signed-off-by: Dmitry Fleytman <dmitry at daynix.com>
>> ---
>> 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
>
>> + }
>> +}
>> +
>> #endif
>>
>> static SpiceUsbDevice *spice_usb_device_ref(SpiceUsbDevice *device)
>> --
>> 2.1.0
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org <mailto:Spice-devel at lists.freedesktop.org>
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel <http://lists.freedesktop.org/mailman/listinfo/spice-devel>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150531/c36b254a/attachment-0001.html>
More information about the Spice-devel
mailing list