Re: [Qemu-devel] [PATCH 2/2] char: use a fixed idx for child muxed chr

[Marc-André Lureau]

Hi

On Tue, Oct 11, 2016 at 6:28 PM Marc-André Lureau <
address@hidden> wrote:

> Hi
>
> On Tue, Oct 11, 2016 at 4:21 PM Claudio Imbrenda <
> address@hidden> 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