Re: [Qemu-devel] [PATCH 2/3] usb-redir: Call qemu_chr_guest_open/close

[Anthony Liguori <anthony@codemonkey.ws>]

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<hdegoede@redhat.com>
>>> ---
>>> 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
>