diff mbox

[2/2] Return usb device to host on exit

Message ID 4C188FD7.2050306@redhat.com
State New
Headers show

Commit Message

Gerd Hoffmann June 16, 2010, 8:48 a.m. UTC
> +static void usb_host_cleanup(struct Notifier* n)
> +{
> +    struct USBHostDevice *s;
> +
> +    QTAILQ_FOREACH(s,&hostdevs, next) {
> +        if (s->fd != -1) {
> +            ioctl(s->fd, USBDEVFS_RESET);
> +        }
> +    }
> +}

Well.  The point of exit notifiers is that you don't need global 
variables for your cleanup work because the notifier function gets 
passed in a handle to your state data.

In that specific case the global hostdevs is needed anyway for other 
reasons.  Nevertheless I don't want usb-linux.c set a bad example, but 
provide a good reference implementation for others to look at.

Patch attached (untested).

cheers,
   Gerd
From 731761de07b73555faf96dc466efd7db2480a694 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Wed, 16 Jun 2010 10:29:59 +0200
Subject: [PATCH] usb-host: make sure we release the device.

Call USBDEVFS_RESET ioctl in usb_host_close.
Use exit notifiers to make sure we do it on exit too.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 usb-linux.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

Comments

Shahar Havivi June 16, 2010, 10:18 a.m. UTC | #1
On Wed, Jun 16, 2010 at 10:48:23AM +0200, Gerd Hoffmann wrote:
> Date: Wed, 16 Jun 2010 10:48:23 +0200
> From: Gerd Hoffmann <kraxel@redhat.com>
> To: Shahar Havivi <shaharh@redhat.com>
> Cc: qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH 2/2] Return usb device to host on exit
> 
> >+static void usb_host_cleanup(struct Notifier* n)
> >+{
> >+    struct USBHostDevice *s;
> >+
> >+    QTAILQ_FOREACH(s,&hostdevs, next) {
> >+        if (s->fd != -1) {
> >+            ioctl(s->fd, USBDEVFS_RESET);
> >+        }
> >+    }
> >+}
> 
> Well.  The point of exit notifiers is that you don't need global
> variables for your cleanup work because the notifier function gets
> passed in a handle to your state data.
> 
> In that specific case the global hostdevs is needed anyway for other
> reasons.  Nevertheless I don't want usb-linux.c set a bad example,
> but provide a good reference implementation for others to look at.
> 
> Patch attached (untested).
> 
> cheers,
>   Gerd
Thanks for the info Gerd,
I will test it.
Shahar.


> From 731761de07b73555faf96dc466efd7db2480a694 Mon Sep 17 00:00:00 2001
> From: Gerd Hoffmann <kraxel@redhat.com>
> Date: Wed, 16 Jun 2010 10:29:59 +0200
> Subject: [PATCH] usb-host: make sure we release the device.
> 
> Call USBDEVFS_RESET ioctl in usb_host_close.
> Use exit notifiers to make sure we do it on exit too.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  usb-linux.c |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/usb-linux.c b/usb-linux.c
> index 88273ff..a089fb6 100644
> --- a/usb-linux.c
> +++ b/usb-linux.c
> @@ -33,6 +33,7 @@
>  #include "qemu-common.h"
>  #include "qemu-timer.h"
>  #include "monitor.h"
> +#include "sysemu.h"
>  
>  #include <dirent.h>
>  #include <sys/ioctl.h>
> @@ -132,6 +133,7 @@ typedef struct USBHostDevice {
>      int       configuration;
>      int       ninterfaces;
>      int       closing;
> +    Notifier  exit;
>  
>      struct ctrl_struct ctrl;
>      struct endp_data endp_table[MAX_ENDPOINTS];
> @@ -404,6 +406,7 @@ static void usb_host_handle_destroy(USBDevice *dev)
>  
>      usb_host_close(s);
>      QTAILQ_REMOVE(&hostdevs, s, next);
> +    qemu_remove_exit_notifier(&s->exit);
>  }
>  
>  static int usb_linux_update_endp_table(USBHostDevice *s);
> @@ -991,11 +994,21 @@ static int usb_host_close(USBHostDevice *dev)
>      async_complete(dev);
>      dev->closing = 0;
>      usb_device_detach(&dev->dev);
> +    ioctl(s->fd, USBDEVFS_RESET);
>      close(dev->fd);
>      dev->fd = -1;
>      return 0;
>  }
>  
> +static void usb_host_exit_notifier(struct Notifier* n)
> +{
> +    USBHostDevice *s = container_of(n, USBHostDevice, exit);
> +
> +    if (s->fd != -1) {
> +        ioctl(s->fd, USBDEVFS_RESET);
> +    }
> +}
> +
>  static int usb_host_initfn(USBDevice *dev)
>  {
>      USBHostDevice *s = DO_UPCAST(USBHostDevice, dev, dev);
> @@ -1003,6 +1016,8 @@ static int usb_host_initfn(USBDevice *dev)
>      dev->auto_attach = 0;
>      s->fd = -1;
>      QTAILQ_INSERT_TAIL(&hostdevs, s, next);
> +    s->exit.notify = usb_host_exit_notifier;
> +    qemu_add_exit_notifier(&s->exit);
>      usb_host_auto_check(NULL);
>      return 0;
>  }
> -- 
> 1.6.5.2
>
diff mbox

Patch

diff --git a/usb-linux.c b/usb-linux.c
index 88273ff..a089fb6 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -33,6 +33,7 @@ 
 #include "qemu-common.h"
 #include "qemu-timer.h"
 #include "monitor.h"
+#include "sysemu.h"
 
 #include <dirent.h>
 #include <sys/ioctl.h>
@@ -132,6 +133,7 @@  typedef struct USBHostDevice {
     int       configuration;
     int       ninterfaces;
     int       closing;
+    Notifier  exit;
 
     struct ctrl_struct ctrl;
     struct endp_data endp_table[MAX_ENDPOINTS];
@@ -404,6 +406,7 @@  static void usb_host_handle_destroy(USBDevice *dev)
 
     usb_host_close(s);
     QTAILQ_REMOVE(&hostdevs, s, next);
+    qemu_remove_exit_notifier(&s->exit);
 }
 
 static int usb_linux_update_endp_table(USBHostDevice *s);
@@ -991,11 +994,21 @@  static int usb_host_close(USBHostDevice *dev)
     async_complete(dev);
     dev->closing = 0;
     usb_device_detach(&dev->dev);
+    ioctl(s->fd, USBDEVFS_RESET);
     close(dev->fd);
     dev->fd = -1;
     return 0;
 }
 
+static void usb_host_exit_notifier(struct Notifier* n)
+{
+    USBHostDevice *s = container_of(n, USBHostDevice, exit);
+
+    if (s->fd != -1) {
+        ioctl(s->fd, USBDEVFS_RESET);
+    }
+}
+
 static int usb_host_initfn(USBDevice *dev)
 {
     USBHostDevice *s = DO_UPCAST(USBHostDevice, dev, dev);
@@ -1003,6 +1016,8 @@  static int usb_host_initfn(USBDevice *dev)
     dev->auto_attach = 0;
     s->fd = -1;
     QTAILQ_INSERT_TAIL(&hostdevs, s, next);
+    s->exit.notify = usb_host_exit_notifier;
+    qemu_add_exit_notifier(&s->exit);
     usb_host_auto_check(NULL);
     return 0;
 }