On Thu, Apr 24, 2014 at 4:37 PM, Jonathon Jongsma <jjongsma@xxxxxxxxxx> wrote:
Perhaps there should be a g_warning here to aid debugging? Or alternately in the function that calls this one?
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.
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.> 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;
> - }
> - }
> -
Yes, that's correct. I added a comment in the commit message (now it retries every time UsbDeviceManager is initialized).
--
Marc-André Lureau
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel