From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:33788) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QqKkf-00089m-Qu for qemu-devel@nongnu.org; Mon, 08 Aug 2011 03:59:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QqKke-0004wM-U9 for qemu-devel@nongnu.org; Mon, 08 Aug 2011 03:59:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:16567) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QqKke-0004wA-Ir for qemu-devel@nongnu.org; Mon, 08 Aug 2011 03:59:56 -0400 Message-ID: <4E3F97F2.2050100@redhat.com> Date: Mon, 08 Aug 2011 10:01:54 +0200 From: Hans de Goede MIME-Version: 1.0 References: <1312723284-7549-1-git-send-email-hdegoede@redhat.com> <1312723284-7549-2-git-send-email-hdegoede@redhat.com> <4E3EB4CE.8030201@codemonkey.ws> <4E3ECE4C.10805@redhat.com> <4E3F03E3.1080706@codemonkey.ws> In-Reply-To: <4E3F03E3.1080706@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/3] usb-redir: Call qemu_chr_guest_open/close List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Gerd Hoffmann , qemu-devel@nongnu.org Hi, On 08/07/2011 11:30 PM, Anthony Liguori wrote: > On 08/07/2011 12:41 PM, Hans de Goede wrote: >> Hi, >> >> On 08/07/2011 05:52 PM, Anthony Liguori wrote: >>> On 08/07/2011 08:21 AM, Hans de Goede wrote: >>>> To let the chardev now we're ready start receiving data. This is >>>> necessary >>>> with the spicevmc chardev to get it registered with the spice-server. >>>> >>>> Signed-off-by: Hans de Goede >>>> --- >>>> usb-redir.c | 3 +++ >>>> 1 files changed, 3 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/usb-redir.c b/usb-redir.c >>>> index e212993..ec88c0b 100644 >>>> --- a/usb-redir.c >>>> +++ b/usb-redir.c >>>> @@ -809,6 +809,8 @@ static int usbredir_initfn(USBDevice *udev) >>>> >>>> qemu_chr_add_handlers(dev->cs, usbredir_chardev_can_read, >>>> usbredir_chardev_read, usbredir_chardev_event, dev); >>>> + /* Let the other side know we are ready */ >>>> + qemu_chr_guest_open(dev->cs); >>> >>> >>> You should do guest_open before adding handlers. >> >> Erm, no, guest_open may lead to a callback in the >> chardev, to which it may respond by immediately queuing a few writes / >> doing a read. > > So after my char-flow changes, you won't be allowed to set handlers unless you've called open. > Why not do it the other way around? So don't allow open until the handlers are set. My reasoning behind this is that eventually we will want to have a struct describing a pipe endpoint, which will contain handlers (by then identical for both sides) and besides the struct a priv / user_data pointer which will get passed by the handlers when called. Then we will have a chardev_create or pipe_create call which will take a struct + user data ptr for both ends (so twice). This matches what currently our set handlers call does. But I would expect the open to come after the creation of the pipe. At least to me it is much more logical to first set the handlers (which are really part of object creation) and then later do the open, this matches the common programming paradigm of having an init/create function and an open function. Also forcing the set handlers after the open does not work well with virtio_console, as these are not open until the port inside the guest is opened. So then it would need to delay its set handlers till the first open, and what should it do at close, do a set handlers NULL before doing the actual close ?? Regards, Hans