<div dir="ltr"><div class="gmail_extra"> <div class="gmail_quote">On Thu, Apr 24, 2014 at 4:37 PM, Jonathon Jongsma <<a href="mailto:jjongsma@redhat.com" target="_blank">jjongsma@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div id=":4qy" class="a3s" style="overflow:hidden">Perhaps there should be a g_warning here to aid debugging? Or alternately in the function that calls this one? <div><div class="h5"> </div></div></div></blockquote><div> </div><div>The spice_usb_device_manager_get() will return NULL , with a GError. Since this function is called many times by spice-gtk or the client, I think it's preferable to let the application print a warning or a dialog (once) instead of flooding with warnings. However, I added a SPICE_DEBUG. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div id=":4qy" class="a3s" style="overflow:hidden"><div><div class="h5"> > return FALSE; > - } > #endif > > /* Initialize libusb */ > diff --git a/gtk/win-usb-driver-install.c b/gtk/win-usb-driver-install.c > index bb18ae4..71b51d7 100644 > --- a/gtk/win-usb-driver-install.c > +++ b/gtk/win-usb-driver-install.c > @@ -53,14 +53,42 @@ struct _SpiceWinUsbDriverPrivate { > }; > > > +static void spice_win_usb_driver_initable_iface_init(GInitableIface *iface); > > -G_DEFINE_TYPE(SpiceWinUsbDriver, spice_win_usb_driver, G_TYPE_OBJECT); > +G_DEFINE_TYPE_WITH_CODE(SpiceWinUsbDriver, spice_win_usb_driver, > G_TYPE_OBJECT, > + G_IMPLEMENT_INTERFACE (G_TYPE_INITABLE, > spice_win_usb_driver_initable_iface_init)); > > static void spice_win_usb_driver_init(SpiceWinUsbDriver *self) > { > self->priv = SPICE_WIN_USB_DRIVER_GET_PRIVATE(self); > } > > +static gboolean spice_win_usb_driver_initable_init(GInitable *initable, > + GCancellable > *cancellable, > + GError **err) > +{ > + SpiceWinUsbDriver *self = SPICE_WIN_USB_DRIVER(initable); > + SpiceWinUsbDriverPrivate *priv = self->priv; > + > + SPICE_DEBUG("win-usb-driver-install: connecting to usbclerk named > pipe"); > + priv->handle = CreateFile(USB_CLERK_PIPE_NAME, > + GENERIC_READ | GENERIC_WRITE, > + 0, NULL, > + OPEN_EXISTING, > + FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OVERLAPPED, > + NULL); > + if (priv->handle == INVALID_HANDLE_VALUE) { > + DWORD errval = GetLastError(); > + gchar *errstr = g_win32_error_message(errval); > + g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_USB_SERVICE, > + "Failed to create service named pipe (%ld) %s", errval, > errstr); > + g_free(errstr); > + return FALSE; > + } > + > + return TRUE; > +} > + > static void spice_win_usb_driver_finalize(GObject *gobject) > { > SpiceWinUsbDriver *self = SPICE_WIN_USB_DRIVER(gobject); > @@ -68,6 +96,7 @@ static void spice_win_usb_driver_finalize(GObject *gobject) > > if (priv->handle) > CloseHandle(priv->handle); > + > g_clear_object(&priv->result); > } > > @@ -80,6 +109,11 @@ static void > spice_win_usb_driver_class_init(SpiceWinUsbDriverClass *klass) > g_type_class_add_private(klass, sizeof(SpiceWinUsbDriverPrivate)); > } > > +static void spice_win_usb_driver_initable_iface_init(GInitableIface *iface) > +{ > + iface->init = spice_win_usb_driver_initable_init; > +} > + > /* ------------------------------------------------------------------ */ > /* callbacks */ > > @@ -244,13 +278,15 @@ void > spice_win_usb_driver_read_reply_async(SpiceWinUsbDriver *self) > > > G_GNUC_INTERNAL > -SpiceWinUsbDriver *spice_win_usb_driver_new(void) > +SpiceWinUsbDriver *spice_win_usb_driver_new(GError **err) > { > - GObject *obj; > + GObject *self; > + > + g_return_val_if_fail(err == NULL || *err == NULL, FALSE); > > - obj = g_object_new(SPICE_TYPE_WIN_USB_DRIVER, NULL); > + self = g_initable_new(SPICE_TYPE_WIN_USB_DRIVER, NULL, err, NULL); > > - return SPICE_WIN_USB_DRIVER(obj); > + return SPICE_WIN_USB_DRIVER(self); > } > > static > @@ -286,26 +322,6 @@ void spice_win_usb_driver_op(SpiceWinUsbDriver *self, > vid = spice_usb_device_get_vid(device); > pid = spice_usb_device_get_pid(device); > > - if (! priv->handle ) { > - SPICE_DEBUG("win-usb-driver-install: connecting to usbclerk named > pipe"); > - priv->handle = CreateFile(USB_CLERK_PIPE_NAME, > - GENERIC_READ | GENERIC_WRITE, > - 0, NULL, > - OPEN_EXISTING, > - FILE_ATTRIBUTE_NORMAL | > FILE_FLAG_OVERLAPPED, > - NULL); > - if (priv->handle == INVALID_HANDLE_VALUE) { > - DWORD errval = GetLastError(); > - gchar *errstr = g_win32_error_message(errval); > - g_warning("failed to create a named pipe to usbclerk (%ld) %s", > - errval,errstr); > - g_simple_async_result_set_error(result, > - G_IO_ERROR, G_IO_ERROR_FAILED, > - "Failed to create named pipe (%ld) %s", errval, > errstr); > - goto failed_request; > - } > - } > - </div></div>One behavioral change to note here: with the old code, if we failed to create the usbclerk pipe, we would re-try to do it again the next time spice_win_usb_driver_op() was called. With the new code, we try it once at the beginning and then never try again. I suspect that re-trying isn't really useful (if it failed the first time, it will likely fail again the next time), but I thought it was worth mentioning the change in behavior.</div> </blockquote></div> </div><div class="gmail_extra">Yes, that's correct. I added a comment in the commit message (now it retries every time UsbDeviceManager is initialized). </div><div class="gmail_extra"> -- Marc-André Lureau </div></div>