From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34923) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UK906-0002V5-8N for qemu-devel@nongnu.org; Mon, 25 Mar 2013 11:07:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UK900-00035I-7W for qemu-devel@nongnu.org; Mon, 25 Mar 2013 11:07:54 -0400 Received: from mail-ia0-x230.google.com ([2607:f8b0:4001:c02::230]:64664) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UK900-000354-3y for qemu-devel@nongnu.org; Mon, 25 Mar 2013 11:07:48 -0400 Received: by mail-ia0-f176.google.com with SMTP id i1so5568662iaa.21 for ; Mon, 25 Mar 2013 08:07:47 -0700 (PDT) From: Anthony Liguori In-Reply-To: <51504E7A.8060006@redhat.com> References: <1364128793-12689-1-git-send-email-hdegoede@redhat.com> <1364128793-12689-5-git-send-email-hdegoede@redhat.com> <87a9prodeg.fsf@codemonkey.ws> <51504E7A.8060006@redhat.com> Date: Mon, 25 Mar 2013 10:07:44 -0500 Message-ID: <87boa7frgf.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: Hans de Goede Cc: Amit Shah , qemu-devel@nongnu.org Hans de Goede writes: > 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. Okay, seems reasonable. REgards, Anthony Liguori > > Regards, > > Hans