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

[Hans de Goede]

Hi,

On 03/25/2013 01:46 PM, Anthony Liguori wrote:
> Hans de Goede <address@hidden> 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.

Regards,

Hans