[Spice-devel] [PATCH v7 06/10] usbdk: Load hide rules for auto-redirected devices
Jonathon Jongsma
jjongsma at redhat.com
Mon Feb 29 19:16:56 UTC 2016
On Sun, 2016-02-28 at 10:37 +0200, Dmitry Fleytman wrote:
>
> > On 21 Feb 2016, at 09:28 AM, Dmitry Fleytman <dmitry at daynix.com> wrote:
> >
> > >
> > > On 19 Feb 2016, at 23:31 PM, Jonathon Jongsma <jjongsma at redhat.com> wrote:
> > >
> > > On Thu, 2016-02-11 at 18:04 +0200, Dmitry Fleytman wrote:
> > > > Hide rules order UsbDk to avoid showing specific USB
> > > > devices to Windows PnP manager.
> > > >
> > > > Spice-gtk loads hide rules for devices that should be
> > > > automatically redirected on connection to prevent Windows
> > > > from showing "New Hardware Found" wizard window for USB
> > > > devices that do not have driver on the local system.
> > > >
> > > > Signed-off-by: Dmitry Fleytman <dmitry at daynix.com>
> > > > ---
> > > > src/usb-device-manager.c | 82
> > > > ++++++++++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 82 insertions(+)
> > > >
> > > > diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> > > > index 9a3df97..ee8e490 100644
> > > > --- a/src/usb-device-manager.c
> > > > +++ b/src/usb-device-manager.c
> > > > @@ -29,6 +29,10 @@
> > > > #include <errno.h>
> > > > #include <libusb.h>
> > > >
> > > > +#ifdef G_OS_WIN32
> > > > +#include "usbdk_api.h"
> > > > +#endif
> > > > +
> > > > #if defined(USE_GUDEV)
> > > > #include <gudev/gudev.h>
> > > > #elif defined(G_OS_WIN32)
> > > > @@ -122,6 +126,8 @@ struct _SpiceUsbDeviceManagerPrivate {
> > > > libusb_hotplug_callback_handle hp_handle;
> > > > #endif
> > > > #ifdef G_OS_WIN32
> > > > + usbdk_api_wrapper *usbdk_api;
> > > > + HANDLE usbdk_hider_handle;
> > > > SpiceWinUsbDriver *installer;
> > > > #endif
> > > > gboolean use_usbclerk;
> > > > @@ -184,6 +190,9 @@ static void spice_usb_device_unref(SpiceUsbDevice
> > > > *device);
> > > > #ifdef G_OS_WIN32
> > > > static guint8 spice_usb_device_get_state(SpiceUsbDevice *device);
> > > > static void spice_usb_device_set_state(SpiceUsbDevice *device, guint8
> > > > s);
> > > > +
> > > > +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,
> > > > @@ -359,6 +368,9 @@ static void
> > > > spice_usb_device_manager_finalize(GObject
> > > > *gobject)
> > > > #ifdef G_OS_WIN32
> > > > if (priv->installer)
> > > > g_object_unref(priv->installer);
> > > > + if (!priv->use_usbclerk) {
> > > > + _usbdk_hider_clear(self);
> > > > + }
> > > > #endif
> > > > #endif
> > > >
> > > > @@ -430,6 +442,11 @@ static void
> > > > spice_usb_device_manager_set_property(GObject
> > > > *gobject,
> > > > break;
> > > > case PROP_AUTO_CONNECT:
> > > > priv->auto_connect = g_value_get_boolean(value);
> > > > +#if defined(G_OS_WIN32) && defined(USE_USBREDIR)
> > > > + if (!priv->use_usbclerk) {
> > > > + _usbdk_hider_update(self);
> > > > + }
> > > > +#endif
> > > > break;
> > > > case PROP_AUTO_CONNECT_FILTER: {
> > > > const gchar *filter = g_value_get_string(value);
> > > > @@ -452,6 +469,12 @@ static void
> > > > spice_usb_device_manager_set_property(GObject
> > > > *gobject,
> > > > #endif
> > > > g_free(priv->auto_connect_filter);
> > > > priv->auto_connect_filter = g_strdup(filter);
> > > > +
> > > > +#if defined(G_OS_WIN32) && defined(USE_USBREDIR)
> > > > + if (!priv->use_usbclerk) {
> > > > + _usbdk_hider_update(self);
> > > > + }
> > > > +#endif
> > > > break;
> > > > }
> > > > case PROP_REDIRECT_ON_CONNECT: {
> > > > @@ -1863,6 +1886,65 @@ guint8 spice_usb_device_get_state(SpiceUsbDevice
> > > > *device)
> > > >
> > > > return info->state;
> > > > }
> > > > +
> > > > +static
> > > > +gboolean _usbdk_hider_prepare(SpiceUsbDeviceManager *manager)
> > > > +{
> > > > + SpiceUsbDeviceManagerPrivate *priv = manager->priv;
> > > > +
> > > > + g_return_val_if_fail(!priv->use_usbclerk, FALSE);
> > > > +
> > > > + if (priv->usbdk_hider_handle == NULL) {
> > > > + priv->usbdk_hider_handle = usbdk_create_hider_handle(priv
> > > > ->usbdk_api);
> > > > + if (priv->usbdk_hider_handle == NULL) {
> > > > + g_warning("Failed to instantiate UsbDk hider interface");
> > > > + return FALSE;
> > > > + }
> > > > + }
> > > > +
> > > > + return TRUE;
> > > > +}
> > > > +
> > > > +static
> > > > +void _usbdk_hider_clear(SpiceUsbDeviceManager *manager)
> > > > +{
> > > > + SpiceUsbDeviceManagerPrivate *priv = manager->priv;
> > > > +
> > > > + g_return_if_fail(!priv->use_usbclerk);
> > > > +
> > > > + if (priv->usbdk_hider_handle != NULL) {
> > > > + usbdk_clear_hide_rules(priv->usbdk_api, priv
> > > > ->usbdk_hider_handle);
> > > > + usbdk_close_hider_handle(priv->usbdk_api, priv
> > > > ->usbdk_hider_handle);
> > > > + priv->usbdk_hider_handle = NULL;
> > > > + }
> > > > +}
> > > > +
> > > > +static
> > > > +void _usbdk_hider_update(SpiceUsbDeviceManager *manager)
> > > > +{
> > > > + SpiceUsbDeviceManagerPrivate *priv = manager->priv;
> > > > +
> > > > + g_return_if_fail(!priv->use_usbclerk);
> > > > +
> > > > + if (priv->auto_connect_filter == NULL) {
> > > > + SPICE_DEBUG("No autoredirect rules, no hider setup needed");
> > > > + _usbdk_hider_clear(manager);
> > > > + return;
> > > > + }
> > > > +
> > > > + if (!priv->auto_connect) {
> > > > + SPICE_DEBUG("Auto-connect disabled, no hider setup needed");
> > > > + _usbdk_hider_clear(manager);
> > > > + return;
> > > > + }
> > > > +
> > > > + if(_usbdk_hider_prepare(manager)) {
> > > > + usbdk_api_set_hide_rules(priv->usbdk_api,
> > > > + priv->usbdk_hider_handle,
> > > > + priv->auto_connect_filter);
> > > So, in the previous version of this patch, you passed priv
> > > ->redirect_on_connect
> > > as the third parameter to usbdk_api_set_hide_rules(). Why did you change
> > > this to
> > > priv->auto_connect_filter?
> > Hi Jonathon,
> >
> > After additional code review we realised that the logic was broken there.
> > Hide rules are loaded for devices that should be redirected on plugin. This
> > is done to avoid client OS attempts to load driver driver that may not be
> > available for such devices. If I understand correctly redirect_on_connect
> > string describes devices that are redirected on session connection i.e.
> > plugged in already with client OS driver installed which is not the case for
> > hide rules. And auto_connect_filter is for devices that are redirected on
> > plug-in which is the case to be covered by hide rules.
> >
> > ~Dmitry
> Hi Jonathon,
>
> Any news regarding this patch?
>
> Thanks,
> Dmitry
So, after looking at the "auto-connect-filter" and "redirect-on-connect"
properties, it seems that you're correct and that priv->auto_connect_filter
should be used here. However, I would feel a bit more comfortable if somebody
else verified this as well, since I'm relatively new to this part of the code.
Jonathon
More information about the Spice-devel
mailing list