[Spice-devel] [PATCH v6 08/10] usbdk: Load hide rules for auto-redirected devices
Dmitry Fleytman
dmitry at daynix.com
Thu Feb 11 14:14:50 UTC 2016
> On 5 Feb 2016, at 18:21 PM, Jonathon Jongsma <jjongsma at redhat.com> wrote:
>
> On Thu, 2016-02-04 at 16:40 -0600, Jonathon Jongsma wrote:
>> On Thu, 2015-10-29 at 17:26 +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 | 58
>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 58 insertions(+)
>>>
>>> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
>>> index dd55276..d5fec8f 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)
>>> @@ -121,6 +125,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;
>>> @@ -183,6 +189,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_autoredir_enable(SpiceUsbDeviceManager *manager);
>>> +static void _usbdk_autoredir_disable(SpiceUsbDeviceManager *manager);
>>> #endif
>>>
>>> static gboolean spice_usb_manager_device_equal_libdev(SpiceUsbDeviceManager
>>> *manager,
>>> @@ -364,6 +373,12 @@ static void spice_usb_device_manager_finalize(GObject
>>> *gobject)
>>> g_free(priv->auto_connect_filter);
>>> g_free(priv->redirect_on_connect);
>>>
>>> +#ifdef G_OS_WIN32
>>> + if (!priv->use_usbclerk) {
>>> + if(priv->auto_connect)
>>> + _usbdk_autoredir_disable(self);
>>> + }
>>> +#endif
>
> Another thing:
>
> Here you introduce this code within an
>
> #ifdef G_OS_WIN32
>
> block, but in the next commit, you change this to
>
> #if defined(G_OS_WIN32) && defined(USE_USBREDIR)
>
> If this change is necessary, it should be done in this commit, not in the next
> one. But it seems that you could simply move it up a few lines to the block
> where you unref the priv->installer variable, no? Or is there some reason this
> has to be done after freeing auto_connect_filter and redirect_on_connect?
Moved to a corresponding commit, thanks.
>
>
>>> /* 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);
>>> @@ -415,6 +430,15 @@ static void
>>> spice_usb_device_manager_set_property(GObject
>>> *gobject,
>>> break;
>>> case PROP_AUTO_CONNECT:
>>> priv->auto_connect = g_value_get_boolean(value);
>>
>> It does not appear that PROP_AUTO_CONNECT is a construct-only property, so
>> this
>> property could theoretically be called multiple times. From looking at the
>> _usb_autoredir_(en|dis)able() functions below, it doesn't look like a good
>> idea
>> to call them multiple times. In addition, it appears racy since
>> _usbdk_autoredir_enable() depends on redirect_on_connect. If that property is
>> not set before this one, it will return without doing anything. To avoid the
>> race, these calls should probably go into a 'constructed' vfunc.
>>
>>> +#ifdef G_OS_WIN32
>>> + if (!priv->use_usbclerk) {
>>> + 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);
>>> @@ -1856,6 +1880,40 @@ guint8 spice_usb_device_get_state(SpiceUsbDevice
>>> *device)
>>>
>>> return info->state;
>>> }
>>> +static
>>> +void _usbdk_autoredir_enable(SpiceUsbDeviceManager *manager)
>>> +{
>>> + SpiceUsbDeviceManagerPrivate *priv = manager->priv;
>>> + g_return_if_fail(!priv->use_usbclerk);
>>> +
>>> + if (priv->redirect_on_connect == NULL) {
>>
>> as I said above, if the 'redirect-on-connect' property is not set before 'auto
>> -connect', this function will return without doing anythign and print a debug
>> statement.
>>
>>> + SPICE_DEBUG("No autoredirect rules, no hider setup needed");
>>> + return;
>>> + }
>>> +
>>> + priv->usbdk_hider_handle = usbdk_create_hider_handle(priv->usbdk_api);
>>
>> Also, if this function is called multiple times, I think we'll leak the handle
>> here.
>>
>>> + if (priv->usbdk_hider_handle == NULL) {
>>> + g_warning("Failed to instantiate UsbDk hider interface");
>>> + return;
>>> + }
>>> +
>>> + usbdk_api_set_hide_rules(priv->usbdk_api,
>>> + priv->usbdk_hider_handle,
>>> + priv->redirect_on_connect);
>>> +}
>>> +
>>> +static
>>> +void _usbdk_autoredir_disable(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;
>>> + }
>>> +}
>>> #endif
>>>
>>> static SpiceUsbDevice *spice_usb_device_ref(SpiceUsbDevice *device)
>>
>>
>> Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>
>> _______________________________________________
>> 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: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160211/3a459269/attachment-0001.html>
More information about the Spice-devel
mailing list