[Spice-devel] [PATCH spice-gtk 14/14] win-usb: try to connect to usbclerk during init

[Marc-André Lureau]

On Thu, Apr 24, 2014 at 4:37 PM, Jonathon Jongsma <jjongsma at redhat.com>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.

>          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;
> > -        }
> > -    }
> > -
>
> 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.
>

Yes, that's correct. I added a comment in the commit message (now it
retries every time UsbDeviceManager is initialized).


-- 
Marc-André Lureau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20140424/1c9203c5/attachment.html>