Release usb devices on shutdown and usb_del command
PATCH v2 1/1 Release usb devices on shutdown and usb_del command
When closig Vm or removing usb on guest via usb_del monitor command, qemu does not return the control to the host, the user have to unplug and plug the device in order to use it on the host.
v2:
added empty methods to usb-bsd and usb-stub.
release usb devices when main is out.
--- hw/usb-bus.c | 4 ++++ hw/usb.h | 2 ++ usb-bsd.c | 10 ++++++++++ usb-linux.c | 21 +++++++++++++++++++++ usb-stub.c | 10 ++++++++++ vl.c | 1 + 6 files changed, 48 insertions(+), 0 deletions(-)
hw/usb-bus.c +4 -0
@@ -207,6 +207,10 @@ int usb_device_delete_addr(int busnr, int addr)return -1 ; dev = port->dev; +if (!strcmp (dev->info->usbdevice_name,"host" )) { +usb_host_device_release (dev); + } +qdev_free (&dev->qdev);return 0 ; }
hw/usb.h +2 -0
@@ -258,6 +258,8 @@ void usb_send_msg(USBDevice *dev, int msg);USBDevice *usb_host_device_open (const char *devname);int usb_host_device_close (const char *devname);void usb_host_info (Monitor *mon); +int usb_host_device_release (USBDevice *dev); +void usb_cleanup (void );/* usb-hid.c */ void usb_hid_datain_cb (USBDevice *dev,void *opaque,void (*datain)(void *));
usb-bsd.c +10 -0
@@ -634,3 +634,13 @@ int usb_host_device_close(const char *devname) {return 0 ; } + +int usb_host_device_release (USBDevice *dev) +{ +return 0 ; +} + +void usb_cleanup (void ) +{ +return 0 ; +}
usb-linux.c +21 -0
@@ -286,6 +286,27 @@ static void async_cancel(USBPacket *unused, void *opaque) } } +void usb_cleanup (void ) +{ +struct USBHostDevice *s; + + QTAILQ_FOREACH(s, &hostdevs, next) { +if (s->fd !=-1 ) { +usb_host_device_release ((USBDevice *)s); + } + } +} + +int usb_host_device_release (USBDevice *dev) +{ +int ret; + +USBHostDevice *s =DO_UPCAST (USBHostDevice, dev, dev); + ret =ioctl (s->fd,USBDEVFS_RESET ); + +return ret; +} +static int usb_host_claim_interfaces (USBHostDevice *dev,int configuration) {int dev_descr_len, config_descr_len;
usb-stub.c +10 -0
@@ -50,3 +50,13 @@ int usb_host_device_close(const char *devname) {return 0 ; } + +int usb_host_device_release (USBDevice *dev) +{ +return 0 ; +} + +void usb_cleanup (void ) +{ +return 0 ; +}
vl.c +1 -0
@@ -3914,6 +3914,7 @@ int main(int argc, char **argv, char **envp)main_loop ();quit_timers ();net_cleanup (); +usb_cleanup ();return 0 ; }
Signature · 1 line
1.7.0.4
Quoted 23 lines · Shahar Havivi
When closig Vm or removing usb on guest via
usb_delmonitor command, qemu does not return the control to the host, the user have to unplug and plug the device in order to use it on the host.v2:
added empty methods to usb-bsd and usb-stub.
release usb devices when main is out.Signed-off-by Shahar Havivi <[email protected]>--- hw/usb-bus.c | 4 ++++ hw/usb.h | 2 ++ usb-bsd.c | 10 ++++++++++ usb-linux.c | 21 +++++++++++++++++++++ usb-stub.c | 10 ++++++++++ vl.c | 1 + 6 files changed, 48 insertions(+), 0 deletions(-)hw/usb-bus.c +0
-0
hw/usb-bus.c int usb_device_delete_addr(int busnr, int addr) · line 213
Quoted 7 lines
hw/usb-bus.c +4
-0@@ -207,6 +207,10 @@ int usb_device_delete_addr(int busnr, int addr)return -1 ; dev = port->dev; +if (!strcmp (dev->info->usbdevice_name,"host" )) { +usb_host_device_release (dev); + } +
Shouldn't this be done through a callback -- say usbdevice_release similar to usbdevice_init -- instead of embedding host specifics here? You wouldn't need the bsd and stub stubs then.
David
Quoted 96 lines
hw/usb-bus.c +0
-0qdev_free (&dev->qdev);return 0 ; }hw/usb.h +2
-0@@ -258,6 +258,8 @@ void usb_send_msg(USBDevice *dev, int msg);USBDevice *usb_host_device_open (const char *devname);int usb_host_device_close (const char *devname);void usb_host_info (Monitor *mon); +int usb_host_device_release (USBDevice *dev); +void usb_cleanup (void );/* usb-hid.c */ void usb_hid_datain_cb (USBDevice *dev,void *opaque,void (*datain)(void *));usb-bsd.c +10
-0@@ -634,3 +634,13 @@ int usb_host_device_close(const char *devname) {return 0 ; } + +int usb_host_device_release (USBDevice *dev) +{ +return 0 ; +} + +void usb_cleanup (void ) +{ +return 0 ; +}usb-linux.c +21
-0@@ -286,6 +286,27 @@ static void async_cancel(USBPacket *unused, void *opaque) } } +void usb_cleanup (void ) +{ +struct USBHostDevice *s; + + QTAILQ_FOREACH(s, &hostdevs, next) { +if (s->fd !=-1 ) { +usb_host_device_release ((USBDevice *)s); + } + } +} + +int usb_host_device_release (USBDevice *dev) +{ +int ret; + +USBHostDevice *s =DO_UPCAST (USBHostDevice, dev, dev); + ret =ioctl (s->fd,USBDEVFS_RESET ); + +return ret; +} +static int usb_host_claim_interfaces (USBHostDevice *dev,int configuration) {int dev_descr_len, config_descr_len;usb-stub.c +10
-0@@ -50,3 +50,13 @@ int usb_host_device_close(const char *devname) {return 0 ; } + +int usb_host_device_release (USBDevice *dev) +{ +return 0 ; +} + +void usb_cleanup (void ) +{ +return 0 ; +}vl.c +1
-0@@ -3914,6 +3914,7 @@ int main(int argc, char **argv, char **envp)main_loop ();quit_timers ();net_cleanup (); +usb_cleanup ();return 0 ; }
Quoted 36 lines · David S. Ahern
Quoted 23 lines · Shahar Havivi
When closig Vm or removing usb on guest via
usb_delmonitor command, qemu does not return the control to the host, the user have to unplug and plug the device in order to use it on the host.v2:
added empty methods to usb-bsd and usb-stub.
release usb devices when main is out.Signed-off-by Shahar Havivi <[email protected]>--- hw/usb-bus.c | 4 ++++ hw/usb.h | 2 ++ usb-bsd.c | 10 ++++++++++ usb-linux.c | 21 +++++++++++++++++++++ usb-stub.c | 10 ++++++++++ vl.c | 1 + 6 files changed, 48 insertions(+), 0 deletions(-)hw/usb-bus.c +0
-0hw/usb-bus.c int usb_device_delete_addr(int busnr, int addr) · line 213
Quoted 7 lines
hw/usb-bus.c +4
-0@@ -207,6 +207,10 @@ int usb_device_delete_addr(int busnr, int addr)return -1 ; dev = port->dev; +if (!strcmp (dev->info->usbdevice_name,"host" )) { +usb_host_device_release (dev); + } +Shouldn't this be done through a callback -- say
usbdevice_releasesimilar tousbdevice_init-- instead of embedding host specifics here? You wouldn't need the bsd and stub stubs then.
Quoted 1 line
David
What about the existing callbacks? Could handle_destroy do?
Note: usbdevice_init() is not for general initialization, just for dealing with the legacy -usbdevice command line.
Quoted 3 lines
Quoted 3 lines
hw/usb-bus.c +0
-0qdev_free (&dev->qdev);return 0 ; }
[...]
Quoted 32 lines
Quoted 32 lines
usb-linux.c +21
-0@@ -286,6 +286,27 @@ static void async_cancel(USBPacket *unused, void *opaque) } } +void usb_cleanup (void ) +{ +struct USBHostDevice *s; + + QTAILQ_FOREACH(s, &hostdevs, next) { +if (s->fd !=-1 ) { +usb_host_device_release ((USBDevice *)s); + } + } +} + +int usb_host_device_release (USBDevice *dev) +{ +int ret; + +USBHostDevice *s =DO_UPCAST (USBHostDevice, dev, dev); + ret =ioctl (s->fd,USBDEVFS_RESET ); + +return ret; +} +static int usb_host_claim_interfaces (USBHostDevice *dev,int configuration) {int dev_descr_len, config_descr_len;
[...]
Quoted 10 lines
Quoted 10 lines
vl.c +1
-0@@ -3914,6 +3914,7 @@ int main(int argc, char **argv, char **envp)main_loop ();quit_timers ();net_cleanup (); +usb_cleanup ();
Quoted 2 lines
vl.c int main(int argc, char **argv, char **envp) · line 3920
Quoted 2 lines
vl.c +0
-0return 0 ; }
Figure we'd have to clean up the qdev tree on exit. Gerd?
Hi,
Quoted 1 line
What about the existing callbacks? Could
handle_destroydo?
For hot-unplug it should do.
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
vl.c +1
-0@@ -3914,6 +3914,7 @@ int main(int argc, char **argv, char **envp)main_loop ();quit_timers ();net_cleanup (); +usb_cleanup ();
Quoted 4 lines
Quoted 2 lines
vl.c int main(int argc, char **argv, char **envp) · line 3920
Quoted 2 lines
vl.c +0
-0return 0 ; }Figure we'd have to clean up the qdev tree on exit. Gerd?
Hmm, yes. Question is how to do that best. There is qdev_free(). Today this is used for hot-unplug only. Using it on exit() too could have unwanted guest-visible side effects as it doesn't just release ressources, but also unplugs the device if possible.
Maybe it is better to add a exit notifier ...
cheers,
Gerd
Remove usb_host_device_release and using usb_host_close to handle usb_del command. Gerd, What do you think about the usb_cleanup()?
If it will be in usb-linux.c we will
have to ad atexit to each device, if we usb-bus.c we will have to implement in bsd and stub...
--- hw/usb.h | 1 + usb-bsd.c | 5 +++++ usb-linux.c | 12 ++++++++++++ usb-stub.c | 5 +++++ vl.c | 1 + 5 files changed, 24 insertions(+), 0 deletions(-)
hw/usb.h +1 -0
@@ -258,6 +258,7 @@ void usb_send_msg(USBDevice *dev, int msg);USBDevice *usb_host_device_open (const char *devname);int usb_host_device_close (const char *devname);void usb_host_info (Monitor *mon); +void usb_cleanup (void );/* usb-hid.c */ void usb_hid_datain_cb (USBDevice *dev,void *opaque,void (*datain)(void *));
usb-bsd.c +5 -0
@@ -634,3 +634,8 @@ int usb_host_device_close(const char *devname) {return 0 ; } + +void usb_cleanup (void ) +{ +return 0 ; +}
usb-linux.c +12 -0
@@ -286,6 +286,17 @@ static void async_cancel(USBPacket *unused, void *opaque) } } +void usb_cleanup (void ) +{ +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;
@@ -991,6 +1002,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 ;
usb-stub.c +5 -0
@@ -50,3 +50,8 @@ int usb_host_device_close(const char *devname) {return 0 ; } + +void usb_cleanup (void ) +{ +return 0 ; +}
vl.c +1 -0
@@ -3914,6 +3914,7 @@ int main(int argc, char **argv, char **envp)main_loop ();quit_timers ();net_cleanup (); +usb_cleanup ();return 0 ; }
Signature · 1 line
1.7.0.4
Quoted 2 lines · Shahar Havivi
Remove
usb_host_device_releaseand usingusb_host_closeto handleusb_delcommand. Gerd, What do you think about theusb_cleanup()?
We need a mechanism to handle this for sure. I don't like that usb-specific approach very much though.
I think we should either do that at qdev level, then at exit walk the whole device tree and call cleanup functions (if present). So every device has the chance to do cleanups when needed.
Or we could have a exit notifier, which can be used for device (and also other) cleanup work.
I tend to think that a exit notifier will be better. We probably have only a few devices which actually have to do some cleanup work (usb passthrough, maybe pci passthrough too), so building qdev infrastructure for that feels a bit like overkill. And exit notifiers are more generic, i.e. it will also work for non-device stuff.
cheers,
Gerd
Quoted 28 lines · Gerd Hoffmann
Date: Tue, 25 May 2010 10:58:50 +0200
From: Gerd Hoffmann <[email protected]>
To: Shahar Havivi <[email protected]>
CC: "David S. Ahern" <[email protected]>,
Markus Armbruster <[email protected]>, [email protected]
Subject: Re: [PATCH] Release usb devices on shutdown andusb_delcommandQuoted 2 lines · Shahar Havivi
Remove
usb_host_device_releaseand usingusb_host_closeto handleusb_delcommand. Gerd, What do you think about theusb_cleanup()?We need a mechanism to handle this for sure. I don't like that usb-specific approach very much though.
I think we should either do that at qdev level, then at exit walk the whole device tree and call cleanup functions (if present). So every device has the chance to do cleanups when needed.
Or we could have a exit notifier, which can be used for device (and also other) cleanup work.
I tend to think that a exit notifier will be better. We probably have only a few devices which actually have to do some cleanup work (usb passthrough, maybe pci passthrough too), so building qdev infrastructure for that feels a bit like overkill. And exit notifiers are more generic, i.e. it will also work for non-device stuff.
Quoted 3 lines
cheers,
Gerd
We can add at exit to the first device that added in usb-linux.c via usb_host_device_open() or usb_host_initfn() and the exit will iterate all usb devices in linux only,
How is that solution?
Quoted 3 lines · Markus Armbruster
Note:
usbdevice_init()is not for general initialization, just for dealing with the legacy -usbdevice command line.
Perhaps the comment before usbdevice_create is misleading or has some conversion not completed? Right now even devices entered through the monitor go through usbdevice_create -- which is the only place the usbdevice_init() hook is invoked.
David
Quoted 8 lines · David S. Ahern
Quoted 3 lines · Markus Armbruster
Note:
usbdevice_init()is not for general initialization, just for dealing with the legacy -usbdevice command line.Perhaps the comment before
usbdevice_createis misleading or has some conversion not completed? Right now even devices entered through the monitor go throughusbdevice_create-- which is the only place theusbdevice_init()hook is invoked.
... just for dealing with the -usbdevice command line and monitor command usb_add (both legacy).
Check out docs/qdev-device-use.txt for the current way to add devices. The monitor equivalent to -device is device_add.