Re: [Qemu-devel] [PATCH 4/8] qemu-char: Automatically do fe_open / fe_close on qemu_chr_add_handlers

[Anthony Liguori <anthony@codemonkey.ws>]

Hans de Goede <hdegoede@redhat.com> writes:

> Hi,
>
> On 03/25/2013 01:46 PM, Anthony Liguori wrote:
>> Hans de Goede <hdegoede@redhat.com> writes:
>>
>>> Most frontends can't really determine if the guest actually has the frontend
>>> side open. So lets automatically generate fe_open / fe_close as soon as a
>>> frontend becomes ready (as signalled by calling qemu_chr_add_handlers) /
>>> becomes non ready (as signalled by setting all handlers to NULL).
>>>
>>> And allow frontends which can actually determine if the guest is listening to
>>> opt-out of this.
>>
>> Could we change virtio-serial to delay calling add_handlers so that we
>> could not introduce this variable?
>
> Hmm, I was trying to avoid opening that can of worms. I've taken a quick look
> and there are 2 issues with doing this:
> 1) It will wreck havoc with CharDriverState.avail_connections, since that
>     gets increased on a qemu_chr_add_handlers( NULL ), while it is decreased
>     only once in hw/qdev-properties-system.c
>
> This can be fixed by moving the avail_connections++ to
> hw/qdev-properties-system.c: release_chr, which seems the sensible thing to
> do wrt nicely balancing out these calls anyways.
>
> 2) It will cause the virtio-console front-end to miss various events, such
> as backend open/close events.
>
> A backend open event will get "replayed" when qemu_chr_add_handlers( stuff )
> is called, causing a potential double open from the virtio-console pov
> (one before it called qemu_chr_add_handlers( NULL ), and one on the next
> add_handlers call). And a close or any other events happening while the
> frontend side is closed will be missed.
>
> I'll gladly add a patch fixing 1). to the next revision of this patchset,
> but given 2) I would prefer to stick with the explicit_fe_open flag and just
> register the handlers once.

Okay, seems reasonable.

REgards,

Anthony Liguori

>
> Regards,
>
> Hans