From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58667) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UK7EI-0001v2-Nt for qemu-devel@nongnu.org; Mon, 25 Mar 2013 09:14:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UK7EF-0004S1-7v for qemu-devel@nongnu.org; Mon, 25 Mar 2013 09:14:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:1247) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UK7EE-0004RE-Ue for qemu-devel@nongnu.org; Mon, 25 Mar 2013 09:14:23 -0400 Message-ID: <51504E7A.8060006@redhat.com> Date: Mon, 25 Mar 2013 14:17:46 +0100 From: Hans de Goede MIME-Version: 1.0 References: <1364128793-12689-1-git-send-email-hdegoede@redhat.com> <1364128793-12689-5-git-send-email-hdegoede@redhat.com> <87a9prodeg.fsf@codemonkey.ws> In-Reply-To: <87a9prodeg.fsf@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/8] qemu-char: Automatically do fe_open / fe_close on qemu_chr_add_handlers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Amit Shah , qemu-devel@nongnu.org Hi, On 03/25/2013 01:46 PM, Anthony Liguori wrote: > Hans de Goede 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