Release usb devices on shutdown and usb_del

PATCH Superseded Shahar Havivi 2 participants 5 messages Jun 15, 2010 – Jun 16, 2010

lore

Share & tools

lore

Shahar Havivi

author

2010-06-15 18:04 UTC #0 lore raw

v4 cover Release usb devices on shutdown and usb_del

v4:
use exit notifier instead of atexit()

Shahar Havivi (2):
Return usb device to host on usb_del command
Return usb device to host on exit

 usb-linux.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

Shahar Havivi

author

2010-06-15 18:05 UTC #1 lore raw

PATCH v4 1/2 Return usb device to host on usb_del command

Signed-off-by Shahar Havivi <[email protected]>

---
 usb-linux.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

usb-linux.c +1 -0

index 88273ff..22a85e3 100644
@@ -991,6 +991,7 @@ static int usb_host_close(USBHostDevice *dev)
     async_complete(dev);
     dev->closing = 0;
     usb_device_detach(&dev->dev);
+    ioctl(dev->fd, USBDEVFS_RESET);
     close(dev->fd);
     dev->fd = -1;
     return 0;

Signature · 1 line

1.7.0.4

Shahar Havivi

author

2010-06-15 18:05 UTC #2 lore raw

PATCH v4 2/2 Return usb device to host on exit

In reply to Shahar Havivi

Signed-off-by Shahar Havivi <[email protected]>

---
 usb-linux.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

usb-linux.c +19 -0

index 22a85e3..4b5aeb6 100644
@@ -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>

@@ -89,6 +90,8 @@ static char *usb_host_device_path;
 #define USB_FS_SYS 3
 
 static int usb_fs_type;
+static int usb_notify_set;
+static Notifier usb_host_notifier;
 
 /* endpoint association data */
 struct endp_data {

@@ -286,6 +289,17 @@ static void async_cancel(USBPacket *unused, void *opaque)
     }
 }
 
+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);
+        }
+    }
+}
+
 static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
 {
     int dev_descr_len, config_descr_len;

@@ -1066,6 +1080,11 @@ USBDevice *usb_host_device_open(const char *devname)
     qdev_prop_set_uint32(&dev->qdev, "vendorid",  filter.vendor_id);
     qdev_prop_set_uint32(&dev->qdev, "productid", filter.product_id);
     qdev_init_nofail(&dev->qdev);
+    if (!usb_notify_set) {
+        usb_notify_set = 1;
+        usb_host_notifier.notify = usb_host_cleanup;
+        qemu_add_exit_notifier(&usb_host_notifier);
+    }
     return dev;
 
 fail:

Signature · 1 line

1.7.0.4

Gerd Hoffmann

2010-06-16 08:48 UTC #3 lore raw

Re: [Qemu-devel] [PATCH 2/2] Return usb device to host on exit

Review

Quoted 10 lines

(quoted diff) +10 -0

+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

Quoted 1 line

From 731761de07b73555faf96dc466efd7db2480a694 Mon Sep 17 00:00:00 2001

From: Gerd Hoffmann <[email protected]>
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 <[email protected]>

---
 usb-linux.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

usb-linux.c +15 -0

index 88273ff..a089fb6 100644
@@ -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;
 }

Signature · 1 line

1.6.5.2

Shahar Havivi

author

2010-06-16 10:17 UTC #4 lore raw

Re: [Qemu-devel] [PATCH 2/2] Return usb device to host on exit

Quoted 21 lines · Gerd Hoffmann

Date: Wed, 16 Jun 2010 10:48:23 +0200
From: Gerd Hoffmann <[email protected]>
To: Shahar Havivi <[email protected]>
Cc: [email protected]
Subject: Re: [Qemu-devel] [PATCH 2/2] Return usb device to host on exit
usb-linux.c static int usb_host_close(USBHostDevice *dev)
Quoted 10 lines
usb-linux.c +10 -0
+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.

Quoted 8 lines

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.

Quoted 75 lines

From 731761de07b73555faf96dc466efd7db2480a694 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <[email protected]>
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 <[email protected]>

---
 usb-linux.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

usb-linux.c +15 -0

index 88273ff..a089fb6 100644
@@ -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;
 }

Signature · 1 line

1.6.5.2