From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:43792) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QqAvI-0002FX-5g for qemu-devel@nongnu.org; Sun, 07 Aug 2011 17:30:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QqAvH-0006Vp-3S for qemu-devel@nongnu.org; Sun, 07 Aug 2011 17:30:16 -0400 Received: from mail-yw0-f45.google.com ([209.85.213.45]:64956) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QqAvG-0006Vl-Uq for qemu-devel@nongnu.org; Sun, 07 Aug 2011 17:30:15 -0400 Received: by ywb3 with SMTP id 3so2797337ywb.4 for ; Sun, 07 Aug 2011 14:30:14 -0700 (PDT) Message-ID: <4E3F03E3.1080706@codemonkey.ws> Date: Sun, 07 Aug 2011 16:30:11 -0500 From: Anthony Liguori 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> In-Reply-To: <4E3ECE4C.10805@redhat.com> 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: Hans de Goede Cc: Gerd Hoffmann , qemu-devel@nongnu.org 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. We want qemu_chr_guest_open() -> qemu_chr_fe_open() and for it to be analogous to a qemu_chr_be_open() which would be called immediately after accept() returned on a socket to signal that the backend is opened. Because there's an intermediate queue, even if a write happens after open, no data will be lost. So conceptionally, it makes sense to set handlers after open IMHO. But most importantly to this series, no backend can possibly generate a write before you get to call add handlers so you've got nothing to worry about here (based on the code today). Regards, Anthony Liguori To me it makes much more sense to actually call guest_open > when we are ready to receive data / to be read from, rather then to do > it before our handlers are hooked up and thus before we are ready. > > Regards, > > Hans >