[Spice-devel] win-usb-driver: use usbclerk new message: USB_CLERK_DRIVER_SESSION_INSTALL

Submitted by Uri Lublin on Oct. 24, 2012, 1:55 p.m.

Details

Message ID 1351086945-6042-1-git-send-email-uril@redhat.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Uri Lublin Oct. 24, 2012, 1:55 p.m.
With this message usbclerk keeps a list of devices for which
a libusb driver was installed (per connection).
When a spice-gtk client exits, the connection is closed, and
usbclerk uninstalls the driver for all devices in the list.

That means we need to keep the connection open, so added
the win-usb driver installer to usb-device-manager's priv.

This prevents the case were the user exits the client, while a usb
device is connected to the guest, and can not use the device from
the client machine.

rhbz#869542
---
 gtk/usb-device-manager.c     |   19 ++++++++++++--
 gtk/win-usb-clerk.h          |    3 +-
 gtk/win-usb-driver-install.c |   56 ++++++++++++++++++++++++-----------------
 3 files changed, 51 insertions(+), 27 deletions(-)

Patch hide | download patch | download mbox

diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c
index 3684485..3f4c272 100644
--- a/gtk/usb-device-manager.c
+++ b/gtk/usb-device-manager.c
@@ -109,6 +109,9 @@  struct _SpiceUsbDeviceManagerPrivate {
     struct usbredirfilter_rule *redirect_on_connect_rules;
     int auto_conn_filter_rules_count;
     int redirect_on_connect_rules_count;
+#ifdef G_OS_WIN32
+    SpiceWinUsbDriver     *installer;
+#endif
 #endif
     GPtrArray *devices;
     GPtrArray *channels;
@@ -302,6 +305,10 @@  static void spice_usb_device_manager_finalize(GObject *gobject)
     if (priv->event_thread)
         g_thread_join(priv->event_thread);
     free(priv->auto_conn_filter_rules);
+#ifdef G_OS_WIN32
+    if (priv->installer)
+        g_object_unref(priv->installer);
+#endif
 #endif

     g_free(priv->auto_connect_filter);
@@ -876,7 +883,6 @@  static void spice_usb_device_manager_drv_install_cb(GObject *gobject,

     status = spice_win_usb_driver_install_finish(installer, res, &err);

-    g_object_unref(installer);
     spice_usb_device_unref(device);

     spice_usb_device_set_state(device, SPICE_USB_DEVICE_STATE_NONE);
@@ -1209,7 +1215,10 @@  void spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
     UsbInstallCbInfo *cbinfo;

     spice_usb_device_set_state(device, SPICE_USB_DEVICE_STATE_INSTALLING);
-    installer = spice_win_usb_driver_new();
+    if (! self->priv->installer) {
+        self->priv->installer = spice_win_usb_driver_new();
+    }
+    installer = self->priv->installer;
     cbinfo = g_new0(UsbInstallCbInfo, 1);
     cbinfo->manager     = self;
     cbinfo->device      = spice_usb_device_ref(device);
@@ -1273,9 +1282,13 @@  void spice_usb_device_manager_disconnect_device(SpiceUsbDeviceManager *self,
     UsbInstallCbInfo *cbinfo;

     g_warn_if_fail(device != NULL);
+    g_warn_if_fail(self->priv->installer != NULL);

     spice_usb_device_set_state(device, SPICE_USB_DEVICE_STATE_UNINSTALLING);
-    installer = spice_win_usb_driver_new();
+    if (! self->priv->installer) {
+        self->priv->installer = spice_win_usb_driver_new();
+    }
+    installer = self->priv->installer;
     cbinfo = g_new0(UsbInstallCbInfo, 1);
     cbinfo->manager     = self;
     cbinfo->device      = spice_usb_device_ref(device);
diff --git a/gtk/win-usb-clerk.h b/gtk/win-usb-clerk.h
index 5b1e3cf..24da3b4 100644
--- a/gtk/win-usb-clerk.h
+++ b/gtk/win-usb-clerk.h
@@ -5,7 +5,7 @@ 

 #define USB_CLERK_PIPE_NAME     TEXT("\\\\.\\pipe\\usbclerkpipe")
 #define USB_CLERK_MAGIC         0xDADA
-#define USB_CLERK_VERSION       0x0002
+#define USB_CLERK_VERSION       0x0003

 typedef struct USBClerkHeader {
     UINT16 magic;
@@ -18,6 +18,7 @@  enum {
     USB_CLERK_DRIVER_INSTALL = 1,
     USB_CLERK_DRIVER_REMOVE,
     USB_CLERK_REPLY,
+    USB_CLERK_DRIVER_SESSION_INSTALL,
     USB_CLERK_END_MESSAGE,
 };

diff --git a/gtk/win-usb-driver-install.c b/gtk/win-usb-driver-install.c
index 02d20d6..1d68296 100644
--- a/gtk/win-usb-driver-install.c
+++ b/gtk/win-usb-driver-install.c
@@ -113,7 +113,6 @@  void win_usb_driver_handle_reply_cb(GObject *gobject,

     g_warn_if_fail(g_input_stream_close(istream, NULL, NULL));
     g_clear_object(&istream);
-    spice_win_usb_driver_close(self);

     if (err) {
         g_warning("failed to read reply from usbclerk (%s)", err->message);
@@ -149,7 +148,10 @@  void win_usb_driver_handle_reply_cb(GObject *gobject,
     if (priv->reply.hdr.version != USB_CLERK_VERSION) {
         g_warning("usbclerk version mismatch: mine=0x%04x  server=0x%04x",
                   USB_CLERK_VERSION, priv->reply.hdr.version);
-        /* For now just warn, do not fail */
+        g_simple_async_result_set_error(priv->result,
+                                        SPICE_WIN_USB_DRIVER_ERROR,
+                                        SPICE_WIN_USB_DRIVER_ERROR_MESSAGE,
+                                        "usbclerk version mismatch");
     }

     if (priv->reply.hdr.type != USB_CLERK_REPLY) {
@@ -265,30 +267,39 @@  void spice_win_usb_driver_op(SpiceWinUsbDriver *self,

     priv = self->priv;

-    g_return_if_fail(priv->result == NULL);
-
     result = g_simple_async_result_new(G_OBJECT(self), callback, user_data,
                                        spice_win_usb_driver_op);

+    if (priv->result) { /* allow one install/uninstall request at a time */
+        g_warning("Another request exists -- try later");
+        g_simple_async_result_set_error(result,
+                  SPICE_WIN_USB_DRIVER_ERROR, SPICE_WIN_USB_DRIVER_ERROR_FAILED,
+                  "Another request exists -- try later");
+        goto failed_request;
+    }
+
+
     vid = spice_usb_device_get_vid(device);
     pid = spice_usb_device_get_pid(device);

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

     if (!spice_win_usb_driver_send_request(self, op_type,
@@ -308,7 +319,6 @@  void spice_win_usb_driver_op(SpiceWinUsbDriver *self,
     return;

  failed_request:
-    spice_win_usb_driver_close(self);
     g_simple_async_result_complete_in_idle(result);
     g_clear_object(&result);
 }
@@ -333,8 +343,8 @@  void spice_win_usb_driver_install(SpiceWinUsbDriver *self,
 {
     SPICE_DEBUG("Win usb driver installation started");

-    spice_win_usb_driver_op(self, device, USB_CLERK_DRIVER_INSTALL, cancellable,
-                            callback, user_data);
+    spice_win_usb_driver_op(self, device, USB_CLERK_DRIVER_SESSION_INSTALL,
+                            cancellable, callback, user_data);
 }

 G_GNUC_INTERNAL

Comments

ack. seems ok, although CreateFile can move to init.


Uri Lublin wrote:
> With this message usbclerk keeps a list of devices for which
> a libusb driver was installed (per connection).
> When a spice-gtk client exits, the connection is closed, and
> usbclerk uninstalls the driver for all devices in the list.
>
> That means we need to keep the connection open, so added
> the win-usb driver installer to usb-device-manager's priv.
>
> This prevents the case were the user exits the client, while a usb
> device is connected to the guest, and can not use the device from
> the client machine.
>
> rhbz#869542
> ---
>  gtk/usb-device-manager.c     |   19 ++++++++++++--
>  gtk/win-usb-clerk.h          |    3 +-
>  gtk/win-usb-driver-install.c |   56 ++++++++++++++++++++++++-----------------
>  3 files changed, 51 insertions(+), 27 deletions(-)
>
> diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c
> index 3684485..3f4c272 100644
> --- a/gtk/usb-device-manager.c
> +++ b/gtk/usb-device-manager.c
> @@ -109,6 +109,9 @@ struct _SpiceUsbDeviceManagerPrivate {
>      struct usbredirfilter_rule *redirect_on_connect_rules;
>      int auto_conn_filter_rules_count;
>      int redirect_on_connect_rules_count;
> +#ifdef G_OS_WIN32
> +    SpiceWinUsbDriver     *installer;
> +#endif
>  #endif
>      GPtrArray *devices;
>      GPtrArray *channels;
> @@ -302,6 +305,10 @@ static void spice_usb_device_manager_finalize(GObject *gobject)
>      if (priv->event_thread)
>          g_thread_join(priv->event_thread);
>      free(priv->auto_conn_filter_rules);
> +#ifdef G_OS_WIN32
> +    if (priv->installer)
> +        g_object_unref(priv->installer);
> +#endif
>  #endif
>
>      g_free(priv->auto_connect_filter);
> @@ -876,7 +883,6 @@ static void spice_usb_device_manager_drv_install_cb(GObject *gobject,
>
>      status = spice_win_usb_driver_install_finish(installer, res, &err);
>
> -    g_object_unref(installer);
>      spice_usb_device_unref(device);
>
>      spice_usb_device_set_state(device, SPICE_USB_DEVICE_STATE_NONE);
> @@ -1209,7 +1215,10 @@ void spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
>      UsbInstallCbInfo *cbinfo;
>
>      spice_usb_device_set_state(device, SPICE_USB_DEVICE_STATE_INSTALLING);
> -    installer = spice_win_usb_driver_new();
> +    if (! self->priv->installer) {
> +        self->priv->installer = spice_win_usb_driver_new();
> +    }
> +    installer = self->priv->installer;
>      cbinfo = g_new0(UsbInstallCbInfo, 1);
>      cbinfo->manager     = self;
>      cbinfo->device      = spice_usb_device_ref(device);
> @@ -1273,9 +1282,13 @@ void spice_usb_device_manager_disconnect_device(SpiceUsbDeviceManager *self,
>      UsbInstallCbInfo *cbinfo;
>
>      g_warn_if_fail(device != NULL);
> +    g_warn_if_fail(self->priv->installer != NULL);
>
>      spice_usb_device_set_state(device, SPICE_USB_DEVICE_STATE_UNINSTALLING);
> -    installer = spice_win_usb_driver_new();
> +    if (! self->priv->installer) {
> +        self->priv->installer = spice_win_usb_driver_new();
> +    }
> +    installer = self->priv->installer;
>      cbinfo = g_new0(UsbInstallCbInfo, 1);
>      cbinfo->manager     = self;
>      cbinfo->device      = spice_usb_device_ref(device);
> diff --git a/gtk/win-usb-clerk.h b/gtk/win-usb-clerk.h
> index 5b1e3cf..24da3b4 100644
> --- a/gtk/win-usb-clerk.h
> +++ b/gtk/win-usb-clerk.h
> @@ -5,7 +5,7 @@
>
>  #define USB_CLERK_PIPE_NAME     TEXT("\\\\.\\pipe\\usbclerkpipe")
>  #define USB_CLERK_MAGIC         0xDADA
> -#define USB_CLERK_VERSION       0x0002
> +#define USB_CLERK_VERSION       0x0003
>
>  typedef struct USBClerkHeader {
>      UINT16 magic;
> @@ -18,6 +18,7 @@ enum {
>      USB_CLERK_DRIVER_INSTALL = 1,
>      USB_CLERK_DRIVER_REMOVE,
>      USB_CLERK_REPLY,
> +    USB_CLERK_DRIVER_SESSION_INSTALL,
>      USB_CLERK_END_MESSAGE,
>  };
>
> diff --git a/gtk/win-usb-driver-install.c b/gtk/win-usb-driver-install.c
> index 02d20d6..1d68296 100644
> --- a/gtk/win-usb-driver-install.c
> +++ b/gtk/win-usb-driver-install.c
> @@ -113,7 +113,6 @@ void win_usb_driver_handle_reply_cb(GObject *gobject,
>
>      g_warn_if_fail(g_input_stream_close(istream, NULL, NULL));
>      g_clear_object(&istream);
> -    spice_win_usb_driver_close(self);
>
>      if (err) {
>          g_warning("failed to read reply from usbclerk (%s)", err->message);
> @@ -149,7 +148,10 @@ void win_usb_driver_handle_reply_cb(GObject *gobject,
>      if (priv->reply.hdr.version != USB_CLERK_VERSION) {
>          g_warning("usbclerk version mismatch: mine=0x%04x  server=0x%04x",
>                    USB_CLERK_VERSION, priv->reply.hdr.version);
> -        /* For now just warn, do not fail */
> +        g_simple_async_result_set_error(priv->result,
> +                                        SPICE_WIN_USB_DRIVER_ERROR,
> +                                        SPICE_WIN_USB_DRIVER_ERROR_MESSAGE,
> +                                        "usbclerk version mismatch");
>      }
>
>      if (priv->reply.hdr.type != USB_CLERK_REPLY) {
> @@ -265,30 +267,39 @@ void spice_win_usb_driver_op(SpiceWinUsbDriver *self,
>
>      priv = self->priv;
>
> -    g_return_if_fail(priv->result == NULL);
> -
>      result = g_simple_async_result_new(G_OBJECT(self), callback, user_data,
>                                         spice_win_usb_driver_op);
>
> +    if (priv->result) { /* allow one install/uninstall request at a time */
> +        g_warning("Another request exists -- try later");
> +        g_simple_async_result_set_error(result,
> +                  SPICE_WIN_USB_DRIVER_ERROR, SPICE_WIN_USB_DRIVER_ERROR_FAILED,
> +                  "Another request exists -- try later");
> +        goto failed_request;
> +    }
> +
> +
>      vid = spice_usb_device_get_vid(device);
>      pid = spice_usb_device_get_pid(device);
>
> -    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;
> +    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;
> +        }
>      }
>
>      if (!spice_win_usb_driver_send_request(self, op_type,
> @@ -308,7 +319,6 @@ void spice_win_usb_driver_op(SpiceWinUsbDriver *self,
>      return;
>
>   failed_request:
> -    spice_win_usb_driver_close(self);
>      g_simple_async_result_complete_in_idle(result);
>      g_clear_object(&result);
>  }
> @@ -333,8 +343,8 @@ void spice_win_usb_driver_install(SpiceWinUsbDriver *self,
>  {
>      SPICE_DEBUG("Win usb driver installation started");
>
> -    spice_win_usb_driver_op(self, device, USB_CLERK_DRIVER_INSTALL, cancellable,
> -                            callback, user_data);
> +    spice_win_usb_driver_op(self, device, USB_CLERK_DRIVER_SESSION_INSTALL,
> +                            cancellable, callback, user_data);
>  }
>
>  G_GNUC_INTERNAL
>