Re: [PATCH v7 06/10] usbdk: Load hide rules for auto-redirected devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 2016-02-28 at 10:37 +0200, Dmitry Fleytman wrote:
> 
> > On 21 Feb 2016, at 09:28 AM, Dmitry Fleytman <dmitry@xxxxxxxxxx> wrote:
> > 
> > > 
> > > On 19 Feb 2016, at 23:31 PM, Jonathon Jongsma <jjongsma@xxxxxxxxxx> 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@xxxxxxxxxx>
> > > > ---
> > > > 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


_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]