<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""> <div><blockquote type="cite" class=""><div class="">On 5 Feb 2016, at 18:31 PM, Jonathon Jongsma <<a href="mailto:jjongsma@redhat.com" class="">jjongsma@redhat.com</a>> wrote:</div> <div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On Thu, 2015-10-29 at 17:26 +0200, Dmitry Fleytman wrote:<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">Signed-off-by: Dmitry Fleytman <<a href="mailto:dmitry@daynix.com" class="">dmitry@daynix.com</a>> --- src/usb-device-manager.c | 50 ++++++++++++++++++++++++++++++----------------- - 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c index d5fec8f..53820b4 100644 --- a/src/usb-device-manager.c +++ b/src/usb-device-manager.c @@ -230,7 +230,7 @@ static void spice_usb_device_manager_init(SpiceUsbDeviceManager *self) priv = SPICE_USB_DEVICE_MANAGER_GET_PRIVATE(self); self->priv = priv; -#ifdef G_OS_WIN32 +#if defined(G_OS_WIN32) && defined(USE_USBREDIR) priv->use_usbclerk = TRUE; #endif </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">This particular change seems a bit unrelated to this commit. It probably should<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">be moved to the patch where priv->use_usbclerk was introduced, no?<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div> </div><div>Moved, thanks.</div> <blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""> priv->channels = g_ptr_array_new(); @@ -255,10 +255,12 @@ static gboolean spice_usb_device_manager_initable_init(GInitable *initable, #endif #ifdef G_OS_WIN32 - priv->installer = spice_win_usb_driver_new(err); - if (!priv->installer) { - SPICE_DEBUG("failed to initialize winusb driver"); - return FALSE; + if (priv->use_usbclerk) { + priv->installer = spice_win_usb_driver_new(err); + if (!priv->installer) { + SPICE_DEBUG("failed to initialize winusb driver"); + return FALSE; + } } #endif @@ -365,15 +367,17 @@ static void spice_usb_device_manager_finalize(GObject *gobject) free(priv->auto_conn_filter_rules); free(priv->redirect_on_connect_rules); #ifdef G_OS_WIN32 - if (priv->installer) + if (priv->installer) { + g_warn_if_fail(priv->use_usbclerk); g_object_unref(priv->installer); + } #endif #endif g_free(priv->auto_connect_filter); g_free(priv->redirect_on_connect); -#ifdef G_OS_WIN32 +#if defined(G_OS_WIN32) && defined(USE_USBREDIR) </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">As I mentioned in the review for patch 8, this change shouldn be done earlier in<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">an earlier patch. it's not directly related to this patch.<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div> </div><div>Moved, thanks.</div> <blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""> if (!priv->use_usbclerk) { if(priv->auto_connect) _usbdk_autoredir_disable(self); @@ -430,7 +434,7 @@ static void spice_usb_device_manager_set_property(GObject *gobject, break; case PROP_AUTO_CONNECT: priv->auto_connect = g_value_get_boolean(value); -#ifdef G_OS_WIN32 +#if defined(G_OS_WIN32) && defined(USE_USBREDIR) </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Same as above. Consider moving to an earlier patch if the change is needed.<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div> </div><div>Moved, thanks.</div> <blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""> if (!priv->use_usbclerk) { if (priv->auto_connect) { _usbdk_autoredir_enable(self); @@ -935,12 +939,14 @@ static void spice_usb_device_manager_remove_dev(SpiceUsbDeviceManager *self, } #ifdef G_OS_WIN32 - const guint8 state = spice_usb_device_get_state(device); - if ((state == SPICE_USB_DEVICE_STATE_INSTALLING) || - (state == SPICE_USB_DEVICE_STATE_UNINSTALLING)) { - SPICE_DEBUG("skipping " DEV_ID_FMT ". It is un/installing its driver", - bus, address); - return; + if (priv->use_usbclerk) { + const guint8 state = spice_usb_device_get_state(device); + if ((state == SPICE_USB_DEVICE_STATE_INSTALLING) || + (state == SPICE_USB_DEVICE_STATE_UNINSTALLING)) { + SPICE_DEBUG("skipping " DEV_ID_FMT ". It is un/installing its driver", + bus, address); + return; + } } #endif @@ -1141,6 +1147,7 @@ static void spice_usb_device_manager_drv_install_cb(GObject *gobject, g_free(cbinfo); g_return_if_fail(SPICE_IS_USB_DEVICE_MANAGER(self)); + g_return_if_fail(self->priv->use_usbclerk); g_return_if_fail(SPICE_IS_WIN_USB_DRIVER(installer)); g_return_if_fail(device!= NULL); @@ -1173,6 +1180,7 @@ static void spice_usb_device_manager_drv_uninstall_cb(GObject *gobject, SPICE_DEBUG("Win USB driver uninstall finished"); g_return_if_fail(SPICE_IS_USB_DEVICE_MANAGER(self)); + g_return_if_fail(self->priv->use_usbclerk); </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Minor issue since these statements are only executed on a programming error, but<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">returning early here will leak cbinfo.<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div> </div><div>Right. This was broken before this patch also.</div><div>I’m adding another patch on top of this series that addresses this issue.</div> <blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""> if (!spice_win_usb_driver_uninstall_finish(cbinfo->installer, res, &err)) { g_warning("win usb driver uninstall failed -- %s", err->message); @@ -1576,15 +1584,18 @@ void spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self, { #if defined(USE_USBREDIR) && defined(G_OS_WIN32) - _spice_usb_device_manager_install_driver_async(self, device, cancellable, - callback, user_data); -#else + if (self->priv->use_usbclerk) { + _spice_usb_device_manager_install_driver_async(self, device, cancellable, + callback, user_data); + return; + } +#endif + _spice_usb_device_manager_connect_device_async(self, device, cancellable, callback, user_data); -#endif } /** @@ -1637,7 +1648,8 @@ void spice_usb_device_manager_disconnect_device(SpiceUsbDeviceManager *self, spice_usbredir_channel_disconnect_device(channel); #ifdef G_OS_WIN32 - _spice_usb_device_manager_uninstall_driver_async(self, device); + if(self->priv->use_usbclerk) + _spice_usb_device_manager_uninstall_driver_async(self, device); #endif #endif </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Reviewed-by: Jonathon Jongsma <<a href="mailto:jjongsma@redhat.com" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">jjongsma@redhat.com</a><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">></div></blockquote></div> </body></html>