Hi On Tue, Oct 11, 2016 at 6:28 PM Marc-André Lureau < marcandre.lur...@gmail.com> wrote:
> Hi > > On Tue, Oct 11, 2016 at 4:21 PM Claudio Imbrenda < > imbre...@linux.vnet.ibm.com> wrote: > > Hi, > > I noticed that this patch kills the Qemu monitor for me. If I start a > text-mode guest with -nographic, then I can't switch to the monitor any > longer with Ctrl+a c . The guest works otherwise, e.g. I can login from > the console. > > Tested on s390, but I think it's a more general issue, since the patch > is not arch-dependent. > > > On 03/10/16 11:47, Marc-André Lureau wrote: > > mux_chr_update_read_handler() is adding a new mux_cnt each time > > mux_chr_update_read_handler() is called, it's not possible to actually > > update the "child" chr callbacks that were set previously. This may lead > > to crashes if the "child" chr is destroyed: > > > > > My understanding was quite wrong, it was assuming that each mux user/fe > was a seperate chardev, that's not how it works. > > The patch should probably be reverted until a better solution comes up. I > am looking at it, but no solution yet. > > (obviously, it would be nice to have some minimal tests for mux, let see > if I get there) > -- > I am quite undecided how to fix this. qemu_chr_add_handlers users have no associated tag: this works ok as long as there is a single user per chardev. But it becomes problematic when there are multiple users, when the backing chardev is a mux: the mux will just grow more fe handlers, And you can't ever remove your fe callback which may lead to a crash. I am surprised this problem didn't raise before. I can imagine 2 solutions, either to associate each fe with it's opaque pointer to lookup the corresponding mux registered callbacks (less intrusive, yet not very clean), or to create a tag when calling qemu_chr_add_handlers() which can be used later with a new function like qemu_chr_remove_handlers(). This would be very intrusive, since all chr fe will have to hold a tag in their struct and pass it accordingly. Other thoughts? -- Marc-André Lureau
