Re: [Qemu-devel] Re: [PATCH v2 3/3] char: emit the OPENED event only when a new char connection is opened

[Amit Shah]

On (Mon) Oct 26 2009 [08:40:12], Jan Kiszka wrote:
> Amit Shah wrote:
> > On (Sat) Oct 24 2009 [12:36:54], Jan Kiszka wrote:
> >> Amit Shah wrote:
> >>> The OPENED event gets sent also when qemu resets its state initially.
> >>> The consumers of the event aren't interested in receiving this event
> >>> on reset.
> >> The monitor was. Now its initial prompt on activation is broken.
> > 
> > The patch in Anthony's queue, titled
> > 
> > 'console: call qemu_chr_reset() in text_console_init'
> 
> You may also want to rename qemu_chr_reset - unless there is still a
> need for real "reset".

Yeah; there isn't a need after my patches -- I've been slowing working
towards renaming it all.

> > 
> > fixed that.
> > 
> > However, with the qcow2 synchronous patch, the monitor prompt doesn't
> > come up again -- which shows there is a problem with the way the bhs
> > work and also the initial resets.
> 
> Then the qcow2 patch is already in? At least applying your patch doesn't
> change the picture.

Yeah; that patch went in just before my patches. Can you try reverting

ef845c3bf421290153154635dc18eaa677cecb43

> > I think the initial resets are a hack to work around something from my
> > reading of it; do you have a better idea of why it's there and how it's
> > all supposed to work?
> 
> From the monitor's POV, it's not a hack, it's simply the requirement to
> receive an indication that the console was opened.

Just an indication that the monitor was opened -- agreed. But git
history shows you added that as 'reset', so I'm wondering if maybe you
wanted it to do something else as well (or you did it that way just
because of the way qemu's bhs are handled?).

> >> Does this patch fix/improve something for a different user? If not,
> >> please let us revert it.
> > 
> > There's another question too: is a separate 'reset' event needed in
> > addition to an 'opened' event?
> 
> Not for the monitor, but I cannot speak for other users. I think it
> would be good to check them in details before changing the reset/open
> semantic.

As far as I could see in the git history, the 'reset' was added for the
monitor. And the others could live with the double 'reset' events they
were getting -- one for the reset and one when the device was opened.

                Amit