From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53154) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V3smw-0007Cc-M9 for qemu-devel@nongnu.org; Mon, 29 Jul 2013 15:07:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V3smp-0006KN-N8 for qemu-devel@nongnu.org; Mon, 29 Jul 2013 15:07:22 -0400 Received: from mail-ob0-x230.google.com ([2607:f8b0:4003:c01::230]:35930) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V3smp-0006KF-Gx for qemu-devel@nongnu.org; Mon, 29 Jul 2013 15:07:15 -0400 Received: by mail-ob0-f176.google.com with SMTP id v19so10274424obq.35 for ; Mon, 29 Jul 2013 12:07:14 -0700 (PDT) Sender: fluxion Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <51F6515C.8040304@msgid.tls.msk.ru> References: <51D3BEBA.4080809@msgid.tls.msk.ru> <51F6515C.8040304@msgid.tls.msk.ru> Message-ID: <20130729190710.16294.14160@loki> Date: Mon, 29 Jul 2013 14:07:10 -0500 Subject: Re: [Qemu-devel] moving CHR_EVENT_OPEN out of BH broke echo for muxed mon List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev Cc: Anthony Liguori , qemu-devel Quoting Michael Tokarev (2013-07-29 06:26:20) > Can we try to fix this for 1.6 please? :) > Or should I try to cook up a patch? Hmm, this seems to be a little less straight-forward than I thought. We can= 't defer OPENED till after MUX_OUT ad infinitum since this will cause the late= st FE to be added to never get the OPENED event. And new FEs can get added all throughout QEMU's lifecycle so there's no way to know when we're on the 'la= st' FE and can thus emit the deferred OPENED event unless we hook into something like qemu_add_machine_init_done_notifier(). Otherwise I don't see a solution other than re-introducing a bottom-half of some sort. If it's via g_idle_timeout() we'll have the same problems as bef= ore though, so this would involve using a QEMUBH which negates the work of being able to drive chardevs via a generic GMainLoop. Also, previous semantics were "on the subsequent iteration of the main loop, send OPENED event for all new FEs that have been added", and whoever had the focus at that time would print it's prompt/banner/etc output for OPENED events, so a QEMUBH is the only way to maintain the semantics exactly. With qemu_add_machine_init_done_notifier() approach, we'd see different behavior if, for instance, 2 FEs were attached to the mux in a single iteration of the main loop, as each would get the OPENED event immediately, as opposed to the previous behavior where only the latest one would get it before MUX_OUT was issued. Personally I think the slight change in semantics is worth avoiding re-introducing a QEMUBH, so I'll go ahead and put something together using qemu_add_machine_init_done_notifer(), but it feels dirty so please chime in if there's might be a better way to do this. > = > Thanks, > = > /mjt > = > 04.07.2013 01:06, Michael Roth wrote: > > On Wed, Jul 3, 2013 at 1:03 AM, Michael Tokarev wrote: > >> Before > >> > >> commit bd5c51ee6c4f1c79cae5ad2516d711a27b4ea8ec > >> Author: Michael Roth > >> Date: Fri Jun 7 15:19:53 2013 -0500 > >> > >> qemu-char: don't issue CHR_EVENT_OPEN in a BH > >> > >> we had no echo by default with -nographic, and it gave > >> the prompt when switching to monitor: > >> > >> $ qemu-system-x86_64 -nographic > >> _ > >> > >> (nothing is on the screen except the command line) > >> > >> (Ctrl+A, c) > >> QEMU 1.5.0 monitor - type 'help' for more information > >> (qemu) _ > >> > >> After this patch, we now have: > >> > >> $ qemu-system-x86_64 -nographic > >> QEMU 1.5.0 monitor - type 'help' for more information > >> (qemu) > >> _ > >> > >> (note the cursor position on the _next_ line after the prompt), > >> and the system is actually in "serial port" mode, not monitor > >> mode (you still need to hit Ctrl+A,c to switch to monitor). > > > > After a bit of unwinding I think I know what's going on. > > > > When we attach a new front-end to a mux (via qemu_chr_add_handlers), we= call the > > mux_chr_update_read_handler() to take those new handlers and stick them= in an > > array of FEs. > > > > We then switch over to the new FE by issuing CHR_EVENT_MUX_OUT to the > > previously active FE (if there was one), updating our focus idx to be t= he > > current FE count, and issuing CHR_EVENT_MUX_IN. > > > > Then, finally, toward the end of qemu_chr_add_handlers(), we issue a > > CHR_EVENT_OPENED if the backend is already open (true in the case of st= dio). > > > > Then we repeat this for subsequent FEs, which in the normal case result= s in > > the monitor FE getting a CHR_EVENT_MUX_OUT shortly after. > > > > - CHR_EVENT_MUX_IN prints the prompt if EVENT_OPENED has already been s= ent > > - CHR_EVENT_OPENED causes the monitor to output the banner and initial = prompt > > - CHR_EVENT_MUX_OUT causes the monitor to print a newline (to delimit a= ny > > subsequent output from another FE. > > > > Previously, due to CHR_EVENT_OPENED getting sent in a BH, the monitor FE > > would've already gotten the MUX_OUT event, so the banner and initial pr= ompt > > would be buffered, and not sent till we switched back to the monitor. N= ow, > > we sent CHR_EVENT_OPENED before the initial MUX_OUT, so the banner actu= ally > > gets display before we switch over to the next FE. > > > > The start-up state is actually exactly what you'd see if you cycled once > > through all the FEs in prior versions of QEMU, resulting in all this > > buffered up output getting flushed. > > > > Fix should be simple: defer CHR_EVENT_OPENED until after MUX_OUT to ret= ain > > the previous behavior. Gonna be out this week but can send a patch when= I > > get back. > > > > Thanks for the catch. > > > >> > >> Thanks, > >> > >> /mjt > >> > >