On Tue, 2006-07-18 at 23:46 +0200, Fabrice Bellard wrote:
> Lonnie Mendez wrote:
> > lo list. I have found the old diff to be incorrect on many levels. New
> > diff has additionally cleanup code for the linux redirector. Please see
> > the attached patch for solution.
>
> Forget my last comment - I understand the problem now. I think it was a
> bad idea to use 'usb_reset' to destroy the usb device. Ideally a
> specific method should be added because it has no relation with the USB
> protocol (it is used to clear resources and it is perfectly possible
> that a given device can be inserted, removed and inserted again without
> having been destroyed). Another point is that the unused message
> 'USB_DESTROY' should be... destroyed.
Attached is a new diff that implements your suggestions. I have
compiled and run tested the patch against cvs.
--- qemu/hw/usb.h 2006-06-26 16:00:51.000000000 -0500
+++ qemu/hw/usb.h 2006-07-18 21:51:17.651436624 -0500
@@ -29,7 +29,6 @@
#define USB_MSG_ATTACH 0x100
#define USB_MSG_DETACH 0x101
#define USB_MSG_RESET 0x102
-#define USB_MSG_DESTROY 0x103
#define USB_RET_NODEV (-1)
#define USB_RET_NAK (-2)
@@ -117,12 +116,14 @@
int (*handle_packet)(USBDevice *dev, int pid,
uint8_t devaddr, uint8_t devep,
uint8_t *data, int len);
+ void (*handle_destroy)(USBDevice *dev);
+
int speed;
/* The following fields are used by the generic USB device
layer. They are here just to avoid creating a new structure for
them. */
- void (*handle_reset)(USBDevice *dev, int destroy);
+ void (*handle_reset)(USBDevice *dev);
int (*handle_control)(USBDevice *dev, int request, int value,
int index, int length, uint8_t *data);
int (*handle_data)(USBDevice *dev, int pid, uint8_t devep,
--- qemu/hw/usb.c 2006-05-25 18:58:51.000000000 -0500
+++ qemu/hw/usb.c 2006-07-18 21:43:26.741025888 -0500
@@ -55,10 +55,7 @@
s->remote_wakeup = 0;
s->addr = 0;
s->state = USB_STATE_DEFAULT;
- s->handle_reset(s, 0);
- break;
- case USB_MSG_DESTROY:
- s->handle_reset(s, 1);
+ s->handle_reset(s);
break;
case USB_TOKEN_SETUP:
if (s->state < USB_STATE_DEFAULT || devaddr != s->addr)
--- qemu/hw/usb-hid.c 2006-06-26 16:00:51.000000000 -0500
+++ qemu/hw/usb-hid.c 2006-07-18 21:52:39.754954992 -0500
@@ -323,16 +323,10 @@
return l;
}
-static void usb_mouse_handle_reset(USBDevice *dev, int destroy)
+static void usb_mouse_handle_reset(USBDevice *dev)
{
USBMouseState *s = (USBMouseState *)dev;
- if (destroy) {
- qemu_add_mouse_event_handler(NULL, NULL, 0);
- qemu_free(s);
- return;
- }
-
s->dx = 0;
s->dy = 0;
s->dz = 0;
@@ -506,6 +500,14 @@
return ret;
}
+static void usb_mouse_handle_destroy(USBDevice *dev)
+{
+ USBMouseState *s = (USBMouseState *)dev;
+
+ qemu_add_mouse_event_handler(NULL, NULL, 0);
+ qemu_free(s);
+}
+
USBDevice *usb_tablet_init(void)
{
USBMouseState *s;
@@ -519,6 +521,7 @@
s->dev.handle_reset = usb_mouse_handle_reset;
s->dev.handle_control = usb_mouse_handle_control;
s->dev.handle_data = usb_mouse_handle_data;
+ s->dev.handle_destroy = usb_mouse_handle_destroy;
s->kind = USB_TABLET;
pstrcpy(s->dev.devname, sizeof(s->dev.devname), "QEMU USB Tablet");
@@ -539,6 +542,7 @@
s->dev.handle_reset = usb_mouse_handle_reset;
s->dev.handle_control = usb_mouse_handle_control;
s->dev.handle_data = usb_mouse_handle_data;
+ s->dev.handle_destroy = usb_mouse_handle_destroy;
s->kind = USB_MOUSE;
pstrcpy(s->dev.devname, sizeof(s->dev.devname), "QEMU USB Mouse");
--- qemu/hw/usb-hub.c 2006-06-26 16:00:51.000000000 -0500
+++ qemu/hw/usb-hub.c 2006-07-18 21:53:24.391169256 -0500
@@ -199,11 +199,9 @@
}
}
-static void usb_hub_handle_reset(USBDevice *dev, int destroy)
+static void usb_hub_handle_reset(USBDevice *dev)
{
/* XXX: do it */
- if (destroy)
- qemu_free(dev);
}
static int usb_hub_handle_control(USBDevice *dev, int request, int value,
@@ -525,6 +523,13 @@
return usb_generic_handle_packet(dev, pid, devaddr, devep, data, len);
}
+static void usb_hub_handle_destroy(USBDevice *dev)
+{
+ USBHubState *s = (USBHubState *)dev;
+
+ qemu_free(s);
+}
+
USBDevice *usb_hub_init(int nb_ports)
{
USBHubState *s;
@@ -543,6 +548,7 @@
s->dev.handle_reset = usb_hub_handle_reset;
s->dev.handle_control = usb_hub_handle_control;
s->dev.handle_data = usb_hub_handle_data;
+ s->dev.handle_destroy = usb_hub_handle_destroy;
pstrcpy(s->dev.devname, sizeof(s->dev.devname), "QEMU USB Hub");
--- qemu/hw/usb-msd.c 2006-06-26 16:00:51.000000000 -0500
+++ qemu/hw/usb-msd.c 2006-07-18 21:54:25.569868680 -0500
@@ -112,16 +112,12 @@
s->mode = USB_MSDM_CSW;
}
-static void usb_msd_handle_reset(USBDevice *dev, int destroy)
+static void usb_msd_handle_reset(USBDevice *dev)
{
MSDState *s = (MSDState *)dev;
DPRINTF("Reset\n");
s->mode = USB_MSDM_CBW;
- if (destroy) {
- scsi_disk_destroy(s->scsi_dev);
- qemu_free(s);
- }
}
static int usb_msd_handle_control(USBDevice *dev, int request, int value,
@@ -369,6 +365,13 @@
return ret;
}
+static void usb_msd_handle_destroy(USBDevice *dev)
+{
+ MSDState *s = (MSDState *)dev;
+
+ scsi_disk_destroy(s->scsi_dev);
+ qemu_free(s);
+}
USBDevice *usb_msd_init(const char *filename)
{
@@ -388,11 +391,12 @@
s->dev.handle_reset = usb_msd_handle_reset;
s->dev.handle_control = usb_msd_handle_control;
s->dev.handle_data = usb_msd_handle_data;
+ s->dev.handle_destroy = usb_msd_handle_destroy;
snprintf(s->dev.devname, sizeof(s->dev.devname), "QEMU USB MSD(%.16s)",
filename);
s->scsi_dev = scsi_disk_init(bdrv, usb_msd_command_complete, s);
- usb_msd_handle_reset((USBDevice *)s, 0);
+ usb_msd_handle_reset((USBDevice *)s);
return (USBDevice *)s;
}
--- qemu/usb-linux.c 2006-06-26 16:00:51.000000000 -0500
+++ qemu/usb-linux.c 2006-07-18 21:54:01.039597848 -0500
@@ -57,7 +57,7 @@
int fd;
} USBHostDevice;
-static void usb_host_handle_reset(USBDevice *dev, int destroy)
+static void usb_host_handle_reset(USBDevice *dev)
{
#if 0
USBHostDevice *s = (USBHostDevice *)dev;
@@ -67,6 +67,15 @@
#endif
}
+static void usb_host_handle_destroy(USBDevice *dev)
+{
+ USBHostDevice *s = (USBHostDevice *)dev;
+
+ if (s->fd >= 0)
+ close(s->fd);
+ qemu_free(s);
+}
+
static int usb_host_handle_control(USBDevice *dev,
int request,
int value,
@@ -235,6 +244,7 @@
dev->dev.handle_reset = usb_host_handle_reset;
dev->dev.handle_control = usb_host_handle_control;
dev->dev.handle_data = usb_host_handle_data;
+ dev->dev.handle_destroy = usb_host_handle_destroy;
if (product_name[0] == '\0')
snprintf(dev->dev.devname, sizeof(dev->dev.devname),
--- qemu/vl.c 2006-07-17 03:16:38.000000000 -0500
+++ qemu/vl.c 2006-07-18 21:51:55.298713368 -0500
@@ -3781,6 +3781,7 @@
{
USBPort *port;
USBPort **lastp;
+ USBDevice *dev;
int bus_num, addr;
const char *p;
@@ -3805,8 +3806,10 @@
if (!port)
return -1;
+ dev = port->dev;
*lastp = port->next;
usb_attach(port, NULL);
+ dev->handle_destroy(dev);
port->next = free_usb_ports;
free_usb_ports = port;
return 0;
_______________________________________________
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel
