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.