Hi
----- Original Message -----
>
>
> On 13/10/2016 13:14, Marc-André Lureau wrote:
> > Hi,
> >
> > Commit 949055a2 "char: use a fixed idx for child muxed chr" introduced
> > a regression in mux usage, since it wrongly interpreted mux as muxing
> > various chr backend. Instead, it muxes frontends.
> >
> > The first patch reverts the broken change, the following patches add
> > tracking to frontend handler, finally the last patch adds some tests
> > that would have helped to track the crash and the regression. There is
> > also a small fix for ringbuf.
>
> In general I like the solution, but I dislike the API.
>
> Would it work if we had something like
>
> struct CharBackend {
> CharDriverState *chr;
> int tag;
> }
>
You mean front-end right?
> and we modified all qemu_chr_fe_* functions (plus
> qemu_chr_add_handlers[1]) to take a struct CharBackend. chardev
> properties would also take a struct CharBackend. Then removing handlers
> can still be done in release_chr, making the patches much smaller.
As long as they use chardev property, it's not always the case.
> The conversion is a bit tedious, but I think it's much easier compared
Yes, it's tedious :) Do you mind if I try to make the change on top? If it
really reduces the patch 4/7, we could try to squash it?
> to patch 4. I feel bad for having you redo everything and in particular
> patch 7, but this is the model that the block layer uses and it works
> very well there.
Which function btw?
>
> [1] while at it, it's probably best to rename qemu_chr_add_handlers
> to qemu_chr_fe_add_handlers as the first patch in the series,
> so that qemu_chr_add_handlers(CharDriverState *chr, ..., int tag)
> can take the role of qemu_chr_set_handlers in this series.