From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42487) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T9WH7-0004mh-Cy for qemu-devel@nongnu.org; Thu, 06 Sep 2012 03:13:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T9WGs-0006Px-KH for qemu-devel@nongnu.org; Thu, 06 Sep 2012 03:13:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:18912) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T9WGs-0006PH-BW for qemu-devel@nongnu.org; Thu, 06 Sep 2012 03:13:02 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q867D1a3019322 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 6 Sep 2012 03:13:01 -0400 From: Gerd Hoffmann Date: Thu, 6 Sep 2012 09:12:19 +0200 Message-Id: <1346915575-12369-19-git-send-email-kraxel@redhat.com> In-Reply-To: <1346915575-12369-1-git-send-email-kraxel@redhat.com> References: <1346915575-12369-1-git-send-email-kraxel@redhat.com> Subject: [Qemu-devel] [PATCH 18/54] usb-redir: Don't delay handling of open events to a bottom half List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Hans de Goede , Gerd Hoffmann From: Hans de Goede There is no need for this, and doing so means that a backend trying to write immediately after an open event will see qemu_chr_be_can_write returning 0, which not all backends handle well as there is no wakeup mechanism to detect when the frontend does become writable. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/redirect.c | 100 ++++++++++++++++++++++++++++------------------------- 1 files changed, 53 insertions(+), 47 deletions(-) diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c index 7f3719b..5cc3334 100644 --- a/hw/usb/redirect.c +++ b/hw/usb/redirect.c @@ -79,8 +79,8 @@ struct USBRedirDevice { /* Data passed from chardev the fd_read cb to the usbredirparser read cb */ const uint8_t *read_buf; int read_buf_size; - /* For async handling of open/close */ - QEMUBH *open_close_bh; + /* For async handling of close */ + QEMUBH *chardev_close_bh; /* To delay the usb attach in case of quick chardev close + open */ QEMUTimer *attach_timer; int64_t next_attach_time; @@ -784,18 +784,11 @@ static int usbredir_handle_control(USBDevice *udev, USBPacket *p, * from within the USBDevice data / control packet callbacks and doing a * usb_detach from within these callbacks is not a good idea. * - * So we use a bh handler to take care of close events. We also handle - * open events from this callback to make sure that a close directly followed - * by an open gets handled in the right order. + * So we use a bh handler to take care of close events. */ -static void usbredir_open_close_bh(void *opaque) +static void usbredir_chardev_close_bh(void *opaque) { USBRedirDevice *dev = opaque; - uint32_t caps[USB_REDIR_CAPS_SIZE] = { 0, }; - char version[32]; - - strcpy(version, "qemu usb-redir guest "); - pstrcat(version, sizeof(version), qemu_get_version()); usbredir_device_disconnect(dev); @@ -803,36 +796,47 @@ static void usbredir_open_close_bh(void *opaque) usbredirparser_destroy(dev->parser); dev->parser = NULL; } +} - if (dev->cs->opened) { - dev->parser = qemu_oom_check(usbredirparser_create()); - dev->parser->priv = dev; - dev->parser->log_func = usbredir_log; - dev->parser->read_func = usbredir_read; - dev->parser->write_func = usbredir_write; - dev->parser->hello_func = usbredir_hello; - dev->parser->device_connect_func = usbredir_device_connect; - dev->parser->device_disconnect_func = usbredir_device_disconnect; - dev->parser->interface_info_func = usbredir_interface_info; - dev->parser->ep_info_func = usbredir_ep_info; - dev->parser->configuration_status_func = usbredir_configuration_status; - dev->parser->alt_setting_status_func = usbredir_alt_setting_status; - dev->parser->iso_stream_status_func = usbredir_iso_stream_status; - dev->parser->interrupt_receiving_status_func = - usbredir_interrupt_receiving_status; - dev->parser->bulk_streams_status_func = usbredir_bulk_streams_status; - dev->parser->control_packet_func = usbredir_control_packet; - dev->parser->bulk_packet_func = usbredir_bulk_packet; - dev->parser->iso_packet_func = usbredir_iso_packet; - dev->parser->interrupt_packet_func = usbredir_interrupt_packet; - dev->read_buf = NULL; - dev->read_buf_size = 0; +static void usbredir_chardev_open(USBRedirDevice *dev) +{ + uint32_t caps[USB_REDIR_CAPS_SIZE] = { 0, }; + char version[32]; - usbredirparser_caps_set_cap(caps, usb_redir_cap_connect_device_version); - usbredirparser_caps_set_cap(caps, usb_redir_cap_filter); - usbredirparser_init(dev->parser, version, caps, USB_REDIR_CAPS_SIZE, 0); - usbredirparser_do_write(dev->parser); - } + /* Make sure any pending closes are handled (no-op if none pending) */ + usbredir_chardev_close_bh(dev); + qemu_bh_cancel(dev->chardev_close_bh); + + strcpy(version, "qemu usb-redir guest "); + pstrcat(version, sizeof(version), qemu_get_version()); + + dev->parser = qemu_oom_check(usbredirparser_create()); + dev->parser->priv = dev; + dev->parser->log_func = usbredir_log; + dev->parser->read_func = usbredir_read; + dev->parser->write_func = usbredir_write; + dev->parser->hello_func = usbredir_hello; + dev->parser->device_connect_func = usbredir_device_connect; + dev->parser->device_disconnect_func = usbredir_device_disconnect; + dev->parser->interface_info_func = usbredir_interface_info; + dev->parser->ep_info_func = usbredir_ep_info; + dev->parser->configuration_status_func = usbredir_configuration_status; + dev->parser->alt_setting_status_func = usbredir_alt_setting_status; + dev->parser->iso_stream_status_func = usbredir_iso_stream_status; + dev->parser->interrupt_receiving_status_func = + usbredir_interrupt_receiving_status; + dev->parser->bulk_streams_status_func = usbredir_bulk_streams_status; + dev->parser->control_packet_func = usbredir_control_packet; + dev->parser->bulk_packet_func = usbredir_bulk_packet; + dev->parser->iso_packet_func = usbredir_iso_packet; + dev->parser->interrupt_packet_func = usbredir_interrupt_packet; + dev->read_buf = NULL; + dev->read_buf_size = 0; + + usbredirparser_caps_set_cap(caps, usb_redir_cap_connect_device_version); + usbredirparser_caps_set_cap(caps, usb_redir_cap_filter); + usbredirparser_init(dev->parser, version, caps, USB_REDIR_CAPS_SIZE, 0); + usbredirparser_do_write(dev->parser); } static void usbredir_do_attach(void *opaque) @@ -856,13 +860,13 @@ static int usbredir_chardev_can_read(void *opaque) { USBRedirDevice *dev = opaque; - if (dev->parser) { - /* usbredir_parser_do_read will consume *all* data we give it */ - return 1024 * 1024; - } else { - /* usbredir_open_close_bh hasn't handled the open event yet */ + if (!dev->parser) { + WARNING("chardev_can_read called on non open chardev!\n"); return 0; } + + /* usbredir_parser_do_read will consume *all* data we give it */ + return 1024 * 1024; } static void usbredir_chardev_read(void *opaque, const uint8_t *buf, int size) @@ -886,8 +890,10 @@ static void usbredir_chardev_event(void *opaque, int event) switch (event) { case CHR_EVENT_OPENED: + usbredir_chardev_open(dev); + break; case CHR_EVENT_CLOSED: - qemu_bh_schedule(dev->open_close_bh); + qemu_bh_schedule(dev->chardev_close_bh); break; } } @@ -917,7 +923,7 @@ static int usbredir_initfn(USBDevice *udev) } } - dev->open_close_bh = qemu_bh_new(usbredir_open_close_bh, dev); + dev->chardev_close_bh = qemu_bh_new(usbredir_chardev_close_bh, dev); dev->attach_timer = qemu_new_timer_ms(vm_clock, usbredir_do_attach, dev); QTAILQ_INIT(&dev->asyncq); @@ -957,7 +963,7 @@ static void usbredir_handle_destroy(USBDevice *udev) qemu_chr_fe_close(dev->cs); qemu_chr_delete(dev->cs); /* Note must be done after qemu_chr_close, as that causes a close event */ - qemu_bh_delete(dev->open_close_bh); + qemu_bh_delete(dev->chardev_close_bh); qemu_del_timer(dev->attach_timer); qemu_free_timer(dev->attach_timer); -- 1.7.1